From: Tomasz Figa <tomasz.figa@gmail.com>
To: Olof Johansson <olof@lixom.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Stephen Warren <swarren@wwwdotorg.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Linux Kernel list <linux-kernel@vger.kernel.org>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Stephen Warren <swarren@nvidia.com>,
Rob Herring <rob.herring@calxeda.com>,
Grant Likely <grant.likely@secretlab.ca>
Subject: Re: "memory" binding issues
Date: Tue, 17 Sep 2013 09:56:39 +0200 [thread overview]
Message-ID: <1815499.tpfeUKmP6V@flatron> (raw)
In-Reply-To: <CAOesGMgE1jiF5OHsmPQz2z66Bcqu0HeAj=taE1vmFK-0-0s2TQ@mail.gmail.com>
On Monday 16 of September 2013 15:48:22 Olof Johansson wrote:
> On Mon, Sep 16, 2013 at 3:46 PM, Benjamin Herrenschmidt
>
> <benh@kernel.crashing.org> wrote:
> > On Mon, 2013-09-16 at 10:17 -0600, Stephen Warren wrote:
> >> On 09/15/2013 08:57 PM, Benjamin Herrenschmidt wrote:
> >> > [resent to the right list this time around]
> >> >
> >> > Hi folks !
> >> >
> >> > So I don't have the bandwidth to follow closely what's going on,
> >> > but I
> >> > just today noticed the crackpot that went into 3.11 as part of
> >> > commit:
> >> >
> >> > 9d8eab7af79cb4ce2de5de39f82c455b1f796963
> >> > drivers: of: add initialization code for dma reserved memory
> >> >
> >> > Fist of all, do NOT add (or change) a binding as part of a patch
> >> > implementing code, it's gross.
> >>
> >> Personally, I would argue the opposite; it's much easier to see
> >> what's
> >> going on when it's all together in one patch.
> >
> > One patch series eventually, but not the same patch.
> >
> >> Ensuring ABI stability can
> >> only be achieved through code review, i.e. splitting into separate
> >> DT/code patches won't achieve that, so that argument doesn't affect
> >> this.
> >>
> >> ...
> >>
> >> > Additionally, it has the following issues:
> >> > - It describes the "memory" node as /memory, which is WRONG
> >> >
> >> > It should be "/memory@unit-address, this is important because the
> >> > Linux
> >> > kernel of_find_device_by_path() isn't smart enough to do partial
> >> > searches (unlike the real OFW one) and thus to ignore the unit
> >> > address
> >> > for search purposes, and you *need* the unit address if you have
> >> > multiple memory nodes (which you typically do on NUMA machines).
> >>
> >> Perhaps /memory should have had a unit-address, but it never has had
> >> on
> >
> >> ARM; see arch/arm/boot/dts/skeleton.dtsi which says:
> > Well, this is a mistake ARM folks might have done from day one but it
> > should still be fixed :-)
> >
> > A node that has a "reg" property should have the corresponding unit
> > address.
>
> No, absolutely _NOT_ a requirement. Unit address is only required if
> needed to disambiguate two properties with the same name.
>
> If there are no ambiguities, then leaving off the unit address is much
> preferred.
I'm afraid that I must disagree. For consistency I'd rather go with what
Ben said. Please see ePAPR chapter 2.2.1.1, which clearly defines how
nodes should be named.
Having unit-address whenever the node has a reg property has the nice
property of eliminating the need to rename any nodes when adding new one.
(Consider the case that you have one subnode somewhere and you omit the
unit-address and then you find out that you have to add another subnode
with the same name, but another reg value.)
Best regards,
Tomasz
next prev parent reply other threads:[~2013-09-17 7:56 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-16 2:57 "memory" binding issues Benjamin Herrenschmidt
2013-09-16 15:22 ` Kumar Gala
2013-09-16 22:42 ` Benjamin Herrenschmidt
2013-09-17 15:53 ` Kumar Gala
2013-09-16 16:17 ` Stephen Warren
2013-09-16 22:46 ` Benjamin Herrenschmidt
2013-09-16 22:48 ` Olof Johansson
2013-09-16 23:47 ` Benjamin Herrenschmidt
2013-09-16 23:48 ` Olof Johansson
2013-09-17 1:37 ` Benjamin Herrenschmidt
2013-09-17 7:56 ` Tomasz Figa [this message]
2013-09-17 16:43 ` Olof Johansson
2013-09-17 21:08 ` Frank Rowand
2013-09-17 21:15 ` Olof Johansson
2013-09-17 21:19 ` Tomasz Figa
2013-09-17 21:33 ` Olof Johansson
2013-09-17 23:04 ` Benjamin Herrenschmidt
2013-09-17 23:25 ` Olof Johansson
2013-09-17 21:56 ` Benjamin Herrenschmidt
2013-09-18 16:28 ` Stephen Warren
2013-09-19 0:29 ` David Gibson
2013-09-18 1:25 ` David Gibson
2013-09-18 1:31 ` Grant Likely
2013-09-18 1:38 ` Grant Likely
2013-09-18 8:08 ` Benjamin Herrenschmidt
2013-09-18 2:57 ` Grant Likely
2013-09-18 8:21 ` Benjamin Herrenschmidt
2013-09-27 15:42 ` Kumar Gala
2013-10-03 15:04 ` Kumar Gala
-- strict thread matches above, loose matches on Subject: below --
2013-09-16 2:41 Benjamin Herrenschmidt
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=1815499.tpfeUKmP6V@flatron \
--to=tomasz.figa@gmail.com \
--cc=benh@kernel.crashing.org \
--cc=devicetree@vger.kernel.org \
--cc=grant.likely@secretlab.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=olof@lixom.net \
--cc=rob.herring@calxeda.com \
--cc=swarren@nvidia.com \
--cc=swarren@wwwdotorg.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).