This patch adds functionality to the gpio-lib subsystem to make it possible to enable the gpio-lib code even if the architecture code didn't request to get it built in. The archtitecture code does still need to implement the gpiolib accessor functions in its asm/gpio.h file. This patch adds the implementations for x86 and PPC. With these changes it is possible to run generic GPIO expansion cards on every architecture that implements the trivial wrapper functions. Support for more architectures can easily be added. Signed-off-by: Michael Buesch <mb@bu3sch.de> --- This patch should be scheduled for the next kernel feature merge window. Index: linux-2.6/arch/x86/Kconfig =================================================================== --- linux-2.6.orig/arch/x86/Kconfig 2008-07-02 23:12:59.000000000 +0200 +++ linux-2.6/arch/x86/Kconfig 2008-07-02 23:15:33.000000000 +0200 @@ -25,6 +25,7 @@ config X86 select HAVE_KRETPROBES select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64) select HAVE_ARCH_KGDB if !X86_VOYAGER + select ARCH_WANT_OPTIONAL_GPIOLIB if !X86_RDC321X config ARCH_DEFCONFIG string Index: linux-2.6/include/asm-x86/gpio.h =================================================================== --- linux-2.6.orig/include/asm-x86/gpio.h 2008-07-02 23:12:59.000000000 +0200 +++ linux-2.6/include/asm-x86/gpio.h 2008-07-02 23:31:42.000000000 +0200 @@ -1,6 +1,62 @@ +/* + * Generic GPIO API implementation for x86. + * + * Derived from the generic GPIO API for powerpc: + * + * Copyright (c) 2007-2008 MontaVista Software, Inc. + * + * Author: Anton Vorontsov <avorontsov@ru.mvista.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + #ifndef _ASM_I386_GPIO_H #define _ASM_I386_GPIO_H +#ifdef ...
On Wed, 2 Jul 2008 23:46:53 +0200 drivers/gpio/gpiolib.c: In function 'gpio_export': drivers/gpio/gpiolib.c:432: error: 'struct class' has no member named 'devices' drivers/gpio/gpiolib.c:456: error: implicit declaration of function 'device_create' drivers/gpio/gpiolib.c:457: warning: assignment makes pointer from integer without a cast drivers/gpio/gpiolib.c: In function 'gpio_unexport': drivers/gpio/gpiolib.c:509: warning: passing argument 2 of 'class_find_device' from incompatible pointer type drivers/gpio/gpiolib.c:509: error: too few arguments to function 'class_find_device' drivers/gpio/gpiolib.c: In function 'gpiochip_export': drivers/gpio/gpiolib.c:536: error: 'struct class' has no member named 'devices' drivers/gpio/gpiolib.c:542: warning: assignment makes pointer from integer without a cast drivers/gpio/gpiolib.c: In function 'gpiochip_unexport': drivers/gpio/gpiolib.c:575: warning: passing argument 2 of 'class_find_device' from incompatible pointer type drivers/gpio/gpiolib.c:575: error: too few arguments to function 'class_find_device' I assume this patch was prepared against some ancient out-of-date kernel such as current Linus mainline. Guys, we have a new development tree now. --
Oh well. Let me diff it against linux-next then. I didn't know Linus' tree doesn't get any development updates anymore in time and so is -- Greetings Michael. --
May be, but shuffling headers around would not have caused that type of breakage. I'm thinking some driver model changes broke the gpio sysfs interface code, and this happens to show up right now because that code wasn't previously getting built. Grumph. I can easily switch the device_create() over to use device_create_drvdata() -- didn't I already send in a patch like that? -- but the other stuff is completely backwards-incompatible. --
beats me ididntdoitnobodysawmedoit. But what I am repeatedly seeing is people cheerfully raising 2.6.27 patches against the 2.6.26 tree when we have a nice 2.6.27 tree for developing against. Those days are over, guys. I'm also seeing obvious signs that developers aren't _testing_ their new code within the context of the 2.6.27 tree. They're obviously testing their stuff against 2.6.26 and then hoping and praying, only it doesn't always work out for them. --
In this case, the contract seems to have changed. Previously, it was that folk doing the API changes would change everyone using the APIs ... in this case, that's not happened. (And I can sort of understand why. No matter how it's done, someone As they most certainly should. That's already a lot of variables in the test process, and anyone wanting to actually *USE* that code right now will probably not want The new "-next" process is still working out. Backwards-incompatible changes will *always* make problems. In this case there seem to have been three such changes: - device_create() vanishing, even for users which did the locking correctly ... this was at least something of a graceful migration, although it would have been better if it were deprecated first. Change can work against the current (2.6.26) code base. - class.devices vanishing ... what I really needed was more of a "is this class initialized yet" call, so testing for class->devices.next non-null can be replaced by a test for class->p non-null. (Or maybe this should be viewed as needing a "real" driver model call, so that code which must run before driver model init can more easily cooperate with driver model stuff that has to run much later, after the guts are initialized.) - Extra parameter to class_find_device(). This could have been done in a backwards-compatible manner, like device_create() was, but ... it wasn't. When I finish pulling linux-next, I'll make an updated patch. And probably send in an updated version of Michaels patch; though I imagine the update will be no more than a s-o-b. - dave --
device_create() did disappear in -next, but when it goes to Linus it will not, it will come back as a #define and everything will be converted back. That was to catch the build errors when I audited the If you need such a function, please let me know and I can provide it, I was not aware of anyone using class.devices, it is an This was simpler as there were less users of this function. And the whole tree was fixed up. If there are trees on top of -next that I can't see, that's pretty hard for me to fix :) thanks, greg k-h --
When the fields are there, it's hard to say what's "internal" in the absense of comments ... :) The scenario is that gpios sometimes get initialized very early, as part of board setup before kmalloc or irqs work, so when they get registered sometimes there's work left over for later. All that's needed is a predicate to test whether that class can be used as a parameter to stuff like device_create_drvdata() and This was in the MM tree with other GPIO patches. - Dave --
Because either way you are going to have to change your code to be able to build properly on top of it. So what could would the compatibility macro do for you now. With the code it way it is, you instantly see the issue, instead of trying to figure out a warning or other parameter error. If you want to see if a class has any devices in it, just do a search for any device instead of poking into that field. Or I can wrap that up in the driver core if needed. thanks, greg k-h --
That would require taking driver model mutexes before tasking is working and driver_init() is called. GPIOs are regularly used *really* early in board setup ... even before IRQs are working. Are driver model searches really expected to work then?? Seems simpler to me to just check "is pointer NULL", when we know that the pointer is initialized by the class initialization, and (because the relevant struct is zero-initialized) null beforehand. That works more or less any point during kernel initialization ... not just after the first task is running. Plus it's not as expensive. :) - Dave --
Developing against -next is even worse than developping against an -rc1. You normally want to be running the code you develop meaning you'd very frequently drown in everyone else's bugs without getting to your own if you do. Development needs to happen against something relatively stable really and the last release would generally seem to be the best choice. Now ofcourse, _porting_ it to -next before submitting makes lots of sense but positioning -next as THE devel tree a bit less I feel... Rene. --
Well one can develop against mainline and port to linux-next and then do a quick test. But it seems like pointless additional overhead to me. What else can we do? People are frequently sending patches which don't even apply to the tree for which they're intended. And some which If one wishes. It's readily obvious that people (ie: top-level maintainers) aren't even compile-testing their own stuff once it's merged into linux-next. You say (but don't provide evidence that) linux-next is too unstable to develop against. Well guess why? Because people are choosing to let it be that way. Now the upshot of this might be that people end up having to spend more time testing and less time developing. It is fairly apparent that this is a desirable outcome. --
Other than developer needing a relatively stable base to develop against and run so as to concentrate on the effects of own code certainly people that said developer wants to test that which is in development want such a base. Fewer testers can/will be testing a particular feature if to do so they need to first drag in megabytes upon megabytes of other changes as well. Last iteration I looked at and tested PNP patches and was using .25 as base which went smoothly. Then at 26-rc1 I needed to start running that since patches were against that which delayed things for a long time while I needed to find time to deal with that many unrelated changes some of which rather violently broke stuff here (ISA DMA most profoundly). So should I have caught the ISA DMA breakage before it hit -rc1? Given that I tend to care about sound/isa which is its main user, perhaps. But the point was that I was trying to test ISAPnP, yet another thing that sound/isa is a main user of... Many users/testers simply do not want to be running bleedin' edge all of the time because they'd be spending way too much time fighting bugs in places that are of no particular interest to them and when features are developed against said edge, you'd have just limited your tester base further to those who either will, or have enough sophistication to take the change in isolation and backport it to their current kernel themselves. And do it again on the next iteration of the feature. As recently as yesterday I stalled on testing an ALSA patch when I saw it fail against mainline and wished for some other model than always having to run the very latest. Not to sound overly pompous (I hope) but not everyone can be responsible for all these integration pains. People in corner A tend to care about corner A and might consider corners B, C and D unwelcome distractions mostly. Linux then has people like you and Linus to stand in the middle and make all the corners get along again. Yes, porting to ...
Should be addressed by the following, at least in
terms of build problems aginst linux-next.
The resulting kernel doesn't seem bootable on any
board I currently have set up for testing gpio calls.
The build dies in the TTY stack.
- Dave
============= CUT HERE
Cope with some backwards-incompatible driver model API changes
now in the linux-next tree:
- device_create() is going away, even for drivers using it safely,
in favor of device_create_drvdata().
- class->devices is gone, but testing class->p serves the same
purpose (non-null when class is usable with driver model calls).
- class_find_device() needs a new argument #2 (NULL)
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
drivers/gpio/gpiolib.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
--- a/drivers/gpio/gpiolib.c 2008-07-02 23:29:58.000000000 -0700
+++ b/drivers/gpio/gpiolib.c 2008-07-03 00:04:57.000000000 -0700
@@ -429,7 +429,7 @@ int gpio_export(unsigned gpio, bool dire
int status = -EINVAL;
/* can't export until sysfs is available ... */
- if (!gpio_class.devices.next) {
+ if (!gpio_class.p) {
pr_debug("%s: called too early!\n", __func__);
return -ENOENT;
}
@@ -453,10 +453,9 @@ int gpio_export(unsigned gpio, bool dire
if (status == 0) {
struct device *dev;
- dev = device_create(&gpio_class, desc->chip->dev, 0,
- "gpio%d", gpio);
+ dev = device_create_drvdata(&gpio_class, desc->chip->dev, 0,
+ desc, "gpio%d", gpio);
if (dev) {
- dev_set_drvdata(dev, desc);
if (direction_may_change)
status = sysfs_create_group(&dev->kobj,
&gpio_attr_group);
@@ -506,7 +505,7 @@ void gpio_unexport(unsigned gpio)
if (test_bit(FLAG_EXPORT, &desc->flags)) {
struct device *dev = NULL;
- dev = class_find_device(&gpio_class, desc, match_export);
+ dev = class_find_device(&gpio_class, NULL, desc, match_export);
if (dev) {
clear_bit(FLAG_EXPORT, &desc->flags);
put_device(dev);
@@ ...Thanks a lot Dave. -- Greetings Michael. --
