On Mon, 20 Jun 2011 01:45:12 -0700 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > On Mon, Jun 20, 2011 at 09:48:15AM +0200, David Jander wrote: > > On Sat, 18 Jun 2011 09:16:45 -0600 > > Grant Likely <grant.likely@xxxxxxxxxxxx> wrote: > > > > > On Sat, Jun 18, 2011 at 07:51:54AM -0700, Dmitry Torokhov wrote: > > > > On Sat, Jun 18, 2011 at 07:18:28AM -0600, Grant Likely wrote: > > > > > On Sat, Jun 18, 2011 at 4:17 AM, Dmitry Torokhov > > > > > <dmitry.torokhov@xxxxxxxxx> wrote: > > > > > > On Thu, Jun 16, 2011 at 01:27:32PM -0600, Grant Likely wrote: > > > > > >> On Tue, Jun 14, 2011 at 11:08:11AM +0200, David Jander wrote: > > > > > >> > Use a threaded interrupt handler in order to permit the handler > > > > > >> > to use a GPIO driver that causes things like I2C transactions > > > > > >> > being done inside the handler context. > > > > > >> > Also, gpio_keys_init needs to be declared as a late_initcall, to > > > > > >> > make sure all needed GPIO drivers have been loaded if the > > > > > >> > drivers are built into the kernel. > > > > > >> > > > > > >> ...which is a horrid hack, but until device dependencies can be > > > > > >> described, it isn't one that can be solved easily. > > > > > >> > > > > > > > > > > > > I really do not want to apply this... Currently the order of > > > > > > initialization does not matter since nothing actually happens until > > > > > > corresponding device appears on the bus. Does the OF code creates > > > > > > devices before all resources are ready? > > > > > > > > > > It's not an OF problem. The problem is that all the platform_devices > > > > > typically get registered all at once at machine_init time (on arm), > > > > > and if the gpio expander isn't a platform_device, (like an i2c gpio > > > > > expander which would end up being a child of a platform_device), then > > > > > it won't be ready. > > > > > > > > Ah, I see. But that can be handled in board code that should ensure > > > > that it registers devices in correct order. > > > > > > Unfortunately, handling it in board code doesn't really work either. > > > It just shuffles the complexity to the board code to implement some > > > kind of deferred mechanism for registering devices, and it has to take > > > into account that it may be a long time before the device actually > > > appears, such as when the driver is configured as a module. > > > > Besides... we don't want anymore board-code, do we? I mean, if a board can > > use a generic board configuration and specify all it needs in the > > device-tree, why should something as trivial as connecting a gpio_keys > > device to a I2C GPIO expander force us to do special board setup all of a > > sudden? IMHO specifying I2C-gpios to be used for gpio_keys should "just > > work", even if declared in a device-tree. > > This is a laudable goal, but then device-tree needs to be able to > express device dependencies better. Until then board-specific code is > needed to register devices in proper order. Hmmm, I am not an expert in OF/DT stuff, but I think that while it would theoretically be possible to add extra properties to the tree, that are handled by the of_platform code, I am not sure if that is an option, since that would be pretty much linux-specific, and could never work on another OS. Grant? > > > I completely agree that shuffling initcall order isn't maintainable > > > though. > > > > I also agree, and if there is a better solution to make this work without > > additional board-support code, please tell me. > > I just think that this patch makes the already cool gpio_keys driver quite > > a bit more awesome. IMO, being able to just hook it all up in the > > device-tree is just fantastic, and we should make it possible. > > > > > A related concern is that changing the device registration order, or > > > the initcall order, does absolutely nothing to tell runtime PM about > > > the dependencies between devices. For instance, how does runtime PM > > > know when it is safe to PM a gpio controller, when it has no reference > > > to devices depending on it, like gpio-keys? (although gpio-keys isn't > > > a great example because it doesn't really have any runtime PM states). > > > > > > I think part of the solution is to give drivers the option of > > > returning a 'defer' code at probe time if it cannot obtain all it's > > > resources, and have the driver core re-probe it when more devices > > > become available, but I haven't had time to prototype it yet. > > > > Sounds interesting. So the probe function could return some sort of > > -ENOTYET or -EAGAIN and have it called again later? > > How about we do not register device until all resources are ready? This > is pretty simple concept - do not create an object until it is usable. Then > nobody needs to bother with -EAGAIN or -ENOTYET or any other similar > garbage. I agree, but DT doesn't permit that (yet). > > But, does that mean that we really need to miss this use-case until > > something like this gets approved and merged? Can't we just declare this > > late_initcall for now and fix it later? Please! > > > > > > > The real problem is that we have no mechanism for > > > > > holding off or deferring a driver probe if it depends on an > > > > > asynchronous resource. > > > > > > > > The mechanism we do have - we should not be creating the device for the > > > > driver to bind to unless all resources that are needed by that device > > > > are ready. > > > > How would we do that in a device-tree? > > > > > > Just shuffling the initcall order is not maintanable. Next there will > > > > be GPIO expander that is for some reason registered as late_initcall > > > > and we'll be back to square one. I am going to take the threaded IRQ > > > > bit but will drop the initcall bit from the patch. > > > > That would destroy the whole purpose of this patch. > > No, it is still useful as it will allow using the driver with GPIOs > accessed over a slow bus. Ok, that's true. Problem is that such slow busses (usually I2C or SPI) most probably have this dependency problem also, so it is a general problem that needs a solution. > > Do you mean to say, what I > > want to do has no acceptable implementation? That would be a pity, since > > IMHO it is a very cool feature, and quite trivial to implement this way. > > Our boards do not need any board setup code. Actually just adding one > > line of code in arch/powerpc/platforms/512x/mpc5121_generic.c or > > arch/powerpc/platforms/52xx/mpc5200_simple.c is enough to support any of > > our boards that need this driver... the rest is done in the device-tree. > > Don't you think this is worth that little bit of (temporary) ugliness? > > Turning the question around, can you add secondary device tree traversal > for gpio_keys to your board code and keep the ugliness there until > device tree can better express dependencies between resources? What do you think, Grant? Would it be possible/acceptable to add some special property to devices that could make them be added in a second round by of_platform code (until there are _real_ dependencies)? Or could the of_platform code be smart and just notice that gpio_keys needs "gpios" (or other resources for that matter) that are depending on another node in the tree, and make sure it gets probed before adding this one? I just don't want to give up on that feature now... besides, now that ARM will hopefully also adopt OF/DT, more and more drivers will need to be adapted, and this problem will repeat more sooner than later I guess. Thanks a lot. Best regards, -- David Jander Protonic Holland. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
