linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/7] ARM: dts: skeleton: add unit name to memory node
Date: Wed, 30 Mar 2016 18:06:46 +0100	[thread overview]
Message-ID: <20160330170645.GA13690@leverpostej> (raw)
In-Reply-To: <CAGhQ9VykiwMWaw3xcJdg2CahNFt+sp7tFC5O8VwA=-_=UoJ8kA@mail.gmail.com>

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.

  reply	other threads:[~2016-03-30 17:06 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29 22:30 [PATCH 0/7] fix unit name warnings on lpc18xx Joachim Eastwood
2016-03-29 22:30 ` [PATCH 1/7] ARM: dts: armv7-m: add unit name to interrupt-controller Joachim Eastwood
2016-04-04  6:05   ` Uwe Kleine-König
2016-04-04  6:42     ` Joachim Eastwood
2016-04-04  6:51       ` Stefan Agner
2016-04-04  7:30       ` Uwe Kleine-König
2016-03-29 22:30 ` [PATCH 2/7] ARM: dts: skeleton: add unit name to memory node Joachim Eastwood
2016-03-30 10:12   ` Vladimir Zapolskiy
2016-03-30 11:06   ` Mark Rutland
2016-03-30 12:36     ` Joachim Eastwood
2016-03-30 13:06     ` Vladimir Zapolskiy
2016-03-30 13:41       ` Mark Rutland
2016-03-30 16:15         ` Joachim Eastwood
2016-03-30 17:06           ` Mark Rutland [this message]
2016-03-30 20:45             ` Joachim Eastwood
2016-03-31 10:38               ` Mark Rutland
2016-03-31 15:21                 ` Joachim Eastwood
2016-03-31 16:34                   ` Joachim Eastwood
2016-04-12 22:45                     ` Rob Herring
2016-04-12 23:03                       ` Joachim Eastwood
2016-03-31 16:27           ` Rob Herring
2016-03-31 16:31             ` Rob Herring
2016-03-31 16:33               ` Joachim Eastwood
2016-03-29 22:30 ` [PATCH 3/7] ARM: dts: lpc18xx: remove unit addresses from creg childs Joachim Eastwood
2016-03-29 22:30 ` [PATCH 4/7] ARM: dts: lpc4357-ea4357: fix unit name warnings from dtc Joachim Eastwood
2016-03-29 22:30 ` [PATCH 5/7] ARM: dts: lpc4350-hitex-eval: " Joachim Eastwood
2016-03-29 22:30 ` [PATCH 6/7] ARM: dts: lpc4337-ciaa:: fix unit name warning " Joachim Eastwood
2016-03-29 22:30 ` [PATCH 7/7] dt-bindings: phy-lpc18xx-usb-otg: remove unit address from binding Joachim Eastwood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160330170645.GA13690@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).