gpiolib: Allow user-selection

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.

Quoted 3 lines · On Wed, 2 Jul 2008 23:46:53 +0200 Michael Buesch <[email protected]> wrote:

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.

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.

Quoted 3 lines · On Wed, 2 Jul 2008 23:46:53 +0200 Michael Buesch <[email protected]> wrote:

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.

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.

Quoted 21 lines · Andrew Morton

Quoted 3 lines · On Wed, 2 Jul 2008 23:46:53 +0200 Michael Buesch <[email protected]> wrote:

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.

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'

Quoted 2 lines

I assume this patch was prepared against some ancient out-of-date kernel such as current Linus mainline.

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.

Quoted 5 lines

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.

Quoted 5 lines

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.

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.

Quoted 9 lines · David Brownell

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.

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 whole tree. The -rc1 merge cycle will handle this properly.

Quoted 8 lines

- 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.)

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 internal-to-the-driver-core field, and has moved private.

Quoted 3 lines

- Extra parameter to class_find_device(). This could have been done in a backwards-compatible manner, like device_create() was, but ... it wasn't.

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 :)

Quoted 11 lines · Greg KH

Quoted 9 lines · David Brownell

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.

Quoted 4 lines

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 whole tree. The -rc1 merge cycle will handle this properly.

Hmm, that's a surprise. Any reason that's not in "-next" already?

Quoted 9 lines

Quoted 8 lines

- 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.)

Quoted 3 lines

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 internal-to-the-driver-core field, and has moved private.

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 sysfs_create_group().

Quoted 6 lines

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 sysfs_create_group().

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.

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.

Quoted 8 lines · Andrew Morton

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.

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...

Quoted 2 lines · On Thu, 03 Jul 2008 10:36:44 +0200 Rene Herman <[email protected]> wrote:

Development needs to happen against something relatively stable really and the last release would generally seem to be the best choice.

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.

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'

Quoted 5 lines · Andrew Morton

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':

Quoted 8 lines

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'

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 <[email protected]>

---
 drivers/gpio/gpiolib.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

drivers/gpio/gpiolib.c +7 -9

@@ -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);

@@ -533,15 +532,14 @@ static int gpiochip_export(struct gpio_c
 	 * export this later, in gpiolib_sysfs_init() ... here we just
 	 * verify that _some_ field of gpio_class got initialized.
 	 */
-	if (!gpio_class.devices.next)
+	if (!gpio_class.p)
 		return 0;
 
 	/* use chip->base for the ID; it's already known to be unique */
 	mutex_lock(&sysfs_lock);
-	dev = device_create(&gpio_class, chip->dev, 0,
+	dev = device_create_drvdata(&gpio_class, chip->dev, 0, chip,
 			"gpiochip%d", chip->base);
 	if (dev) {
-		dev_set_drvdata(dev, chip);
 		status = sysfs_create_group(&dev->kobj,
 				&gpiochip_attr_group);
 	} else

@@ -572,7 +570,7 @@ static void gpiochip_unexport(struct gpi
 	struct device		*dev;
 
 	mutex_lock(&sysfs_lock);
-	dev = class_find_device(&gpio_class, chip, match_export);
+	dev = class_find_device(&gpio_class, NULL, chip, match_export);
 	if (dev) {
 		put_device(dev);
 		device_unregister(dev);