[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Jean-Christophe PLAGNIOL-VILLARD]

this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 include/asm-generic/gpio.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 1ff4e22..c08b758 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -122,7 +122,6 @@ struct gpio_chip {
 	unsigned		can_sleep:1;
 	unsigned		exported:1;
 
-#if defined(CONFIG_OF_GPIO)
 	/*
 	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
 	 * device tree automatically may have an OF translation
@@ -131,7 +130,6 @@ struct gpio_chip {
 	int of_gpio_n_cells;
 	int (*of_xlate)(struct gpio_chip *gc,
 		        const struct of_phandle_args *gpiospec, u32 *flags);
-#endif
 };
 
 extern const char *gpiochip_is_requested(struct gpio_chip *chip,
-- 
1.7.7

Re: [PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Nicolas Ferre]

On 02/13/2012 10:23 AM, Jean-Christophe PLAGNIOL-VILLARD :
> this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> Cc: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>

Added devicetree-discuss ml.

> ---
>  include/asm-generic/gpio.h |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 1ff4e22..c08b758 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -122,7 +122,6 @@ struct gpio_chip {
>  	unsigned		can_sleep:1;
>  	unsigned		exported:1;
>  
> -#if defined(CONFIG_OF_GPIO)
>  	/*
>  	 * If CONFIG_OF is enabled, then all GPIO controllers described in the

Maybe you will need to rework this comment.

>  	 * device tree automatically may have an OF translation
> @@ -131,7 +130,6 @@ struct gpio_chip {
>  	int of_gpio_n_cells;
>  	int (*of_xlate)(struct gpio_chip *gc,
>  		        const struct of_phandle_args *gpiospec, u32 *flags);
> -#endif

I do not know if we can remove the ifdef completely of just move of_node
out of it.


>  };
>  
>  extern const char *gpiochip_is_requested(struct gpio_chip *chip,


-- 
Nicolas Ferre

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Nicolas Ferre]

On 02/13/2012 10:23 AM, Jean-Christophe PLAGNIOL-VILLARD :
> this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>

Added devicetree-discuss ml.

> ---
>  include/asm-generic/gpio.h |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 1ff4e22..c08b758 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -122,7 +122,6 @@ struct gpio_chip {
>  	unsigned		can_sleep:1;
>  	unsigned		exported:1;
>  
> -#if defined(CONFIG_OF_GPIO)
>  	/*
>  	 * If CONFIG_OF is enabled, then all GPIO controllers described in the

Maybe you will need to rework this comment.

>  	 * device tree automatically may have an OF translation
> @@ -131,7 +130,6 @@ struct gpio_chip {
>  	int of_gpio_n_cells;
>  	int (*of_xlate)(struct gpio_chip *gc,
>  		        const struct of_phandle_args *gpiospec, u32 *flags);
> -#endif

I do not know if we can remove the ifdef completely of just move of_node
out of it.


>  };
>  
>  extern const char *gpiochip_is_requested(struct gpio_chip *chip,


-- 
Nicolas Ferre

Re: [PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:18 Mon 13 Feb     , Nicolas Ferre wrote:
> On 02/13/2012 10:23 AM, Jean-Christophe PLAGNIOL-VILLARD :
> > this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Added devicetree-discuss ml.
> 
> > ---
> >  include/asm-generic/gpio.h |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index 1ff4e22..c08b758 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -122,7 +122,6 @@ struct gpio_chip {
> >  	unsigned		can_sleep:1;
> >  	unsigned		exported:1;
> >  
> > -#if defined(CONFIG_OF_GPIO)
> >  	/*
> >  	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
> 
> Maybe you will need to rework this comment.
> 
> >  	 * device tree automatically may have an OF translation
> > @@ -131,7 +130,6 @@ struct gpio_chip {
> >  	int of_gpio_n_cells;
> >  	int (*of_xlate)(struct gpio_chip *gc,
> >  		        const struct of_phandle_args *gpiospec, u32 *flags);
> > -#endif
> 
> I do not know if we can remove the ifdef completely of just move of_node
> out of it.
basicaly ifdef on a struct is so so
so yes

Best Regards,
J.

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:18 Mon 13 Feb     , Nicolas Ferre wrote:
> On 02/13/2012 10:23 AM, Jean-Christophe PLAGNIOL-VILLARD :
> > this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Added devicetree-discuss ml.
> 
> > ---
> >  include/asm-generic/gpio.h |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index 1ff4e22..c08b758 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -122,7 +122,6 @@ struct gpio_chip {
> >  	unsigned		can_sleep:1;
> >  	unsigned		exported:1;
> >  
> > -#if defined(CONFIG_OF_GPIO)
> >  	/*
> >  	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
> 
> Maybe you will need to rework this comment.
> 
> >  	 * device tree automatically may have an OF translation
> > @@ -131,7 +130,6 @@ struct gpio_chip {
> >  	int of_gpio_n_cells;
> >  	int (*of_xlate)(struct gpio_chip *gc,
> >  		        const struct of_phandle_args *gpiospec, u32 *flags);
> > -#endif
> 
> I do not know if we can remove the ifdef completely of just move of_node
> out of it.
basicaly ifdef on a struct is so so
so yes

Best Regards,
J.

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Rob Herring]

On 02/13/2012 03:23 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
> 

So would always enabling CONFIG_OF on at91 and then only 1 sub-arch is
paying the price for additional space. Then we would only have ifdefs in
the truly cross-platform gpio drivers.

Rob

> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>  include/asm-generic/gpio.h |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 1ff4e22..c08b758 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -122,7 +122,6 @@ struct gpio_chip {
>  	unsigned		can_sleep:1;
>  	unsigned		exported:1;
>  
> -#if defined(CONFIG_OF_GPIO)
>  	/*
>  	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
>  	 * device tree automatically may have an OF translation
> @@ -131,7 +130,6 @@ struct gpio_chip {
>  	int of_gpio_n_cells;
>  	int (*of_xlate)(struct gpio_chip *gc,
>  		        const struct of_phandle_args *gpiospec, u32 *flags);
> -#endif
>  };
>  
>  extern const char *gpiochip_is_requested(struct gpio_chip *chip,

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Jean-Christophe PLAGNIOL-VILLARD]

On 08:00 Mon 13 Feb     , Rob Herring wrote:
> On 02/13/2012 03:23 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
> > 
> 
> So would always enabling CONFIG_OF on at91 and then only 1 sub-arch is
> paying the price for additional space. Then we would only have ifdefs in
> the truly cross-platform gpio drivers.
we talk about 12bytes and here it's force to put a ifdef in the gpio drivers
in platform_device we don't do so. So why should be do it here??

Drop the ifdef will ensure evenif we compile the code without OF_GPIO that all
the work can compile

we recently introduce IS_ENABLED & co to drop the maximum of ifdef
here it's the same idea

Best Regards,
J.
> 
> Rob
> 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> > ---
> >  include/asm-generic/gpio.h |    2 --
> >  1 files changed, 0 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index 1ff4e22..c08b758 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -122,7 +122,6 @@ struct gpio_chip {
> >  	unsigned		can_sleep:1;
> >  	unsigned		exported:1;
> >  
> > -#if defined(CONFIG_OF_GPIO)
> >  	/*
> >  	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
> >  	 * device tree automatically may have an OF translation
> > @@ -131,7 +130,6 @@ struct gpio_chip {
> >  	int of_gpio_n_cells;
> >  	int (*of_xlate)(struct gpio_chip *gc,
> >  		        const struct of_phandle_args *gpiospec, u32 *flags);
> > -#endif
> >  };
> >  
> >  extern const char *gpiochip_is_requested(struct gpio_chip *chip,

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Rob Herring]

On 02/13/2012 08:12 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:00 Mon 13 Feb     , Rob Herring wrote:
>> On 02/13/2012 03:23 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
>>>
>>
>> So would always enabling CONFIG_OF on at91 and then only 1 sub-arch is
>> paying the price for additional space. Then we would only have ifdefs in
>> the truly cross-platform gpio drivers.
> we talk about 12bytes and here it's force to put a ifdef in the gpio drivers
> in platform_device we don't do so. So why should be do it here??

But you are affecting everyone. I'm not saying we shouldn't do this. I'm
just highlighting the reason this is a problem and gpio is just one
instance. I think sub-arches should either always select OF or not.
Trying to do both is just going to be a pain to maintain.

> Drop the ifdef will ensure evenif we compile the code without OF_GPIO that all
> the work can compile

Yes, I understand the concept. To look at it another way, this has been
here for a long time without issue because CONFIG_OF was designed to be
selected at arch level.

> we recently introduce IS_ENABLED & co to drop the maximum of ifdef
> here it's the same idea
> 

I have no idea what you are referring to.

Rob

> Best Regards,
> J.
>>
>> Rob
>>
>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> ---
>>>  include/asm-generic/gpio.h |    2 --
>>>  1 files changed, 0 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
>>> index 1ff4e22..c08b758 100644
>>> --- a/include/asm-generic/gpio.h
>>> +++ b/include/asm-generic/gpio.h
>>> @@ -122,7 +122,6 @@ struct gpio_chip {
>>>  	unsigned		can_sleep:1;
>>>  	unsigned		exported:1;
>>>  
>>> -#if defined(CONFIG_OF_GPIO)
>>>  	/*
>>>  	 * If CONFIG_OF is enabled, then all GPIO controllers described in the
>>>  	 * device tree automatically may have an OF translation
>>> @@ -131,7 +130,6 @@ struct gpio_chip {
>>>  	int of_gpio_n_cells;
>>>  	int (*of_xlate)(struct gpio_chip *gc,
>>>  		        const struct of_phandle_args *gpiospec, u32 *flags);
>>> -#endif
>>>  };
>>>  
>>>  extern const char *gpiochip_is_requested(struct gpio_chip *chip,

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Nicolas Ferre]

On 02/13/2012 03:33 PM, Rob Herring :
> On 02/13/2012 08:12 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 08:00 Mon 13 Feb     , Rob Herring wrote:
>>> On 02/13/2012 03:23 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
>>>>
>>>
>>> So would always enabling CONFIG_OF on at91 and then only 1 sub-arch is
>>> paying the price for additional space. Then we would only have ifdefs in
>>> the truly cross-platform gpio drivers.
>> we talk about 12bytes and here it's force to put a ifdef in the gpio drivers
>> in platform_device we don't do so. So why should be do it here??
> 
> But you are affecting everyone. I'm not saying we shouldn't do this. I'm
> just highlighting the reason this is a problem and gpio is just one
> instance. I think sub-arches should either always select OF or not.
> Trying to do both is just going to be a pain to maintain.

(nothing related to the particular case discussed here, but...)

Rob,

It may be a pain to maintain (and I can tell it is), but we do not have
the choice: the AT91 family is simply too widely spread in terms of
number of SoC and release date of those SoC. We must maintain all of
them working while we are doing the effort to switch most of the
material to DT. As this work takes time I fear that the transition
period will last for a long time.

Older SoCs will surely never be converted to device-tree as both the
lack of workforce, customer habits and amount of boards existing and
working correctly in mainline will go against this.

Best regards,
-- 
Nicolas Ferre

[PATCH 1/1] gpio: drop CONFIG_OF_GPIO in the definition of the struct gpio_chip

[Rob Herring]

On 02/13/2012 08:59 AM, Nicolas Ferre wrote:
> On 02/13/2012 03:33 PM, Rob Herring :
>> On 02/13/2012 08:12 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 08:00 Mon 13 Feb     , Rob Herring wrote:
>>>> On 02/13/2012 03:23 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> this will allow to avoid the ifdef CONFIG_OF_GPIO in gpio drivers
>>>>>
>>>>
>>>> So would always enabling CONFIG_OF on at91 and then only 1 sub-arch is
>>>> paying the price for additional space. Then we would only have ifdefs in
>>>> the truly cross-platform gpio drivers.
>>> we talk about 12bytes and here it's force to put a ifdef in the gpio drivers
>>> in platform_device we don't do so. So why should be do it here??
>>
>> But you are affecting everyone. I'm not saying we shouldn't do this. I'm
>> just highlighting the reason this is a problem and gpio is just one
>> instance. I think sub-arches should either always select OF or not.
>> Trying to do both is just going to be a pain to maintain.
> 
> (nothing related to the particular case discussed here, but...)
> 
> Rob,
> 
> It may be a pain to maintain (and I can tell it is), but we do not have
> the choice: the AT91 family is simply too widely spread in terms of
> number of SoC and release date of those SoC. We must maintain all of
> them working while we are doing the effort to switch most of the
> material to DT. As this work takes time I fear that the transition
> period will last for a long time.
> 
> Older SoCs will surely never be converted to device-tree as both the
> lack of workforce, customer habits and amount of boards existing and
> working correctly in mainline will go against this.

I'm not saying convert all the boards to DT, just unconditionally enable
the config option. The only impact with that is kernel size, but I
haven't quantified how much of an increase that is though. Is it a big
deal if the boards are only minimally being maintained compared to the
maintenance?

Rob