linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v3,1/3] soc: fsl: handle RCPM errata A-008646 on SoC LS1021A
@ 2019-09-24  2:45 Biwen Li
  2019-09-24  2:45 ` [v3,2/3] arm: dts: ls1021a: fix that FlexTimer cannot wakeup system in deep sleep Biwen Li
  2019-09-24  2:45 ` [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property Biwen Li
  0 siblings, 2 replies; 15+ messages in thread
From: Biwen Li @ 2019-09-24  2:45 UTC (permalink / raw)
  To: leoyang.li, shawnguo, robh+dt, mark.rutland, ran.wang_1
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree, Biwen Li

Description:
	- Reading configuration register RCPM_IPPDEXPCR1
	  always return zero

Workaround:
	- Save register RCPM_IPPDEXPCR1's value to
	  register SCFG_SPARECR8.(uboot's psci also
	  need reading value from the register SCFG_SPARECR8
	  to set register RCPM_IPPDEXPCR1)

Impact:
	- FlexTimer module will cannot wakeup system in
	  deep sleep on SoC LS1021A

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v3:
	- update commit message
	- rename property name
	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr

Change in v2:
	- fix stype problems

 drivers/soc/fsl/rcpm.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
index 82c0ad5e663e..7f42b17d3f29 100644
--- a/drivers/soc/fsl/rcpm.c
+++ b/drivers/soc/fsl/rcpm.c
@@ -13,6 +13,8 @@
 #include <linux/slab.h>
 #include <linux/suspend.h>
 #include <linux/kernel.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #define RCPM_WAKEUP_CELL_MAX_SIZE	7
 
@@ -29,6 +31,9 @@ static int rcpm_pm_prepare(struct device *dev)
 	struct rcpm		*rcpm;
 	u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp;
 	int i, ret, idx;
+	struct regmap * scfg_addr_regmap = NULL;
+	u32 reg_offset[RCPM_WAKEUP_CELL_MAX_SIZE + 1];
+	u32 reg_value = 0;
 
 	rcpm = dev_get_drvdata(dev);
 	if (!rcpm)
@@ -63,6 +68,22 @@ static int rcpm_pm_prepare(struct device *dev)
 					tmp |= value[i + 1];
 					iowrite32be(tmp, rcpm->ippdexpcr_base + i * 4);
 				}
+				/* Workaround of errata A-008646 on SoC LS1021A: There is a bug of
+				 * register ippdexpcr1. Reading configuration register RCPM_IPPDEXPCR1
+				 * always return zero. So save ippdexpcr1's value to register SCFG_SPARECR8.
+				 * And the value of ippdexpcr1 will be read from SCFG_SPARECR8.
+				 */
+				scfg_addr_regmap = syscon_regmap_lookup_by_phandle(np, "fsl,ippdexpcr-alt-addr");
+				if (scfg_addr_regmap) {
+					if (of_property_read_u32_array(dev->of_node,
+					    "fsl,ippdexpcr-alt-addr", reg_offset, rcpm->wakeup_cells + 1)) {
+						scfg_addr_regmap = NULL;
+						continue;
+					}
+					regmap_read(scfg_addr_regmap, reg_offset[i + 1], &reg_value);
+					/* Write value to register SCFG_SPARECR8 */
+					regmap_write(scfg_addr_regmap, reg_offset[i + 1], tmp | reg_value);
+				}
 			}
 		}
 	} while (ws = wakeup_source_get_next(ws));
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [v3,2/3] arm: dts: ls1021a: fix that FlexTimer cannot wakeup system in deep sleep
  2019-09-24  2:45 [v3,1/3] soc: fsl: handle RCPM errata A-008646 on SoC LS1021A Biwen Li
@ 2019-09-24  2:45 ` Biwen Li
  2019-09-24  2:45 ` [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property Biwen Li
  1 sibling, 0 replies; 15+ messages in thread
From: Biwen Li @ 2019-09-24  2:45 UTC (permalink / raw)
  To: leoyang.li, shawnguo, robh+dt, mark.rutland, ran.wang_1
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree, Biwen Li

The patch fix a bug that FlexTimer cannot
wakeup system in deep sleep.

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v3:
	- update property name
	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
	  
Change in v2:
	- None
	 
 arch/arm/boot/dts/ls1021a.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index e3973b611c3a..383b2dcd5720 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -1000,6 +1000,7 @@
 			compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
 			reg = <0x0 0x1ee2140 0x0 0x8>;
 			#fsl,rcpm-wakeup-cells = <2>;
+			fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /* SCFG_SPARECR8 */
 		};
 
 		ftm_alarm0: timer0@29d0000 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-24  2:45 [v3,1/3] soc: fsl: handle RCPM errata A-008646 on SoC LS1021A Biwen Li
  2019-09-24  2:45 ` [v3,2/3] arm: dts: ls1021a: fix that FlexTimer cannot wakeup system in deep sleep Biwen Li
@ 2019-09-24  2:45 ` Biwen Li
  2019-09-24 15:58   ` Leo Li
  1 sibling, 1 reply; 15+ messages in thread
From: Biwen Li @ 2019-09-24  2:45 UTC (permalink / raw)
  To: leoyang.li, shawnguo, robh+dt, mark.rutland, ran.wang_1
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree, Biwen Li

The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata A-008646
on LS1021A

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v3:
	- rename property name
	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr

Change in v2:
	- update desc of the property 'fsl,rcpm-scfg'

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

diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
index 5a33619d881d..157dcf6da17c 100644
--- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
@@ -34,6 +34,11 @@ Chassis Version		Example Chips
 Optional properties:
  - little-endian : RCPM register block is Little Endian. Without it RCPM
    will be Big Endian (default case).
+ - fsl,ippdexpcr-alt-addr : Must add the property for SoC LS1021A,
+   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
+   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 entries).
+   The first entry must be a link to the SCFG device node.
+   The non-first entry must be offset of registers of SCFG.
 
 Example:
 The RCPM node for T4240:
@@ -43,6 +48,15 @@ The RCPM node for T4240:
 		#fsl,rcpm-wakeup-cells = <2>;
 	};
 
+The RCPM node for LS1021A:
+	rcpm: rcpm@1ee2140 {
+		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
+		reg = <0x0 0x1ee2140 0x0 0x8>;
+		#fsl,rcpm-wakeup-cells = <2>;
+		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /* SCFG_SPARECR8 */
+	};
+
+
 * Freescale RCPM Wakeup Source Device Tree Bindings
 -------------------------------------------
 Required fsl,rcpm-wakeup property should be added to a device node if the device
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-24  2:45 ` [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property Biwen Li
@ 2019-09-24 15:58   ` Leo Li
  2019-09-25  3:13     ` Biwen Li
  0 siblings, 1 reply; 15+ messages in thread
From: Leo Li @ 2019-09-24 15:58 UTC (permalink / raw)
  To: Biwen Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree, Biwen Li



> -----Original Message-----
> From: Biwen Li <biwen.li@nxp.com>
> Sent: Monday, September 23, 2019 9:46 PM
> To: Leo Li <leoyang.li@nxp.com>; shawnguo@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Ran Wang
> <ran.wang_1@nxp.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Biwen Li
> <biwen.li@nxp.com>
> Subject: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr'
> property
> 
> The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata A-008646 on
> LS1021A
> 
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
> Change in v3:
> 	- rename property name
> 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> 
> Change in v2:
> 	- update desc of the property 'fsl,rcpm-scfg'
> 
>  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> index 5a33619d881d..157dcf6da17c 100644
> --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> @@ -34,6 +34,11 @@ Chassis Version		Example Chips
>  Optional properties:
>   - little-endian : RCPM register block is Little Endian. Without it RCPM
>     will be Big Endian (default case).
> + - fsl,ippdexpcr-alt-addr : Must add the property for SoC LS1021A,

You probably should mention this is related to a hardware issue on LS1021a and only needed on LS1021a.

> +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 entries).

#fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an SoC.  However you are defining an offset to scfg registers here.  Why these two are related?  The length here should actually be related to the #address-cells of the soc/.  But since this is only needed for LS1021, you can just make it 3.

> +   The first entry must be a link to the SCFG device node.
> +   The non-first entry must be offset of registers of SCFG.
> 
>  Example:
>  The RCPM node for T4240:
> @@ -43,6 +48,15 @@ The RCPM node for T4240:
>  		#fsl,rcpm-wakeup-cells = <2>;
>  	};
> 
> +The RCPM node for LS1021A:
> +	rcpm: rcpm@1ee2140 {
> +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
> +		reg = <0x0 0x1ee2140 0x0 0x8>;
> +		#fsl,rcpm-wakeup-cells = <2>;
> +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> SCFG_SPARECR8 */
> +	};
> +
> +
>  * Freescale RCPM Wakeup Source Device Tree Bindings
>  -------------------------------------------
>  Required fsl,rcpm-wakeup property should be added to a device node if the
> device
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-24 15:58   ` Leo Li
@ 2019-09-25  3:13     ` Biwen Li
  2019-09-25  3:26       ` Leo Li
  0 siblings, 1 reply; 15+ messages in thread
From: Biwen Li @ 2019-09-25  3:13 UTC (permalink / raw)
  To: Leo Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree

> >
> > The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata
> > A-008646 on LS1021A
> >
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > ---
> > Change in v3:
> > 	- rename property name
> > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> >
> > Change in v2:
> > 	- update desc of the property 'fsl,rcpm-scfg'
> >
> >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > index 5a33619d881d..157dcf6da17c 100644
> > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > @@ -34,6 +34,11 @@ Chassis Version		Example Chips
> >  Optional properties:
> >   - little-endian : RCPM register block is Little Endian. Without it RCPM
> >     will be Big Endian (default case).
> > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC LS1021A,
> 
> You probably should mention this is related to a hardware issue on LS1021a
> and only needed on LS1021a.
Okay, got it, thanks, I will add this in v4.
> 
> > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 entries).
> 
> #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an SoC.
> However you are defining an offset to scfg registers here.  Why these two
> are related?  The length here should actually be related to the #address-cells
> of the soc/.  But since this is only needed for LS1021, you can just make it 3.
I need set the value of IPPDEXPCR resgiters from ftm_alarm0 device node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for IPPDEXPCR1).
But because of the hardware issue on LS1021A, I need store the value of IPPDEXPCR registers
to an alt address. So I defining an offset to scfg registers, then RCPM driver get an abosolute address from offset,
 RCPM driver write the value of IPPDEXPCR registers to these abosolute addresses(backup the value of IPPDEXPCR registers).
> 
> > +   The first entry must be a link to the SCFG device node.
> > +   The non-first entry must be offset of registers of SCFG.
> >
> >  Example:
> >  The RCPM node for T4240:
> > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> >  		#fsl,rcpm-wakeup-cells = <2>;
> >  	};
> >
> > +The RCPM node for LS1021A:
> > +	rcpm: rcpm@1ee2140 {
> > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
> > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > +		#fsl,rcpm-wakeup-cells = <2>;
> > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > SCFG_SPARECR8 */
> > +	};
> > +
> > +
> >  * Freescale RCPM Wakeup Source Device Tree Bindings
> >  -------------------------------------------
> >  Required fsl,rcpm-wakeup property should be added to a device node if
> > the device
> > --
> > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  3:13     ` Biwen Li
@ 2019-09-25  3:26       ` Leo Li
  2019-09-25  3:30         ` Biwen Li
  0 siblings, 1 reply; 15+ messages in thread
From: Leo Li @ 2019-09-25  3:26 UTC (permalink / raw)
  To: Biwen Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree



> -----Original Message-----
> From: Biwen Li
> Sent: Tuesday, September 24, 2019 10:13 PM
> To: Leo Li <leoyang.li@nxp.com>; shawnguo@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Ran Wang
> <ran.wang_1@nxp.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-
> addr' property
> 
> > >
> > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata
> > > A-008646 on LS1021A
> > >
> > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > ---
> > > Change in v3:
> > > 	- rename property name
> > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > >
> > > Change in v2:
> > > 	- update desc of the property 'fsl,rcpm-scfg'
> > >
> > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > index 5a33619d881d..157dcf6da17c 100644
> > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > @@ -34,6 +34,11 @@ Chassis Version		Example Chips
> > >  Optional properties:
> > >   - little-endian : RCPM register block is Little Endian. Without it RCPM
> > >     will be Big Endian (default case).
> > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC LS1021A,
> >
> > You probably should mention this is related to a hardware issue on
> > LS1021a and only needed on LS1021a.
> Okay, got it, thanks, I will add this in v4.
> >
> > > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 entries).
> >
> > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an SoC.
> > However you are defining an offset to scfg registers here.  Why these
> > two are related?  The length here should actually be related to the
> > #address-cells of the soc/.  But since this is only needed for LS1021, you can
> just make it 3.
> I need set the value of IPPDEXPCR resgiters from ftm_alarm0 device
> node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for IPPDEXPCR1).
> But because of the hardware issue on LS1021A, I need store the value of
> IPPDEXPCR registers to an alt address. So I defining an offset to scfg registers,
> then RCPM driver get an abosolute address from offset,  RCPM driver write
> the value of IPPDEXPCR registers to these abosolute addresses(backup the
> value of IPPDEXPCR registers).

I understand what you are trying to do.  The problem is that the new fsl,ippdexpcr-alt-addr property contains a phandle and an offset.  The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.

> >
> > > +   The first entry must be a link to the SCFG device node.
> > > +   The non-first entry must be offset of registers of SCFG.
> > >
> > >  Example:
> > >  The RCPM node for T4240:
> > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > >  		#fsl,rcpm-wakeup-cells = <2>;
> > >  	};
> > >
> > > +The RCPM node for LS1021A:
> > > +	rcpm: rcpm@1ee2140 {
> > > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
> > > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > > +		#fsl,rcpm-wakeup-cells = <2>;
> > > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > SCFG_SPARECR8 */
> > > +	};
> > > +
> > > +
> > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > >  -------------------------------------------
> > >  Required fsl,rcpm-wakeup property should be added to a device node
> > > if the device
> > > --
> > > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  3:26       ` Leo Li
@ 2019-09-25  3:30         ` Biwen Li
  2019-09-25  3:34           ` Leo Li
  0 siblings, 1 reply; 15+ messages in thread
From: Biwen Li @ 2019-09-25  3:30 UTC (permalink / raw)
  To: Leo Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree

> > > >
> > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata
> > > > A-008646 on LS1021A
> > > >
> > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > ---
> > > > Change in v3:
> > > > 	- rename property name
> > > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > >
> > > > Change in v2:
> > > > 	- update desc of the property 'fsl,rcpm-scfg'
> > > >
> > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > index 5a33619d881d..157dcf6da17c 100644
> > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > @@ -34,6 +34,11 @@ Chassis Version		Example Chips
> > > >  Optional properties:
> > > >   - little-endian : RCPM register block is Little Endian. Without it RCPM
> > > >     will be Big Endian (default case).
> > > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC
> > > > + LS1021A,
> > >
> > > You probably should mention this is related to a hardware issue on
> > > LS1021a and only needed on LS1021a.
> > Okay, got it, thanks, I will add this in v4.
> > >
> > > > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 entries).
> > >
> > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an SoC.
> > > However you are defining an offset to scfg registers here.  Why
> > > these two are related?  The length here should actually be related
> > > to the #address-cells of the soc/.  But since this is only needed
> > > for LS1021, you can
> > just make it 3.
> > I need set the value of IPPDEXPCR resgiters from ftm_alarm0 device
> > node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for IPPDEXPCR1).
> > But because of the hardware issue on LS1021A, I need store the value
> > of IPPDEXPCR registers to an alt address. So I defining an offset to
> > scfg registers, then RCPM driver get an abosolute address from offset,
> > RCPM driver write the value of IPPDEXPCR registers to these abosolute
> > addresses(backup the value of IPPDEXPCR registers).
> 
> I understand what you are trying to do.  The problem is that the new
> fsl,ippdexpcr-alt-addr property contains a phandle and an offset.  The size
> of it shouldn't be related to #fsl,rcpm-wakeup-cells.
You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/* SCFG_SPARECR8 */
> 
> > >
> > > > +   The first entry must be a link to the SCFG device node.
> > > > +   The non-first entry must be offset of registers of SCFG.
> > > >
> > > >  Example:
> > > >  The RCPM node for T4240:
> > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > >  		#fsl,rcpm-wakeup-cells = <2>;
> > > >  	};
> > > >
> > > > +The RCPM node for LS1021A:
> > > > +	rcpm: rcpm@1ee2140 {
> > > > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
> > > > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > +		#fsl,rcpm-wakeup-cells = <2>;
> > > > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > > SCFG_SPARECR8 */
> > > > +	};
> > > > +
> > > > +
> > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > >  -------------------------------------------
> > > >  Required fsl,rcpm-wakeup property should be added to a device
> > > > node if the device
> > > > --
> > > > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  3:30         ` Biwen Li
@ 2019-09-25  3:34           ` Leo Li
  2019-09-25  3:47             ` Biwen Li
  2019-09-27 18:28             ` Rob Herring
  0 siblings, 2 replies; 15+ messages in thread
From: Leo Li @ 2019-09-25  3:34 UTC (permalink / raw)
  To: Biwen Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree



> -----Original Message-----
> From: Biwen Li
> Sent: Tuesday, September 24, 2019 10:30 PM
> To: Leo Li <leoyang.li@nxp.com>; shawnguo@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Ran Wang
> <ran.wang_1@nxp.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-
> addr' property
> 
> > > > >
> > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an
> > > > > errata
> > > > > A-008646 on LS1021A
> > > > >
> > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > ---
> > > > > Change in v3:
> > > > > 	- rename property name
> > > > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > >
> > > > > Change in v2:
> > > > > 	- update desc of the property 'fsl,rcpm-scfg'
> > > > >
> > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > ++++++++++++++
> > > > >  1 file changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > @@ -34,6 +34,11 @@ Chassis Version		Example Chips
> > > > >  Optional properties:
> > > > >   - little-endian : RCPM register block is Little Endian. Without it RCPM
> > > > >     will be Big Endian (default case).
> > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC
> > > > > + LS1021A,
> > > >
> > > > You probably should mention this is related to a hardware issue on
> > > > LS1021a and only needed on LS1021a.
> > > Okay, got it, thanks, I will add this in v4.
> > > >
> > > > > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 entries).
> > > >
> > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an SoC.
> > > > However you are defining an offset to scfg registers here.  Why
> > > > these two are related?  The length here should actually be related
> > > > to the #address-cells of the soc/.  But since this is only needed
> > > > for LS1021, you can
> > > just make it 3.
> > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0 device
> > > node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for IPPDEXPCR1).
> > > But because of the hardware issue on LS1021A, I need store the value
> > > of IPPDEXPCR registers to an alt address. So I defining an offset to
> > > scfg registers, then RCPM driver get an abosolute address from
> > > offset, RCPM driver write the value of IPPDEXPCR registers to these
> > > abosolute addresses(backup the value of IPPDEXPCR registers).
> >
> > I understand what you are trying to do.  The problem is that the new
> > fsl,ippdexpcr-alt-addr property contains a phandle and an offset.  The
> > size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/*
> SCFG_SPARECR8 */

No.  The #address-cell for the soc/ is 2, so the offset to scfg should be 0x0 0x51c.  The total size should be 3, but it shouldn't be coming from #fsl,rcpm-wakeup-cells like you mentioned in the binding.

> >
> > > >
> > > > > +   The first entry must be a link to the SCFG device node.
> > > > > +   The non-first entry must be offset of registers of SCFG.
> > > > >
> > > > >  Example:
> > > > >  The RCPM node for T4240:
> > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > >  		#fsl,rcpm-wakeup-cells = <2>;
> > > > >  	};
> > > > >
> > > > > +The RCPM node for LS1021A:
> > > > > +	rcpm: rcpm@1ee2140 {
> > > > > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-
> 2.1+";
> > > > > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > +		#fsl,rcpm-wakeup-cells = <2>;
> > > > > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > > > SCFG_SPARECR8 */
> > > > > +	};
> > > > > +
> > > > > +
> > > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > >  -------------------------------------------
> > > > >  Required fsl,rcpm-wakeup property should be added to a device
> > > > > node if the device
> > > > > --
> > > > > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  3:34           ` Leo Li
@ 2019-09-25  3:47             ` Biwen Li
  2019-09-25  3:48               ` Leo Li
  2019-09-27 18:28             ` Rob Herring
  1 sibling, 1 reply; 15+ messages in thread
From: Biwen Li @ 2019-09-25  3:47 UTC (permalink / raw)
  To: Leo Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree

> > > > > >
> > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an
> > > > > > errata
> > > > > > A-008646 on LS1021A
> > > > > >
> > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > ---
> > > > > > Change in v3:
> > > > > > 	- rename property name
> > > > > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > >
> > > > > > Change in v2:
> > > > > > 	- update desc of the property 'fsl,rcpm-scfg'
> > > > > >
> > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > > ++++++++++++++
> > > > > >  1 file changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > @@ -34,6 +34,11 @@ Chassis Version		Example Chips
> > > > > >  Optional properties:
> > > > > >   - little-endian : RCPM register block is Little Endian. Without it
> RCPM
> > > > > >     will be Big Endian (default case).
> > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC
> > > > > > + LS1021A,
> > > > >
> > > > > You probably should mention this is related to a hardware issue
> > > > > on LS1021a and only needed on LS1021a.
> > > > Okay, got it, thanks, I will add this in v4.
> > > > >
> > > > > > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1
> entries).
> > > > >
> > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an
> SoC.
> > > > > However you are defining an offset to scfg registers here.  Why
> > > > > these two are related?  The length here should actually be
> > > > > related to the #address-cells of the soc/.  But since this is
> > > > > only needed for LS1021, you can
> > > > just make it 3.
> > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0 device
> > > > node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for
> IPPDEXPCR1).
> > > > But because of the hardware issue on LS1021A, I need store the
> > > > value of IPPDEXPCR registers to an alt address. So I defining an
> > > > offset to scfg registers, then RCPM driver get an abosolute
> > > > address from offset, RCPM driver write the value of IPPDEXPCR
> > > > registers to these abosolute addresses(backup the value of IPPDEXPCR
> registers).
> > >
> > > I understand what you are trying to do.  The problem is that the new
> > > fsl,ippdexpcr-alt-addr property contains a phandle and an offset.
> > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/*
> > SCFG_SPARECR8 */
> 
> No.  The #address-cell for the soc/ is 2, so the offset to scfg should be 0x0
> 0x51c.  The total size should be 3, but it shouldn't be coming from
> #fsl,rcpm-wakeup-cells like you mentioned in the binding.
Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative with #address-cells instead of #fsl,rcpm-wakeup-cells.
> 
> > >
> > > > >
> > > > > > +   The first entry must be a link to the SCFG device node.
> > > > > > +   The non-first entry must be offset of registers of SCFG.
> > > > > >
> > > > > >  Example:
> > > > > >  The RCPM node for T4240:
> > > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > > >  		#fsl,rcpm-wakeup-cells = <2>;
> > > > > >  	};
> > > > > >
> > > > > > +The RCPM node for LS1021A:
> > > > > > +	rcpm: rcpm@1ee2140 {
> > > > > > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-
> > 2.1+";
> > > > > > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > > +		#fsl,rcpm-wakeup-cells = <2>;
> > > > > > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > > > > SCFG_SPARECR8 */
> > > > > > +	};
> > > > > > +
> > > > > > +
> > > > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > > >  -------------------------------------------
> > > > > >  Required fsl,rcpm-wakeup property should be added to a device
> > > > > > node if the device
> > > > > > --
> > > > > > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  3:47             ` Biwen Li
@ 2019-09-25  3:48               ` Leo Li
  2019-09-25  4:06                 ` Biwen Li
  0 siblings, 1 reply; 15+ messages in thread
From: Leo Li @ 2019-09-25  3:48 UTC (permalink / raw)
  To: Biwen Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree



> -----Original Message-----
> From: Biwen Li
> Sent: Tuesday, September 24, 2019 10:47 PM
> To: Leo Li <leoyang.li@nxp.com>; shawnguo@kernel.org;
> robh+dt@kernel.org; mark.rutland@arm.com; Ran Wang
> <ran.wang_1@nxp.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-
> addr' property
> 
> > > > > > >
> > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an
> > > > > > > errata
> > > > > > > A-008646 on LS1021A
> > > > > > >
> > > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > > ---
> > > > > > > Change in v3:
> > > > > > > 	- rename property name
> > > > > > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > > >
> > > > > > > Change in v2:
> > > > > > > 	- update desc of the property 'fsl,rcpm-scfg'
> > > > > > >
> > > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > > > ++++++++++++++
> > > > > > >  1 file changed, 14 insertions(+)
> > > > > > >
> > > > > > > diff --git
> > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > @@ -34,6 +34,11 @@ Chassis Version		Example
> Chips
> > > > > > >  Optional properties:
> > > > > > >   - little-endian : RCPM register block is Little Endian.
> > > > > > > Without it
> > RCPM
> > > > > > >     will be Big Endian (default case).
> > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC
> > > > > > > + LS1021A,
> > > > > >
> > > > > > You probably should mention this is related to a hardware
> > > > > > issue on LS1021a and only needed on LS1021a.
> > > > > Okay, got it, thanks, I will add this in v4.
> > > > > >
> > > > > > > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> > > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 +
> > > > > > > + 1
> > entries).
> > > > > >
> > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on
> > > > > > an
> > SoC.
> > > > > > However you are defining an offset to scfg registers here.
> > > > > > Why these two are related?  The length here should actually be
> > > > > > related to the #address-cells of the soc/.  But since this is
> > > > > > only needed for LS1021, you can
> > > > > just make it 3.
> > > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0
> > > > > device node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for
> > IPPDEXPCR1).
> > > > > But because of the hardware issue on LS1021A, I need store the
> > > > > value of IPPDEXPCR registers to an alt address. So I defining an
> > > > > offset to scfg registers, then RCPM driver get an abosolute
> > > > > address from offset, RCPM driver write the value of IPPDEXPCR
> > > > > registers to these abosolute addresses(backup the value of
> > > > > IPPDEXPCR
> > registers).
> > > >
> > > > I understand what you are trying to do.  The problem is that the
> > > > new fsl,ippdexpcr-alt-addr property contains a phandle and an offset.
> > > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/*
> > > SCFG_SPARECR8 */
> >
> > No.  The #address-cell for the soc/ is 2, so the offset to scfg should
> > be 0x0 0x51c.  The total size should be 3, but it shouldn't be coming
> > from #fsl,rcpm-wakeup-cells like you mentioned in the binding.
> Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative with #address-cells
> instead of #fsl,rcpm-wakeup-cells.

Yes.

Regards,
Leo
> >
> > > >
> > > > > >
> > > > > > > +   The first entry must be a link to the SCFG device node.
> > > > > > > +   The non-first entry must be offset of registers of SCFG.
> > > > > > >
> > > > > > >  Example:
> > > > > > >  The RCPM node for T4240:
> > > > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > > > >  		#fsl,rcpm-wakeup-cells = <2>;
> > > > > > >  	};
> > > > > > >
> > > > > > > +The RCPM node for LS1021A:
> > > > > > > +	rcpm: rcpm@1ee2140 {
> > > > > > > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-
> > > 2.1+";
> > > > > > > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > > > +		#fsl,rcpm-wakeup-cells = <2>;
> > > > > > > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > > > > > SCFG_SPARECR8 */
> > > > > > > +	};
> > > > > > > +
> > > > > > > +
> > > > > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > > > >  -------------------------------------------
> > > > > > >  Required fsl,rcpm-wakeup property should be added to a
> > > > > > > device node if the device
> > > > > > > --
> > > > > > > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  3:48               ` Leo Li
@ 2019-09-25  4:06                 ` Biwen Li
  2019-09-25  4:21                   ` Biwen Li
  0 siblings, 1 reply; 15+ messages in thread
From: Biwen Li @ 2019-09-25  4:06 UTC (permalink / raw)
  To: Leo Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree

> >
> > > > > > > >
> > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an
> > > > > > > > errata
> > > > > > > > A-008646 on LS1021A
> > > > > > > >
> > > > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > > > ---
> > > > > > > > Change in v3:
> > > > > > > > 	- rename property name
> > > > > > > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > > > >
> > > > > > > > Change in v2:
> > > > > > > > 	- update desc of the property 'fsl,rcpm-scfg'
> > > > > > > >
> > > > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > > > > ++++++++++++++
> > > > > > > >  1 file changed, 14 insertions(+)
> > > > > > > >
> > > > > > > > diff --git
> > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > @@ -34,6 +34,11 @@ Chassis Version		Example
> > Chips
> > > > > > > >  Optional properties:
> > > > > > > >   - little-endian : RCPM register block is Little Endian.
> > > > > > > > Without it
> > > RCPM
> > > > > > > >     will be Big Endian (default case).
> > > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC
> > > > > > > > + LS1021A,
> > > > > > >
> > > > > > > You probably should mention this is related to a hardware
> > > > > > > issue on LS1021a and only needed on LS1021a.
> > > > > > Okay, got it, thanks, I will add this in v4.
> > > > > > >
> > > > > > > > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such
> as:
> > > > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2
> > > > > > > > + +
> > > > > > > > + 1
> > > entries).
> > > > > > >
> > > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers
> > > > > > > on an
> > > SoC.
> > > > > > > However you are defining an offset to scfg registers here.
> > > > > > > Why these two are related?  The length here should actually
> > > > > > > be related to the #address-cells of the soc/.  But since
> > > > > > > this is only needed for LS1021, you can
> > > > > > just make it 3.
> > > > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0
> > > > > > device node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > > > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for
> > > IPPDEXPCR1).
> > > > > > But because of the hardware issue on LS1021A, I need store the
> > > > > > value of IPPDEXPCR registers to an alt address. So I defining
> > > > > > an offset to scfg registers, then RCPM driver get an abosolute
> > > > > > address from offset, RCPM driver write the value of IPPDEXPCR
> > > > > > registers to these abosolute addresses(backup the value of
> > > > > > IPPDEXPCR
> > > registers).
> > > > >
> > > > > I understand what you are trying to do.  The problem is that the
> > > > > new fsl,ippdexpcr-alt-addr property contains a phandle and an offset.
> > > > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > > > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/*
> > > > SCFG_SPARECR8 */
> > >
> > > No.  The #address-cell for the soc/ is 2, so the offset to scfg
> > > should be 0x0 0x51c.  The total size should be 3, but it shouldn't
> > > be coming from #fsl,rcpm-wakeup-cells like you mentioned in the binding.
> > Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative with
> > #address-cells instead of #fsl,rcpm-wakeup-cells.
> 
> Yes.
I got an example from drivers/pci/controller/dwc/pci-layerscape.c
and arch/arm/boot/dts/ls1021a.dtsi as follows:
fsl,pcie-scfg = <&scfg 0>, 0 is an index

In my fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>,
It means that 0x0 is an alt offset address for IPPDEXPCR0, 0x51c is an alt offset address
For IPPDEXPCR1 instead of 0x0 and 0x51c compose to an alt address of SCFG_SPARECR8.
> 
> Regards,
> Leo
> > >
> > > > >
> > > > > > >
> > > > > > > > +   The first entry must be a link to the SCFG device node.
> > > > > > > > +   The non-first entry must be offset of registers of SCFG.
> > > > > > > >
> > > > > > > >  Example:
> > > > > > > >  The RCPM node for T4240:
> > > > > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > > > > >  		#fsl,rcpm-wakeup-cells = <2>;
> > > > > > > >  	};
> > > > > > > >
> > > > > > > > +The RCPM node for LS1021A:
> > > > > > > > +	rcpm: rcpm@1ee2140 {
> > > > > > > > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-
> > > > 2.1+";
> > > > > > > > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > > > > +		#fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > > > > > > SCFG_SPARECR8 */
> > > > > > > > +	};
> > > > > > > > +
> > > > > > > > +
> > > > > > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > > > > >  -------------------------------------------
> > > > > > > >  Required fsl,rcpm-wakeup property should be added to a
> > > > > > > > device node if the device
> > > > > > > > --
> > > > > > > > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  4:06                 ` Biwen Li
@ 2019-09-25  4:21                   ` Biwen Li
  2019-09-25 14:07                     ` Li Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Biwen Li @ 2019-09-25  4:21 UTC (permalink / raw)
  To: Leo Li, shawnguo, robh+dt, mark.rutland, Ran Wang
  Cc: linuxppc-dev, linux-arm-kernel, linux-kernel, devicetree

> > >
> > > > > > > > >
> > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle
> > > > > > > > > an errata
> > > > > > > > > A-008646 on LS1021A
> > > > > > > > >
> > > > > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > > > > ---
> > > > > > > > > Change in v3:
> > > > > > > > > 	- rename property name
> > > > > > > > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > > > > >
> > > > > > > > > Change in v2:
> > > > > > > > > 	- update desc of the property 'fsl,rcpm-scfg'
> > > > > > > > >
> > > > > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > > > > > ++++++++++++++
> > > > > > > > >  1 file changed, 14 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git
> > > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > @@ -34,6 +34,11 @@ Chassis Version		Example
> > > Chips
> > > > > > > > >  Optional properties:
> > > > > > > > >   - little-endian : RCPM register block is Little Endian.
> > > > > > > > > Without it
> > > > RCPM
> > > > > > > > >     will be Big Endian (default case).
> > > > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for
> > > > > > > > > + SoC LS1021A,
> > > > > > > >
> > > > > > > > You probably should mention this is related to a hardware
> > > > > > > > issue on LS1021a and only needed on LS1021a.
> > > > > > > Okay, got it, thanks, I will add this in v4.
> > > > > > > >
> > > > > > > > > +   Must include n + 1 entries (n =
> > > > > > > > > + #fsl,rcpm-wakeup-cells, such
> > as:
> > > > > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include
> > > > > > > > > + 2
> > > > > > > > > + +
> > > > > > > > > + 1
> > > > entries).
> > > > > > > >
> > > > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR
> > > > > > > > registers on an
> > > > SoC.
> > > > > > > > However you are defining an offset to scfg registers here.
> > > > > > > > Why these two are related?  The length here should
> > > > > > > > actually be related to the #address-cells of the soc/.
> > > > > > > > But since this is only needed for LS1021, you can
> > > > > > > just make it 3.
> > > > > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0
> > > > > > > device node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > > > > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for
> > > > IPPDEXPCR1).
> > > > > > > But because of the hardware issue on LS1021A, I need store
> > > > > > > the value of IPPDEXPCR registers to an alt address. So I
> > > > > > > defining an offset to scfg registers, then RCPM driver get
> > > > > > > an abosolute address from offset, RCPM driver write the
> > > > > > > value of IPPDEXPCR registers to these abosolute
> > > > > > > addresses(backup the value of IPPDEXPCR
> > > > registers).
> > > > > >
> > > > > > I understand what you are trying to do.  The problem is that
> > > > > > the new fsl,ippdexpcr-alt-addr property contains a phandle and an
> offset.
> > > > > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > > > > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/*
> > > > > SCFG_SPARECR8 */
> > > >
> > > > No.  The #address-cell for the soc/ is 2, so the offset to scfg
> > > > should be 0x0 0x51c.  The total size should be 3, but it shouldn't
> > > > be coming from #fsl,rcpm-wakeup-cells like you mentioned in the
> binding.
> > > Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative with
> > > #address-cells instead of #fsl,rcpm-wakeup-cells.
> >
> > Yes.
> I got an example from drivers/pci/controller/dwc/pci-layerscape.c
> and arch/arm/boot/dts/ls1021a.dtsi as follows:
> fsl,pcie-scfg = <&scfg 0>, 0 is an index
> 
> In my fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>, It means that 0x0 is an alt
> offset address for IPPDEXPCR0, 0x51c is an alt offset address For
> IPPDEXPCR1 instead of 0x0 and 0x51c compose to an alt address of
> SCFG_SPARECR8.
Maybe I need write it as:
fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x0 0x0 0x51c>;
first two 0x0 compose an alt offset address for IPPDEXPCR0,
last 0x0 and 0x51c compose an alt address for IPPDEXPCR1,

Best Regards,
Biwen Li 
> >
> > Regards,
> > Leo
> > > >
> > > > > >
> > > > > > > >
> > > > > > > > > +   The first entry must be a link to the SCFG device node.
> > > > > > > > > +   The non-first entry must be offset of registers of SCFG.
> > > > > > > > >
> > > > > > > > >  Example:
> > > > > > > > >  The RCPM node for T4240:
> > > > > > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > > > > > >  		#fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > >  	};
> > > > > > > > >
> > > > > > > > > +The RCPM node for LS1021A:
> > > > > > > > > +	rcpm: rcpm@1ee2140 {
> > > > > > > > > +		compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-
> > > > > 2.1+";
> > > > > > > > > +		reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > > > > > +		#fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > > +		fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > > > > > > > SCFG_SPARECR8 */
> > > > > > > > > +	};
> > > > > > > > > +
> > > > > > > > > +
> > > > > > > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > > > > > >  -------------------------------------------
> > > > > > > > >  Required fsl,rcpm-wakeup property should be added to a
> > > > > > > > > device node if the device
> > > > > > > > > --
> > > > > > > > > 2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  4:21                   ` Biwen Li
@ 2019-09-25 14:07                     ` Li Yang
  2019-09-26  1:54                       ` [EXT] " Biwen Li
  0 siblings, 1 reply; 15+ messages in thread
From: Li Yang @ 2019-09-25 14:07 UTC (permalink / raw)
  To: Biwen Li
  Cc: shawnguo, robh+dt, mark.rutland, Ran Wang, devicetree,
	linuxppc-dev, linux-kernel, linux-arm-kernel

On Tue, Sep 24, 2019 at 11:27 PM Biwen Li <biwen.li@nxp.com> wrote:
>
> > > >
> > > > > > > > > >
> > > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle
> > > > > > > > > > an errata
> > > > > > > > > > A-008646 on LS1021A
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > > > > > ---
> > > > > > > > > > Change in v3:
> > > > > > > > > >       - rename property name
> > > > > > > > > >         fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > > > > > >
> > > > > > > > > > Change in v2:
> > > > > > > > > >       - update desc of the property 'fsl,rcpm-scfg'
> > > > > > > > > >
> > > > > > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > > > > > > ++++++++++++++
> > > > > > > > > >  1 file changed, 14 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git
> > > > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > @@ -34,6 +34,11 @@ Chassis Version            Example
> > > > Chips
> > > > > > > > > >  Optional properties:
> > > > > > > > > >   - little-endian : RCPM register block is Little Endian.
> > > > > > > > > > Without it
> > > > > RCPM
> > > > > > > > > >     will be Big Endian (default case).
> > > > > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for
> > > > > > > > > > + SoC LS1021A,
> > > > > > > > >
> > > > > > > > > You probably should mention this is related to a hardware
> > > > > > > > > issue on LS1021a and only needed on LS1021a.
> > > > > > > > Okay, got it, thanks, I will add this in v4.
> > > > > > > > >
> > > > > > > > > > +   Must include n + 1 entries (n =
> > > > > > > > > > + #fsl,rcpm-wakeup-cells, such
> > > as:
> > > > > > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include
> > > > > > > > > > + 2
> > > > > > > > > > + +
> > > > > > > > > > + 1
> > > > > entries).
> > > > > > > > >
> > > > > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR
> > > > > > > > > registers on an
> > > > > SoC.
> > > > > > > > > However you are defining an offset to scfg registers here.
> > > > > > > > > Why these two are related?  The length here should
> > > > > > > > > actually be related to the #address-cells of the soc/.
> > > > > > > > > But since this is only needed for LS1021, you can
> > > > > > > > just make it 3.
> > > > > > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0
> > > > > > > > device node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > > > > > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for
> > > > > IPPDEXPCR1).
> > > > > > > > But because of the hardware issue on LS1021A, I need store
> > > > > > > > the value of IPPDEXPCR registers to an alt address. So I
> > > > > > > > defining an offset to scfg registers, then RCPM driver get
> > > > > > > > an abosolute address from offset, RCPM driver write the
> > > > > > > > value of IPPDEXPCR registers to these abosolute
> > > > > > > > addresses(backup the value of IPPDEXPCR
> > > > > registers).
> > > > > > >
> > > > > > > I understand what you are trying to do.  The problem is that
> > > > > > > the new fsl,ippdexpcr-alt-addr property contains a phandle and an
> > offset.
> > > > > > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > > > > > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/*
> > > > > > SCFG_SPARECR8 */
> > > > >
> > > > > No.  The #address-cell for the soc/ is 2, so the offset to scfg
> > > > > should be 0x0 0x51c.  The total size should be 3, but it shouldn't
> > > > > be coming from #fsl,rcpm-wakeup-cells like you mentioned in the
> > binding.
> > > > Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative with
> > > > #address-cells instead of #fsl,rcpm-wakeup-cells.
> > >
> > > Yes.
> > I got an example from drivers/pci/controller/dwc/pci-layerscape.c
> > and arch/arm/boot/dts/ls1021a.dtsi as follows:
> > fsl,pcie-scfg = <&scfg 0>, 0 is an index
> >
> > In my fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>, It means that 0x0 is an alt
> > offset address for IPPDEXPCR0, 0x51c is an alt offset address For
> > IPPDEXPCR1 instead of 0x0 and 0x51c compose to an alt address of
> > SCFG_SPARECR8.
> Maybe I need write it as:
> fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x0 0x0 0x51c>;
> first two 0x0 compose an alt offset address for IPPDEXPCR0,
> last 0x0 and 0x51c compose an alt address for IPPDEXPCR1,

I remember the hardware issue is only is only related to IPPDEXPCR1
register, no idea why you need to define IPPDEXPCR0 in the binding.

>
> Best Regards,
> Biwen Li
> > >
> > > Regards,
> > > Leo
> > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > > +   The first entry must be a link to the SCFG device node.
> > > > > > > > > > +   The non-first entry must be offset of registers of SCFG.
> > > > > > > > > >
> > > > > > > > > >  Example:
> > > > > > > > > >  The RCPM node for T4240:
> > > > > > > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > > > > > > >               #fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > > >       };
> > > > > > > > > >
> > > > > > > > > > +The RCPM node for LS1021A:
> > > > > > > > > > +     rcpm: rcpm@1ee2140 {
> > > > > > > > > > +             compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-
> > > > > > 2.1+";
> > > > > > > > > > +             reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > > > > > > +             #fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > > > +             fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>; /*
> > > > > > > > > > SCFG_SPARECR8 */
> > > > > > > > > > +     };
> > > > > > > > > > +
> > > > > > > > > > +
> > > > > > > > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > > > > > > >  -------------------------------------------
> > > > > > > > > >  Required fsl,rcpm-wakeup property should be added to a
> > > > > > > > > > device node if the device
> > > > > > > > > > --
> > > > > > > > > > 2.17.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [EXT] Re: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25 14:07                     ` Li Yang
@ 2019-09-26  1:54                       ` Biwen Li
  0 siblings, 0 replies; 15+ messages in thread
From: Biwen Li @ 2019-09-26  1:54 UTC (permalink / raw)
  To: Leo Li
  Cc: shawnguo, robh+dt, mark.rutland, Ran Wang, devicetree,
	linuxppc-dev, linux-kernel, linux-arm-kernel

> Caution: EXT Email
> 
> On Tue, Sep 24, 2019 at 11:27 PM Biwen Li <biwen.li@nxp.com> wrote:
> >
> > > > >
> > > > > > > > > > >
> > > > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to
> > > > > > > > > > > handle an errata
> > > > > > > > > > > A-008646 on LS1021A
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > > > > > > ---
> > > > > > > > > > > Change in v3:
> > > > > > > > > > >       - rename property name
> > > > > > > > > > >         fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > > > > > > >
> > > > > > > > > > > Change in v2:
> > > > > > > > > > >       - update desc of the property 'fsl,rcpm-scfg'
> > > > > > > > > > >
> > > > > > > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > | 14
> > > > > > > > > > > ++++++++++++++
> > > > > > > > > > >  1 file changed, 14 insertions(+)
> > > > > > > > > > >
> > > > > > > > > > > diff --git
> > > > > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > > > > > > ---
> > > > > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm
> > > > > > > > > > > +++ .txt
> > > > > > > > > > > @@ -34,6 +34,11 @@ Chassis Version            Example
> > > > > Chips
> > > > > > > > > > >  Optional properties:
> > > > > > > > > > >   - little-endian : RCPM register block is Little Endian.
> > > > > > > > > > > Without it
> > > > > > RCPM
> > > > > > > > > > >     will be Big Endian (default case).
> > > > > > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property
> > > > > > > > > > > + for SoC LS1021A,
> > > > > > > > > >
> > > > > > > > > > You probably should mention this is related to a
> > > > > > > > > > hardware issue on LS1021a and only needed on LS1021a.
> > > > > > > > > Okay, got it, thanks, I will add this in v4.
> > > > > > > > > >
> > > > > > > > > > > +   Must include n + 1 entries (n =
> > > > > > > > > > > + #fsl,rcpm-wakeup-cells, such
> > > > as:
> > > > > > > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must
> > > > > > > > > > > + include
> > > > > > > > > > > + 2
> > > > > > > > > > > + +
> > > > > > > > > > > + 1
> > > > > > entries).
> > > > > > > > > >
> > > > > > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR
> > > > > > > > > > registers on an
> > > > > > SoC.
> > > > > > > > > > However you are defining an offset to scfg registers here.
> > > > > > > > > > Why these two are related?  The length here should
> > > > > > > > > > actually be related to the #address-cells of the soc/.
> > > > > > > > > > But since this is only needed for LS1021, you can
> > > > > > > > > just make it 3.
> > > > > > > > > I need set the value of IPPDEXPCR resgiters from
> > > > > > > > > ftm_alarm0 device node(fsl,rcpm-wakeup = <&rcpm 0x0
> > > > > > > > > 0x20000000>;
> > > > > > > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for
> > > > > > IPPDEXPCR1).
> > > > > > > > > But because of the hardware issue on LS1021A, I need
> > > > > > > > > store the value of IPPDEXPCR registers to an alt
> > > > > > > > > address. So I defining an offset to scfg registers, then
> > > > > > > > > RCPM driver get an abosolute address from offset, RCPM
> > > > > > > > > driver write the value of IPPDEXPCR registers to these
> > > > > > > > > abosolute addresses(backup the value of IPPDEXPCR
> > > > > > registers).
> > > > > > > >
> > > > > > > > I understand what you are trying to do.  The problem is
> > > > > > > > that the new fsl,ippdexpcr-alt-addr property contains a
> > > > > > > > phandle and an
> > > offset.
> > > > > > > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > > > > > > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg
> > > > > > > 0x51c>;/*
> > > > > > > SCFG_SPARECR8 */
> > > > > >
> > > > > > No.  The #address-cell for the soc/ is 2, so the offset to
> > > > > > scfg should be 0x0 0x51c.  The total size should be 3, but it
> > > > > > shouldn't be coming from #fsl,rcpm-wakeup-cells like you
> > > > > > mentioned in the
> > > binding.
> > > > > Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative
> > > > > with #address-cells instead of #fsl,rcpm-wakeup-cells.
> > > >
> > > > Yes.
> > > I got an example from drivers/pci/controller/dwc/pci-layerscape.c
> > > and arch/arm/boot/dts/ls1021a.dtsi as follows:
> > > fsl,pcie-scfg = <&scfg 0>, 0 is an index
> > >
> > > In my fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>, It means that 0x0
> > > is an alt offset address for IPPDEXPCR0, 0x51c is an alt offset
> > > address For
> > > IPPDEXPCR1 instead of 0x0 and 0x51c compose to an alt address of
> > > SCFG_SPARECR8.
> > Maybe I need write it as:
> > fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x0 0x0 0x51c>; first two 0x0
> > compose an alt offset address for IPPDEXPCR0, last 0x0 and 0x51c
> > compose an alt address for IPPDEXPCR1,
> 
> I remember the hardware issue is only is only related to IPPDEXPCR1 register, no
> idea why you need to define IPPDEXPCR0 in the binding.
Okay, got it, thanks.
Best Regards,
Biwen Li
> 
> >
> > Best Regards,
> > Biwen Li
> > > >
> > > > Regards,
> > > > Leo
> > > > > >
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > +   The first entry must be a link to the SCFG device node.
> > > > > > > > > > > +   The non-first entry must be offset of registers of SCFG.
> > > > > > > > > > >
> > > > > > > > > > >  Example:
> > > > > > > > > > >  The RCPM node for T4240:
> > > > > > > > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > > > > > > > >               #fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > > > >       };
> > > > > > > > > > >
> > > > > > > > > > > +The RCPM node for LS1021A:
> > > > > > > > > > > +     rcpm: rcpm@1ee2140 {
> > > > > > > > > > > +             compatible = "fsl,ls1021a-rcpm",
> > > > > > > > > > > +"fsl,qoriq-rcpm-
> > > > > > > 2.1+";
> > > > > > > > > > > +             reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > > > > > > > +             #fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > > > > +             fsl,ippdexpcr-alt-addr = <&scfg 0x0
> > > > > > > > > > > + 0x51c>; /*
> > > > > > > > > > > SCFG_SPARECR8 */
> > > > > > > > > > > +     };
> > > > > > > > > > > +
> > > > > > > > > > > +
> > > > > > > > > > >  * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > > > > > > > >  -------------------------------------------
> > > > > > > > > > >  Required fsl,rcpm-wakeup property should be added
> > > > > > > > > > > to a device node if the device
> > > > > > > > > > > --
> > > > > > > > > > > 2.17.1
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
  2019-09-25  3:34           ` Leo Li
  2019-09-25  3:47             ` Biwen Li
@ 2019-09-27 18:28             ` Rob Herring
  1 sibling, 0 replies; 15+ messages in thread
From: Rob Herring @ 2019-09-27 18:28 UTC (permalink / raw)
  To: Leo Li
  Cc: Biwen Li, shawnguo, mark.rutland, Ran Wang, linuxppc-dev,
	linux-arm-kernel, linux-kernel, devicetree

On Wed, Sep 25, 2019 at 03:34:47AM +0000, Leo Li wrote:
> 
> 
> > -----Original Message-----
> > From: Biwen Li
> > Sent: Tuesday, September 24, 2019 10:30 PM
> > To: Leo Li <leoyang.li@nxp.com>; shawnguo@kernel.org;
> > robh+dt@kernel.org; mark.rutland@arm.com; Ran Wang
> > <ran.wang_1@nxp.com>
> > Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: RE: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-
> > addr' property
> > 
> > > > > >
> > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an
> > > > > > errata
> > > > > > A-008646 on LS1021A
> > > > > >
> > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > ---
> > > > > > Change in v3:
> > > > > > 	- rename property name
> > > > > > 	  fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > >
> > > > > > Change in v2:
> > > > > > 	- update desc of the property 'fsl,rcpm-scfg'
> > > > > >
> > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > > ++++++++++++++
> > > > > >  1 file changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > @@ -34,6 +34,11 @@ Chassis Version		Example Chips
> > > > > >  Optional properties:
> > > > > >   - little-endian : RCPM register block is Little Endian. Without it RCPM
> > > > > >     will be Big Endian (default case).
> > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for SoC
> > > > > > + LS1021A,
> > > > >
> > > > > You probably should mention this is related to a hardware issue on
> > > > > LS1021a and only needed on LS1021a.
> > > > Okay, got it, thanks, I will add this in v4.
> > > > >
> > > > > > +   Must include n + 1 entries (n = #fsl,rcpm-wakeup-cells, such as:
> > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include 2 + 1 entries).
> > > > >
> > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR registers on an SoC.
> > > > > However you are defining an offset to scfg registers here.  Why
> > > > > these two are related?  The length here should actually be related
> > > > > to the #address-cells of the soc/.  But since this is only needed
> > > > > for LS1021, you can
> > > > just make it 3.
> > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0 device
> > > > node(fsl,rcpm-wakeup = <&rcpm 0x0 0x20000000>;
> > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for IPPDEXPCR1).
> > > > But because of the hardware issue on LS1021A, I need store the value
> > > > of IPPDEXPCR registers to an alt address. So I defining an offset to
> > > > scfg registers, then RCPM driver get an abosolute address from
> > > > offset, RCPM driver write the value of IPPDEXPCR registers to these
> > > > abosolute addresses(backup the value of IPPDEXPCR registers).
> > >
> > > I understand what you are trying to do.  The problem is that the new
> > > fsl,ippdexpcr-alt-addr property contains a phandle and an offset.  The
> > > size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg 0x51c>;/*
> > SCFG_SPARECR8 */
> 
> No.  The #address-cell for the soc/ is 2, so the offset to scfg 
> should be 0x0 0x51c.  The total size should be 3, but it shouldn't be 
> coming from #fsl,rcpm-wakeup-cells like you mentioned in the binding.

Um, no. #address-cells applies to reg and ranges, not some vendor 
specific property. You can just define how many cells you need if that's 
fixed. 

However, I suggested doing this another way in the next version of this.

Rob

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-09-27 18:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-24  2:45 [v3,1/3] soc: fsl: handle RCPM errata A-008646 on SoC LS1021A Biwen Li
2019-09-24  2:45 ` [v3,2/3] arm: dts: ls1021a: fix that FlexTimer cannot wakeup system in deep sleep Biwen Li
2019-09-24  2:45 ` [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property Biwen Li
2019-09-24 15:58   ` Leo Li
2019-09-25  3:13     ` Biwen Li
2019-09-25  3:26       ` Leo Li
2019-09-25  3:30         ` Biwen Li
2019-09-25  3:34           ` Leo Li
2019-09-25  3:47             ` Biwen Li
2019-09-25  3:48               ` Leo Li
2019-09-25  4:06                 ` Biwen Li
2019-09-25  4:21                   ` Biwen Li
2019-09-25 14:07                     ` Li Yang
2019-09-26  1:54                       ` [EXT] " Biwen Li
2019-09-27 18:28             ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).