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. > 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? Because everybody else does differently ? If you have things like NUMA configurations where some memory ranges pertain to different nodes, you need a memory node per NUMA node so you can add the other node-local properties there. The above should have been memory@0 The best way to fix this in a backward compatible manner is to once and for all for our implementation of path-lookup to be able to work with partial path components like a real OFW does, ie, name only, unit address only, or both. In anycase, just "/memory" will break on at least powerpc. Ben. > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/