Hey Rob! On Wed, Aug 31, 2016 at 1:01 AM, Rob Herring <robh+dt@xxxxxxxxxx> wrote: > On Tue, Aug 30, 2016 at 4:12 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: >> Currently I see at least 3 paths that might add such nodes: >> >> - of_platform_populate() > > If we assume the node is only under chosen, then that would require a > call to of_platform_populate with /chosen as the root which shouldn't > happen. More generally the assumption is of_platform_populate is only > called once from for the root node and only on sub-trees by the driver > of sub-tree's root device. I see. >> - of_node_attach() (via the notifier) > > A node getting attached would be harmless. Nothing really would happen > until a handler calls of_platform_populate. So this is the same as the > 1st case. drivers/of/platform.c has a notifier callback and registers the platform devices when added. It has a check that the parent is a populated bus, which I guess is what you refer to? >> - simplefb_init() > > Actually, I'd prefer to move it out of there and into the core. I > don't think drivers should look at /chosen and only the core and arch > code should. Of course, the only enforcing of that is policy. For > overlays though, there's been some discussion on limiting where > overlays can be applied. > >> Should I just ignore anything but simplefb_init()? I understand that >> it's the only one used by normal code-paths, but isn't it kinda ugly >> to silently introduce race conditions if a node just happens to be >> introduced via one of the other methods? Or are errors in the DT not >> meant to be caught? > > In general there's no limit to how wrong a DT could be. I could write > a DT that causes every DT enabled driver in the kernel to probe (that > would be an interesting test case). The kernel is limited in knowing > what is correct, and the whole point of DT is to move that information > out of the kernel. This is case is just one compatible string out of > thousands and location in the tree is just one thing to check. > > This is a major reason why there is not yet a userspace interface for > applying DT overlays as who knows what random crap could be in the DT. > We're nervous about what could happen from frying h/w to creating > security holes. Evidently the ACPI folks were not so nervous and added > an interface. I see. Thanks a lot for the clarifications! I will put the of_platform_device_destroy() right next to the x86 handler, making sure it is properly locked. I'm not so sure about the of_chosen instantiation, though. x86 has the instantiation in arch/x86/kernel/sysfb_simplefb.c, so maybe an equivalent for arm would be fine? Thanks David -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
