From: Stephen Warren <swarren@wwwdotorg.org> To: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: devicetree@vger.kernel.org, Linux Kernel list <linux-kernel@vger.kernel.org>, m.szyprowski@samsung.com, swarren@nvidia.com, rob.herring@calxeda.com, Grant Likely <grant.likely@secretlab.ca> Subject: Re: "memory" binding issues Date: Mon, 16 Sep 2013 10:17:46 -0600 [thread overview] Message-ID: <52372F2A.2050003@wwwdotorg.org> (raw) In-Reply-To: <1379300274.4098.77.camel@pasglop> 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. 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: memory { device_type = "memory"; reg = <0 0>; }; ... and the fact that reg in /memory can have multiple entries seems to support the expectation we only have a single node here. I'm not sure how we could possibly change this now it's become so entrenched?
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> To: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux Kernel list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org, swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> Subject: Re: "memory" binding issues Date: Mon, 16 Sep 2013 10:17:46 -0600 [thread overview] Message-ID: <52372F2A.2050003@wwwdotorg.org> (raw) In-Reply-To: <1379300274.4098.77.camel@pasglop> 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. 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: memory { device_type = "memory"; reg = <0 0>; }; ... and the fact that reg in /memory can have multiple entries seems to support the expectation we only have a single node here. I'm not sure how we could possibly change this now it's become so entrenched? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2013-09-16 16:17 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-09-16 2:57 "memory" binding issues Benjamin Herrenschmidt 2013-09-16 2:57 ` Benjamin Herrenschmidt 2013-09-16 15:22 ` Kumar Gala 2013-09-16 15:22 ` Kumar Gala 2013-09-16 22:42 ` Benjamin Herrenschmidt 2013-09-17 15:53 ` Kumar Gala 2013-09-17 15:53 ` Kumar Gala 2013-09-16 16:17 ` Stephen Warren [this message] 2013-09-16 16:17 ` Stephen Warren 2013-09-16 22:46 ` Benjamin Herrenschmidt 2013-09-16 22:46 ` Benjamin Herrenschmidt 2013-09-16 22:48 ` Olof Johansson 2013-09-16 22:48 ` Olof Johansson 2013-09-16 23:47 ` Benjamin Herrenschmidt 2013-09-16 23:47 ` Benjamin Herrenschmidt 2013-09-16 23:48 ` Olof Johansson 2013-09-17 1:37 ` Benjamin Herrenschmidt 2013-09-17 1:37 ` Benjamin Herrenschmidt 2013-09-17 7:56 ` Tomasz Figa 2013-09-17 7:56 ` Tomasz Figa 2013-09-17 16:43 ` Olof Johansson 2013-09-17 16:43 ` Olof Johansson 2013-09-17 21:08 ` Frank Rowand 2013-09-17 21:08 ` Frank Rowand 2013-09-17 21:15 ` Olof Johansson 2013-09-17 21:15 ` Olof Johansson 2013-09-17 21:19 ` Tomasz Figa 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:04 ` Benjamin Herrenschmidt 2013-09-17 23:25 ` Olof Johansson 2013-09-17 23:25 ` Olof Johansson 2013-09-17 21:56 ` Benjamin Herrenschmidt 2013-09-17 21:56 ` Benjamin Herrenschmidt 2013-09-18 16:28 ` Stephen Warren 2013-09-18 16:28 ` Stephen Warren 2013-09-19 0:29 ` David Gibson 2013-09-19 0:29 ` David Gibson 2013-09-18 1:25 ` David Gibson 2013-09-18 1:25 ` David Gibson 2013-09-18 1:31 ` Grant Likely 2013-09-18 1:31 ` Grant Likely 2013-09-18 1:38 ` Grant Likely 2013-09-18 8:08 ` Benjamin Herrenschmidt 2013-09-18 8:08 ` Benjamin Herrenschmidt 2013-09-18 2:57 ` Grant Likely 2013-09-18 2:57 ` Grant Likely 2013-09-18 8:21 ` Benjamin Herrenschmidt 2013-09-18 8:21 ` Benjamin Herrenschmidt 2013-09-27 15:42 ` Kumar Gala 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=52372F2A.2050003@wwwdotorg.org \ --to=swarren@wwwdotorg.org \ --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=rob.herring@calxeda.com \ --cc=swarren@nvidia.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.