[U-Boot,09/14] fdt: Add polarity-aware gpio functions to fdtdec

From: Sean Paul <seanpaul@chromium.org>

Add get and set gpio functions to fdtdec that take into account the
polarity field in fdtdec_gpio_state.flags.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
 include/fdtdec.h |   16 ++++++++++++++++
 lib/fdtdec.c     |   20 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 0 deletions(-)

Am Donnerstag, den 25.10.2012, 19:31 -0700 schrieb Simon Glass:
> From: Sean Paul <seanpaul@chromium.org>
> 
> Add get and set gpio functions to fdtdec that take into account the
> polarity field in fdtdec_gpio_state.flags.
> 
In another thread Stephen Warren and I came to the conclusion that we
most likely should remove this polarity flag from the GPIO bindings.

Currently it is only for the USB VBUS GPIO which should move over to
regulators once they land in U-Boot. Do you have any other applications
for this flag, so we might reconsider removing it?

> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  include/fdtdec.h |   16 ++++++++++++++++
>  lib/fdtdec.c     |   20 ++++++++++++++++++++
>  2 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 12f73a7..17daa99 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -90,6 +90,22 @@ struct fdt_gpio_state {
>  #define fdt_gpio_isvalid(x) ((x)->gpio != FDT_GPIO_NONE)
>  
>  /**
> + * Read the GPIO taking into account the polarity of the pin.
> + *
> + * @param gpio		pointer to the decoded gpio
> + * @return value of the gpio if successful, < 0 if unsuccessful
> + */
> +int fdtdec_get_gpio(struct fdt_gpio_state *gpio);
> +
> +/**
> + * Write the GPIO taking into account the polarity of the pin.
> + *
> + * @param gpio		pointer to the decoded gpio
> + * @return 0 if successful
> + */
> +int fdtdec_set_gpio(struct fdt_gpio_state *gpio, int val);
> +
> +/**
>   * Find the next numbered alias for a peripheral. This is used to enumerate
>   * all the peripherals of a certain type.
>   *
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 6c417d2..91ba558 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -487,6 +487,26 @@ int fdtdec_decode_gpio(const void *blob, int node, const char *prop_name,
>  	return err == 1 ? 0 : err;
>  }
>  
> +int fdtdec_get_gpio(struct fdt_gpio_state *gpio)
> +{
> +	int val;
> +
> +	if (!fdt_gpio_isvalid(gpio))
> +		return -1;
> +
> +	val = gpio_get_value(gpio->gpio);
> +	return gpio->flags & FDT_GPIO_ACTIVE_LOW ? val ^ 1 : val;
> +}
> +
> +int fdtdec_set_gpio(struct fdt_gpio_state *gpio, int val)
> +{
> +	if (!fdt_gpio_isvalid(gpio))
> +		return -1;
> +
> +	val = gpio->flags & FDT_GPIO_ACTIVE_LOW ? val ^ 1 : val;
> +	return gpio_set_value(gpio->gpio, val);
> +}
> +
>  int fdtdec_setup_gpio(struct fdt_gpio_state *gpio)
>  {
>  	/*

Hi,

On Fri, Oct 26, 2012 at 12:17 AM, Lucas Stach <dev@lynxeye.de> wrote:
> Am Donnerstag, den 25.10.2012, 19:31 -0700 schrieb Simon Glass:
>> From: Sean Paul <seanpaul@chromium.org>
>>
>> Add get and set gpio functions to fdtdec that take into account the
>> polarity field in fdtdec_gpio_state.flags.
>>
> In another thread Stephen Warren and I came to the conclusion that we
> most likely should remove this polarity flag from the GPIO bindings.
>
> Currently it is only for the USB VBUS GPIO which should move over to
> regulators once they land in U-Boot. Do you have any other applications
> for this flag, so we might reconsider removing it?
>

Well, any time you have a flag which is inverted in meaning, it can be
useful. We have several switches on the board which can be active high
or low, and polarity is used for that.

In fact, it would be nice IMO to be able to specify input/output as
well. I know the exynos bindings do this. There is a noddy function
called fdtdec_setup_gpio() in U-Boot which really needs to be sorted
out. I discussed with Stephen some time ago how GPIOs should be
SOC-specific and it should be possible to set up a GPIO with a single
call, as Linux does. The more information there is in the binding, the
more it can do automatically.

Does the Tegra Linux GPIO binding still have a polarity?

Regards,
Simon

On 10/31/2012 05:59 PM, Simon Glass wrote:
> Hi,
> 
> On Fri, Oct 26, 2012 at 12:17 AM, Lucas Stach <dev@lynxeye.de> wrote:
>> Am Donnerstag, den 25.10.2012, 19:31 -0700 schrieb Simon Glass:
>>> From: Sean Paul <seanpaul@chromium.org>
>>>
>>> Add get and set gpio functions to fdtdec that take into account the
>>> polarity field in fdtdec_gpio_state.flags.
>>>
>> In another thread Stephen Warren and I came to the conclusion that we
>> most likely should remove this polarity flag from the GPIO bindings.
>>
>> Currently it is only for the USB VBUS GPIO which should move over to
>> regulators once they land in U-Boot. Do you have any other applications
>> for this flag, so we might reconsider removing it?
>>
> 
> Well, any time you have a flag which is inverted in meaning, it can be
> useful. We have several switches on the board which can be active high
> or low, and polarity is used for that.
> 
> In fact, it would be nice IMO to be able to specify input/output as
> well. I know the exynos bindings do this. There is a noddy function
> called fdtdec_setup_gpio() in U-Boot which really needs to be sorted
> out. I discussed with Stephen some time ago how GPIOs should be
> SOC-specific and it should be possible to set up a GPIO with a single
> call, as Linux does. The more information there is in the binding, the
> more it can do automatically.
> 
> Does the Tegra Linux GPIO binding still have a polarity?

Yes it does, although in practice it can't be used (and hence should
really be removed), since not all GPIO bindings have such a flag, so
there is always a need for a separate property to indicate the polarity
(c.f. fixed-regulator with GPIO control bindings for example).

Hi Stephen,

On Wed, Oct 31, 2012 at 9:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/31/2012 05:59 PM, Simon Glass wrote:
>> Hi,
>>
>> On Fri, Oct 26, 2012 at 12:17 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>> Am Donnerstag, den 25.10.2012, 19:31 -0700 schrieb Simon Glass:
>>>> From: Sean Paul <seanpaul@chromium.org>
>>>>
>>>> Add get and set gpio functions to fdtdec that take into account the
>>>> polarity field in fdtdec_gpio_state.flags.
>>>>
>>> In another thread Stephen Warren and I came to the conclusion that we
>>> most likely should remove this polarity flag from the GPIO bindings.
>>>
>>> Currently it is only for the USB VBUS GPIO which should move over to
>>> regulators once they land in U-Boot. Do you have any other applications
>>> for this flag, so we might reconsider removing it?
>>>
>>
>> Well, any time you have a flag which is inverted in meaning, it can be
>> useful. We have several switches on the board which can be active high
>> or low, and polarity is used for that.
>>
>> In fact, it would be nice IMO to be able to specify input/output as
>> well. I know the exynos bindings do this. There is a noddy function
>> called fdtdec_setup_gpio() in U-Boot which really needs to be sorted
>> out. I discussed with Stephen some time ago how GPIOs should be
>> SOC-specific and it should be possible to set up a GPIO with a single
>> call, as Linux does. The more information there is in the binding, the
>> more it can do automatically.
>>
>> Does the Tegra Linux GPIO binding still have a polarity?
>
> Yes it does, although in practice it can't be used (and hence should
> really be removed), since not all GPIO bindings have such a flag, so
> there is always a need for a separate property to indicate the polarity
> (c.f. fixed-regulator with GPIO control bindings for example).
>

I've had a bit of time to look into this. I see that the regulator
framework in the kernel seems to be used for various control purposes,
and provides useful polarity stuff. I was rather hoping that GPIOs
could be a bit more high level in U-Boot, with information about:

- input/output
- drive strength
- polarity
- pull up/down

In fact most of these are actually supported in most kernel bindings,
but of course it is binding-specific. Would it be useful to ask for a
polarity setting in the GPIO. When it is not available, the polarity
would then always be normal.

This might avoid moving polarity and input/output selecting down into
each client of the gpio, which seems undesirable in general.

Should we consider a second level of indirection for GPIOs to support
these non-binding features? It seems a bit complicated though.

However, if it is too late to do this, or not desirable for some
reason, then we should just drop this patch.

Regards,
Simon

On 11/15/2012 04:31 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Wed, Oct 31, 2012 at 9:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/31/2012 05:59 PM, Simon Glass wrote:
>>> Hi,
>>>
>>> On Fri, Oct 26, 2012 at 12:17 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>> Am Donnerstag, den 25.10.2012, 19:31 -0700 schrieb Simon Glass:
>>>>> From: Sean Paul <seanpaul@chromium.org>
>>>>>
>>>>> Add get and set gpio functions to fdtdec that take into account the
>>>>> polarity field in fdtdec_gpio_state.flags.
>>>>>
>>>> In another thread Stephen Warren and I came to the conclusion that we
>>>> most likely should remove this polarity flag from the GPIO bindings.
>>>>
>>>> Currently it is only for the USB VBUS GPIO which should move over to
>>>> regulators once they land in U-Boot. Do you have any other applications
>>>> for this flag, so we might reconsider removing it?
>>>>
>>>
>>> Well, any time you have a flag which is inverted in meaning, it can be
>>> useful. We have several switches on the board which can be active high
>>> or low, and polarity is used for that.
>>>
>>> In fact, it would be nice IMO to be able to specify input/output as
>>> well. I know the exynos bindings do this. There is a noddy function
>>> called fdtdec_setup_gpio() in U-Boot which really needs to be sorted
>>> out. I discussed with Stephen some time ago how GPIOs should be
>>> SOC-specific and it should be possible to set up a GPIO with a single
>>> call, as Linux does. The more information there is in the binding, the
>>> more it can do automatically.
>>>
>>> Does the Tegra Linux GPIO binding still have a polarity?
>>
>> Yes it does, although in practice it can't be used (and hence should
>> really be removed), since not all GPIO bindings have such a flag, so
>> there is always a need for a separate property to indicate the polarity
>> (c.f. fixed-regulator with GPIO control bindings for example).
>>
> 
> I've had a bit of time to look into this. I see that the regulator
> framework in the kernel seems to be used for various control purposes,
> and provides useful polarity stuff. I was rather hoping that GPIOs
> could be a bit more high level in U-Boot, with information about:
> 
> - input/output
> - drive strength
> - polarity
> - pull up/down
> 
> In fact most of these are actually supported in most kernel bindings,
> but of course it is binding-specific. Would it be useful to ask for a
> polarity setting in the GPIO. When it is not available, the polarity
> would then always be normal.
> 
> This might avoid moving polarity and input/output selecting down into
> each client of the gpio, which seems undesirable in general.
> 
> Should we consider a second level of indirection for GPIOs to support
> these non-binding features? It seems a bit complicated though.
> 
> However, if it is too late to do this, or not desirable for some
> reason, then we should just drop this patch.

The issue may not be bad enough we have to drop flag usage. It's also
being discussed at:

http://www.spinics.net/lists/arm-kernel/msg206299.html

I'd recommend seeing how that pans out before making a decision whether
to start/keep using flags or not.

Hi Stephen,

On Thu, Nov 15, 2012 at 3:46 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 11/15/2012 04:31 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Wed, Oct 31, 2012 at 9:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 10/31/2012 05:59 PM, Simon Glass wrote:
>>>> Hi,
>>>>
>>>> On Fri, Oct 26, 2012 at 12:17 AM, Lucas Stach <dev@lynxeye.de> wrote:
>>>>> Am Donnerstag, den 25.10.2012, 19:31 -0700 schrieb Simon Glass:
>>>>>> From: Sean Paul <seanpaul@chromium.org>
>>>>>>
>>>>>> Add get and set gpio functions to fdtdec that take into account the
>>>>>> polarity field in fdtdec_gpio_state.flags.
>>>>>>
>>>>> In another thread Stephen Warren and I came to the conclusion that we
>>>>> most likely should remove this polarity flag from the GPIO bindings.
>>>>>
>>>>> Currently it is only for the USB VBUS GPIO which should move over to
>>>>> regulators once they land in U-Boot. Do you have any other applications
>>>>> for this flag, so we might reconsider removing it?
>>>>>
>>>>
>>>> Well, any time you have a flag which is inverted in meaning, it can be
>>>> useful. We have several switches on the board which can be active high
>>>> or low, and polarity is used for that.
>>>>
>>>> In fact, it would be nice IMO to be able to specify input/output as
>>>> well. I know the exynos bindings do this. There is a noddy function
>>>> called fdtdec_setup_gpio() in U-Boot which really needs to be sorted
>>>> out. I discussed with Stephen some time ago how GPIOs should be
>>>> SOC-specific and it should be possible to set up a GPIO with a single
>>>> call, as Linux does. The more information there is in the binding, the
>>>> more it can do automatically.
>>>>
>>>> Does the Tegra Linux GPIO binding still have a polarity?
>>>
>>> Yes it does, although in practice it can't be used (and hence should
>>> really be removed), since not all GPIO bindings have such a flag, so
>>> there is always a need for a separate property to indicate the polarity
>>> (c.f. fixed-regulator with GPIO control bindings for example).
>>>
>>
>> I've had a bit of time to look into this. I see that the regulator
>> framework in the kernel seems to be used for various control purposes,
>> and provides useful polarity stuff. I was rather hoping that GPIOs
>> could be a bit more high level in U-Boot, with information about:
>>
>> - input/output
>> - drive strength
>> - polarity
>> - pull up/down
>>
>> In fact most of these are actually supported in most kernel bindings,
>> but of course it is binding-specific. Would it be useful to ask for a
>> polarity setting in the GPIO. When it is not available, the polarity
>> would then always be normal.
>>
>> This might avoid moving polarity and input/output selecting down into
>> each client of the gpio, which seems undesirable in general.
>>
>> Should we consider a second level of indirection for GPIOs to support
>> these non-binding features? It seems a bit complicated though.
>>
>> However, if it is too late to do this, or not desirable for some
>> reason, then we should just drop this patch.
>
> The issue may not be bad enough we have to drop flag usage. It's also
> being discussed at:
>
> http://www.spinics.net/lists/arm-kernel/msg206299.html
>
> I'd recommend seeing how that pans out before making a decision whether
> to start/keep using flags or not.
>

Thanks, will await news.

Regards,
Simon