Re: [alsa-devel] linux-next: Tree for May 23 (sound/soc/codecs)
On Mon, 23 May 2011 15:45:18 +1000 Stephen Rothwell wrote:
Hi all,
Changes since 20110520:
when CONFIG_GPIOLIB is not enabled and CONFIG_GENERIC_GPIO is not enabled:
sound/soc/codecs/ak4641.c:524: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function) sound/soc/codecs/wm8915.c:2857: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function)
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Mon, May 23, 2011 at 01:48:15PM -0700, Randy Dunlap wrote:
On Mon, 23 May 2011 15:45:18 +1000 Stephen Rothwell wrote:
when CONFIG_GPIOLIB is not enabled and CONFIG_GENERIC_GPIO is not enabled:
sound/soc/codecs/ak4641.c:524: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function) sound/soc/codecs/wm8915.c:2857: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function)
What architecture is this on (you should always include information like this anyway so people can try to reproduce what you're seeing)? In any case, please talk to the architecture maintainers about this - it's an issue in the architecture GPIO support (or lack thereof) rather than a driver problem.
Also adding Dmitry who submitted the driver - Randy, please try to remember to CC relevant people.
On Tue, 24 May 2011 06:47:28 +0800 Mark Brown wrote:
On Mon, May 23, 2011 at 01:48:15PM -0700, Randy Dunlap wrote:
On Mon, 23 May 2011 15:45:18 +1000 Stephen Rothwell wrote:
when CONFIG_GPIOLIB is not enabled and CONFIG_GENERIC_GPIO is not enabled:
sound/soc/codecs/ak4641.c:524: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function) sound/soc/codecs/wm8915.c:2857: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function)
What architecture is this on (you should always include information like this anyway so people can try to reproduce what you're seeing)? In any
on x86_64 [adding x86@kernel.org == arch maintainers]
case, please talk to the architecture maintainers about this - it's an issue in the architecture GPIO support (or lack thereof) rather than a driver problem.
except that a driver should not assume that defines like GPIOF_OUT_INIT_LOW are always available.
Also adding Dmitry who submitted the driver - Randy, please try to remember to CC relevant people.
Which driver did Dmitry submit? how would I know that? I don't download every linux-next git tree -- just linux-next tarballs.
ak4641.c says: MODULE_AUTHOR("Harald Welte laforge@gnufiish.org"); [which bounces]
and wm8915.c says: MODULE_AUTHOR("Mark Brown broonie@opensource.wolfsonmicro.com");
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Mon, May 23, 2011 at 03:53:43PM -0700, Randy Dunlap wrote:
On Tue, 24 May 2011 06:47:28 +0800 Mark Brown wrote:
case, please talk to the architecture maintainers about this - it's an issue in the architecture GPIO support (or lack thereof) rather than a driver problem.
except that a driver should not assume that defines like GPIOF_OUT_INIT_LOW are always available.
No, really we should. The GPIO APIs are stubbed out when not in use for a very good reason, think about the usability here. The goal here isn't to litter the code with ifdefs - if architectures aren't able to keep up with API changes they should convert to using gpiolib so this stuff happens automatically (indeed, I can't think of any good reason for an architecture to not be using gpiolib at this point).
Also adding Dmitry who submitted the driver - Randy, please try to remember to CC relevant people.
Which driver did Dmitry submit? how would I know that? I don't download every linux-next git tree -- just linux-next tarballs.
I *strongly* suggest looking at git if you want to find relevant people to mail; the internal documentation in the code really isn't a terribly useful guide, the authors listed in the code often bear no relation to who's actually working on it at the current time.
and wm8915.c says: MODULE_AUTHOR("Mark Brown broonie@opensource.wolfsonmicro.com");
You've clearly not looked at MAINTAINERS for this one.
On 05/23/11 17:08, Mark Brown wrote:
On Mon, May 23, 2011 at 03:53:43PM -0700, Randy Dunlap wrote:
On Tue, 24 May 2011 06:47:28 +0800 Mark Brown wrote:
case, please talk to the architecture maintainers about this - it's an issue in the architecture GPIO support (or lack thereof) rather than a driver problem.
except that a driver should not assume that defines like GPIOF_OUT_INIT_LOW are always available.
No, really we should. The GPIO APIs are stubbed out when not in use for a very good reason, think about the usability here. The goal here isn't to litter the code with ifdefs - if architectures aren't able to keep up with API changes they should convert to using gpiolib so this stuff happens automatically (indeed, I can't think of any good reason for an architecture to not be using gpiolib at this point).
No, I would say that there are a lot of drivers in sound/soc/codecs/ that are missing some GPIO pieces in the Kconfig file.
Also adding Dmitry who submitted the driver - Randy, please try to remember to CC relevant people.
Which driver did Dmitry submit? how would I know that? I don't download every linux-next git tree -- just linux-next tarballs.
I *strongly* suggest looking at git if you want to find relevant people to mail; the internal documentation in the code really isn't a terribly useful guide, the authors listed in the code often bear no relation to who's actually working on it at the current time.
and wm8915.c says: MODULE_AUTHOR("Mark Brown broonie@opensource.wolfsonmicro.com");
You've clearly not looked at MAINTAINERS for this one.
It's not listed in the MAINTAINERS file. But maybe you mean scripts/get_maintainer.pl, which I did try. I found that using git log <that source file name> was better info than using scripts/get_maintainer.pl.
On Mon, May 23, 2011 at 06:21:51PM -0700, Randy Dunlap wrote:
On 05/23/11 17:08, Mark Brown wrote:
No, really we should. The GPIO APIs are stubbed out when not in use for a very good reason, think about the usability here. The goal here isn't to litter the code with ifdefs - if architectures aren't able to keep up with API changes they should convert to using gpiolib so this stuff happens automatically (indeed, I can't think of any good reason for an architecture to not be using gpiolib at this point).
No, I would say that there are a lot of drivers in sound/soc/codecs/ that are missing some GPIO pieces in the Kconfig file.
Have you actually looked at the code here? Vanishingly few of the drivers need GPIOs at all, they can just optionally use GPIOs if the system makes them available. There is absolutely no dependency on GPIOs for them, anything in Kconfig would be entirely unconstructive.
MODULE_AUTHOR("Mark Brown broonie@opensource.wolfsonmicro.com");
You've clearly not looked at MAINTAINERS for this one.
It's not listed in the MAINTAINERS file.
MAINTAINERS has a pattern sound/soc/codecs/wm*.
But maybe you mean scripts/get_maintainer.pl, which I did try. I found that using git log <that source file name> was better info than using scripts/get_maintainer.pl.
get_maintainers is just a script that reads MAINTAINERS and trawls logs for it; the reason I mentioned MAINTAINERs was that you were saying you didn't use git. In general you're better off doing things by hand rather than using get_maintainers.
On 05/23/11 18:50, Mark Brown wrote:
On Mon, May 23, 2011 at 06:21:51PM -0700, Randy Dunlap wrote:
On 05/23/11 17:08, Mark Brown wrote:
No, really we should. The GPIO APIs are stubbed out when not in use for a very good reason, think about the usability here. The goal here isn't to litter the code with ifdefs - if architectures aren't able to keep up with API changes they should convert to using gpiolib so this stuff happens automatically (indeed, I can't think of any good reason for an architecture to not be using gpiolib at this point).
No, I would say that there are a lot of drivers in sound/soc/codecs/ that are missing some GPIO pieces in the Kconfig file.
Have you actually looked at the code here? Vanishingly few of the drivers need GPIOs at all, they can just optionally use GPIOs if the system makes them available. There is absolutely no dependency on GPIOs for them, anything in Kconfig would be entirely unconstructive.
MODULE_AUTHOR("Mark Brown broonie@opensource.wolfsonmicro.com");
You've clearly not looked at MAINTAINERS for this one.
It's not listed in the MAINTAINERS file.
MAINTAINERS has a pattern sound/soc/codecs/wm*.
Thanks for that hint.
But maybe you mean scripts/get_maintainer.pl, which I did try. I found that using git log <that source file name> was better info than using scripts/get_maintainer.pl.
get_maintainers is just a script that reads MAINTAINERS and trawls logs for it; the reason I mentioned MAINTAINERs was that you were saying you didn't use git. In general you're better off doing things by hand rather than using get_maintainers.
Yes.
OK, I didn't mean to get into a blame game on this. You mentioned stubs earlier and that's what is not working AFAICT.
Below is a patch that makes the 2 reported drivers build when CONFIG_GPIOLIB is disabled and CONFIG_GENERIC_GPIO is disabled. What do you think of the patch?
--- From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com --- include/asm-generic/gpio.h | 10 ---------- include/linux/gpio.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-)
--- linux-next-20110523.orig/include/asm-generic/gpio.h +++ linux-next-20110523/include/asm-generic/gpio.h @@ -170,16 +170,6 @@ extern int __gpio_cansleep(unsigned gpio
extern int __gpio_to_irq(unsigned gpio);
-#define GPIOF_DIR_OUT (0 << 0) -#define GPIOF_DIR_IN (1 << 0) - -#define GPIOF_INIT_LOW (0 << 1) -#define GPIOF_INIT_HIGH (1 << 1) - -#define GPIOF_IN (GPIOF_DIR_IN) -#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) -#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH) - /** * struct gpio - a structure describing a GPIO with configuration * @gpio: the GPIO number --- linux-next-20110523.orig/include/linux/gpio.h +++ linux-next-20110523/include/linux/gpio.h @@ -3,6 +3,17 @@
/* see Documentation/gpio.txt */
+/* make these flag values available regardless of GPIO kconfig options */ +#define GPIOF_DIR_OUT (0 << 0) +#define GPIOF_DIR_IN (1 << 0) + +#define GPIOF_INIT_LOW (0 << 1) +#define GPIOF_INIT_HIGH (1 << 1) + +#define GPIOF_IN (GPIOF_DIR_IN) +#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) +#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH) + #ifdef CONFIG_GENERIC_GPIO #include <asm/gpio.h>
On 21:58 Mon 23 May , Randy Dunlap wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com
Looks good.
We probably may also want to move definition of struct gpio into include/linux/gpio.h to make things like this work as well:
static struct gpio some_gpios[] = { { GPIO_BLAH, GPIOF_IN, "BLAH"}, { GPIO_BLAH2, GPIOF_OUT_INIT_LOW, "BLAH2"}, };
static int some_init_function(void) { /* ... */
gpio_request_array(some_gpios, ARRAY_SIZE(some_gpios));
/* ... */ }
These gpio_request_one() and gpio_request_array() are quite handy, so I suppose more and more drivers will use it as we go...
On Tue, 24 May 2011 09:23:42 +0400 Dmitry Artamonow wrote:
On 21:58 Mon 23 May , Randy Dunlap wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com
Looks good.
We probably may also want to move definition of struct gpio into include/linux/gpio.h to make things like this work as well:
static struct gpio some_gpios[] = { { GPIO_BLAH, GPIOF_IN, "BLAH"}, { GPIO_BLAH2, GPIOF_OUT_INIT_LOW, "BLAH2"}, };
static int some_init_function(void) { /* ... */
gpio_request_array(some_gpios, ARRAY_SIZE(some_gpios));
/* ... */ }
These gpio_request_one() and gpio_request_array() are quite handy, so I suppose more and more drivers will use it as we go...
That could help this one:
linux-next-20110524/include/linux/mfd/tps65910.h:774: error: field 'gpio' has incomplete type
and then add some way to handle (e.g.):
struct tps65910 *tps65910 = container_of(gc, struct tps65910, gpio); => linux-next-20110524/drivers/gpio/tps65910-gpio.c:25: warning: type defaults to 'int' in declaration of '__mptr'
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Mon, May 23, 2011 at 09:58:39PM -0700, Randy Dunlap wrote:
Below is a patch that makes the 2 reported drivers build when CONFIG_GPIOLIB is disabled and CONFIG_GENERIC_GPIO is disabled. What do you think of the patch?
Looks OK for me on a first scan, though probably the best approach is to go through and just enable gpiolib on all the architectures that don't have it already yet - that's the more generally useful approach as it means that if plugin boards for the platform need to use gpiolib they can. I already did this for Alpha, I guess I'll try to look see what other architectures could use it.
On Tue, May 24, 2011 at 08:52:48AM +0100, Mark Brown wrote:
On Mon, May 23, 2011 at 09:58:39PM -0700, Randy Dunlap wrote:
Below is a patch that makes the 2 reported drivers build when CONFIG_GPIOLIB is disabled and CONFIG_GENERIC_GPIO is disabled. What do you think of the patch?
Looks OK for me on a first scan, though probably the best approach is to go through and just enable gpiolib on all the architectures that don't have it already yet - that's the more generally useful approach as it means that if plugin boards for the platform need to use gpiolib they can. I already did this for Alpha, I guess I'll try to look see what other architectures could use it.
Looks okay. Please repost with proper s-o-b so I can pick it up.
g.
From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com --- include/asm-generic/gpio.h | 10 ---------- include/linux/gpio.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-)
--- linux-next-20110523.orig/include/asm-generic/gpio.h +++ linux-next-20110523/include/asm-generic/gpio.h @@ -170,16 +170,6 @@ extern int __gpio_cansleep(unsigned gpio
extern int __gpio_to_irq(unsigned gpio);
-#define GPIOF_DIR_OUT (0 << 0) -#define GPIOF_DIR_IN (1 << 0) - -#define GPIOF_INIT_LOW (0 << 1) -#define GPIOF_INIT_HIGH (1 << 1) - -#define GPIOF_IN (GPIOF_DIR_IN) -#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) -#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH) - /** * struct gpio - a structure describing a GPIO with configuration * @gpio: the GPIO number --- linux-next-20110523.orig/include/linux/gpio.h +++ linux-next-20110523/include/linux/gpio.h @@ -3,6 +3,17 @@
/* see Documentation/gpio.txt */
+/* make these flag values available regardless of GPIO kconfig options */ +#define GPIOF_DIR_OUT (0 << 0) +#define GPIOF_DIR_IN (1 << 0) + +#define GPIOF_INIT_LOW (0 << 1) +#define GPIOF_INIT_HIGH (1 << 1) + +#define GPIOF_IN (GPIOF_DIR_IN) +#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) +#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH) + #ifdef CONFIG_GENERIC_GPIO #include <asm/gpio.h>
On Fri, May 27, 2011 at 10:46:57AM -0700, Randy Dunlap wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com
Applied, thanks.
g.
include/asm-generic/gpio.h | 10 ---------- include/linux/gpio.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-)
--- linux-next-20110523.orig/include/asm-generic/gpio.h +++ linux-next-20110523/include/asm-generic/gpio.h @@ -170,16 +170,6 @@ extern int __gpio_cansleep(unsigned gpio
extern int __gpio_to_irq(unsigned gpio);
-#define GPIOF_DIR_OUT (0 << 0) -#define GPIOF_DIR_IN (1 << 0)
-#define GPIOF_INIT_LOW (0 << 1) -#define GPIOF_INIT_HIGH (1 << 1)
-#define GPIOF_IN (GPIOF_DIR_IN) -#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) -#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH)
/**
- struct gpio - a structure describing a GPIO with configuration
- @gpio: the GPIO number
--- linux-next-20110523.orig/include/linux/gpio.h +++ linux-next-20110523/include/linux/gpio.h @@ -3,6 +3,17 @@
/* see Documentation/gpio.txt */
+/* make these flag values available regardless of GPIO kconfig options */ +#define GPIOF_DIR_OUT (0 << 0) +#define GPIOF_DIR_IN (1 << 0)
+#define GPIOF_INIT_LOW (0 << 1) +#define GPIOF_INIT_HIGH (1 << 1)
+#define GPIOF_IN (GPIOF_DIR_IN) +#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) +#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH)
#ifdef CONFIG_GENERIC_GPIO #include <asm/gpio.h>
On Fri, May 27, 2011 at 2:12 PM, Grant Likely grant.likely@secretlab.ca wrote:
On Fri, May 27, 2011 at 10:46:57AM -0700, Randy Dunlap wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com
Applied, thanks.
Hi Randy,
I ended up not pushing this one to Linus. Turns out it causes other breakage on other platforms that don't include include/linux/gpio.h. Since I don't have confidence that I'll be able to find all the offenders, I'm dropping it. I recommend making any drivers that are breaking on these symbols depend on GPIOLIB. Platforms not using gpiolib are strongly discouraged now anyways, and there only a handful of files in drivers/ that reference GPIOF_*.
g.
On Fri, Jun 03, 2011 at 11:04:52AM -0600, Grant Likely wrote:
I ended up not pushing this one to Linus. Turns out it causes other breakage on other platforms that don't include include/linux/gpio.h. Since I don't have confidence that I'll be able to find all the offenders, I'm dropping it. I recommend making any drivers that are
So, this originally came about because I pushed back on adding random dependencies like this for features which are pretty much optional in drivers - their use of GPIOs is totally optional and the dependencies are just too fragile, leading to noise with all the randconfigs. It seems better to get the architectures to keep up with enhancements to gpiolib (or convert to it) than to have to worry about this in drivers.
breaking on these symbols depend on GPIOLIB. Platforms not using gpiolib are strongly discouraged now anyways, and there only a handful of files in drivers/ that reference GPIOF_*.
That's more a result of it being a pretty new feature than anything else I think.
On Fri, Jun 3, 2011 at 11:18 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Jun 03, 2011 at 11:04:52AM -0600, Grant Likely wrote:
I ended up not pushing this one to Linus. Turns out it causes other breakage on other platforms that don't include include/linux/gpio.h. Since I don't have confidence that I'll be able to find all the offenders, I'm dropping it. I recommend making any drivers that are
So, this originally came about because I pushed back on adding random dependencies like this for features which are pretty much optional in drivers - their use of GPIOs is totally optional and the dependencies are just too fragile, leading to noise with all the randconfigs. It seems better to get the architectures to keep up with enhancements to gpiolib (or convert to it) than to have to worry about this in drivers.
Fair enough. Randy, if you or someone else can check that all GPIOF_ users have the required #include <linux/gpio.h>, then I'm okay with this patch.
g.
On 06/03/11 10:42, Grant Likely wrote:
On Fri, Jun 3, 2011 at 11:18 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Jun 03, 2011 at 11:04:52AM -0600, Grant Likely wrote:
I ended up not pushing this one to Linus. Turns out it causes other breakage on other platforms that don't include include/linux/gpio.h. Since I don't have confidence that I'll be able to find all the offenders, I'm dropping it. I recommend making any drivers that are
So, this originally came about because I pushed back on adding random dependencies like this for features which are pretty much optional in drivers - their use of GPIOs is totally optional and the dependencies are just too fragile, leading to noise with all the randconfigs. It seems better to get the architectures to keep up with enhancements to gpiolib (or convert to it) than to have to worry about this in drivers.
Fair enough. Randy, if you or someone else can check that all GPIOF_ users have the required #include <linux/gpio.h>, then I'm okay with this patch.
OK, I'll look at that.
Do you have any examples of builds that failed with this patch?
thanks,
On Sun, 05 Jun 2011 20:45:43 -0700 Randy Dunlap wrote:
On 06/03/11 10:42, Grant Likely wrote:
On Fri, Jun 3, 2011 at 11:18 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Jun 03, 2011 at 11:04:52AM -0600, Grant Likely wrote:
I ended up not pushing this one to Linus. Turns out it causes other breakage on other platforms that don't include include/linux/gpio.h. Since I don't have confidence that I'll be able to find all the offenders, I'm dropping it. I recommend making any drivers that are
So, this originally came about because I pushed back on adding random dependencies like this for features which are pretty much optional in drivers - their use of GPIOs is totally optional and the dependencies are just too fragile, leading to noise with all the randconfigs. It seems better to get the architectures to keep up with enhancements to gpiolib (or convert to it) than to have to worry about this in drivers.
Fair enough. Randy, if you or someone else can check that all GPIOF_ users have the required #include <linux/gpio.h>, then I'm okay with this patch.
OK, I'll look at that.
Of the 70 files that use GPIOF_ macros:
hdr missing in: ./arch/arm/mach-pxa/spitz_pm.c hdr missing in: ./arch/avr32/mach-at32ap/pio.c hdr missing in: ./arch/avr32/mach-at32ap/include/mach/portmux.h hdr missing in: ./arch/avr32/boards/hammerhead/setup.c hdr missing in: ./arch/avr32/boards/hammerhead/flash.c hdr missing in: ./arch/avr32/boards/mimc200/setup.c hdr missing in: ./arch/avr32/boards/atstk1000/setup.c hdr missing in: ./drivers/pcmcia/pxa2xx_vpac270.c
Do you have any examples of builds that failed with this patch?
and would you merge patch(es) that add this header file to those source files? If not, I'll need to make 3 separate patches for them.
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Tue, Jun 14, 2011 at 09:12:11AM -0700, Randy Dunlap wrote:
On Sun, 05 Jun 2011 20:45:43 -0700 Randy Dunlap wrote:
On 06/03/11 10:42, Grant Likely wrote:
On Fri, Jun 3, 2011 at 11:18 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Jun 03, 2011 at 11:04:52AM -0600, Grant Likely wrote:
I ended up not pushing this one to Linus. Turns out it causes other breakage on other platforms that don't include include/linux/gpio.h. Since I don't have confidence that I'll be able to find all the offenders, I'm dropping it. I recommend making any drivers that are
So, this originally came about because I pushed back on adding random dependencies like this for features which are pretty much optional in drivers - their use of GPIOs is totally optional and the dependencies are just too fragile, leading to noise with all the randconfigs. It seems better to get the architectures to keep up with enhancements to gpiolib (or convert to it) than to have to worry about this in drivers.
Fair enough. Randy, if you or someone else can check that all GPIOF_ users have the required #include <linux/gpio.h>, then I'm okay with this patch.
OK, I'll look at that.
Of the 70 files that use GPIOF_ macros:
hdr missing in: ./arch/arm/mach-pxa/spitz_pm.c hdr missing in: ./arch/avr32/mach-at32ap/pio.c hdr missing in: ./arch/avr32/mach-at32ap/include/mach/portmux.h hdr missing in: ./arch/avr32/boards/hammerhead/setup.c hdr missing in: ./arch/avr32/boards/hammerhead/flash.c hdr missing in: ./arch/avr32/boards/mimc200/setup.c hdr missing in: ./arch/avr32/boards/atstk1000/setup.c hdr missing in: ./drivers/pcmcia/pxa2xx_vpac270.c
Do you have any examples of builds that failed with this patch?
and would you merge patch(es) that add this header file to those source files? If not, I'll need to make 3 separate patches for them.
Yes I would. I'm fine with it all being one patch.
g.
On Tue, 14 Jun 2011 10:13:57 -0600 Grant Likely wrote:
On Tue, Jun 14, 2011 at 09:12:11AM -0700, Randy Dunlap wrote:
On Sun, 05 Jun 2011 20:45:43 -0700 Randy Dunlap wrote:
On 06/03/11 10:42, Grant Likely wrote:
On Fri, Jun 3, 2011 at 11:18 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Jun 03, 2011 at 11:04:52AM -0600, Grant Likely wrote:
I ended up not pushing this one to Linus. Turns out it causes other breakage on other platforms that don't include include/linux/gpio.h. Since I don't have confidence that I'll be able to find all the offenders, I'm dropping it. I recommend making any drivers that are
So, this originally came about because I pushed back on adding random dependencies like this for features which are pretty much optional in drivers - their use of GPIOs is totally optional and the dependencies are just too fragile, leading to noise with all the randconfigs. It seems better to get the architectures to keep up with enhancements to gpiolib (or convert to it) than to have to worry about this in drivers.
Fair enough. Randy, if you or someone else can check that all GPIOF_ users have the required #include <linux/gpio.h>, then I'm okay with this patch.
OK, I'll look at that.
Of the 70 files that use GPIOF_ macros:
hdr missing in: ./arch/arm/mach-pxa/spitz_pm.c hdr missing in: ./arch/avr32/mach-at32ap/pio.c hdr missing in: ./arch/avr32/mach-at32ap/include/mach/portmux.h hdr missing in: ./arch/avr32/boards/hammerhead/setup.c hdr missing in: ./arch/avr32/boards/hammerhead/flash.c hdr missing in: ./arch/avr32/boards/mimc200/setup.c hdr missing in: ./arch/avr32/boards/atstk1000/setup.c hdr missing in: ./drivers/pcmcia/pxa2xx_vpac270.c
Do you have any examples of builds that failed with this patch?
and would you merge patch(es) that add this header file to those source files? If not, I'll need to make 3 separate patches for them.
That list of files above is incorrect. Bad grep pattern. Drop all of the avr32 files. Ending patch is small -- sending it soon.
Yes I would. I'm fine with it all being one patch.
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Fixes these build errors in linux-next: sound/soc/codecs/ak4641.c:524: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function) sound/soc/codecs/wm8915.c:2921: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function)
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com --- include/asm-generic/gpio.h | 10 ---------- include/linux/gpio.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-)
--- linux-next-20110614.orig/include/asm-generic/gpio.h +++ linux-next-20110614/include/asm-generic/gpio.h @@ -170,16 +170,6 @@ extern int __gpio_cansleep(unsigned gpio
extern int __gpio_to_irq(unsigned gpio);
-#define GPIOF_DIR_OUT (0 << 0) -#define GPIOF_DIR_IN (1 << 0) - -#define GPIOF_INIT_LOW (0 << 1) -#define GPIOF_INIT_HIGH (1 << 1) - -#define GPIOF_IN (GPIOF_DIR_IN) -#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) -#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH) - /** * struct gpio - a structure describing a GPIO with configuration * @gpio: the GPIO number --- linux-next-20110614.orig/include/linux/gpio.h +++ linux-next-20110614/include/linux/gpio.h @@ -3,6 +3,17 @@
/* see Documentation/gpio.txt */
+/* make these flag values available regardless of GPIO kconfig options */ +#define GPIOF_DIR_OUT (0 << 0) +#define GPIOF_DIR_IN (1 << 0) + +#define GPIOF_INIT_LOW (0 << 1) +#define GPIOF_INIT_HIGH (1 << 1) + +#define GPIOF_IN (GPIOF_DIR_IN) +#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) +#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH) + #ifdef CONFIG_GENERIC_GPIO #include <asm/gpio.h>
On Tue, Jun 14, 2011 at 05:05:11PM -0700, Randy Dunlap wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Make GPIOF_ defined values available even when GPIOLIB nor GENERIC_GPIO is enabled by moving them to <linux/gpio.h>.
Fixes these build errors in linux-next: sound/soc/codecs/ak4641.c:524: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function) sound/soc/codecs/wm8915.c:2921: error: 'GPIOF_OUT_INIT_LOW' undeclared (first use in this function)
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com
Applied, thanks.
g.
include/asm-generic/gpio.h | 10 ---------- include/linux/gpio.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 10 deletions(-)
--- linux-next-20110614.orig/include/asm-generic/gpio.h +++ linux-next-20110614/include/asm-generic/gpio.h @@ -170,16 +170,6 @@ extern int __gpio_cansleep(unsigned gpio
extern int __gpio_to_irq(unsigned gpio);
-#define GPIOF_DIR_OUT (0 << 0) -#define GPIOF_DIR_IN (1 << 0)
-#define GPIOF_INIT_LOW (0 << 1) -#define GPIOF_INIT_HIGH (1 << 1)
-#define GPIOF_IN (GPIOF_DIR_IN) -#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) -#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH)
/**
- struct gpio - a structure describing a GPIO with configuration
- @gpio: the GPIO number
--- linux-next-20110614.orig/include/linux/gpio.h +++ linux-next-20110614/include/linux/gpio.h @@ -3,6 +3,17 @@
/* see Documentation/gpio.txt */
+/* make these flag values available regardless of GPIO kconfig options */ +#define GPIOF_DIR_OUT (0 << 0) +#define GPIOF_DIR_IN (1 << 0)
+#define GPIOF_INIT_LOW (0 << 1) +#define GPIOF_INIT_HIGH (1 << 1)
+#define GPIOF_IN (GPIOF_DIR_IN) +#define GPIOF_OUT_INIT_LOW (GPIOF_DIR_OUT | GPIOF_INIT_LOW) +#define GPIOF_OUT_INIT_HIGH (GPIOF_DIR_OUT | GPIOF_INIT_HIGH)
#ifdef CONFIG_GENERIC_GPIO #include <asm/gpio.h>
From: Randy Dunlap randy.dunlap@oracle.com
Some files use GPIOF_ macros but don't include the header file for them. These macros are being moved to <linux/gpio.h>, so add includes for <linux/gpio.h> where needed.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com --- arch/arm/mach-pxa/spitz_pm.c | 1 + drivers/pcmcia/pxa2xx_vpac270.c | 1 + 2 files changed, 2 insertions(+)
--- linux-next-20110614.orig/arch/arm/mach-pxa/spitz_pm.c +++ linux-next-20110614/arch/arm/mach-pxa/spitz_pm.c @@ -14,6 +14,7 @@ #include <linux/init.h> #include <linux/kernel.h> #include <linux/delay.h> +#include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/platform_device.h> #include <linux/apm-emulation.h> --- linux-next-20110614.orig/drivers/pcmcia/pxa2xx_vpac270.c +++ linux-next-20110614/drivers/pcmcia/pxa2xx_vpac270.c @@ -11,6 +11,7 @@ * */
+#include <linux/gpio.h> #include <linux/module.h> #include <linux/platform_device.h>
Hi Randy,
On Tue, 14 Jun 2011 17:06:02 -0700 Randy Dunlap randy.dunlap@oracle.com wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Some files use GPIOF_ macros but don't include the header file for them. These macros are being moved to <linux/gpio.h>, so add includes for <linux/gpio.h> where needed.
Shouldn't these patches be in the other order to avoid bisection problems?
On 06/14/11 17:34, Stephen Rothwell wrote:
Hi Randy,
On Tue, 14 Jun 2011 17:06:02 -0700 Randy Dunlap randy.dunlap@oracle.com wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Some files use GPIOF_ macros but don't include the header file for them. These macros are being moved to <linux/gpio.h>, so add includes for <linux/gpio.h> where needed.
Shouldn't these patches be in the other order to avoid bisection problems?
Hm, I suppose so.
Grant, can you apply these with patch 2/2 first or do I need to resend the patches?
thanks,
On Wed, Jun 15, 2011 at 11:59 AM, Randy Dunlap randy.dunlap@oracle.com wrote:
On 06/14/11 17:34, Stephen Rothwell wrote:
Hi Randy,
On Tue, 14 Jun 2011 17:06:02 -0700 Randy Dunlap randy.dunlap@oracle.com wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Some files use GPIOF_ macros but don't include the header file for them. These macros are being moved to <linux/gpio.h>, so add includes for <linux/gpio.h> where needed.
Shouldn't these patches be in the other order to avoid bisection problems?
Hm, I suppose so.
Grant, can you apply these with patch 2/2 first or do I need to resend the patches?
You don't need to resend. I'll apply them in the correct order.
g.
On Tue, Jun 14, 2011 at 05:06:02PM -0700, Randy Dunlap wrote:
From: Randy Dunlap randy.dunlap@oracle.com
Some files use GPIOF_ macros but don't include the header file for them. These macros are being moved to <linux/gpio.h>, so add includes for <linux/gpio.h> where needed.
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com
Applied, thanks.
g.
arch/arm/mach-pxa/spitz_pm.c | 1 + drivers/pcmcia/pxa2xx_vpac270.c | 1 + 2 files changed, 2 insertions(+)
--- linux-next-20110614.orig/arch/arm/mach-pxa/spitz_pm.c +++ linux-next-20110614/arch/arm/mach-pxa/spitz_pm.c @@ -14,6 +14,7 @@ #include <linux/init.h> #include <linux/kernel.h> #include <linux/delay.h> +#include <linux/gpio.h> #include <linux/interrupt.h> #include <linux/platform_device.h> #include <linux/apm-emulation.h> --- linux-next-20110614.orig/drivers/pcmcia/pxa2xx_vpac270.c +++ linux-next-20110614/drivers/pcmcia/pxa2xx_vpac270.c @@ -11,6 +11,7 @@
*/
+#include <linux/gpio.h> #include <linux/module.h> #include <linux/platform_device.h>
participants (5)
-
Dmitry Artamonow -
Grant Likely -
Mark Brown -
Randy Dunlap -
Stephen Rothwell