On Tue, Nov 13, 2012 at 8:09 AM, Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> wrote: > On Nov 13, 2012, at 9:25 AM, David Gibson wrote: > Not good to rely on userspace kicking off dtc and compiling from source. > Some capes/expansion boards might have your root fs device, for example > there is an eMMC cape coming up, while networking capes are common too. > > However I have a compromise. > > I agree that compiling from source can be an option for a runtime definable > cape, but for built-in capes we could do a bit better. > > In particular, I don't see any particular need to have a DT fragment > reference anything that dependent of the runtime device tree. It should > be possible to compile the DT fragment in kernel, against the generated > flattened device tree, producing a flattened DT fragment with the phandles > already resolved. Do you mean linking dtc into the kernel? > So the sequence could be something like this: > > $ dtc -O dtb -o am335x-bone.dtb -b 0 am335x-bone.dts -@ ${LAST_PHANDLE_FILE} > $ dtc -O dtbf -R am335x-bone.dtb -o weather-cape.dtb -b 0 weather-cape.dts -@ ${LAST_PHANDLE_FILE} > $ dtc -O dtbf -R am335x-bone.dtb -o geiger-cape.dtb -b 0 geiger-cape.dts -@ ${LAST_PHANDLE_FILE} > > The ${LAST_PHANDLE_FILE} can be updated with the last phandle value generated. > > Alternatively we could have a way to statically assign a phandle range > for well known capes. All the others will have to use the runtime compile > mechanism. > $ dtc -O dtb -o am335x-bone.dtb -b 0 am335x-bone.dts > $ dtc -O dtbf -R am335x-bone.dtb -o weather-cape.dtb -b 0 weather-cape.dts > $ dtc -O dtbf -R am335x-bone.dtb -o geiger-cape.dtb -b 0 geiger-cape.dts > > With the cape dtses having a /phandle-range/ statement at the top. I really think the whole phandle allocation thing is a non problem. Generating phandles is the easy part, and using a good hash function pretty much solves it entirely. I know people are worried about collisions, but there are only two scenarios where collisions may happen: 1) base and overlay try to define same phandle - Easy to detect - kernel can complain and refuse to load if it happens - Will never happen when overlay is compiled against the base used to boot (dtc can resolve the conflict) - Hash phandle generation makes this exceptionally rare 2) two overlays try to define same phandle - Also easy to detect on loading the second overlay - kernel will - Hash phandle generation still makes this exceptionally rare In both cases a collision is extremely rare and if it does happen it will not fail silently. Besides, in the odd scenario where it does happen, a node can be manually assigned a phandle. It is far better to do the manual override maybe once or twice (actually, I'd be surprised if it is ever needed) than to get into any kind of numberspace management for phandles. That's just a maintenance nightmare. The hard bit to solve has always been when the overlay expects different phandles than the base is using, and that problem only occurs if the overlay is compiled against a different base dtb than was used to boot the system. Hashed phandle generation even makes phandles very stable when the base dtb is recompiled, and if they do change, then the kernel can detect it and report an error. Again, no silent failures. Phandle fixup does make the problem disappears entirely but I'm concerned that it is still incomplete (see below) and would be conceptually expensive. Once of the requests is to support one overlay .dtb with many different baseboards, but now that I've thought about it more I realize that it just won't work for anything beyond the most trivial case. Phandle fixup isn's sufficient. The format of GPIO, Interrupt, clock, etc specifiers may change if the base board nodes change. The specifiers are not portable. So, I no longer think that plain dtb overlays alone will work against any base board. Something more is needed. It may be the mechanism for loading new data into the kernel, but something really generic does require either being compiled at runtime (as David suggests) or each expansion interface (ie. the BeagleBone expansion or the RPi expansion) to really specify what kinds of references are allowed and run all specifiers through translation nodes. Similar to how interrupt-map works. > i2c2: i2c@4819c000 { > compatible = "ti,omap4-i2c"; > #address-cells = <1>; > #size-cells = <0>; > ti,hwmods = "i2c3"; > reg = <0x4819c000 0x1000>; > interrupt-parent = <&intc>; > interrupts = <30>; > status = "disabled"; > }; > > And in the cape definition (when compiled with the special mode I describe > below) > > / { > plugin-bundle; > compatible = "cco,weather-cape"; > version = <00A0>; > > i2c2-graft = { > compatible = <dt,graft>; > graft-point = <&i2c2>; Since overlays are different, we can interpret them slightly differently. The node name itself could be the graft point, and the name could be either a full path "/foo/bar/i2c@10002000" or an alias reference "i2c2". I think that is a bit better than a new graft-point property. g. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html