52xx build error

PATCH In review Benjamin Herrenschmidt 3 participants 7 messages Aug 24, 2010 – Sep 1, 2010 GPIO

Share & tools

Benjamin Herrenschmidt

author

2010-08-24 06:26 UTC #0 lore raw

I get that with my current stuff:

cc1: warnings being treated as errors
In file included from /home/benh/linux-powerpc-test/arch/powerpc/platforms/52xx/mpc52xx_common.c:19:
/home/benh/linux-powerpc-test/include/linux/of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list /home/benh/linux-powerpc-test/include/linux/of_gpio.h:74: error: its scope is only this definition or declaration, which is probably not what you want /home/benh/linux-powerpc-test/include/linux/of_gpio.h:75: error: ‘struct gpio_chip’ declared inside parameter list
CC mm/bootmem.o
make[3]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1
make[3]: *** Waiting for unfinished jobs....

Cheers,
Ben.

Anton Vorontsov

2010-08-24 09:26 UTC #1 lore raw

PATCH 1/1 gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case

With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC:

cc1: warnings being treated as errors
In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19:
include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared
inside parameter list
include/linux/of_gpio.h:74: warning: its scope is only this definition
or declaration, which is probably not what you want
include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared
inside parameter list
make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1

This patch fixes the issue by providing the proper forward declaration.

Signed-off-by Anton Vorontsov <[email protected]>

---

Quoted 5 lines · Benjamin Herrenschmidt

I get that with my current stuff:
cc1: warnings being treated as errors
In file included from [..]/mpc52xx_common.c:19:
of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list

[...]

Quoted 1 line

make[3]: *** Waiting for unfinished jobs....

That's because with GPIOCHIP=n no one declares struct gpio_chip.

It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as this feels more generic, and we already have some !GPIOLIB handling in there.

 include/linux/gpio.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

include/linux/gpio.h +6 -0

index 03f616b..85207d2 100644
@@ -15,6 +15,12 @@
 struct device;
 
 /*
+ * Some code might rely on the declaration. Still, it is illegal
+ * to dereference it for !GPIOLIB case.
+ */
+struct gpio_chip;
+
+/*
  * Some platforms don't support the GPIO programming interface.
  *
  * In case some driver uses it anyway (it should normally have

Signature · 1 line

1.7.0.5

Grant Likely

2010-08-31 08:03 UTC #2 lore raw

Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case

Quoted 14 lines · Anton Vorontsov

With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC:
cc1: warnings being treated as errors
In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19:
include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared
inside parameter list
include/linux/of_gpio.h:74: warning: its scope is only this definition
or declaration, which is probably not what you want
include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared
inside parameter list
make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1

Quoted 3 lines

This patch fixes the issue by providing the proper forward declaration.
Signed-off-by Anton Vorontsov <[email protected]>

This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=n to catch exactly these build failures. The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig.

Quoted 40 lines

---
Quoted 5 lines · Benjamin Herrenschmidt
I get that with my current stuff:
cc1: warnings being treated as errors
In file included from [..]/mpc52xx_common.c:19:
of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list
[...]
Quoted 1 line
make[3]: *** Waiting for unfinished jobs....
That's because with GPIOCHIP=n no one declares struct gpio_chip.
It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as this feels more generic, and we already have some !GPIOLIB handling in there.
 include/linux/gpio.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
include/linux/gpio.h +6 -0
index 03f616b..85207d2 100644
@@ -15,6 +15,12 @@
 struct device;
 
 /*
+ * Some code might rely on the declaration. Still, it is illegal
+ * to dereference it for !GPIOLIB case.
+ */
+struct gpio_chip;
+
+/*
  * Some platforms don't support the GPIO programming interface.
  *
  * In case some driver uses it anyway (it should normally have
Signature · 1 line
1.7.0.5

Anton Vorontsov

2010-08-31 08:37 UTC #3 lore raw

Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case

Quoted 17 lines · Grant Likely

Quoted 16 lines · Anton Vorontsov
With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC:
cc1: warnings being treated as errors
In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19:
include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared
inside parameter list
include/linux/of_gpio.h:74: warning: its scope is only this definition
or declaration, which is probably not what you want
include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared
inside parameter list
make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1
This patch fixes the issue by providing the proper forward declaration.

Quoted 7 lines

Quoted 1 line
Signed-off-by Anton Vorontsov <[email protected]>
This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=n to catch exactly these build failures. The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig.

No, look closer. The error is in of_gpio.h, and it's perfectly fine to include it w/o GPIOLIB=y.

Quoted 40 lines

Quoted 40 lines
---
Quoted 5 lines · Benjamin Herrenschmidt
I get that with my current stuff:
cc1: warnings being treated as errors
In file included from [..]/mpc52xx_common.c:19:
of_gpio.h:74: error: ‘struct gpio_chip’ declared inside parameter list
[...]
Quoted 1 line
make[3]: *** Waiting for unfinished jobs....
That's because with GPIOCHIP=n no one declares struct gpio_chip.
It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as this feels more generic, and we already have some !GPIOLIB handling in there.
 include/linux/gpio.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
include/linux/gpio.h +6 -0
index 03f616b..85207d2 100644
@@ -15,6 +15,12 @@
 struct device;
 
 /*
+ * Some code might rely on the declaration. Still, it is illegal
+ * to dereference it for !GPIOLIB case.
+ */
+struct gpio_chip;
+
+/*
  * Some platforms don't support the GPIO programming interface.
  *
  * In case some driver uses it anyway (it should normally have
Signature · 1 line
1.7.0.5

Signature · 3 lines

Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

Grant Likely

2010-08-31 16:44 UTC #4 lore raw

Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case

On Tue, Aug 31, 2010 at 2:37 AM, Anton Vorontsov <[email protected]> w= rote:

Quoted 7 lines · Grant Likely

Quoted 6 lines · Anton Vorontsov
Quoted 5 lines
With CONFIG_GPIOLIB=3Dn, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC:
=A0 cc1: warnings being treated as errors
=A0 In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c=

:19:

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
=A0 include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 inside par=

ameter list

Quoted 1 line

Quoted 1 line
Quoted 1 line
=A0 include/linux/of_gpio.h:74: warning: its scope is only this defini=

tion

Quoted 1 line

Quoted 1 line
Quoted 1 line
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 or declara=

tion, which is probably not what

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 you want =A0 include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 inside par=

ameter list

Quoted 1 line

Quoted 1 line
Quoted 1 line
=A0 make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error =

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
This patch fixes the issue by providing the proper forward declaration=

Quoted 11 lines

Quoted 8 lines
Quoted 2 lines
Signed-off-by Anton Vorontsov <[email protected]>
This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=3Dn to catch exactly these build failures. =A0The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig.
No, look closer. The error is in of_gpio.h, and it's perfectly fine to include it w/o GPIOLIB=3Dy.

Looking even closer, we're both wrong. You're right I didn't look carefully enough, and the error is in of_gpio.h, not the .c file.

However, it is not okay to get the definitions from of_gpio.h when CONFIG_GPIOLIB=3Dn. If GPIOLIB, or more specifically OF_GPIO isn't set, then the of_gpio.h definitions should either not be defined, or should be defined as empty stubs (where appropriate).

So, instead of adding a forward declarations of the struct, the correct thing I think to do is to #ifdef out the contents of of_gpio.h when GPIOLIB isn't selected so that extra #includes are completely benign. I've been doing the same thing on the other linux/of*.h headers. I've got a patch in my tree that I'm testing right now and I'll post later today.

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
---
On Tue, Aug 24, 2010 at 04:26:08PM +1000, Benjamin Herrenschmidt wrote=

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
I get that with my current stuff:
cc1: warnings being treated as errors
In file included from [..]/mpc52xx_common.c:19:
of_gpio.h:74: error: =91struct gpio_chip=92 declared inside paramete=

r list

Quoted 29 lines

Quoted 29 lines
Quoted 29 lines
[...]
Quoted 1 line
make[3]: *** Waiting for unfinished jobs....
That's because with GPIOCHIP=3Dn no one declares struct gpio_chip.
It should be either of_gpio.h or gpio.h. Let's make it gpio.h, as this feels more generic, and we already have some !GPIOLIB handling in there.
=A0include/linux/gpio.h | =A0 =A06 ++++++ =A01 files changed, 6 insertions(+), 0 deletions(-)
include/linux/gpio.h +0 -0
index 03f616b..85207d2 100644
@@ -15,6 +15,12 @@
 =A0struct device;
 
=A0/*
+ * Some code might rely on the declaration. Still, it is illegal
+ * to dereference it for !GPIOLIB case.
+ */
include/linux/gpio.h +3 -0
+struct gpio_chip;
+
+/*
=A0 * Some platforms don't support the GPIO programming interface. =A0 *
=A0 * In case some driver uses it anyway (it should normally have

Quoted 9 lines

Quoted 3 lines
Quoted 3 lines
--
1.7.0.5
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

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

Anton Vorontsov

2010-08-31 17:08 UTC #5 lore raw

Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case

Quoted 30 lines · Grant Likely

Quoted 20 lines · Anton Vorontsov
Quoted 19 lines · Grant Likely
Quoted 17 lines · Anton Vorontsov
With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC:
cc1: warnings being treated as errors
In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19:
include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared inside parameter list include/linux/of_gpio.h:74: warning: its scope is only this definition or declaration, which is probably not what you want include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared inside parameter list
make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1
This patch fixes the issue by providing the proper forward declaration.
Signed-off-by Anton Vorontsov <[email protected]>
Quoted 8 lines
Quoted 5 lines
This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=n to catch exactly these build failures. The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig.
No, look closer. The error is in of_gpio.h, and it's perfectly fine to include it w/o GPIOLIB=y.

Quoted 7 lines

Looking even closer, we're both wrong. You're right I didn't look carefully enough, and the error is in of_gpio.h, not the .c file.
However, it is not okay to get the definitions from of_gpio.h when CONFIG_GPIOLIB=n. If GPIOLIB, or more specifically OF_GPIO isn't set, then the of_gpio.h definitions should either not be defined, or should be defined as empty stubs (where appropriate).

Grrr. Grant, look again, even closer than you did.

They are stubs!

#else /* CONFIG_OF_GPIO */ <<<<<< !OF_GPIO (or !GPIOLIB) case.

/* Drivers may not strictly depend on the GPIO support, so let them link. */ static inline int of_get_gpio_flags(struct device_node *np, int index,
enum of_gpio_flags *flags)

{
        return -ENOSYS;
}

static inline unsigned int of_gpio_count(struct device_node *np)

{
        return 0;
}

static inline void of_gpiochip_add(struct gpio_chip *gc) { } static inline void of_gpiochip_remove(struct gpio_chip *gc) { }

#endif /* CONFIG_OF_GPIO */

The errors are triggered by the of_gpiochip_*() stubs, which are needed by the drivers/gpio/gpiolib.c.

Do you want to add another #ifdef CONFIG_GPIOLIB around of_gpiochip_*()? That would be ugly.

There's nothing wrong in providing the forward decls, you can't dereference it anyway (so you would still catch the bogus users). And at the same time it's will work great for these stubs.

Signature · 3 lines

Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

Grant Likely

2010-09-01 15:26 UTC #6 lore raw

Re: [PATCH] gpiolib: Add 'struct gpio_chip' forward declaration for !GPIOLIB case

Quoted 34 lines · Anton Vorontsov

Quoted 33 lines · Grant Likely
Quoted 20 lines · Anton Vorontsov
Quoted 19 lines · Grant Likely
Quoted 17 lines · Anton Vorontsov
With CONFIG_GPIOLIB=n, the 'struct gpio_chip' is not declared, so the following pops up on PowerPC:
cc1: warnings being treated as errors
In file included from arch/powerpc/platforms/52xx/mpc52xx_common.c:19:
include/linux/of_gpio.h:74: warning: 'struct gpio_chip' declared inside parameter list include/linux/of_gpio.h:74: warning: its scope is only this definition or declaration, which is probably not what you want include/linux/of_gpio.h:75: warning: 'struct gpio_chip' declared inside parameter list
make[2]: *** [arch/powerpc/platforms/52xx/mpc52xx_common.o] Error 1
This patch fixes the issue by providing the proper forward declaration.
Signed-off-by Anton Vorontsov <[email protected]>
Quoted 8 lines
Quoted 5 lines
This doesn't actually solve the problem, and gpiochip should remain undefined when CONFIG_GPIOLIB=n to catch exactly these build failures. The real problem is that I merged a change into the mpc5200 code that required CONFIG_GPIOLIB be enabled without reflecting that requirement in Kconfig.
No, look closer. The error is in of_gpio.h, and it's perfectly fine to include it w/o GPIOLIB=y.
Looking even closer, we're both wrong. You're right I didn't look carefully enough, and the error is in of_gpio.h, not the .c file.

Quoted 6 lines

Quoted 4 lines
However, it is not okay to get the definitions from of_gpio.h when CONFIG_GPIOLIB=n. If GPIOLIB, or more specifically OF_GPIO isn't set, then the of_gpio.h definitions should either not be defined, or should be defined as empty stubs (where appropriate).
Grrr. Grant, look again, even closer than you did.

Gah. Very bad day yesterday. Applied and sorry for the noise.