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 <[email protected]>
---
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 <[email protected]>
+ *
+ * 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 CONFIG_X86_RDC321X
#include <gpio.h>
+#else /* CONFIG_X86_RDC321X */
+
+#include <asm-generic/gpio.h>
+
+#ifdef CONFIG_GPIOLIB
+
+/*
+ * Just call gpiolib.
+ */
+static inline int gpio_get_value(unsigned int gpio)
+{
+ return __gpio_get_value(gpio);
+}
+
+static inline void gpio_set_value(unsigned int gpio, int value)
+{
+ __gpio_set_value(gpio, value);
+}
+
+static inline int gpio_cansleep(unsigned int gpio)
+{
+ return __gpio_cansleep(gpio);
+}
+
+/*
+ * Not implemented, yet.
+ */
+static inline int gpio_to_irq(unsigned int gpio)
+{
+ return -ENOSYS;
+}
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+ return -EINVAL;
+}
+
+#endif /* CONFIG_GPIOLIB */
+
+#endif /* CONFIG_X86_RDC321X */
#endif /* _ASM_I386_GPIO_H */
Index: linux-2.6/arch/powerpc/Kconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/Kconfig 2008-07-02 23:12:59.000000000 +0200
+++ linux-2.6/arch/powerpc/Kconfig 2008-07-02 23:15:33.000000000 +0200
@@ -110,6 +110,7 @@ config PPC
select HAVE_KPROBES
select HAVE_KRETPROBES
select HAVE_LMB
+ select ARCH_WANT_OPTIONAL_GPIOLIB
config EARLY_PRINTK
bool
Index: linux-2.6/drivers/gpio/Kconfig
===================================================================
--- linux-2.6.orig/drivers/gpio/Kconfig 2008-07-02 23:12:59.000000000 +0200
+++ linux-2.6/drivers/gpio/Kconfig 2008-07-02 23:28:57.000000000 +0200
@@ -2,15 +2,40 @@
# GPIO infrastructure and expanders
#
-config HAVE_GPIO_LIB
+config ARCH_WANT_OPTIONAL_GPIOLIB
bool
help
+ Select this config option from the architecture Kconfig, if
+ it is possible to use gpiolib on the architecture, but let the
+ user decide whether to actually build it or not.
+ Select this instead of ARCH_REQUIRE_GPIOLIB, if your architecture does
+ not depend on GPIOs being available, but rather let the user
+ decide whether he needs it or not.
+
+config ARCH_REQUIRE_GPIOLIB
+ bool
+ select GPIOLIB
+ help
Platforms select gpiolib if they use this infrastructure
for all their GPIOs, usually starting with ones integrated
into SOC processors.
+ Selecting this from the architecture code will cause the gpiolib
+ code to always get built in.
+
+
+
+menuconfig GPIOLIB
+ bool "GPIO Support"
+ depends on ARCH_WANT_OPTIONAL_GPIOLIB || ARCH_REQUIRE_GPIOLIB
+ select GENERIC_GPIO
+ help
+ This enables GPIO support through the generic GPIO library.
+ You only need to enable this, if you also want to enable
+ one or more of the GPIO expansion card drivers below.
+
+ If unsure, say N.
-menu "GPIO Support"
- depends on HAVE_GPIO_LIB
+if GPIOLIB
config DEBUG_GPIO
bool "Debug GPIO calls"
@@ -70,4 +95,4 @@ config GPIO_MCP23S08
SPI driver for Microchip MCP23S08 I/O expander. This provides
a GPIO interface supporting inputs and outputs.
-endmenu
+endif
Index: linux-2.6/drivers/gpio/Makefile
===================================================================
--- linux-2.6.orig/drivers/gpio/Makefile 2008-07-02 23:12:59.000000000 +0200
+++ linux-2.6/drivers/gpio/Makefile 2008-07-02 23:15:33.000000000 +0200
@@ -2,7 +2,7 @@
ccflags-$(CONFIG_DEBUG_GPIO) += -DDEBUG
-obj-$(CONFIG_HAVE_GPIO_LIB) += gpiolib.o
+obj-$(CONFIG_GPIOLIB) += gpiolib.o
obj-$(CONFIG_GPIO_MCP23S08) += mcp23s08.o
obj-$(CONFIG_GPIO_PCA953X) += pca953x.o
Index: linux-2.6/Documentation/gpio.txt
===================================================================
--- linux-2.6.orig/Documentation/gpio.txt 2008-05-01 13:19:44.000000000 +0200
+++ linux-2.6/Documentation/gpio.txt 2008-07-02 23:35:48.000000000 +0200
@@ -392,11 +392,21 @@ either NULL or the label associated with
Platform Support
----------------
-To support this framework, a platform's Kconfig will "select HAVE_GPIO_LIB"
+To support this framework, a platform's Kconfig will "select" either
+ARCH_REQUIRE_GPIOLIB or ARCH_WANT_OPTIONAL_GPIOLIB
and arrange that its <asm/gpio.h> includes <asm-generic/gpio.h> and defines
three functions: gpio_get_value(), gpio_set_value(), and gpio_cansleep().
They may also want to provide a custom value for ARCH_NR_GPIOS.
+ARCH_REQUIRE_GPIOLIB means that the gpio-lib code will always get compiled
+into the kernel on that architecture.
+
+ARCH_WANT_OPTIONAL_GPIOLIB means the gpio-lib code defaults to off and the user
+can enable it and build it into the kernel optionally.
+
+If neither of these options are selected, the platform does not support
+GPIOs through GPIO-lib and the code cannot be enabled by the user.
+
Trivial implementations of those functions can directly use framework
code, which always dispatches through the gpio_chip:
Index: linux-2.6/arch/arm/Kconfig
===================================================================
--- linux-2.6.orig/arch/arm/Kconfig 2008-05-01 13:19:44.000000000 +0200
+++ linux-2.6/arch/arm/Kconfig 2008-07-02 23:23:59.000000000 +0200
@@ -254,7 +254,7 @@ config ARCH_EP93XX
select ARM_AMBA
select ARM_VIC
select GENERIC_GPIO
- select HAVE_GPIO_LIB
+ select ARCH_REQUIRE_GPIOLIB
help
This enables support for the Cirrus EP93xx series of CPUs.
@@ -393,7 +393,7 @@ config ARCH_PXA
depends on MMU
select ARCH_MTD_XIP
select GENERIC_GPIO
- select HAVE_GPIO_LIB
+ select ARCH_REQUIRE_GPIOLIB
select GENERIC_TIME
select GENERIC_CLOCKEVENTS
select TICK_ONESHOT
@@ -423,7 +423,7 @@ config ARCH_SA1100
select GENERIC_TIME
select GENERIC_CLOCKEVENTS
select TICK_ONESHOT
- select HAVE_GPIO_LIB
+ select ARCH_REQUIRE_GPIOLIB
help
Support for StrongARM 11x0 based boards.
@@ -463,7 +463,7 @@ config ARCH_DAVINCI
config ARCH_OMAP
bool "TI OMAP"
select GENERIC_GPIO
- select HAVE_GPIO_LIB
+ select ARCH_REQUIRE_GPIOLIB
select GENERIC_TIME
select GENERIC_CLOCKEVENTS
help
Index: linux-2.6/arch/arm/configs/am200epdkit_defconfig
===================================================================
--- linux-2.6.orig/arch/arm/configs/am200epdkit_defconfig 2008-05-01 13:19:44.000000000 +0200
+++ linux-2.6/arch/arm/configs/am200epdkit_defconfig 2008-07-02 23:23:34.000000000 +0200
@@ -668,7 +668,7 @@ CONFIG_UNIX98_PTYS=y
#
# CONFIG_SPI is not set
# CONFIG_SPI_MASTER is not set
-CONFIG_HAVE_GPIO_LIB=y
+CONFIG_ARCH_REQUIRE_GPIOLIB=y
#
# GPIO Support
Index: linux-2.6/arch/avr32/Kconfig
===================================================================
--- linux-2.6.orig/arch/avr32/Kconfig 2008-05-01 13:19:44.000000000 +0200
+++ linux-2.6/arch/avr32/Kconfig 2008-07-02 23:26:20.000000000 +0200
@@ -87,7 +87,7 @@ config PLATFORM_AT32AP
select SUBARCH_AVR32B
select MMU
select PERFORMANCE_COUNTERS
- select HAVE_GPIO_LIB
+ select ARCH_REQUIRE_GPIOLIB
#
# CPU types
Index: linux-2.6/arch/avr32/configs/atngw100_defconfig
===================================================================
--- linux-2.6.orig/arch/avr32/configs/atngw100_defconfig 2008-06-13 13:13:38.000000000 +0200
+++ linux-2.6/arch/avr32/configs/atngw100_defconfig 2008-07-02 23:25:22.000000000 +0200
@@ -646,7 +646,7 @@ CONFIG_SPI_ATMEL=y
# CONFIG_SPI_AT25 is not set
CONFIG_SPI_SPIDEV=m
# CONFIG_SPI_TLE62X0 is not set
-CONFIG_HAVE_GPIO_LIB=y
+CONFIG_ARCH_REQUIRE_GPIOLIB=y
#
# GPIO Support
Index: linux-2.6/arch/avr32/configs/atstk1002_defconfig
===================================================================
--- linux-2.6.orig/arch/avr32/configs/atstk1002_defconfig 2008-06-13 13:13:38.000000000 +0200
+++ linux-2.6/arch/avr32/configs/atstk1002_defconfig 2008-07-02 23:25:02.000000000 +0200
@@ -664,7 +664,7 @@ CONFIG_SPI_ATMEL=y
# CONFIG_SPI_AT25 is not set
CONFIG_SPI_SPIDEV=m
# CONFIG_SPI_TLE62X0 is not set
-CONFIG_HAVE_GPIO_LIB=y
+CONFIG_ARCH_REQUIRE_GPIOLIB=y
#
# GPIO Support
Index: linux-2.6/arch/avr32/configs/atstk1003_defconfig
===================================================================
--- linux-2.6.orig/arch/avr32/configs/atstk1003_defconfig 2008-06-13 13:13:38.000000000 +0200
+++ linux-2.6/arch/avr32/configs/atstk1003_defconfig 2008-07-02 23:25:55.000000000 +0200
@@ -614,7 +614,7 @@ CONFIG_SPI_ATMEL=y
# CONFIG_SPI_AT25 is not set
CONFIG_SPI_SPIDEV=m
# CONFIG_SPI_TLE62X0 is not set
-CONFIG_HAVE_GPIO_LIB=y
+CONFIG_ARCH_REQUIRE_GPIOLIB=y
#
# GPIO Support
Index: linux-2.6/arch/avr32/configs/atstk1004_defconfig
===================================================================
--- linux-2.6.orig/arch/avr32/configs/atstk1004_defconfig 2008-06-13 13:13:38.000000000 +0200
+++ linux-2.6/arch/avr32/configs/atstk1004_defconfig 2008-07-02 23:26:04.000000000 +0200
@@ -390,7 +390,7 @@ CONFIG_SPI_ATMEL=y
# CONFIG_SPI_AT25 is not set
# CONFIG_SPI_SPIDEV is not set
# CONFIG_SPI_TLE62X0 is not set
-CONFIG_HAVE_GPIO_LIB=y
+CONFIG_ARCH_REQUIRE_GPIOLIB=y
#
# GPIO Support
Index: linux-2.6/arch/mips/Kconfig
===================================================================
--- linux-2.6.orig/arch/mips/Kconfig 2008-05-01 13:19:45.000000000 +0200
+++ linux-2.6/arch/mips/Kconfig 2008-07-02 23:24:28.000000000 +0200
@@ -800,7 +800,7 @@ config CSRC_SB1250
config GPIO_TXX9
select GENERIC_GPIO
- select HAVE_GPIO_LIB
+ select ARCH_REQUIRE_GPIOLIB
bool
config CFE
Index: linux-2.6/arch/powerpc/platforms/52xx/Kconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/52xx/Kconfig 2008-05-01 13:19:46.000000000 +0200
+++ linux-2.6/arch/powerpc/platforms/52xx/Kconfig 2008-07-02 23:26:41.000000000 +0200
@@ -47,6 +47,6 @@ config PPC_MPC5200_BUGFIX
config PPC_MPC5200_GPIO
bool "MPC5200 GPIO support"
depends on PPC_MPC52xx
- select HAVE_GPIO_LIB
+ select ARCH_REQUIRE_GPIOLIB
help
Enable gpiolib support for mpc5200 based boards
Index: linux-2.6/drivers/Makefile
===================================================================
--- linux-2.6.orig/drivers/Makefile 2008-05-01 13:19:48.000000000 +0200
+++ linux-2.6/drivers/Makefile 2008-07-02 23:29:33.000000000 +0200
@@ -5,7 +5,7 @@
# Rewritten to use lists instead of if-statements.
#
-obj-$(CONFIG_HAVE_GPIO_LIB) += gpio/
+obj-y += gpio/
obj-$(CONFIG_PCI) += pci/
obj-$(CONFIG_PARISC) += parisc/
obj-$(CONFIG_RAPIDIO) += rapidio/
Index: linux-2.6/drivers/i2c/chips/Kconfig
===================================================================
--- linux-2.6.orig/drivers/i2c/chips/Kconfig 2008-05-01 13:19:49.000000000 +0200
+++ linux-2.6/drivers/i2c/chips/Kconfig 2008-07-02 23:27:49.000000000 +0200
@@ -93,7 +93,7 @@ config ISP1301_OMAP
config TPS65010
tristate "TPS6501x Power Management chips"
- depends on HAVE_GPIO_LIB
+ depends on GPIOLIB
default y if MACH_OMAP_H2 || MACH_OMAP_H3 || MACH_OMAP_OSK
help
If you say yes here you get support for the TPS6501x series of
Index: linux-2.6/drivers/mfd/Kconfig
===================================================================
--- linux-2.6.orig/drivers/mfd/Kconfig 2008-06-13 13:13:40.000000000 +0200
+++ linux-2.6/drivers/mfd/Kconfig 2008-07-02 23:28:09.000000000 +0200
@@ -24,7 +24,7 @@ config MFD_ASIC3
config HTC_EGPIO
bool "HTC EGPIO support"
- depends on GENERIC_HARDIRQS && HAVE_GPIO_LIB && ARM
+ depends on GENERIC_HARDIRQS && GPIOLIB && ARM
help
This driver supports the CPLD egpio chip present on
several HTC phones. It provides basic support for input
Index: linux-2.6/drivers/of/Kconfig
===================================================================
--- linux-2.6.orig/drivers/of/Kconfig 2008-05-01 13:19:51.000000000 +0200
+++ linux-2.6/drivers/of/Kconfig 2008-07-02 23:27:28.000000000 +0200
@@ -4,7 +4,7 @@ config OF_DEVICE
config OF_GPIO
def_bool y
- depends on OF && PPC_OF && HAVE_GPIO_LIB
+ depends on OF && PPC_OF && GPIOLIB
help
OpenFirmware GPIO accessors
Index: linux-2.6/include/asm-generic/gpio.h
===================================================================
--- linux-2.6.orig/include/asm-generic/gpio.h 2008-06-13 13:13:45.000000000 +0200
+++ linux-2.6/include/asm-generic/gpio.h 2008-07-02 23:32:19.000000000 +0200
@@ -3,7 +3,7 @@
#include <linux/types.h>
-#ifdef CONFIG_HAVE_GPIO_LIB
+#ifdef CONFIG_GPIOLIB
#include <linux/compiler.h>
Index: linux-2.6/include/asm-mips/mach-generic/gpio.h
===================================================================
--- linux-2.6.orig/include/asm-mips/mach-generic/gpio.h 2008-05-01 13:19:58.000000000 +0200
+++ linux-2.6/include/asm-mips/mach-generic/gpio.h 2008-07-02 23:31:17.000000000 +0200
@@ -1,7 +1,7 @@
#ifndef __ASM_MACH_GENERIC_GPIO_H
#define __ASM_MACH_GENERIC_GPIO_H
-#ifdef CONFIG_HAVE_GPIO_LIB
+#ifdef CONFIG_GPIOLIB
#define gpio_get_value __gpio_get_value
#define gpio_set_value __gpio_set_value
#define gpio_cansleep __gpio_cansleep
Index: linux-2.6/include/asm-powerpc/gpio.h
===================================================================
--- linux-2.6.orig/include/asm-powerpc/gpio.h 2008-05-01 13:19:58.000000000 +0200
+++ linux-2.6/include/asm-powerpc/gpio.h 2008-07-02 23:31:58.000000000 +0200
@@ -17,7 +17,7 @@
#include <linux/errno.h>
#include <asm-generic/gpio.h>
-#ifdef CONFIG_HAVE_GPIO_LIB
+#ifdef CONFIG_GPIOLIB
/*
* We don't (yet) implement inlined/rapid versions for on-chip gpios.
@@ -51,6 +51,6 @@ static inline int irq_to_gpio(unsigned i
return -EINVAL;
}
-#endif /* CONFIG_HAVE_GPIO_LIB */
+#endif /* CONFIG_GPIOLIB */
#endif /* __ASM_POWERPC_GPIO_H */
--
Greetings Michael.
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.
On Thursday 03 July 2008 02:04:31 Andrew Morton wrote:
> 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.
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
considered "ancient".
> Guys, we have a new development tree now.
--
Greetings Michael.
On Wednesday 02 July 2008, Andrew Morton wrote:
> 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.
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.
- Dave
>
> Guys, we have a new development tree now.
>
On Wed, 2 Jul 2008 22:00:49 -0700 David Brownell <[email protected]> wrote:
> On Wednesday 02 July 2008, Andrew Morton wrote:
> > 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.
>
> 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.
On Wednesday 02 July 2008, Andrew Morton wrote:
>
> > 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.
I hope you got the T-shirt while you weren't doing all that! :)
> 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.
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
is going to get extra work because of those changes.)
> 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
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
all the added turnover and variables of the -next tree.
> 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.
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
On Thu, 03 Jul 2008 10:36:44 +0200 Rene Herman <[email protected]> wrote:
> On 03-07-08 07:08, Andrew Morton wrote:
>
> > 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.
Stuff happens. linux-next is generally OKish.
> 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.
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
simply fail at compile-time or runtime when they are applied to that tree.
> Now ofcourse, _porting_ it to -next before submitting makes lots of
> sense but positioning -next as THE devel tree a bit less I feel...
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.
On Wednesday 02 July 2008, Andrew Morton wrote:
> 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'
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(-)
--- 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);
@@ -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);
On 03-07-08 07:08, Andrew Morton wrote:
> 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...
Rene.
On 03-07-08 11:01, Andrew Morton wrote:
> 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.
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 and checking -next before submission makes perfect sense
but developing against it -- please not...
Rene.
On Thursday 03 July 2008 10:25:08 David Brownell wrote:
> On Wednesday 02 July 2008, Andrew Morton wrote:
> > 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'
>
> 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.
Thanks a lot Dave.
I'm going to try this with my bt878 card, once I got linux-next pulled and built.
> - 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(-)
>
> --- 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);
> @@ -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);
>
>
>
--
Greetings Michael.
On Wed, Jul 02, 2008 at 10:41:27PM -0700, David Brownell wrote:
> 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.
> - 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.
> - 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 :)
thanks,
greg k-h
On Thursday 03 July 2008, Greg KH wrote:
> On Wed, Jul 02, 2008 at 10:41:27PM -0700, David Brownell wrote:
> > 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.
Hmm, that's a surprise. Any reason that's not in "-next" already?
> > - 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.
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().
> > - 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 :)
This was in the MM tree with other GPIO patches.
- Dave
On Thu, Jul 03, 2008 at 02:28:00PM -0700, David Brownell wrote:
> On Thursday 03 July 2008, Greg KH wrote:
> > On Wed, Jul 02, 2008 at 10:41:27PM -0700, David Brownell wrote:
> > > 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.
>
> Hmm, that's a surprise. Any reason that's not in "-next" already?
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.
I was trying to make it obvious, which I guess worked :)
> > > - 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.
>
> When the fields are there, it's hard to say what's "internal"
> in the absense of comments ... :)
That's why I have been commenting the changes :)
> 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.
thanks,
greg k-h
On Thursday 03 July 2008, Greg KH wrote:
> > 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.
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