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.l...@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@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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to