Re: linux-next: Tree for May 23 (sound/soc/codecs)

Re: linux-next: Tree for May 23 (sound/soc/codecs)

[Randy Dunlap]

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

Re: linux-next: Tree for May 23 (sound/soc/codecs)

[Mark Brown]

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.

Re: linux-next: Tree for May 23 (sound/soc/codecs)

[Randy Dunlap]

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

Re: linux-next: Tree for May 23 (sound/soc/codecs)

[Mark Brown]

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.

Re: linux-next: Tree for May 23 (sound/soc/codecs)

[Randy Dunlap]

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.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: linux-next: Tree for May 23 (sound/soc/codecs)

[Mark Brown]

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.

[PATCH/RFC] gpio: add GPIOF_ values regardless on kconfig settings

[Randy Dunlap]

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>
 

Re: [PATCH/RFC] gpio: add GPIOF_ values regardless on kconfig settings

[Dmitry Artamonow]

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

-- 
Best regards,
Dmitry "MAD" Artamonow

Re: [PATCH/RFC] gpio: add GPIOF_ values regardless on kconfig settings

[Randy Dunlap]

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

Re: [PATCH/RFC] gpio: add GPIOF_ values regardless on kconfig settings

[Mark Brown]

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.

Re: [alsa-devel] [PATCH/RFC] gpio: add GPIOF_ values regardless on kconfig settings

[Grant Likely]

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.

[PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Randy Dunlap]

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>
 

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Grant Likely]

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

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Grant Likely]

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.

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Mark Brown]

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.

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Grant Likely]

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.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Randy Dunlap]

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,
-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Randy Dunlap]

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

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Grant Likely]

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.

Re: [PATCH] gpio: add GPIOF_ values regardless on kconfig settings

[Randy Dunlap]

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

[PATCH 1/2] gpio: add GPIOF_ values regardless on kconfig settings

[Randy Dunlap]

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>
 

Re: [PATCH 1/2] gpio: add GPIOF_ values regardless on kconfig settings

[Grant Likely]

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

[PATCH 2/2] gpio: include linux/gpio.h where needed

[Randy Dunlap]

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>
 

Re: [PATCH 2/2] gpio: include linux/gpio.h where needed

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

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?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 2/2] gpio: include linux/gpio.h where needed

[Randy Dunlap]

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,
-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 2/2] gpio: include linux/gpio.h where needed

[Grant Likely]

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.

Re: [PATCH 2/2] gpio: include linux/gpio.h where needed

[Grant Likely]

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