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. > > > 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. > > 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. > 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? Thanks. -- Dmitry -- 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
