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 <[email protected]>
---
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], ®_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
The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata A-008646
on LS1021A
Signed-off-by: Biwen Li <[email protected]>
---
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
> -----Original Message-----
> From: Biwen Li <[email protected]>
> Sent: Monday, September 23, 2019 9:46 PM
> To: Leo Li <[email protected]>; [email protected];
> [email protected]; [email protected]; Ran Wang
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Biwen Li
> <[email protected]>
> 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 <[email protected]>
> ---
> 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
> >
> > The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata
> > A-008646 on LS1021A
> >
> > Signed-off-by: Biwen Li <[email protected]>
> > ---
> > 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
> >
> > > > > > > >
> > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an
> > > > > > > > errata
> > > > > > > > A-008646 on LS1021A
> > > > > > > >
> > > > > > > > Signed-off-by: Biwen Li <[email protected]>
> > > > > > > > ---
> > > > > > > > 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
> > > >
> > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an errata
> > > > A-008646 on LS1021A
> > > >
> > > > Signed-off-by: Biwen Li <[email protected]>
> > > > ---
> > > > 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
> -----Original Message-----
> From: Biwen Li
> Sent: Tuesday, September 24, 2019 10:30 PM
> To: Leo Li <[email protected]>; [email protected];
> [email protected]; [email protected]; Ran Wang
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> > > > > ---
> > > > > 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
> > > > > >
> > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle an
> > > > > > errata
> > > > > > A-008646 on LS1021A
> > > > > >
> > > > > > Signed-off-by: Biwen Li <[email protected]>
> > > > > > ---
> > > > > > 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
> -----Original Message-----
> From: Biwen Li
> Sent: Tuesday, September 24, 2019 10:47 PM
> To: Leo Li <[email protected]>; [email protected];
> [email protected]; [email protected]; Ran Wang
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> > > > > > > ---
> > > > > > > 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
> > >
> > > > > > > > >
> > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle
> > > > > > > > > an errata
> > > > > > > > > A-008646 on LS1021A
> > > > > > > > >
> > > > > > > > > Signed-off-by: Biwen Li <[email protected]>
> > > > > > > > > ---
> > > > > > > > > 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
On Tue, Sep 24, 2019 at 11:27 PM Biwen Li <[email protected]> wrote:
>
> > > >
> > > > > > > > > >
> > > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle
> > > > > > > > > > an errata
> > > > > > > > > > A-008646 on LS1021A
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Biwen Li <[email protected]>
> > > > > > > > > > ---
> > > > > > > > > > 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
>
> Caution: EXT Email
>
> On Tue, Sep 24, 2019 at 11:27 PM Biwen Li <[email protected]> wrote:
> >
> > > > >
> > > > > > > > > > >
> > > > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to
> > > > > > > > > > > handle an errata
> > > > > > > > > > > A-008646 on LS1021A
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Biwen Li <[email protected]>
> > > > > > > > > > > ---
> > > > > > > > > > > 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
> >
> -----Original Message-----
> From: Biwen Li
> Sent: Tuesday, September 24, 2019 10:13 PM
> To: Leo Li <[email protected]>; [email protected];
> [email protected]; [email protected]; Ran Wang
> <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]
> 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 <[email protected]>
> > > ---
> > > 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
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 <[email protected]>; [email protected];
> > [email protected]; [email protected]; Ran Wang
> > <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected]
> > 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 <[email protected]>
> > > > > > ---
> > > > > > 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