[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@nxp.com>

Sparse complains when a unit address is provided without a
corresponding 'reg' field.
   
Fix these build warnings by removing the unneeded unit-addresses of
the anatop regulators.

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/boot/dts/imx6sl.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx6sx.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx6ul.dtsi |  6 +++---
 arch/arm/boot/dts/imx7d.dtsi  |  2 +-
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index d12b250..224c0d0 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -461,7 +461,7 @@
 					     <0 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 127 IRQ_TYPE_LEVEL_HIGH>;
 
-				regulator-1p1 at 110 {
+				regulator-1p1 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p1";
 					regulator-min-microvolt = <800000>;
@@ -475,7 +475,7 @@
 					anatop-max-voltage = <1375000>;
 				};
 
-				regulator-3p0 at 120 {
+				regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2800000>;
@@ -489,7 +489,7 @@
 					anatop-max-voltage = <3400000>;
 				};
 
-				regulator-2p5 at 130 {
+				regulator-2p5 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd2p5";
 					regulator-min-microvolt = <2100000>;
@@ -503,7 +503,7 @@
 					anatop-max-voltage = <2850000>;
 				};
 
-				reg_arm: regulator-vddcore at 140 {
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddarm";
 					regulator-min-microvolt = <725000>;
@@ -520,7 +520,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_pu: regulator-vddpu at 140 {
+				reg_pu: regulator-vddpu {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddpu";
 					regulator-min-microvolt = <725000>;
@@ -537,7 +537,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc at 140 {
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 6a993bfda..2863c52 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -547,7 +547,7 @@
 					     <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
 
-				regulator-1p1 at 110 {
+				regulator-1p1 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p1";
 					regulator-min-microvolt = <800000>;
@@ -561,7 +561,7 @@
 					anatop-max-voltage = <1375000>;
 				};
 
-				regulator-3p0 at 120 {
+				regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2800000>;
@@ -575,7 +575,7 @@
 					anatop-max-voltage = <3400000>;
 				};
 
-				regulator-2p5 at 130 {
+				regulator-2p5 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd2p5";
 					regulator-min-microvolt = <2100000>;
@@ -589,7 +589,7 @@
 					anatop-max-voltage = <2875000>;
 				};
 
-				reg_arm: regulator-vddcore at 140 {
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddarm";
 					regulator-min-microvolt = <725000>;
@@ -606,7 +606,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_pcie: regulator-vddpcie at 140 {
+				reg_pcie: regulator-vddpcie {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddpcie";
 					regulator-min-microvolt = <725000>;
@@ -622,7 +622,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc at 140 {
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 4356b65..b15ed37 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -512,7 +512,7 @@
 					     <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
 
-				reg_3p0: regulator-3p0 at 120 {
+				reg_3p0: regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2625000>;
@@ -526,7 +526,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				reg_arm: regulator-vddcore at 140 {
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "cpu";
 					regulator-min-microvolt = <725000>;
@@ -543,7 +543,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc at 140 {
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 6b3faa2..d70d55d 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -522,7 +522,7 @@
 				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
 
-				reg_1p0d: regulator-vdd1p0d at 210 {
+				reg_1p0d: regulator-vdd1p0d {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p0d";
 					regulator-min-microvolt = <800000>;
-- 
1.9.1

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Shawn Guo]

On Sun, Jun 12, 2016 at 11:15:43AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Sparse complains when a unit address is provided without a
> corresponding 'reg' field.
>    
> Fix these build warnings by removing the unneeded unit-addresses of
> the anatop regulators.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

Is there any changes from this one [1], which I already applied?

Shawn

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/497467/

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Fabio Estevam]

On Wed, Jun 15, 2016 at 10:29 PM, Shawn Guo <shawnguo@kernel.org> wrote:

> Is there any changes from this one [1], which I already applied?

Ops, I haven't noticed it was already applied. Thanks

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Shawn Guo]

On Tue, May 03, 2016 at 10:57:31AM -0300, Fabio Estevam wrote:
> Building with W=1 option leads to several warnings like:
> 
> Warning (unit_address_vs_reg): Node /soc/aips-bus at 02000000/anatop at 020c8000/regulator-1p1 at 110 has a unit name, but no reg property
> 
> Fix them by removing the unneeded unit-addresses.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

Applied, thanks.

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Rob Herring]

On Fri, May 6, 2016 at 2:19 PM, Joshua Clayton <stillcompiling@gmail.com> wrote:
> On Tue, 3 May 2016 12:45:07 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
>
>> On Tue, May 3, 2016 at 12:42 PM, Fabio Estevam <festevam@gmail.com>
>> wrote:
>> > On Tue, May 3, 2016 at 12:37 PM, Rob Herring <robh@kernel.org>
>> > wrote:
>> >> Presumably these are register offsets which could be useful to have
>> >> even if the Linux driver doesn't use them. But either way is fine
>> >> with me.
>> >
>> > Yes, these are register offsets.
>> >
>> >                 reg_3p0: regulator-3p0 {
>> >                     compatible = "fsl,anatop-regulator";
>> >                     regulator-name = "vdd3p0";
>> >                     regulator-min-microvolt = <2625000>;
>> >                     regulator-max-microvolt = <3400000>;
>> >                     anatop-reg-offset = <0x120>;
>>
>> Sorry, pressed the 'send' button too quickly.
>>
>> There is a "anatop-reg-offset = <0x120>;" property that is used to
>> pass the offset to the anatop regulator driver, so that's why I
>> preferred to remove the unneeded unit-address.
>>
>> Thanks
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
> There are actually 2 registers (on some of them),
> anatop-reg-offset, and anatop-delay-reg-offset...
> Converting those to use reg and deprecating the anatop specific
> items would seem like a Good Thing (tm) to me.
>
> ...wandering into less safe territory:
> I know reg is usually a memory offset in bytes, but
> each of the anatop memory offsets also include a bit-shift
> and a bit-width item, which really narrowly defines the
> register bits that pertain to each regulator.
>
> would it be totally inappropriate to use
> #address-cells = 2 and
> #size-cells =1
> where address cells would contain the byte offset and the bit shift, and
> the size cell would contain the bit offset?

Technically that is fine. However, then the standard address
translation functions will not work.

Given it is also encoding bit offsets, I'd just keep this patch as is.
We could also debate just not putting this in the DT at all.

Acked-by: Rob Herring <robh@kernel.org>

Rob

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Joshua Clayton]

On Tue, 3 May 2016 12:45:07 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> On Tue, May 3, 2016 at 12:42 PM, Fabio Estevam <festevam@gmail.com>
> wrote:
> > On Tue, May 3, 2016 at 12:37 PM, Rob Herring <robh@kernel.org>
> > wrote: 
> >> Presumably these are register offsets which could be useful to have
> >> even if the Linux driver doesn't use them. But either way is fine
> >> with me.  
> >
> > Yes, these are register offsets.
> >
> >                 reg_3p0: regulator-3p0 {
> >                     compatible = "fsl,anatop-regulator";
> >                     regulator-name = "vdd3p0";
> >                     regulator-min-microvolt = <2625000>;
> >                     regulator-max-microvolt = <3400000>;
> >                     anatop-reg-offset = <0x120>;  
> 
> Sorry, pressed the 'send' button too quickly.
> 
> There is a "anatop-reg-offset = <0x120>;" property that is used to
> pass the offset to the anatop regulator driver, so that's why I
> preferred to remove the unneeded unit-address.
> 
> Thanks
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

There are actually 2 registers (on some of them),
anatop-reg-offset, and anatop-delay-reg-offset...
Converting those to use reg and deprecating the anatop specific
items would seem like a Good Thing (tm) to me.

...wandering into less safe territory:
I know reg is usually a memory offset in bytes, but
each of the anatop memory offsets also include a bit-shift
and a bit-width item, which really narrowly defines the 
register bits that pertain to each regulator.

would it be totally inappropriate to use
#address-cells = 2 and
#size-cells =1 
where address cells would contain the byte offset and the bit shift, and
the size cell would contain the bit offset?

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Fabio Estevam]

On Tue, May 3, 2016 at 12:42 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Tue, May 3, 2016 at 12:37 PM, Rob Herring <robh@kernel.org> wrote:
>
>> Presumably these are register offsets which could be useful to have
>> even if the Linux driver doesn't use them. But either way is fine with
>> me.
>
> Yes, these are register offsets.
>
>                 reg_3p0: regulator-3p0 {
>                     compatible = "fsl,anatop-regulator";
>                     regulator-name = "vdd3p0";
>                     regulator-min-microvolt = <2625000>;
>                     regulator-max-microvolt = <3400000>;
>                     anatop-reg-offset = <0x120>;

Sorry, pressed the 'send' button too quickly.

There is a "anatop-reg-offset = <0x120>;" property that is used to
pass the offset to the anatop regulator driver, so that's why I
preferred to remove the unneeded unit-address.

Thanks

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Fabio Estevam]

On Tue, May 3, 2016 at 12:37 PM, Rob Herring <robh@kernel.org> wrote:

> Presumably these are register offsets which could be useful to have
> even if the Linux driver doesn't use them. But either way is fine with
> me.

Yes, these are register offsets.

                reg_3p0: regulator-3p0 {
                    compatible = "fsl,anatop-regulator";
                    regulator-name = "vdd3p0";
                    regulator-min-microvolt = <2625000>;
                    regulator-max-microvolt = <3400000>;
                    anatop-reg-offset = <0x120>;

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Rob Herring]

On Tue, May 3, 2016 at 8:57 AM, Fabio Estevam <fabio.estevam@nxp.com> wrote:
> Building with W=1 option leads to several warnings like:
>
> Warning (unit_address_vs_reg): Node /soc/aips-bus at 02000000/anatop at 020c8000/regulator-1p1 at 110 has a unit name, but no reg property
>
> Fix them by removing the unneeded unit-addresses.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  arch/arm/boot/dts/imx6sl.dtsi | 12 ++++++------
>  arch/arm/boot/dts/imx6sx.dtsi | 12 ++++++------
>  arch/arm/boot/dts/imx6ul.dtsi |  6 +++---
>  arch/arm/boot/dts/imx7d.dtsi  |  2 +-
>  4 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> index d12b250..224c0d0 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -461,7 +461,7 @@
>                                              <0 54 IRQ_TYPE_LEVEL_HIGH>,
>                                              <0 127 IRQ_TYPE_LEVEL_HIGH>;
>
> -                               regulator-1p1 at 110 {
> +                               regulator-1p1 {

Presumably these are register offsets which could be useful to have
even if the Linux driver doesn't use them. But either way is fine with
me.

Rob

[PATCH] ARM: dts: imx: Remove unneeded unit-addresses

[Fabio Estevam]

Building with W=1 option leads to several warnings like:

Warning (unit_address_vs_reg): Node /soc/aips-bus at 02000000/anatop at 020c8000/regulator-1p1 at 110 has a unit name, but no reg property

Fix them by removing the unneeded unit-addresses.

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/boot/dts/imx6sl.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx6sx.dtsi | 12 ++++++------
 arch/arm/boot/dts/imx6ul.dtsi |  6 +++---
 arch/arm/boot/dts/imx7d.dtsi  |  2 +-
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index d12b250..224c0d0 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -461,7 +461,7 @@
 					     <0 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <0 127 IRQ_TYPE_LEVEL_HIGH>;
 
-				regulator-1p1 at 110 {
+				regulator-1p1 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p1";
 					regulator-min-microvolt = <800000>;
@@ -475,7 +475,7 @@
 					anatop-max-voltage = <1375000>;
 				};
 
-				regulator-3p0 at 120 {
+				regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2800000>;
@@ -489,7 +489,7 @@
 					anatop-max-voltage = <3400000>;
 				};
 
-				regulator-2p5 at 130 {
+				regulator-2p5 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd2p5";
 					regulator-min-microvolt = <2100000>;
@@ -503,7 +503,7 @@
 					anatop-max-voltage = <2850000>;
 				};
 
-				reg_arm: regulator-vddcore at 140 {
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddarm";
 					regulator-min-microvolt = <725000>;
@@ -520,7 +520,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_pu: regulator-vddpu at 140 {
+				reg_pu: regulator-vddpu {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddpu";
 					regulator-min-microvolt = <725000>;
@@ -537,7 +537,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc at 140 {
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 6a993bfda..2863c52 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -547,7 +547,7 @@
 					     <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
 
-				regulator-1p1 at 110 {
+				regulator-1p1 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p1";
 					regulator-min-microvolt = <800000>;
@@ -561,7 +561,7 @@
 					anatop-max-voltage = <1375000>;
 				};
 
-				regulator-3p0 at 120 {
+				regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2800000>;
@@ -575,7 +575,7 @@
 					anatop-max-voltage = <3400000>;
 				};
 
-				regulator-2p5 at 130 {
+				regulator-2p5 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd2p5";
 					regulator-min-microvolt = <2100000>;
@@ -589,7 +589,7 @@
 					anatop-max-voltage = <2875000>;
 				};
 
-				reg_arm: regulator-vddcore at 140 {
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddarm";
 					regulator-min-microvolt = <725000>;
@@ -606,7 +606,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_pcie: regulator-vddpcie at 140 {
+				reg_pcie: regulator-vddpcie {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddpcie";
 					regulator-min-microvolt = <725000>;
@@ -622,7 +622,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc at 140 {
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 4356b65..b15ed37 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -512,7 +512,7 @@
 					     <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
 
-				reg_3p0: regulator-3p0 at 120 {
+				reg_3p0: regulator-3p0 {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd3p0";
 					regulator-min-microvolt = <2625000>;
@@ -526,7 +526,7 @@
 					anatop-enable-bit = <0>;
 				};
 
-				reg_arm: regulator-vddcore at 140 {
+				reg_arm: regulator-vddcore {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "cpu";
 					regulator-min-microvolt = <725000>;
@@ -543,7 +543,7 @@
 					anatop-max-voltage = <1450000>;
 				};
 
-				reg_soc: regulator-vddsoc at 140 {
+				reg_soc: regulator-vddsoc {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vddsoc";
 					regulator-min-microvolt = <725000>;
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 6b3faa2..d70d55d 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -522,7 +522,7 @@
 				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
 
-				reg_1p0d: regulator-vdd1p0d at 210 {
+				reg_1p0d: regulator-vdd1p0d {
 					compatible = "fsl,anatop-regulator";
 					regulator-name = "vdd1p0d";
 					regulator-min-microvolt = <800000>;
-- 
1.9.1