[PATCH 3/5] powerpc/dts: update MSI bindings doc for MPIC v4.3

[Scott Wood]

On 06/17/2013 12:07:41 AM, Lian Minghuan-b31939 wrote:
> Hi Soctt,
> 
> please see my comments.
> 
> On 06/15/2013 06:06 AM, Scott Wood wrote:
>> On 06/14/2013 02:15:57 AM, Minghuan Lian wrote:
>>> Add compatible "fsl,mpic-msi-v4.3" for MPIC v4.3. MPIC v4.3 contains
>>> MSIIR and MSIIR1. MSIIR supports 8 MSI registers and MSIIR1 supports
>>> 16 MSI registers, but uses different IBS and SRS shift. When using
>>> MSIR1, the interrupt number is not consecutive. It is hard to use
>>> 'msi-available-ranges' to describe the ranges of the available
>>> interrupt and the ranges are related to the application, rather than
>>> the description of the hardware. this patch also removes
>>> 'msi-available-ranges' property.
>>> 
>>> Signed-off-by: Minghuan Lian <Minghuan.Lian at freescale.com>
>>> ---
>>>  .../devicetree/bindings/powerpc/fsl/msi-pic.txt    | 49  
>>> ++++++++++------------
>>>  1 file changed, 22 insertions(+), 27 deletions(-)
>>> 
>>> diff --git  
>>> a/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt  
>>> b/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt
>>> index 5693877..e851e93 100644
>>> --- a/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt
>>> +++ b/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt
>>> @@ -1,26 +1,23 @@
>>>  * Freescale MSI interrupt controller
>>> 
>>>  Required properties:
>>> -- compatible : compatible list, contains 2 entries,
>>> +- compatible : compatible list, may contains one or two entries,
>>>    first is "fsl,CHIP-msi", where CHIP is the processor(mpc8610,  
>>> mpc8572,
>>> -  etc.) and the second is "fsl,mpic-msi" or "fsl,ipic-msi"  
>>> depending on
>>> -  the parent type.
>>> +  etc.) and the second is "fsl,mpic-msi" or "fsl,ipic-msi" or
>>> +  "fsl,mpic-msi-v4.3" depending on the parent type and version. If  
>>> mpic
>>> +  version is 4.3, the number of MSI registers is increased to 16,  
>>> MSIIR1 is
>>> +  provided to access these 16 registers, compatible  
>>> "fsl,mpic-msi-v4.3"
>>> +  should be used.
>> 
>> Why "one or two"?  What does it look like in the case where there's  
>> just one?
>> 
> [Minghuan] The original doc said 'contains 2 entries', but I notcie  
> pq3-mpic.dtsi and qoriq-mpic.dtsi have only one entry "fsl,mpic-msi",  
> do not have "fsl,CHIP-msi".
> for example:
> mpc8610_hpcd.dts: compatible = "fsl,mpc8610-msi", "fsl,mpic-msi";
> fsl/qoriq-mpic.dtsi:  compatible = "fsl,mpic-msi"
> 
> Maybe I should say " For some platforms, "fsl,CHIP-msi' is optional."

Well, this is more a matter of some device trees not complying with the  
binding, rather than an update for MPIC v4.3.

In any case, if the plan is to update the binding to match what we've  
been doing in the actual trees, at least word it so that it's clear  
which one of the two is optional.

>> Why are you removing msi-available-ranges?  It's not valid for MPIC  
>> v4.3, but it's still valid for older MPICs.  It should move to the  
>> optional section, though.
> [Minghuan] Because I would like to add kernel parameter 'msiregs'  
> instead of "msi-available-ranges", for all the MPICs, we will have a  
> uniform way to configure

I've responded elsewhere to this, but I'd also like to add that we  
don't break compatibility with older device tree bindings just for "a  
uniform way".

>>>  Example:
>>>      msi at 41600 {
>>> -        compatible = "fsl,mpc8610-msi", "fsl,mpic-msi";
>>> -        reg = <0x41600 0x80>;
>>> -        msi-available-ranges = <0 0x100>;
>>> -        interrupts = <
>>> -            0xe0 0
>>> -            0xe1 0
>>> -            0xe2 0
>>> -            0xe3 0
>>> -            0xe4 0
>>> -            0xe5 0
>>> -            0xe6 0
>>> -            0xe7 0>;
>>> -        interrupt-parent = <&mpic>;
>>> -    };
>>> +    compatible = "fsl,mpic-msi";
>>> +    reg = <0x41600 0x200 0x44140 4>;
>> 
>> Why 0x200?
>> 
> [Minghuan] The offsets of the MSIA registers are from 0x41600 to  
> 0x417ff, and the size is 0x200.
> offset 0x41600-0x4170 are MSIIRA1-7.
> 0x41720 is MSISRA,
> 0x41750 is MSIIR.
> The others are reserved.

There is no MSIIRA on fsl,mpic-msi.

If you want to show an fsl,mpic-msi-v4.3 example, update the compatible  
and add the extra 8 interrupts.  We should probably show an example of  
each.

BTW, why are you changing/breaking the whitespace in the example?

-Scott