[PATCH 0/7] fix unit name warnings on lpc18xx

[PATCH 0/7] fix unit name warnings on lpc18xx

[Joachim Eastwood]

This patch set fix all the unit name warnings from the new dtc
on lpc18xx. Note that this dtc version is not yet upstream.

Also note that the two first patches are not stricly for lpc18xx
but are needed to make dtc happy when compiling. If there are no
objections I can take them through the lpc18xx dt branch. My
plan is to send a pull request for these patches together with
some other stuff to arm-soc soon.

Joachim Eastwood (7):
  ARM: dts: armv7-m: add unit name to interrupt-controller
  ARM: dts: skeleton: add unit name to memory node
  ARM: dts: lpc18xx: remove unit addresses from creg childs
  ARM: dts: lpc4357-ea4357: fix unit name warnings from dtc
  ARM: dts: lpc4350-hitex-eval: fix unit name warnings from dtc
  ARM: dts: lpc4337-ciaa:: fix unit name warning from dtc
  dt-bindings: phy-lpc18xx-usb-otg: remove unit address from binding

 .../devicetree/bindings/phy/phy-lpc18xx-usb-otg.txt  |  2 +-
 arch/arm/boot/dts/armv7-m.dtsi                       |  2 +-
 arch/arm/boot/dts/lpc18xx.dtsi                       |  4 ++--
 arch/arm/boot/dts/lpc4337-ciaa.dts                   |  2 +-
 arch/arm/boot/dts/lpc4350-hitex-eval.dts             | 20 ++++++++++----------
 arch/arm/boot/dts/lpc4357-ea4357-devkit.dts          | 14 +++++++-------
 arch/arm/boot/dts/skeleton.dtsi                      |  2 +-
 arch/arm/boot/dts/skeleton64.dtsi                    |  2 +-
 8 files changed, 24 insertions(+), 24 deletions(-)

-- 
2.7.4

[PATCH 1/7] ARM: dts: armv7-m: add unit name to interrupt-controller

[Joachim Eastwood]

Add unit name to nvic to remove the following warning:
 Warning (unit_address_vs_reg): Node /nv-interrupt-controller has a reg
				or ranges property, but no unit name

Also correct the node name to 'interrupt-controller' while changing the
line.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/boot/dts/armv7-m.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armv7-m.dtsi b/arch/arm/boot/dts/armv7-m.dtsi
index b1ad7cf..16331aa 100644
--- a/arch/arm/boot/dts/armv7-m.dtsi
+++ b/arch/arm/boot/dts/armv7-m.dtsi
@@ -1,7 +1,7 @@
 #include "skeleton.dtsi"
 
 / {
-	nvic: nv-interrupt-controller  {
+	nvic: interrupt-controller at e000e100  {
 		compatible = "arm,armv7m-nvic";
 		interrupt-controller;
 		#interrupt-cells = <1>;
-- 
2.7.4

[PATCH 1/7] ARM: dts: armv7-m: add unit name to interrupt-controller

[Uwe Kleine-König]

Hello Joachim,

On Wed, Mar 30, 2016 at 12:30:40AM +0200, Joachim Eastwood wrote:
> Add unit name to nvic to remove the following warning:
>  Warning (unit_address_vs_reg): Node /nv-interrupt-controller has a reg
> 				or ranges property, but no unit name
> 
> Also correct the node name to 'interrupt-controller' while changing the
> line.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Looks right,

Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 1/7] ARM: dts: armv7-m: add unit name to interrupt-controller

[Joachim Eastwood]

On 4 April 2016 at 08:05, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:
> Hello Joachim,
>
> On Wed, Mar 30, 2016 at 12:30:40AM +0200, Joachim Eastwood wrote:
>> Add unit name to nvic to remove the following warning:
>>  Warning (unit_address_vs_reg): Node /nv-interrupt-controller has a reg
>>                               or ranges property, but no unit name
>>
>> Also correct the node name to 'interrupt-controller' while changing the
>> line.
>>
>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> Looks right,
>
> Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Thanks, Uwe.


Unless any of the other Cortex-M platform maintainers (added more CCs)
has any objections I'll take this through the lpc18xx tree. I will
send a pull request to arm-soc with this soon.


regards,
Joachim Eastwood

[PATCH 1/7] ARM: dts: armv7-m: add unit name to interrupt-controller

[Stefan Agner]

On 2016-04-03 23:42, Joachim  Eastwood wrote:
> On 4 April 2016 at 08:05, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
>> Hello Joachim,
>>
>> On Wed, Mar 30, 2016 at 12:30:40AM +0200, Joachim Eastwood wrote:
>>> Add unit name to nvic to remove the following warning:
>>>  Warning (unit_address_vs_reg): Node /nv-interrupt-controller has a reg
>>>                               or ranges property, but no unit name
>>>
>>> Also correct the node name to 'interrupt-controller' while changing the
>>> line.
>>>
>>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> Looks right,
>>
>> Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> 
> Thanks, Uwe.
> 
> 
> Unless any of the other Cortex-M platform maintainers (added more CCs)
> has any objections I'll take this through the lpc18xx tree. I will
> send a pull request to arm-soc with this soon.

Your patch looks right, I agree we should merge yours.

Acked-by: Stefan Agner <stefan@agner.ch>

--
Stefan

[PATCH 1/7] ARM: dts: armv7-m: add unit name to interrupt-controller

[Uwe Kleine-König]

Hello Joachim,

On Mon, Apr 04, 2016 at 08:42:48AM +0200, Joachim Eastwood wrote:
> Unless any of the other Cortex-M platform maintainers (added more CCs)
> has any objections I'll take this through the lpc18xx tree. I will
> send a pull request to arm-soc with this soon.

that is fine for me.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

Add unit name to memory to remove the following warning:
 Warning (unit_address_vs_reg): Node /memory has a reg or ranges
				property, but no unit name

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/boot/dts/skeleton.dtsi   | 2 +-
 arch/arm/boot/dts/skeleton64.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/skeleton.dtsi b/arch/arm/boot/dts/skeleton.dtsi
index b41d241..a20da0a 100644
--- a/arch/arm/boot/dts/skeleton.dtsi
+++ b/arch/arm/boot/dts/skeleton.dtsi
@@ -9,5 +9,5 @@
 	#size-cells = <1>;
 	chosen { };
 	aliases { };
-	memory { device_type = "memory"; reg = <0 0>; };
+	memory at 0 { device_type = "memory"; reg = <0 0>; };
 };
diff --git a/arch/arm/boot/dts/skeleton64.dtsi b/arch/arm/boot/dts/skeleton64.dtsi
index b5d7f36..6dbe9f9 100644
--- a/arch/arm/boot/dts/skeleton64.dtsi
+++ b/arch/arm/boot/dts/skeleton64.dtsi
@@ -9,5 +9,5 @@
 	#size-cells = <2>;
 	chosen { };
 	aliases { };
-	memory { device_type = "memory"; reg = <0 0 0 0>; };
+	memory at 0 { device_type = "memory"; reg = <0 0 0 0>; };
 };
-- 
2.7.4

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Vladimir Zapolskiy]

Hi Joachim,

On 30.03.2016 01:30, Joachim Eastwood wrote:
> Add unit name to memory to remove the following warning:
>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
> 				property, but no unit name
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
>  arch/arm/boot/dts/skeleton.dtsi   | 2 +-
>  arch/arm/boot/dts/skeleton64.dtsi | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/skeleton.dtsi b/arch/arm/boot/dts/skeleton.dtsi
> index b41d241..a20da0a 100644
> --- a/arch/arm/boot/dts/skeleton.dtsi
> +++ b/arch/arm/boot/dts/skeleton.dtsi
> @@ -9,5 +9,5 @@
>  	#size-cells = <1>;
>  	chosen { };
>  	aliases { };
> -	memory { device_type = "memory"; reg = <0 0>; };
> +	memory at 0 { device_type = "memory"; reg = <0 0>; };
>  };
> diff --git a/arch/arm/boot/dts/skeleton64.dtsi b/arch/arm/boot/dts/skeleton64.dtsi
> index b5d7f36..6dbe9f9 100644
> --- a/arch/arm/boot/dts/skeleton64.dtsi
> +++ b/arch/arm/boot/dts/skeleton64.dtsi
> @@ -9,5 +9,5 @@
>  	#size-cells = <2>;
>  	chosen { };
>  	aliases { };
> -	memory { device_type = "memory"; reg = <0 0 0 0>; };
> +	memory at 0 { device_type = "memory"; reg = <0 0 0 0>; };
>  };
> 

this looks wrong, because all inherited compiled dtbs will contain memory at 0
device node, for example (with applied 4/7 from this series):

% scripts/dtc/dtc -I dtb -O dts arch/arm/boot/dts/lpc4357-ea4357-devkit.dtb
| grep -A3 memory@

	memory at 0 {
		device_type = "memory";
		reg = <0x0 0x0>;
	};
--
	memory at 28000000 {
		device_type = "memory";
		reg = <0x28000000 0x2000000>;
	};

--
With best wishes,
Vladimir

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Mark Rutland]

On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
> Add unit name to memory to remove the following warning:
>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
> 				property, but no unit name

If anything, it would be better to get rid of the memory node from the
skeleton DTs.

For DTs which have a memory node there's no problem, and DTs which
expect a bootlaoder to fill things in have a logical place to document
that fact.

Thanks,
Mark.

> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
>  arch/arm/boot/dts/skeleton.dtsi   | 2 +-
>  arch/arm/boot/dts/skeleton64.dtsi | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/skeleton.dtsi b/arch/arm/boot/dts/skeleton.dtsi
> index b41d241..a20da0a 100644
> --- a/arch/arm/boot/dts/skeleton.dtsi
> +++ b/arch/arm/boot/dts/skeleton.dtsi
> @@ -9,5 +9,5 @@
>  	#size-cells = <1>;
>  	chosen { };
>  	aliases { };
> -	memory { device_type = "memory"; reg = <0 0>; };
> +	memory at 0 { device_type = "memory"; reg = <0 0>; };
>  };
> diff --git a/arch/arm/boot/dts/skeleton64.dtsi b/arch/arm/boot/dts/skeleton64.dtsi
> index b5d7f36..6dbe9f9 100644
> --- a/arch/arm/boot/dts/skeleton64.dtsi
> +++ b/arch/arm/boot/dts/skeleton64.dtsi
> @@ -9,5 +9,5 @@
>  	#size-cells = <2>;
>  	chosen { };
>  	aliases { };
> -	memory { device_type = "memory"; reg = <0 0 0 0>; };
> +	memory at 0 { device_type = "memory"; reg = <0 0 0 0>; };
>  };
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

Hi Mark,

On 30 March 2016 at 13:06, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
>> Add unit name to memory to remove the following warning:
>>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>>                               property, but no unit name
>
> If anything, it would be better to get rid of the memory node from the
> skeleton DTs.

Yes, I agree and as Vladimir notes just by adding a unit name we
create duplicate memory nodes.

I'll send an updated patch set once Rob has a change to look at it also.


> For DTs which have a memory node there's no problem, and DTs which
> expect a bootlaoder to fill things in have a logical place to document
> that fact.

I'll take a look at which DTs that currently doesn't have a memory and add it.


Thanks for looking.


regards,
Joachim Eastwood

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Vladimir Zapolskiy]

On 30.03.2016 14:06, Mark Rutland wrote:
> On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
>> Add unit name to memory to remove the following warning:
>>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>> 				property, but no unit name
> 
> If anything, it would be better to get rid of the memory node from the
> skeleton DTs.
> 
> For DTs which have a memory node there's no problem, and DTs which
> expect a bootlaoder to fill things in have a logical place to document
> that fact.

Generally I support this.

U-boot still creates or fixes up "/memory" node only, assuming that
a bootloader is updated rarely the kernel should continue to process
in expected way "/memory" device node, also note that ePAPR says

ePAPR> If a system has multiple ranges of memory, multiple memory nodes
ePAPR> can be created, or the ranges can be specified in the reg property
ePAPR> of a single memory node.

Having just a DT on hand it won't be possible to make assumptions about
a bootloader, also "/memory" without a given unit address is used by
other kernels and a bootloader can respect this.

The only problem I see if DTB is updated on a board but a board bootloader
on fix-up is capable to fill a preexisting "/memory" device node in only,
otherwise it is not clear why the device node is present in skeleton.dtsi.

>> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
>> ---
>>  arch/arm/boot/dts/skeleton.dtsi   | 2 +-
>>  arch/arm/boot/dts/skeleton64.dtsi | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/skeleton.dtsi b/arch/arm/boot/dts/skeleton.dtsi
>> index b41d241..a20da0a 100644
>> --- a/arch/arm/boot/dts/skeleton.dtsi
>> +++ b/arch/arm/boot/dts/skeleton.dtsi
>> @@ -9,5 +9,5 @@
>>  	#size-cells = <1>;
>>  	chosen { };
>>  	aliases { };
>> -	memory { device_type = "memory"; reg = <0 0>; };
>> +	memory at 0 { device_type = "memory"; reg = <0 0>; };
>>  };
>> diff --git a/arch/arm/boot/dts/skeleton64.dtsi b/arch/arm/boot/dts/skeleton64.dtsi
>> index b5d7f36..6dbe9f9 100644
>> --- a/arch/arm/boot/dts/skeleton64.dtsi
>> +++ b/arch/arm/boot/dts/skeleton64.dtsi
>> @@ -9,5 +9,5 @@
>>  	#size-cells = <2>;
>>  	chosen { };
>>  	aliases { };
>> -	memory { device_type = "memory"; reg = <0 0 0 0>; };
>> +	memory at 0 { device_type = "memory"; reg = <0 0 0 0>; };
>>  };
>> -- 
>> 2.7.4
>>

--
With best wishes,
Vladimir

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Mark Rutland]

On Wed, Mar 30, 2016 at 04:06:56PM +0300, Vladimir Zapolskiy wrote:
> On 30.03.2016 14:06, Mark Rutland wrote:
> > On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
> >> Add unit name to memory to remove the following warning:
> >>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
> >> 				property, but no unit name
> > 
> > If anything, it would be better to get rid of the memory node from the
> > skeleton DTs.
> > 
> > For DTs which have a memory node there's no problem, and DTs which
> > expect a bootlaoder to fill things in have a logical place to document
> > that fact.

> The only problem I see if DTB is updated on a board but a board bootloader
> on fix-up is capable to fill a preexisting "/memory" device node in only,
> otherwise it is not clear why the device node is present in skeleton.dtsi.

Sure. To clarify the above, what I expect that for this case is that the
empty memory node would exist in the dts for that particular board,
along with a comment, e.g.

	/* The firmware/bootloader for $BOARD fills this in */
	memory {
		device_type = "memory";
		reg = <0 0 0 0>;
	};

That way you can tell at a glance that the lack of memory information in
the DT for a board is intentional, and the bootloader still gets the
node it expects.

Thanks,
Mark.

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

Hi Mark,

On 30 March 2016 at 15:41, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 30, 2016 at 04:06:56PM +0300, Vladimir Zapolskiy wrote:
>> On 30.03.2016 14:06, Mark Rutland wrote:
>> > On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
>> >> Add unit name to memory to remove the following warning:
>> >>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>> >>                            property, but no unit name
>> >
>> > If anything, it would be better to get rid of the memory node from the
>> > skeleton DTs.
>> >
>> > For DTs which have a memory node there's no problem, and DTs which
>> > expect a bootlaoder to fill things in have a logical place to document
>> > that fact.
>
>> The only problem I see if DTB is updated on a board but a board bootloader
>> on fix-up is capable to fill a preexisting "/memory" device node in only,
>> otherwise it is not clear why the device node is present in skeleton.dtsi.
>
> Sure. To clarify the above, what I expect that for this case is that the
> empty memory node would exist in the dts for that particular board,
> along with a comment, e.g.
>
>         /* The firmware/bootloader for $BOARD fills this in */
>         memory {
>                 device_type = "memory";
>                 reg = <0 0 0 0>;
>         };

To avoid the warning with the new dtc this would need to be memory at 0.


> That way you can tell at a glance that the lack of memory information in
> the DT for a board is intentional, and the bootloader still gets the
> node it expects.


But this doesn't seem to be a "problem" with any of the DTs in
arch/arm/boot as they all defined a memory node.

I used the following script to check for the memory node in all built dtb's.
  make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
  for i in $(ls arch/arm/boot/dts/*.dtb); do
         m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
         if [ -z "$m" ]; then
                 echo "Missing memory node in $i"
          fi
  done

So it should be pretty safe to just remove the memory node entry in
the skeleton files. Unless I have missed something with the script
above.


regards,
Joachim Eastwood

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Mark Rutland]

On Wed, Mar 30, 2016 at 06:15:35PM +0200, Joachim Eastwood wrote:
> Hi Mark,
> 
> On 30 March 2016 at 15:41, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Mar 30, 2016 at 04:06:56PM +0300, Vladimir Zapolskiy wrote:
> >> On 30.03.2016 14:06, Mark Rutland wrote:
> >> > On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
> >> >> Add unit name to memory to remove the following warning:
> >> >>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
> >> >>                            property, but no unit name
> >> >
> >> > If anything, it would be better to get rid of the memory node from the
> >> > skeleton DTs.
> >> >
> >> > For DTs which have a memory node there's no problem, and DTs which
> >> > expect a bootlaoder to fill things in have a logical place to document
> >> > that fact.
> >
> >> The only problem I see if DTB is updated on a board but a board bootloader
> >> on fix-up is capable to fill a preexisting "/memory" device node in only,
> >> otherwise it is not clear why the device node is present in skeleton.dtsi.
> >
> > Sure. To clarify the above, what I expect that for this case is that the
> > empty memory node would exist in the dts for that particular board,
> > along with a comment, e.g.
> >
> >         /* The firmware/bootloader for $BOARD fills this in */
> >         memory {
> >                 device_type = "memory";
> >                 reg = <0 0 0 0>;
> >         };
> 
> To avoid the warning with the new dtc this would need to be memory at 0.

Hmm... That's a little sub-optimal in the case that a bootloader is
patching this. Presumably a bootloader that needs an existing node won't
patch the unit-address to match the reg (which might not start at 0).

I'd rather not have the unit-address than have an incorrect
unit-address, though I guess we don't have much of a choice here, unless
there's some override we can place in the dts.

> > That way you can tell at a glance that the lack of memory information in
> > the DT for a board is intentional, and the bootloader still gets the
> > node it expects.
> 
> But this doesn't seem to be a "problem" with any of the DTs in
> arch/arm/boot as they all defined a memory node.
> 
> I used the following script to check for the memory node in all built dtb's.
>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>          if [ -z "$m" ]; then
>                  echo "Missing memory node in $i"
>           fi
>   done
> 
> So it should be pretty safe to just remove the memory node entry in
> the skeleton files. Unless I have missed something with the script
> above.

The above might match reserved-memory nodes; it might be better to check
for 'device_type\s*=\s*"memory"'.

I assume that was run after deleting the memory node from the skeletons?

Otherwise, that looks fairly convincing!

Thanks,
Mark.

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

On 30 March 2016 at 19:06, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 30, 2016 at 06:15:35PM +0200, Joachim Eastwood wrote:
>> Hi Mark,
>>
>> On 30 March 2016 at 15:41, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Wed, Mar 30, 2016 at 04:06:56PM +0300, Vladimir Zapolskiy wrote:
>> >> On 30.03.2016 14:06, Mark Rutland wrote:
>> >> > On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
>> >> >> Add unit name to memory to remove the following warning:
>> >> >>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>> >> >>                            property, but no unit name
>> >> >
>> >> > If anything, it would be better to get rid of the memory node from the
>> >> > skeleton DTs.
>> >> >
>> >> > For DTs which have a memory node there's no problem, and DTs which
>> >> > expect a bootlaoder to fill things in have a logical place to document
>> >> > that fact.
>> >
>> >> The only problem I see if DTB is updated on a board but a board bootloader
>> >> on fix-up is capable to fill a preexisting "/memory" device node in only,
>> >> otherwise it is not clear why the device node is present in skeleton.dtsi.
>> >
>> > Sure. To clarify the above, what I expect that for this case is that the
>> > empty memory node would exist in the dts for that particular board,
>> > along with a comment, e.g.
>> >
>> >         /* The firmware/bootloader for $BOARD fills this in */
>> >         memory {
>> >                 device_type = "memory";
>> >                 reg = <0 0 0 0>;
>> >         };
>>
>> To avoid the warning with the new dtc this would need to be memory at 0.
>
> Hmm... That's a little sub-optimal in the case that a bootloader is
> patching this. Presumably a bootloader that needs an existing node won't
> patch the unit-address to match the reg (which might not start at 0).
>
> I'd rather not have the unit-address than have an incorrect
> unit-address, though I guess we don't have much of a choice here, unless
> there's some override we can place in the dts.
>
>> > That way you can tell at a glance that the lack of memory information in
>> > the DT for a board is intentional, and the bootloader still gets the
>> > node it expects.
>>
>> But this doesn't seem to be a "problem" with any of the DTs in
>> arch/arm/boot as they all defined a memory node.
>>
>> I used the following script to check for the memory node in all built dtb's.
>>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>>          if [ -z "$m" ]; then
>>                  echo "Missing memory node in $i"
>>           fi
>>   done
>>
>> So it should be pretty safe to just remove the memory node entry in
>> the skeleton files. Unless I have missed something with the script
>> above.
>
> The above might match reserved-memory nodes; it might be better to check
> for 'device_type\s*=\s*"memory"'.

I did check the output of the grep and it looks good. But there are
indeed DTs that are missing the 'device_type = "memory"' parameter.
Actually; _a lot_ or 438 of 741 to be exact. ugh...

I guess all those should be fixed up before we can remove the memory
node from skeleton. :/


> I assume that was run after deleting the memory node from the skeletons?

Yes :)


regards,
Joachim Eastwood

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Mark Rutland]

On Wed, Mar 30, 2016 at 10:45:06PM +0200, Joachim Eastwood wrote:
> On 30 March 2016 at 19:06, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Mar 30, 2016 at 06:15:35PM +0200, Joachim Eastwood wrote:
> >> I used the following script to check for the memory node in all built dtb's.
> >>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
> >>   for i in $(ls arch/arm/boot/dts/*.dtb); do
> >>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
> >>          if [ -z "$m" ]; then
> >>                  echo "Missing memory node in $i"
> >>           fi
> >>   done
> >>
> >> So it should be pretty safe to just remove the memory node entry in
> >> the skeleton files. Unless I have missed something with the script
> >> above.
> >
> > The above might match reserved-memory nodes; it might be better to check
> > for 'device_type\s*=\s*"memory"'.
> 
> I did check the output of the grep and it looks good. But there are
> indeed DTs that are missing the 'device_type = "memory"' parameter.
> Actually; _a lot_ or 438 of 741 to be exact. ugh...
> 
> I guess all those should be fixed up before we can remove the memory
> node from skeleton. :/

Ouch, yes. :(

That said, the cahnges don't need to be an atomic operation. We could
start adding device_type = "memory" to dts immediately (in as whatever
size batches maintainers are happy with), as a duplicate device_type
shouldn't be problematic.

When we hit critical mass, we could then remove the skeleton memory
nodes, fixing up any remaining fallout.

As for the mechanical changes, it sounds like we need coccinelle for DT.

That, or a laptop, a long flight, and a gin and tonic.

Thanks,
Mark.

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

On 31 March 2016 at 12:38, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 30, 2016 at 10:45:06PM +0200, Joachim Eastwood wrote:
>> On 30 March 2016 at 19:06, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Wed, Mar 30, 2016 at 06:15:35PM +0200, Joachim Eastwood wrote:
>> >> I used the following script to check for the memory node in all built dtb's.
>> >>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>> >>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>> >>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>> >>          if [ -z "$m" ]; then
>> >>                  echo "Missing memory node in $i"
>> >>           fi
>> >>   done
>> >>
>> >> So it should be pretty safe to just remove the memory node entry in
>> >> the skeleton files. Unless I have missed something with the script
>> >> above.
>> >
>> > The above might match reserved-memory nodes; it might be better to check
>> > for 'device_type\s*=\s*"memory"'.
>>
>> I did check the output of the grep and it looks good. But there are
>> indeed DTs that are missing the 'device_type = "memory"' parameter.
>> Actually; _a lot_ or 438 of 741 to be exact. ugh...
>>
>> I guess all those should be fixed up before we can remove the memory
>> node from skeleton. :/
>
> Ouch, yes. :(
>
> That said, the cahnges don't need to be an atomic operation. We could
> start adding device_type = "memory" to dts immediately (in as whatever
> size batches maintainers are happy with), as a duplicate device_type
> shouldn't be problematic.

Yes, that is true.


> When we hit critical mass, we could then remove the skeleton memory
> nodes, fixing up any remaining fallout.
>
> As for the mechanical changes, it sounds like we need coccinelle for DT.
>
> That, or a laptop, a long flight, and a gin and tonic.

:-)

I'll see if I can cook up something with awk.


Anyway, I am dropping the memory node changes from this patch set so I
can get a pull request sent to arm-soc for lpc18xx sooner rather than
later.


regards,
Joachim Eastwood

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

On 31 March 2016 at 17:21, Joachim  Eastwood <manabian@gmail.com> wrote:
> On 31 March 2016 at 12:38, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Wed, Mar 30, 2016 at 10:45:06PM +0200, Joachim Eastwood wrote:
>>> On 30 March 2016 at 19:06, Mark Rutland <mark.rutland@arm.com> wrote:
>>> > On Wed, Mar 30, 2016 at 06:15:35PM +0200, Joachim Eastwood wrote:
>>> >> I used the following script to check for the memory node in all built dtb's.
>>> >>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>>> >>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>>> >>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>>> >>          if [ -z "$m" ]; then
>>> >>                  echo "Missing memory node in $i"
>>> >>           fi
>>> >>   done
>>> >>
>>> >> So it should be pretty safe to just remove the memory node entry in
>>> >> the skeleton files. Unless I have missed something with the script
>>> >> above.
>>> >
>>> > The above might match reserved-memory nodes; it might be better to check
>>> > for 'device_type\s*=\s*"memory"'.
>>>
>>> I did check the output of the grep and it looks good. But there are
>>> indeed DTs that are missing the 'device_type = "memory"' parameter.
>>> Actually; _a lot_ or 438 of 741 to be exact. ugh...
>>>
>>> I guess all those should be fixed up before we can remove the memory
>>> node from skeleton. :/
>>
>> Ouch, yes. :(
>>
>> That said, the cahnges don't need to be an atomic operation. We could
>> start adding device_type = "memory" to dts immediately (in as whatever
>> size batches maintainers are happy with), as a duplicate device_type
>> shouldn't be problematic.
>
> Yes, that is true.
>
>
>> When we hit critical mass, we could then remove the skeleton memory
>> nodes, fixing up any remaining fallout.
>>
>> As for the mechanical changes, it sounds like we need coccinelle for DT.
>>
>> That, or a laptop, a long flight, and a gin and tonic.
>
> :-)
>
> I'll see if I can cook up something with awk.

Something like this should do it:
#!/usr/bin/gawk -f
# find arch/arm/boot/dts/ -type f -name *.dts* | xargs
./mem_node_add_dev_type.awk -i inplace
BEGIN { go = 0 }
/[^-]memory ?{/ { go = 1; idx = 0; device_type = 0 }

go == 1 {
        buf[idx++] = $0
        if ($0 ~ /device_type/)
                device_type = 1

        if ($0 ~ /};/) {
                print buf[0]

                if (!device_type) {
                        buf[0] = buf[1]
                        gsub(/[^\t]*/, "", buf[0])
                        buf[0] = buf[0] "device_type = \"memory\";"
                } else {
                        delete buf[0]
                }

                for (i in buf)
                        print buf[i]
                delete buf

                go = 0
                device_type = 0
        }
        next
}

{ print }

Gives me: 249 files changed, 249 insertions(+), 1 deletion(-) on v4.6-rc1.

Running the my check script after this reveals that 83 DTs lack the
device_type parameter, but these seem to also lack the memory node as
well. Wonder why my script didn't pick up the missing memory node in
those... I also doubt that the memory node is set by the boot loader
in some of these boards.


regards,
Joachim Eastwood

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Rob Herring]

On Thu, Mar 31, 2016 at 11:34 AM, Joachim  Eastwood <manabian@gmail.com> wrote:
> On 31 March 2016 at 17:21, Joachim  Eastwood <manabian@gmail.com> wrote:
>> On 31 March 2016 at 12:38, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Wed, Mar 30, 2016 at 10:45:06PM +0200, Joachim Eastwood wrote:
>>>> On 30 March 2016 at 19:06, Mark Rutland <mark.rutland@arm.com> wrote:
>>>> > On Wed, Mar 30, 2016 at 06:15:35PM +0200, Joachim Eastwood wrote:
>>>> >> I used the following script to check for the memory node in all built dtb's.
>>>> >>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>>>> >>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>>>> >>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>>>> >>          if [ -z "$m" ]; then
>>>> >>                  echo "Missing memory node in $i"
>>>> >>           fi
>>>> >>   done
>>>> >>
>>>> >> So it should be pretty safe to just remove the memory node entry in
>>>> >> the skeleton files. Unless I have missed something with the script
>>>> >> above.
>>>> >
>>>> > The above might match reserved-memory nodes; it might be better to check
>>>> > for 'device_type\s*=\s*"memory"'.
>>>>
>>>> I did check the output of the grep and it looks good. But there are
>>>> indeed DTs that are missing the 'device_type = "memory"' parameter.
>>>> Actually; _a lot_ or 438 of 741 to be exact. ugh...
>>>>
>>>> I guess all those should be fixed up before we can remove the memory
>>>> node from skeleton. :/
>>>
>>> Ouch, yes. :(
>>>
>>> That said, the cahnges don't need to be an atomic operation. We could
>>> start adding device_type = "memory" to dts immediately (in as whatever
>>> size batches maintainers are happy with), as a duplicate device_type
>>> shouldn't be problematic.
>>
>> Yes, that is true.
>>
>>
>>> When we hit critical mass, we could then remove the skeleton memory
>>> nodes, fixing up any remaining fallout.
>>>
>>> As for the mechanical changes, it sounds like we need coccinelle for DT.
>>>
>>> That, or a laptop, a long flight, and a gin and tonic.
>>
>> :-)
>>
>> I'll see if I can cook up something with awk.
>
> Something like this should do it:
> #!/usr/bin/gawk -f
> # find arch/arm/boot/dts/ -type f -name *.dts* | xargs
> ./mem_node_add_dev_type.awk -i inplace
> BEGIN { go = 0 }
> /[^-]memory ?{/ { go = 1; idx = 0; device_type = 0 }
>
> go == 1 {
>         buf[idx++] = $0
>         if ($0 ~ /device_type/)
>                 device_type = 1
>
>         if ($0 ~ /};/) {
>                 print buf[0]
>
>                 if (!device_type) {
>                         buf[0] = buf[1]
>                         gsub(/[^\t]*/, "", buf[0])
>                         buf[0] = buf[0] "device_type = \"memory\";"
>                 } else {
>                         delete buf[0]
>                 }
>
>                 for (i in buf)
>                         print buf[i]
>                 delete buf
>
>                 go = 0
>                 device_type = 0
>         }
>         next
> }
>
> { print }
>
> Gives me: 249 files changed, 249 insertions(+), 1 deletion(-) on v4.6-rc1.
>
> Running the my check script after this reveals that 83 DTs lack the
> device_type parameter, but these seem to also lack the memory node as
> well. Wonder why my script didn't pick up the missing memory node in
> those... I also doubt that the memory node is set by the boot loader
> in some of these boards.

I don't think this script is right. It will not work if the memory
node is spread across dtsi files. You would need to compiler
everything to dtb and then back to a flat dts file before running thru
this script.

I don't see the rest of the series in -next. Is that going to happen soon?

Rob

>
>
> regards,
> Joachim Eastwood

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

On 13 April 2016 at 00:45, Rob Herring <robh+dt@kernel.org> wrote:
> On Thu, Mar 31, 2016 at 11:34 AM, Joachim  Eastwood <manabian@gmail.com> wrote:
>> On 31 March 2016 at 17:21, Joachim  Eastwood <manabian@gmail.com> wrote:
>>> On 31 March 2016 at 12:38, Mark Rutland <mark.rutland@arm.com> wrote:
>>>> On Wed, Mar 30, 2016 at 10:45:06PM +0200, Joachim Eastwood wrote:
>>>>> On 30 March 2016 at 19:06, Mark Rutland <mark.rutland@arm.com> wrote:
>>>>> > On Wed, Mar 30, 2016 at 06:15:35PM +0200, Joachim Eastwood wrote:
>>>>> >> I used the following script to check for the memory node in all built dtb's.
>>>>> >>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>>>>> >>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>>>>> >>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>>>>> >>          if [ -z "$m" ]; then
>>>>> >>                  echo "Missing memory node in $i"
>>>>> >>           fi
>>>>> >>   done
>>>>> >>
>>>>> >> So it should be pretty safe to just remove the memory node entry in
>>>>> >> the skeleton files. Unless I have missed something with the script
>>>>> >> above.
>>>>> >
>>>>> > The above might match reserved-memory nodes; it might be better to check
>>>>> > for 'device_type\s*=\s*"memory"'.
>>>>>
>>>>> I did check the output of the grep and it looks good. But there are
>>>>> indeed DTs that are missing the 'device_type = "memory"' parameter.
>>>>> Actually; _a lot_ or 438 of 741 to be exact. ugh...
>>>>>
>>>>> I guess all those should be fixed up before we can remove the memory
>>>>> node from skeleton. :/
>>>>
>>>> Ouch, yes. :(
>>>>
>>>> That said, the cahnges don't need to be an atomic operation. We could
>>>> start adding device_type = "memory" to dts immediately (in as whatever
>>>> size batches maintainers are happy with), as a duplicate device_type
>>>> shouldn't be problematic.
>>>
>>> Yes, that is true.
>>>
>>>
>>>> When we hit critical mass, we could then remove the skeleton memory
>>>> nodes, fixing up any remaining fallout.
>>>>
>>>> As for the mechanical changes, it sounds like we need coccinelle for DT.
>>>>
>>>> That, or a laptop, a long flight, and a gin and tonic.
>>>
>>> :-)
>>>
>>> I'll see if I can cook up something with awk.
>>
>> Something like this should do it:
>> #!/usr/bin/gawk -f
>> # find arch/arm/boot/dts/ -type f -name *.dts* | xargs
>> ./mem_node_add_dev_type.awk -i inplace
>> BEGIN { go = 0 }
>> /[^-]memory ?{/ { go = 1; idx = 0; device_type = 0 }
>>
>> go == 1 {
>>         buf[idx++] = $0
>>         if ($0 ~ /device_type/)
>>                 device_type = 1
>>
>>         if ($0 ~ /};/) {
>>                 print buf[0]
>>
>>                 if (!device_type) {
>>                         buf[0] = buf[1]
>>                         gsub(/[^\t]*/, "", buf[0])
>>                         buf[0] = buf[0] "device_type = \"memory\";"
>>                 } else {
>>                         delete buf[0]
>>                 }
>>
>>                 for (i in buf)
>>                         print buf[i]
>>                 delete buf
>>
>>                 go = 0
>>                 device_type = 0
>>         }
>>         next
>> }
>>
>> { print }
>>
>> Gives me: 249 files changed, 249 insertions(+), 1 deletion(-) on v4.6-rc1.
>>
>> Running the my check script after this reveals that 83 DTs lack the
>> device_type parameter, but these seem to also lack the memory node as
>> well. Wonder why my script didn't pick up the missing memory node in
>> those... I also doubt that the memory node is set by the boot loader
>> in some of these boards.
>
> I don't think this script is right. It will not work if the memory
> node is spread across dtsi files. You would need to compiler
> everything to dtb and then back to a flat dts file before running thru
> this script.

The script will simply add 'device_type = memory' to all dts and dtsi
files that are missing this property.
What do you by spread across dtsi files? Got an example DT that I
could take a look at?


> I don't see the rest of the series in -next. Is that going to happen soon?

Pull request was sent to arm-soc some time ago, but it seems they
haven't pulled in anything for 4.7 yet.
Link: http://permalink.gmane.org/gmane.linux.ports.arm.kernel/490303
(All memory node "fixes" was removed from this pull request)


regards,
Joachim Eastwood

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Rob Herring]

On Wed, Mar 30, 2016 at 11:15 AM, Joachim  Eastwood <manabian@gmail.com> wrote:
> Hi Mark,
>
> On 30 March 2016 at 15:41, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Wed, Mar 30, 2016 at 04:06:56PM +0300, Vladimir Zapolskiy wrote:
>>> On 30.03.2016 14:06, Mark Rutland wrote:
>>> > On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
>>> >> Add unit name to memory to remove the following warning:
>>> >>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>>> >>                            property, but no unit name
>>> >
>>> > If anything, it would be better to get rid of the memory node from the
>>> > skeleton DTs.
>>> >
>>> > For DTs which have a memory node there's no problem, and DTs which
>>> > expect a bootlaoder to fill things in have a logical place to document
>>> > that fact.
>>
>>> The only problem I see if DTB is updated on a board but a board bootloader
>>> on fix-up is capable to fill a preexisting "/memory" device node in only,
>>> otherwise it is not clear why the device node is present in skeleton.dtsi.
>>
>> Sure. To clarify the above, what I expect that for this case is that the
>> empty memory node would exist in the dts for that particular board,
>> along with a comment, e.g.
>>
>>         /* The firmware/bootloader for $BOARD fills this in */
>>         memory {
>>                 device_type = "memory";
>>                 reg = <0 0 0 0>;
>>         };
>
> To avoid the warning with the new dtc this would need to be memory at 0.

Except memory is probably not actually at 0 here. This has come up
before and been beaten to death. Bottom line is plain "memory" is
allowed, and I plan to add that exception to dtc.

>> That way you can tell at a glance that the lack of memory information in
>> the DT for a board is intentional, and the bootloader still gets the
>> node it expects.
>
>
> But this doesn't seem to be a "problem" with any of the DTs in
> arch/arm/boot as they all defined a memory node.
>
> I used the following script to check for the memory node in all built dtb's.
>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>          if [ -z "$m" ]; then
>                  echo "Missing memory node in $i"
>           fi
>   done
>
> So it should be pretty safe to just remove the memory node entry in
> the skeleton files. Unless I have missed something with the script
> above.

You've got to make sure they have 'device_type = "memory";' which they
could rely on inheriting.

Rob

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Rob Herring]

On Thu, Mar 31, 2016 at 11:27 AM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, Mar 30, 2016 at 11:15 AM, Joachim  Eastwood <manabian@gmail.com> wrote:
>> Hi Mark,
>>
>> On 30 March 2016 at 15:41, Mark Rutland <mark.rutland@arm.com> wrote:
>>> On Wed, Mar 30, 2016 at 04:06:56PM +0300, Vladimir Zapolskiy wrote:
>>>> On 30.03.2016 14:06, Mark Rutland wrote:
>>>> > On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
>>>> >> Add unit name to memory to remove the following warning:
>>>> >>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>>>> >>                            property, but no unit name
>>>> >
>>>> > If anything, it would be better to get rid of the memory node from the
>>>> > skeleton DTs.
>>>> >
>>>> > For DTs which have a memory node there's no problem, and DTs which
>>>> > expect a bootlaoder to fill things in have a logical place to document
>>>> > that fact.
>>>
>>>> The only problem I see if DTB is updated on a board but a board bootloader
>>>> on fix-up is capable to fill a preexisting "/memory" device node in only,
>>>> otherwise it is not clear why the device node is present in skeleton.dtsi.
>>>
>>> Sure. To clarify the above, what I expect that for this case is that the
>>> empty memory node would exist in the dts for that particular board,
>>> along with a comment, e.g.
>>>
>>>         /* The firmware/bootloader for $BOARD fills this in */
>>>         memory {
>>>                 device_type = "memory";
>>>                 reg = <0 0 0 0>;
>>>         };
>>
>> To avoid the warning with the new dtc this would need to be memory at 0.
>
> Except memory is probably not actually at 0 here. This has come up
> before and been beaten to death. Bottom line is plain "memory" is
> allowed, and I plan to add that exception to dtc.
>
>>> That way you can tell at a glance that the lack of memory information in
>>> the DT for a board is intentional, and the bootloader still gets the
>>> node it expects.
>>
>>
>> But this doesn't seem to be a "problem" with any of the DTs in
>> arch/arm/boot as they all defined a memory node.
>>
>> I used the following script to check for the memory node in all built dtb's.
>>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>>          if [ -z "$m" ]; then
>>                  echo "Missing memory node in $i"
>>           fi
>>   done
>>
>> So it should be pretty safe to just remove the memory node entry in
>> the skeleton files. Unless I have missed something with the script
>> above.
>
> You've got to make sure they have 'device_type = "memory";' which they
> could rely on inheriting.

BTW, you could add this as a check to dtc and then it will find all
the missing ones for you.

Rob

[PATCH 2/7] ARM: dts: skeleton: add unit name to memory node

[Joachim Eastwood]

On 31 March 2016 at 18:31, Rob Herring <robh+dt@kernel.org> wrote:
> On Thu, Mar 31, 2016 at 11:27 AM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Wed, Mar 30, 2016 at 11:15 AM, Joachim  Eastwood <manabian@gmail.com> wrote:
>>> Hi Mark,
>>>
>>> On 30 March 2016 at 15:41, Mark Rutland <mark.rutland@arm.com> wrote:
>>>> On Wed, Mar 30, 2016 at 04:06:56PM +0300, Vladimir Zapolskiy wrote:
>>>>> On 30.03.2016 14:06, Mark Rutland wrote:
>>>>> > On Wed, Mar 30, 2016 at 12:30:41AM +0200, Joachim Eastwood wrote:
>>>>> >> Add unit name to memory to remove the following warning:
>>>>> >>  Warning (unit_address_vs_reg): Node /memory has a reg or ranges
>>>>> >>                            property, but no unit name
>>>>> >
>>>>> > If anything, it would be better to get rid of the memory node from the
>>>>> > skeleton DTs.
>>>>> >
>>>>> > For DTs which have a memory node there's no problem, and DTs which
>>>>> > expect a bootlaoder to fill things in have a logical place to document
>>>>> > that fact.
>>>>
>>>>> The only problem I see if DTB is updated on a board but a board bootloader
>>>>> on fix-up is capable to fill a preexisting "/memory" device node in only,
>>>>> otherwise it is not clear why the device node is present in skeleton.dtsi.
>>>>
>>>> Sure. To clarify the above, what I expect that for this case is that the
>>>> empty memory node would exist in the dts for that particular board,
>>>> along with a comment, e.g.
>>>>
>>>>         /* The firmware/bootloader for $BOARD fills this in */
>>>>         memory {
>>>>                 device_type = "memory";
>>>>                 reg = <0 0 0 0>;
>>>>         };
>>>
>>> To avoid the warning with the new dtc this would need to be memory at 0.
>>
>> Except memory is probably not actually at 0 here. This has come up
>> before and been beaten to death. Bottom line is plain "memory" is
>> allowed, and I plan to add that exception to dtc.

Okey, good.


>>>> That way you can tell at a glance that the lack of memory information in
>>>> the DT for a board is intentional, and the bootloader still gets the
>>>> node it expects.
>>>
>>>
>>> But this doesn't seem to be a "problem" with any of the DTs in
>>> arch/arm/boot as they all defined a memory node.
>>>
>>> I used the following script to check for the memory node in all built dtb's.
>>>   make ARCH=arm CONFIG_OF_ALL_DTBS=y dtbs
>>>   for i in $(ls arch/arm/boot/dts/*.dtb); do
>>>          m=$(scripts/dtc/dtc -I dtb -O dts $i | grep -m1 'memory.*{')
>>>          if [ -z "$m" ]; then
>>>                  echo "Missing memory node in $i"
>>>           fi
>>>   done
>>>
>>> So it should be pretty safe to just remove the memory node entry in
>>> the skeleton files. Unless I have missed something with the script
>>> above.
>>
>> You've got to make sure they have 'device_type = "memory";' which they
>> could rely on inheriting.
>
> BTW, you could add this as a check to dtc and then it will find all
> the missing ones for you.

I have created a script now that will check and add it if it is missing.


regards,
Joachim Eastwood

[PATCH 3/7] ARM: dts: lpc18xx: remove unit addresses from creg childs

[Joachim Eastwood]

DT nodes without reg properties should not have a unit address. This
fixes the following warnings from dtc.

 Warning (unit_address_vs_reg): Node /soc/syscon at 40043000/phy at 004 has a
 unit name, but no reg property
 Warning (unit_address_vs_reg): Node /soc/syscon at 40043000/dma-mux at 11c has
 a unit name, but no reg property

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/boot/dts/lpc18xx.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/lpc18xx.dtsi b/arch/arm/boot/dts/lpc18xx.dtsi
index fdc991b..fdb736c 100644
--- a/arch/arm/boot/dts/lpc18xx.dtsi
+++ b/arch/arm/boot/dts/lpc18xx.dtsi
@@ -201,13 +201,13 @@
 				#clock-cells = <1>;
 			};
 
-			usb0_otg_phy: phy at 004 {
+			usb0_otg_phy: phy {
 				compatible = "nxp,lpc1850-usb-otg-phy";
 				clocks = <&ccu1 CLK_USB0>;
 				#phy-cells = <0>;
 			};
 
-			dmamux: dma-mux at 11c {
+			dmamux: dma-mux {
 				compatible = "nxp,lpc1850-dmamux";
 				#dma-cells = <3>;
 				dma-requests = <64>;
-- 
2.7.4

[PATCH 4/7] ARM: dts: lpc4357-ea4357: fix unit name warnings from dtc

[Joachim Eastwood]

Fix the following warnings from dtc by either adding or removing
the unit name from the node.

Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Warning (unit_address_vs_reg): Node /soc/flash-controller at 40003000/flash at 0 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /gpio_joystick/button at 0 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /gpio_joystick/button at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /gpio_joystick/button at 2 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /gpio_joystick/button at 3 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /gpio_joystick/button at 4 has a unit name, but no reg property

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/boot/dts/lpc4357-ea4357-devkit.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/lpc4357-ea4357-devkit.dts b/arch/arm/boot/dts/lpc4357-ea4357-devkit.dts
index a211b6e..9b5fad6 100644
--- a/arch/arm/boot/dts/lpc4357-ea4357-devkit.dts
+++ b/arch/arm/boot/dts/lpc4357-ea4357-devkit.dts
@@ -33,7 +33,7 @@
 		stdout-path = &uart0;
 	};
 
-	memory {
+	memory at 28000000 {
 		device_type = "memory";
 		reg = <0x28000000 0x2000000>; /* 32 MB */
 	};
@@ -62,31 +62,31 @@
 		poll-interval = <100>;
 		autorepeat;
 
-		button at 0 {
+		button0 {
 			label = "joy_enter";
 			linux,code = <KEY_ENTER>;
 			gpios = <&gpio LPC_GPIO(4,8) GPIO_ACTIVE_LOW>;
 		};
 
-		button at 1 {
+		button1 {
 			label = "joy_left";
 			linux,code = <KEY_LEFT>;
 			gpios = <&gpio LPC_GPIO(4,9) GPIO_ACTIVE_LOW>;
 		};
 
-		button at 2 {
+		button2 {
 			label = "joy_up";
 			linux,code = <KEY_UP>;
 			gpios = <&gpio LPC_GPIO(4,10) GPIO_ACTIVE_LOW>;
 		};
 
-		button at 3 {
+		button3 {
 			label = "joy_right";
 			linux,code = <KEY_RIGHT>;
 			gpios = <&gpio LPC_GPIO(4,12) GPIO_ACTIVE_LOW>;
 		};
 
-		button at 4 {
+		button4 {
 			label = "joy_down";
 			linux,code = <KEY_DOWN>;
 			gpios = <&gpio LPC_GPIO(4,13) GPIO_ACTIVE_LOW>;
@@ -584,7 +584,7 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&spifi_pins>;
 
-	flash at 0 {
+	flash {
 		compatible = "jedec,spi-nor";
 		spi-cpol;
 		spi-cpha;
-- 
2.7.4

[PATCH 5/7] ARM: dts: lpc4350-hitex-eval: fix unit name warnings from dtc

[Joachim Eastwood]

Fix the following warnings from dtc by either adding or removing
the unit name from the node.

Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
Warning (unit_address_vs_reg): Node /soc/flash-controller at 40003000/flash at 0 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 0 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 2 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 3 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 4 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 5 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 6 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /pca_buttons/button at 7 has a unit name, but no reg property

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/boot/dts/lpc4350-hitex-eval.dts | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/lpc4350-hitex-eval.dts b/arch/arm/boot/dts/lpc4350-hitex-eval.dts
index 6d0c698..cb65352 100644
--- a/arch/arm/boot/dts/lpc4350-hitex-eval.dts
+++ b/arch/arm/boot/dts/lpc4350-hitex-eval.dts
@@ -33,7 +33,7 @@
 		stdout-path = &uart0;
 	};
 
-	memory {
+	memory at 28000000 {
 		device_type = "memory";
 		reg = <0x28000000 0x800000>; /* 8 MB */
 	};
@@ -45,50 +45,50 @@
 		poll-interval = <100>;
 		autorepeat;
 
-		button at 0 {
+		button0 {
 			label = "joy:right";
 			linux,code = <KEY_RIGHT>;
 			gpios = <&pca_gpio 8 GPIO_ACTIVE_LOW>;
 		};
 
-		button at 1 {
+		button1 {
 			label = "joy:up";
 			linux,code = <KEY_UP>;
 			gpios = <&pca_gpio 9 GPIO_ACTIVE_LOW>;
 		};
 
 
-		button at 2 {
+		button2 {
 			label = "joy:enter";
 			linux,code = <KEY_ENTER>;
 			gpios = <&pca_gpio 10 GPIO_ACTIVE_LOW>;
 		};
 
-		button at 3 {
+		button3 {
 			label = "joy:left";
 			linux,code = <KEY_LEFT>;
 			gpios = <&pca_gpio 11 GPIO_ACTIVE_LOW>;
 		};
 
-		button at 4 {
+		button4 {
 			label = "joy:down";
 			linux,code = <KEY_DOWN>;
 			gpios = <&pca_gpio 12 GPIO_ACTIVE_LOW>;
 		};
 
-		button at 5 {
+		button5 {
 			label = "user:sw3";
 			linux,code = <KEY_F1>;
 			gpios = <&pca_gpio 13 GPIO_ACTIVE_LOW>;
 		};
 
-		button at 6 {
+		button6 {
 			label = "user:sw4";
 			linux,code = <KEY_F2>;
 			gpios = <&pca_gpio 14 GPIO_ACTIVE_LOW>;
 		};
 
-		button at 7 {
+		button7 {
 			label = "user:sw5";
 			linux,code = <KEY_F3>;
 			gpios = <&pca_gpio 15 GPIO_ACTIVE_LOW>;
@@ -453,7 +453,7 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&spifi_pins>;
 
-	flash at 0 {
+	flash {
 		compatible = "jedec,spi-nor";
 		spi-rx-bus-width = <4>;
 		#address-cells = <1>;
-- 
2.7.4

[PATCH 6/7] ARM: dts: lpc4337-ciaa:: fix unit name warning from dtc

[Joachim Eastwood]

Fix the following warning from dtc by either adding a unit name
to the memory node.

Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 arch/arm/boot/dts/lpc4337-ciaa.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/lpc4337-ciaa.dts b/arch/arm/boot/dts/lpc4337-ciaa.dts
index 5cfadb0..7c16d63 100644
--- a/arch/arm/boot/dts/lpc4337-ciaa.dts
+++ b/arch/arm/boot/dts/lpc4337-ciaa.dts
@@ -30,7 +30,7 @@
 		stdout-path = &uart2;
 	};
 
-	memory {
+	memory at 28000000 {
 		device_type = "memory";
 		reg = <0x28000000 0x0800000>; /* 8 MB */
 	};
-- 
2.7.4

[PATCH 7/7] dt-bindings: phy-lpc18xx-usb-otg: remove unit address from binding

[Joachim Eastwood]

DT bindings should either use a unit address and a reg property
or none of them. Since either of them aren't really useful for
such a simple device like this one remove the unit address.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/phy/phy-lpc18xx-usb-otg.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-lpc18xx-usb-otg.txt b/Documentation/devicetree/bindings/phy/phy-lpc18xx-usb-otg.txt
index bd61b46..3bb821c 100644
--- a/Documentation/devicetree/bindings/phy/phy-lpc18xx-usb-otg.txt
+++ b/Documentation/devicetree/bindings/phy/phy-lpc18xx-usb-otg.txt
@@ -18,7 +18,7 @@ creg: syscon at 40043000 {
 	compatible = "nxp,lpc1850-creg", "syscon", "simple-mfd";
 	reg = <0x40043000 0x1000>;
 
-	usb0_otg_phy: phy at 004 {
+	usb0_otg_phy: phy {
 		compatible = "nxp,lpc1850-usb-otg-phy";
 		clocks = <&ccu1 CLK_USB0>;
 		#phy-cells = <0>;
-- 
2.7.4