On Mon, Apr 30, 2012 at 8:52 AM, Sachin Kamat <[email protected]> wrote:
> +#if defined(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5)
> +#endif
> -static struct samsung_gpio_chip exynos4_gpios_1[] = {
> ?#ifdef CONFIG_ARCH_EXYNOS4
> +static struct samsung_gpio_chip exynos4_gpios_1[] = {
> -#endif
> +#endif
> -static struct samsung_gpio_chip exynos4_gpios_2[] = {
> ?#ifdef CONFIG_ARCH_EXYNOS4
> +static struct samsung_gpio_chip exynos4_gpios_2[] = {
> -#endif
> +#endif
> -static struct samsung_gpio_chip exynos4_gpios_3[] = {
> ?#ifdef CONFIG_ARCH_EXYNOS4
> +static struct samsung_gpio_chip exynos4_gpios_3[] = {
> -#endif
> +#endif
> ?#ifdef CONFIG_ARCH_EXYNOS5
> +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
> +#endif
> +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
> +#endif
We really want to get rid of this kind of stuff from all drivers, #ifdefs are
declared ugly already in Documentation/CodingStyle.
Any chance you could solve this problem by reworking the driver to
pass some flag in platform data tell which exynos it's for
and jist adapt at runtime instead of the compile-time quirkiness?
Besides looking better, it helps us to get to a single zImage for the
exynoses too..
Yours,
Linus Walleij
On 02/05/2012, Linus Walleij <[email protected]> wrote:
> On Mon, Apr 30, 2012 at 8:52 AM, Sachin Kamat <[email protected]>
> wrote:
>
>> +#if defined(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5)
>> +#endif
>> -static struct samsung_gpio_chip exynos4_gpios_1[] = {
>> #ifdef CONFIG_ARCH_EXYNOS4
>> +static struct samsung_gpio_chip exynos4_gpios_1[] = {
>> -#endif
>> +#endif
>> -static struct samsung_gpio_chip exynos4_gpios_2[] = {
>> #ifdef CONFIG_ARCH_EXYNOS4
>> +static struct samsung_gpio_chip exynos4_gpios_2[] = {
>> -#endif
>> +#endif
>> -static struct samsung_gpio_chip exynos4_gpios_3[] = {
>> #ifdef CONFIG_ARCH_EXYNOS4
>> +static struct samsung_gpio_chip exynos4_gpios_3[] = {
>> -#endif
>> +#endif
>> #ifdef CONFIG_ARCH_EXYNOS5
>> +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
>> +#endif
>> +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
>> +#endif
>
> We really want to get rid of this kind of stuff from all drivers, #ifdefs
> are
> declared ugly already in Documentation/CodingStyle.
Yes, you are right. #Ifdefs should be avoided as far as possible and
more importantly to get a single zImage.
>
> Any chance you could solve this problem by reworking the driver to
> pass some flag in platform data tell which exynos it's for
> and jist adapt at runtime instead of the compile-time quirkiness?
No. I wouldn't be working on this, hence provided this patch to get
rid of the nasty compiler warnings until someone fixes the same
(compile-time to runtime adaptation).
>
> Besides looking better, it helps us to get to a single zImage for the
> exynoses too..
Definitely.
>
> Yours,
> Linus Walleij
>
--
With warm regards,
Sachin
On Wed, 2 May 2012 01:26:12 +0200, Linus Walleij <[email protected]> wrote:
> On Mon, Apr 30, 2012 at 8:52 AM, Sachin Kamat <[email protected]> wrote:
>
> > +#if defined(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5)
> > +#endif
> > -static struct samsung_gpio_chip exynos4_gpios_1[] = {
> > #ifdef CONFIG_ARCH_EXYNOS4
> > +static struct samsung_gpio_chip exynos4_gpios_1[] = {
> > -#endif
> > +#endif
> > -static struct samsung_gpio_chip exynos4_gpios_2[] = {
> > #ifdef CONFIG_ARCH_EXYNOS4
> > +static struct samsung_gpio_chip exynos4_gpios_2[] = {
> > -#endif
> > +#endif
> > -static struct samsung_gpio_chip exynos4_gpios_3[] = {
> > #ifdef CONFIG_ARCH_EXYNOS4
> > +static struct samsung_gpio_chip exynos4_gpios_3[] = {
> > -#endif
> > +#endif
> > #ifdef CONFIG_ARCH_EXYNOS5
> > +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
> > +#endif
> > +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
> > +#endif
>
> We really want to get rid of this kind of stuff from all drivers, #ifdefs are
> declared ugly already in Documentation/CodingStyle.
>
> Any chance you could solve this problem by reworking the driver to
> pass some flag in platform data tell which exynos it's for
> and jist adapt at runtime instead of the compile-time quirkiness?
>
> Besides looking better, it helps us to get to a single zImage for the
> exynoses too..
I don't see any single-zImage issues here. There are no #else clauses
in the #ifdef blocks so all it does it compile out unused code when
exynos4 & 5 is not enabled. I'm going to apply it.
g.
On Saturday 12 May 2012, Grant Likely wrote:
> On Wed, 2 May 2012 01:26:12 +0200, Linus Walleij <[email protected]> wrote:
> > On Mon, Apr 30, 2012 at 8:52 AM, Sachin Kamat <[email protected]> wrote:
> >
> > > +#if defined(CONFIG_ARCH_EXYNOS4) || defined(CONFIG_ARCH_EXYNOS5)
> > > +#endif
> > > -static struct samsung_gpio_chip exynos4_gpios_1[] = {
> > > ? #ifdef CONFIG_ARCH_EXYNOS4
> > > +static struct samsung_gpio_chip exynos4_gpios_1[] = {
> > > -#endif
> > > +#endif
> > > -static struct samsung_gpio_chip exynos4_gpios_2[] = {
> > > ? #ifdef CONFIG_ARCH_EXYNOS4
> > > +static struct samsung_gpio_chip exynos4_gpios_2[] = {
> > > -#endif
> > > +#endif
> > > -static struct samsung_gpio_chip exynos4_gpios_3[] = {
> > > ? #ifdef CONFIG_ARCH_EXYNOS4
> > > +static struct samsung_gpio_chip exynos4_gpios_3[] = {
> > > -#endif
> > > +#endif
> > > ? #ifdef CONFIG_ARCH_EXYNOS5
> > > +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
> > > +#endif
> > > +#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
> > > +#endif
> >
> > We really want to get rid of this kind of stuff from all drivers, #ifdefs are
> > declared ugly already in Documentation/CodingStyle.
> >
> > Any chance you could solve this problem by reworking the driver to
> > pass some flag in platform data tell which exynos it's for
> > and jist adapt at runtime instead of the compile-time quirkiness?
> >
> > Besides looking better, it helps us to get to a single zImage for the
> > exynoses too..
>
> I don't see any single-zImage issues here. There are no #else clauses
> in the #ifdef blocks so all it does it compile out unused code when
> exynos4 & 5 is not enabled. I'm going to apply it.
I needed another fixup on top of this to build exynos4_defconfig without
warnings.
8<---
gpio/samsung: define gpio_base4 variable only for exynos5
Avoids this warning:
drivers/gpio/gpio-samsung.c: In function 'samsung_gpiolib_init':
drivers/gpio/gpio-samsung.c:2979:1: warning: label 'err_ioremap4' defined but not used [-Wunused-label]
drivers/gpio/gpio-samsung.c:2725:47: warning: unused variable 'gpio_base4' [-Wunused-variable]
Signed-off-by: Arnd Bergmann <[email protected]>
diff --git a/drivers/gpio/gpio-samsung.c b/drivers/gpio/gpio-samsung.c
index e991d91..c80356d 100644
--- a/drivers/gpio/gpio-samsung.c
+++ b/drivers/gpio/gpio-samsung.c
@@ -2722,7 +2722,10 @@ static __init int samsung_gpiolib_init(void)
struct samsung_gpio_chip *chip;
int i, nr_chips;
#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
- void __iomem *gpio_base1, *gpio_base2, *gpio_base3, *gpio_base4;
+ void __iomem *gpio_base1, *gpio_base2, *gpio_base3;
+#endif
+#if defined(CONFIG_SOC_EXYNOS5250)
+ void __iomem *gpio_base4;
#endif
int group = 0;
@@ -2975,9 +2978,11 @@ static __init int samsung_gpiolib_init(void)
return 0;
-#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
+#if defined(CONFIG_SOC_EXYNOS5250)
err_ioremap4:
iounmap(gpio_base3);
+#endif
+#if defined(CONFIG_CPU_EXYNOS4210) || defined(CONFIG_SOC_EXYNOS5250)
err_ioremap3:
iounmap(gpio_base2);
err_ioremap2: