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

From: Scott Wood <[email-masked]>
Date: Fri, 14 Jun 2013 17:06:29 -0500

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.
>=20
> Signed-off-by: Minghuan Lian <[email-masked]>
> ---
>  .../devicetree/bindings/powerpc/fsl/msi-pic.txt    | 49 =20
> ++++++++++------------
>  1 file changed, 22 insertions(+), 27 deletions(-)
>=20
> diff --git =20
> a/Documentation/devicetree/bindings/powerpc/fsl/msi-pic.txt =20
> 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
>=20
>  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, =20
> mpc8572,
> -  etc.) and the second is "fsl,mpic-msi" or "fsl,ipic-msi" depending =20
> 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 =20
> mpic
> +  version is 4.3, the number of MSI registers is increased to 16, =20
> MSIIR1 is
> +  provided to access these 16 registers, compatible =20
> "fsl,mpic-msi-v4.3"
> +  should be used.

Why "one or two"?  What does it look like in the case where there's =20
just one?

>  - reg : It may contain one or two regions. The first region should =20
> contain
>    the address and the length of the shared message interrupt =20
> register set.
> -  The second region should contain the address of aliased MSIIR =20
> register for
> -  platforms that have such an alias.
> -
> -- msi-available-ranges: use <start count> style section to define =20
> which
> -  msi interrupt can be used in the 256 msi interrupts. This property =20
> is
> -  optional, without this, all the 256 MSI interrupts can be used.
> -  Each available range must begin and end on a multiple of 32 (i.e.
> -  no splitting an individual MSI register or the associated PIC =20
> interrupt).
> +  The second region should contain the address of aliased MSIIR or =20
> MSIIR1
> +  register for platforms that have such an alias, if using MSIIR1, =20
> the second
> +  region must be added because different MSI group has different =20
> MSIRR1 offset.

Why are you removing msi-available-ranges?  It's not valid for MPIC =20
v4.3, but it's still valid for older MPICs.  It should move to the =20
optional section, though.

>  - interrupts : each one of the interrupts here is one entry per 32 =20
> MSIs,
>    and routed to the host interrupt controller. the interrupts should
> -  be set as edge sensitive.  If msi-available-ranges is present, only
> -  the interrupts that correspond to available ranges shall be =20
> present.
> +  be set as edge sensitive.
>=20
>  - interrupt-parent: the phandle for the interrupt controller
>    that services interrupts for this device. for 83xx cpu, the =20
> interrupts
> @@ -39,20 +36,18 @@ Optional properties:
>=20
>  Example:
>  	msi@41600 {
> -		compatible =3D "fsl,mpc8610-msi", "fsl,mpic-msi";
> -		reg =3D <0x41600 0x80>;
> -		msi-available-ranges =3D <0 0x100>;
> -		interrupts =3D <
> -			0xe0 0
> -			0xe1 0
> -			0xe2 0
> -			0xe3 0
> -			0xe4 0
> -			0xe5 0
> -			0xe6 0
> -			0xe7 0>;
> -		interrupt-parent =3D <&mpic>;
> -	};
> +	compatible =3D "fsl,mpic-msi";
> +	reg =3D <0x41600 0x200 0x44140 4>;

Why 0x200?

-Scott=