Hi On Tue, Aug 30, 2016 at 10:58 PM, Rob Herring <robh+dt@xxxxxxxxxx> wrote: > On Tue, Aug 30, 2016 at 2:30 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: >> Sure, all those functions are not meant to be called in parallel by >> multiple tasks. They are rather meant to have a single holder which >> preferably is the one instantiating and destroying the >> node/device/foobar. But the framebuffer eviction code somehow needs to >> trigger the removal, and thus needs some hook, that can be called in >> parallel by any driver that is loaded. >> >> I can make sure the simple-framebuffer eviction code is locked >> properly. That'll work, for now. But if someone ends up providing >> simple-framebuffer devices via overlays or any other dynamic >> mechanism, they will race the removal. > > No doubt someone will come up with some usecase wanting to do that, > but IMO that is not a supported usecase and should not be. > simple-framebuffer is for providing a firmware setup framebuffer. Sure. Any sensible simple-framebuffer use would follow what we have now. But it feels wrong to me that if some node is added that just happens to have "simple-framebuffer" as name, suddenly things will go wrong. I mean, yeah, DT is not a userspace API, but I still would like the code to catch misuses rather than fail. It is an API after all. Or is that being overly pedantic? >> And it will be completely >> non-obvious to them. I really don't want to be responsible to have >> submitted that code. If anyone cares for proper device hand-over on >> ARM, please come up with an idea to fix it. Otherwise, I will just >> limit the simpledrm code to x86. >> >> IOW the device handover code somehow needs to know who was responsible >> for the instantiation of the simple-framebuffer device, so it can tell >> them to remove it again. On x86 there is only one place where those >> can be instantiated. But on OF-based systems, it can be dynamically >> instantiated in many places right now. > > What do you mean by all over the place? It is only in simplefb_init > ATM. I haven't looked at what simpledrm is doing, but we can move the > device creation to of_platform_default_populate_init if we need a > single spot. Currently I see at least 3 paths that might add such nodes: - of_platform_populate() - of_node_attach() (via the notifier) - simplefb_init() 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? 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
