[PATCH 2/6] arm64: dts: renesas: r8a77965: Add #address-cells and #size-cells

[PATCH 2/6] arm64: dts: renesas: r8a77965: Add #address-cells and #size-cells

[Jacopo Mondi]

Add "#address-cells" and "#size-cells" properties to all place-holder nodes
that have children nodes defined by salvator-x[s].dtsi device tree.

This silences the following DTC compiler warnings:
Warning (reg_format): "reg" property in /soc/.. has invalid length (4 bytes)
(#address-cells == 2, #size-cells == 1)
Warning (avoid_default_addr_size): Relying on default #address-cells
value for /soc/...
Warning (avoid_default_addr_size): Relying on default #size-cells value
for /soc/...

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 4d9bba9..ab46cd2 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -521,6 +521,9 @@
 		};
 
 		avb: ethernet at e6800000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
 			reg = <0 0xe6800000 0 0x800>, <0 0xe6a00000 0 0x10000>;
 			/* placeholder */
 		};
@@ -528,11 +531,21 @@
 		csi20: csi2 at fea80000 {
 			reg = <0 0xfea80000 0 0x10000>;
 			/* placeholder */
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
 		};
 
 		csi40: csi2 at feaa0000 {
 			reg = <0 0xfeaa0000 0 0x10000>;
 			/* placeholder */
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
 		};
 
 		vin0: video at e6ef0000 {
@@ -611,6 +624,9 @@
 		};
 
 		i2c2: i2c at e6510000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
 			reg = <0 0xe6510000 0 0x40>;
 			/* placeholder */
 		};
@@ -621,6 +637,9 @@
 		};
 
 		i2c4: i2c at e66d8000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
 			reg = <0 0xe66d8000 0 0x40>;
 			/* placeholder */
 		};
@@ -636,6 +655,9 @@
 		};
 
 		i2c_dvfs: i2c at e60b0000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
 			reg = <0 0xe60b0000 0 0x425>;
 			/* placeholder */
 		};
@@ -681,6 +703,9 @@
 			/* placeholder */
 
 			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
 				port at 0 {
 					reg = <0>;
 					du_out_rgb: endpoint {
-- 
2.7.4

[PATCH 4/6] arm64: dts: renesas: r8a77965: Add #phy-cells property

[Jacopo Mondi]

Add "#phy-cells" property to "usb-phy at e65ee000" device node.

This silences the following DTC compiler warning:
Warning (phys_property): Missing property '#phy-cells' in node
/soc/usb-phy at e65ee000 or bad phandle (referred from
/soc/usb at ee020000:phys[0])

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index be4f25c..cd12176 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -795,6 +795,7 @@
 
 		usb3_phy0: usb-phy at e65ee000 {
 			reg = <0 0xe65ee000 0 0x90>;
+			#phy-cells = <0>;
 			/* placeholder */
 		};
 
-- 
2.7.4

[PATCH 1/6] arm64: dts: renesas: r8a77965: Add "reg" properties

[Geert Uytterhoeven]

Hi Jacopo,

On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add "reg" properties to place-holder nodes with unit address defined for
> R-Car M3-N SoC.
>
> This silences the following DTC compiler warning:
> Warning (unit_address_vs_reg): Node /soc/... has a unit name,
> but no reg property
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

A few minor nits below...

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi

> @@ -520,130 +521,163 @@
>                 };
>
>                 avb: ethernet at e6800000 {
> +                       reg = <0 0xe6800000 0 0x800>, <0 0xe6a00000 0 0x10000>;

The first reg part would be sufficient for a placeholder.
I think even setting the size to zero would work...

>                 du: display at feb00000 {
> +                       reg = <0 0xfeb00000 0 0x70000>,

Unlike M3-W, the M3-N DU has a third channel, so the size is too small.

> +                             <0 0xfeb90000 0 0x14>;
>                         /* placeholder */

> @@ -703,38 +745,47 @@
>                 };
>
>                 usb2_phy1: usb-phy at ee0a0200 {
> +                       reg = <0 0xee0a0200 0 0x700>;
>                         /* placeholder */
>                 };

To be moved up, next to the other usb2 devices?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 2/6] arm64: dts: renesas: r8a77965: Add #address-cells and #size-cells

[Geert Uytterhoeven]

On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add "#address-cells" and "#size-cells" properties to all place-holder nodes
> that have children nodes defined by salvator-x[s].dtsi device tree.
>
> This silences the following DTC compiler warnings:
> Warning (reg_format): "reg" property in /soc/.. has invalid length (4 bytes)
> (#address-cells == 2, #size-cells == 1)
> Warning (avoid_default_addr_size): Relying on default #address-cells
> value for /soc/...
> Warning (avoid_default_addr_size): Relying on default #size-cells value
> for /soc/...
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 3/6] arm64: dts: renesas: r8a77965: Remove stale reg property

[Geert Uytterhoeven]

On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Remove "reg" property from cache-controller-0 device node as it does not
> have any unit address.
>
> This silences the following DTC compiler warning:
> Warning (unit_address_vs_reg): Node /cpus/cache-controller-0 has a reg
> or ranges property, but no unit name
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 4/6] arm64: dts: renesas: r8a77965: Add #phy-cells property

[Geert Uytterhoeven]

On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add "#phy-cells" property to "usb-phy at e65ee000" device node.
>
> This silences the following DTC compiler warning:
> Warning (phys_property): Missing property '#phy-cells' in node
> /soc/usb-phy at e65ee000 or bad phandle (referred from
> /soc/usb at ee020000:phys[0])
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 5/6] arm64: dts: renesas: r8a77965: Add #pwm-cells property

[Geert Uytterhoeven]

On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add "#pwm-cells" property to "pwm at e6e31000" device node.
>
> This silences the following DTC compiler warning:
> Warning (pwms_property): Missing property '#pwm-cells' in node
> /soc/pwm at e6e31000 or bad phandle (referred from /backlight:pwms[0])
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 6/6] arm64: dts: renesas: r8a77965: Add #interrupt-cells property

[Geert Uytterhoeven]

On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> Add "#interrupt-cells" property and "interrupt-controller" label to
> "interrupt-controller at e61c0000" device node.
>
> This silences the following DTC compiler warnings:
> Warning (interrupts_property): Missing interrupt-controller or
> interrupt-map property in /soc/interrupt-controller at e61c0000
> Warning (interrupts_property): Missing #interrupt-cells in
> interrupt-parent /soc/interrupt-controller at e61c000
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -316,6 +316,8 @@
>                 };
>
>                 intc_ex: interrupt-controller at e61c0000 {
> +                       #interrupt-cells = <2>;
> +                       interrupt-controller;
>                         reg = <0 0xe61c0000 0 0x200>;
>                         /* placeholder */

(bikeshedding) Should the placeholder be at the top of the node?

>                 };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 6/6] arm64: dts: renesas: r8a77965: Add #interrupt-cells property

[jacopo mondi]

Hi Geert,

On Fri, Feb 23, 2018 at 10:27:16AM +0100, Geert Uytterhoeven wrote:
> On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > Add "#interrupt-cells" property and "interrupt-controller" label to
> > "interrupt-controller at e61c0000" device node.
> >
> > This silences the following DTC compiler warnings:
> > Warning (interrupts_property): Missing interrupt-controller or
> > interrupt-map property in /soc/interrupt-controller at e61c0000
> > Warning (interrupts_property): Missing #interrupt-cells in
> > interrupt-parent /soc/interrupt-controller at e61c000
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > @@ -316,6 +316,8 @@
> >                 };
> >
> >                 intc_ex: interrupt-controller at e61c0000 {
> > +                       #interrupt-cells = <2>;
> > +                       interrupt-controller;
> >                         reg = <0 0xe61c0000 0 0x200>;
> >                         /* placeholder */
>
> (bikeshedding) Should the placeholder be at the top of the node?

I would like it more too, but in M3-W dtsi:

		pciec0: pcie at fe000000 {
			reg = <0 0xfe000000 0 0x80000>;
			/* placeholder */
		};

So I kept using the same style everywhere here.

I can change it everywhere in this series to have placeholder at the
beginning of the node definition if you agree with that!

Thanks
   j

>
> >                 };
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

[PATCH 6/6] arm64: dts: renesas: r8a77965: Add #interrupt-cells property

[Simon Horman]

On Fri, Feb 23, 2018 at 11:06:07AM +0100, jacopo mondi wrote:
> Hi Geert,
> 
> On Fri, Feb 23, 2018 at 10:27:16AM +0100, Geert Uytterhoeven wrote:
> > On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > > Add "#interrupt-cells" property and "interrupt-controller" label to
> > > "interrupt-controller at e61c0000" device node.
> > >
> > > This silences the following DTC compiler warnings:
> > > Warning (interrupts_property): Missing interrupt-controller or
> > > interrupt-map property in /soc/interrupt-controller at e61c0000
> > > Warning (interrupts_property): Missing #interrupt-cells in
> > > interrupt-parent /soc/interrupt-controller at e61c000
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > > @@ -316,6 +316,8 @@
> > >                 };
> > >
> > >                 intc_ex: interrupt-controller at e61c0000 {
> > > +                       #interrupt-cells = <2>;
> > > +                       interrupt-controller;
> > >                         reg = <0 0xe61c0000 0 0x200>;
> > >                         /* placeholder */
> >
> > (bikeshedding) Should the placeholder be at the top of the node?
> 
> I would like it more too, but in M3-W dtsi:
> 
> 		pciec0: pcie at fe000000 {
> 			reg = <0 0xfe000000 0 0x80000>;
> 			/* placeholder */
> 		};
> 
> So I kept using the same style everywhere here.
> 
> I can change it everywhere in this series to have placeholder at the
> beginning of the node definition if you agree with that!

Either way is fine by me. But if we do change it lets change it everywhere.

[PATCH 1/6] arm64: dts: renesas: r8a77965: Add "reg" properties

[jacopo mondi]

Hi Geert,

On Fri, Feb 23, 2018 at 10:19:21AM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> > Add "reg" properties to place-holder nodes with unit address defined for
> > R-Car M3-N SoC.
> >
> > This silences the following DTC compiler warning:
> > Warning (unit_address_vs_reg): Node /soc/... has a unit name,
> > but no reg property
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> A few minor nits below...
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>
> > @@ -520,130 +521,163 @@
> >                 };
> >
> >                 avb: ethernet at e6800000 {
> > +                       reg = <0 0xe6800000 0 0x800>, <0 0xe6a00000 0 0x10000>;
>
> The first reg part would be sufficient for a placeholder.
> I think even setting the size to zero would work...

Not sure I fully got your point here.

Do you mean I have to remove the second (and third, fourth...)  addresses range block
in all placeholder device nodes with multiple blocks (avb, du, rcar-sound) ?

Do you mean I have to set to 0 the address length in all placeholder nodes?
What's the point if they have to be added back once the device is
enabled?

>
> >                 du: display at feb00000 {
> > +                       reg = <0 0xfeb00000 0 0x70000>,
>
> Unlike M3-W, the M3-N DU has a third channel, so the size is too small.
>

Will fix in v2

> > +                             <0 0xfeb90000 0 0x14>;
> >                         /* placeholder */
>
> > @@ -703,38 +745,47 @@
> >                 };
> >
> >                 usb2_phy1: usb-phy at ee0a0200 {
> > +                       reg = <0 0xee0a0200 0 0x700>;
> >                         /* placeholder */
> >                 };
>
> To be moved up, next to the other usb2 devices?

Will add a patch to move this next to other usb2 device nodes in v2.

I will send v2 as soon as first review comment gets clarified.

Thanks
   j

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

[PATCH 1/6] arm64: dts: renesas: r8a77965: Add "reg" properties

[Geert Uytterhoeven]

Hi Jacopo,

On Fri, Feb 23, 2018 at 1:22 PM, jacopo mondi <jacopo@jmondi.org> wrote:
> On Fri, Feb 23, 2018 at 10:19:21AM +0100, Geert Uytterhoeven wrote:
>> On Thu, Feb 22, 2018 at 8:06 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>> > Add "reg" properties to place-holder nodes with unit address defined for
>> > R-Car M3-N SoC.
>> >
>> > This silences the following DTC compiler warning:
>> > Warning (unit_address_vs_reg): Node /soc/... has a unit name,
>> > but no reg property
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>>
>> A few minor nits below...
>>
>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>> > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
>>
>> > @@ -520,130 +521,163 @@
>> >                 };
>> >
>> >                 avb: ethernet at e6800000 {
>> > +                       reg = <0 0xe6800000 0 0x800>, <0 0xe6a00000 0 0x10000>;
>>
>> The first reg part would be sufficient for a placeholder.
>> I think even setting the size to zero would work...
>
> Not sure I fully got your point here.
>
> Do you mean I have to remove the second (and third, fourth...)  addresses range block
> in all placeholder device nodes with multiple blocks (avb, du, rcar-sound) ?

No, you don't have to.

> Do you mean I have to set to 0 the address length in all placeholder nodes?
> What's the point if they have to be added back once the device is
> enabled?

No, you don't have to.

The point is to do the minimum amount of work for the placeholders.
But since you've already done the work (and I've done the work to review it),
it doesn't make sense to remove all of this, to be redone later.

For a new future SoC+board combination, we may consider this.
Actually we could get rid of the "reg" requirement as well, if would drop
the unit addresses from the placeholders (e.g. "avb: ethernet { ... }", and
"vin2: video-2 { ...}"). Even less to look up in the datasheet...
(to be broken by Rob adding more dtc checks ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds