[v7,2/3] Documentation: dt: binding: fsl: Add 'little-endian' and update Chassis define
diff mbox series

Message ID 20191021034927.19300-2-ran.wang_1@nxp.com
State New
Headers show
Series
  • [v7,1/3] PM: wakeup: Add routine to help fetch wakeup source object.
Related show

Commit Message

Ran Wang Oct. 21, 2019, 3:49 a.m. UTC
By default, QorIQ SoC's RCPM register block is Big Endian. But
there are some exceptions, such as LS1088A and LS2088A, are
Little Endian. So add this optional property to help identify
them.

Actually LS2021A and other Layerscapes won't totally follow Chassis
2.1, so separate them from powerpc SoC.

Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Change in v7:
	- None.

Change in v6:
	- None.

Change in v5:
	- Add 'Reviewed-by: Rob Herring <robh@kernel.org>' to commit message.
	- Rename property 'fsl,#rcpm-wakeup-cells' to '#fsl,rcpm-wakeup-cells'.
	please see https://lore.kernel.org/patchwork/patch/1101022/

Change in v4:
	- Adjust indectation of 'ls1021a, ls1012a, ls1043a, ls1046a'.

Change in v3:
	- None.

Change in v2:
	- None.

 Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Scott Wood Oct. 24, 2019, 6:33 p.m. UTC | #1
On Mon, 2019-10-21 at 11:49 +0800, Ran Wang wrote:
> By default, QorIQ SoC's RCPM register block is Big Endian. But
> there are some exceptions, such as LS1088A and LS2088A, are
> Little Endian. So add this optional property to help identify
> them.
> 
> Actually LS2021A and other Layerscapes won't totally follow Chassis
> 2.1, so separate them from powerpc SoC.

Did you mean LS1021A and "don't" instead of "won't", given the change to the
examples?

> Change in v5:
> 	- Add 'Reviewed-by: Rob Herring <robh@kernel.org>' to commit message.
> 	- Rename property 'fsl,#rcpm-wakeup-cells' to '#fsl,rcpm-wakeup-
> cells'.
> 	please see https://lore.kernel.org/patchwork/patch/1101022/

I'm not sure why Rob considers this the "correct form" -- there are other
examples of the current form, such as ibm,#dma-address-cells and ti,#tlb-
entries, and the current form makes more logical sense (# is part of the
property name, not the vendor).  Oh well.

> Required properites:
>    - reg : Offset and length of the register set of the RCPM block.
> -  - fsl,#rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the
> +  - #fsl,rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the
>  	fsl,rcpm-wakeup property.
>    - compatible : Must contain a chip-specific RCPM block compatible string
>  	and (if applicable) may contain a chassis-version RCPM compatible
> @@ -20,6 +20,7 @@ Required properites:
>  	* "fsl,qoriq-rcpm-1.0": for chassis 1.0 rcpm
>  	* "fsl,qoriq-rcpm-2.0": for chassis 2.0 rcpm
>  	* "fsl,qoriq-rcpm-2.1": for chassis 2.1 rcpm
> +	* "fsl,qoriq-rcpm-2.1+": for chassis 2.1+ rcpm

Is there something actually called "2.1+"?  It looks a bit like an attempt to
claim compatibility with all future versions.  If the former, is it a name
that comes from the hardware side with an intent for it to describe a stable
interface, or are we later going to see a patch changing some by-then-existing 
device trees from "2.1+" to "2.1++" when some new incompatibility is found?

Perhaps it would be better to bind to the specific chip compatibles.

-Scott
Ran Wang Oct. 25, 2019, 2:40 a.m. UTC | #2
Hi Scott,

On Friday, October 25, 2019 02:34, Scott Wood wrote
> 
> On Mon, 2019-10-21 at 11:49 +0800, Ran Wang wrote:
> > By default, QorIQ SoC's RCPM register block is Big Endian. But there
> > are some exceptions, such as LS1088A and LS2088A, are Little Endian.
> > So add this optional property to help identify them.
> >
> > Actually LS2021A and other Layerscapes won't totally follow Chassis
> > 2.1, so separate them from powerpc SoC.
> 
> Did you mean LS1021A and "don't" instead of "won't", given the change to the
> examples?

OK, I will change it to don't to just tel current situation.
 
> > Change in v5:
> > 	- Add 'Reviewed-by: Rob Herring <robh@kernel.org>' to commit
> message.
> > 	- Rename property 'fsl,#rcpm-wakeup-cells' to '#fsl,rcpm-wakeup-
> > cells'.
> > 	please see https://lore.kernel.org/patchwork/patch/1101022/
> 
> I'm not sure why Rob considers this the "correct form" -- there are other
> examples of the current form, such as ibm,#dma-address-cells and ti,#tlb-
> entries, and the current form makes more logical sense (# is part of the property
> name, not the vendor).  Oh well.
> 
> > Required properites:
> >    - reg : Offset and length of the register set of the RCPM block.
> > -  - fsl,#rcpm-wakeup-cells : The number of IPPDEXPCR register cells
> > in the
> > +  - #fsl,rcpm-wakeup-cells : The number of IPPDEXPCR register cells
> > + in the
> >  	fsl,rcpm-wakeup property.
> >    - compatible : Must contain a chip-specific RCPM block compatible string
> >  	and (if applicable) may contain a chassis-version RCPM compatible @@
> > -20,6 +20,7 @@ Required properites:
> >  	* "fsl,qoriq-rcpm-1.0": for chassis 1.0 rcpm
> >  	* "fsl,qoriq-rcpm-2.0": for chassis 2.0 rcpm
> >  	* "fsl,qoriq-rcpm-2.1": for chassis 2.1 rcpm
> > +	* "fsl,qoriq-rcpm-2.1+": for chassis 2.1+ rcpm
> 
> Is there something actually called "2.1+"?  It looks a bit like an attempt to claim
> compatibility with all future versions.  If the former, is it a name that comes
> from the hardware side with an intent for it to describe a stable interface, or are
> we later going to see a patch changing some by-then-existing device trees from
> "2.1+" to "2.1++" when some new incompatibility is found?
>
> Perhaps it would be better to bind to the specific chip compatibles.

According to SoC data sheets, powerPC SoC T1040 and current ARM based Layerscape
SoCs (LS1021A, LS1012A, LS1043A, etc)'s arch designs are both basing on Chassis spec 2.1.
However, for Layerscape, their data sheets are also explicitly telling that some minor
changes have been made(basing on Chassis 2.1 spec). And in parallel, the SW arch designs
between T1040 and Layerscape family are also different: For Layerscape, part of RCPM
programming job has been moved from kernel driver to firmware/bootloader (through
PSCI interface). That's why I have to name a new compatible string to distinguish them.
They cannot use the same driver. I don’t think we will add another sting like 2.1++ in the
future. If the Chassis spec keep evolving and requiring different programming logic,
we can add more like 3.0, 4.0, ..., I think.

Regards,
Ran

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
index e284e4e..5a33619 100644
--- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
@@ -5,7 +5,7 @@  and power management.
 
 Required properites:
   - reg : Offset and length of the register set of the RCPM block.
-  - fsl,#rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the
+  - #fsl,rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the
 	fsl,rcpm-wakeup property.
   - compatible : Must contain a chip-specific RCPM block compatible string
 	and (if applicable) may contain a chassis-version RCPM compatible
@@ -20,6 +20,7 @@  Required properites:
 	* "fsl,qoriq-rcpm-1.0": for chassis 1.0 rcpm
 	* "fsl,qoriq-rcpm-2.0": for chassis 2.0 rcpm
 	* "fsl,qoriq-rcpm-2.1": for chassis 2.1 rcpm
+	* "fsl,qoriq-rcpm-2.1+": for chassis 2.1+ rcpm
 
 All references to "1.0" and "2.0" refer to the QorIQ chassis version to
 which the chip complies.
@@ -27,14 +28,19 @@  Chassis Version		Example Chips
 ---------------		-------------------------------
 1.0				p4080, p5020, p5040, p2041, p3041
 2.0				t4240, b4860, b4420
-2.1				t1040, ls1021
+2.1				t1040,
+2.1+				ls1021a, ls1012a, ls1043a, ls1046a
+
+Optional properties:
+ - little-endian : RCPM register block is Little Endian. Without it RCPM
+   will be Big Endian (default case).
 
 Example:
 The RCPM node for T4240:
 	rcpm: global-utilities@e2000 {
 		compatible = "fsl,t4240-rcpm", "fsl,qoriq-rcpm-2.0";
 		reg = <0xe2000 0x1000>;
-		fsl,#rcpm-wakeup-cells = <2>;
+		#fsl,rcpm-wakeup-cells = <2>;
 	};
 
 * Freescale RCPM Wakeup Source Device Tree Bindings
@@ -44,7 +50,7 @@  can be used as a wakeup source.
 
   - fsl,rcpm-wakeup: Consists of a phandle to the rcpm node and the IPPDEXPCR
 	register cells. The number of IPPDEXPCR register cells is defined in
-	"fsl,#rcpm-wakeup-cells" in the rcpm node. The first register cell is
+	"#fsl,rcpm-wakeup-cells" in the rcpm node. The first register cell is
 	the bit mask that should be set in IPPDEXPCR0, and the second register
 	cell is for IPPDEXPCR1, and so on.