[U-Boot,7/7] gpio: tegra: remove flags parsing in xlate routine

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass, it is
no longer necessary for the tegra-specific xlate function to do this.

Signed-off-by: Eric Nelson <eric@nelint.com>
---
 drivers/gpio/tegra_gpio.c | 1 -
 1 file changed, 1 deletion(-)

On 04/01/2016 09:47 AM, Eric Nelson wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass, it is
> no longer necessary for the tegra-specific xlate function to do this.

> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c

> @@ -246,7 +246,6 @@ static int tegra_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
>   	if (ret)
>   		return ret;
>   	desc->offset = gpio % TEGRA_GPIOS_PER_PORT;
> -	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;

I expect that after that, you can also remove the following at the top 
of the file:

#include <dt-bindings/gpio/gpio.h>

I expect that's true of other GPIO drivers too.

Hi Stephen,

On 5 April 2016 at 16:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/01/2016 09:47 AM, Eric Nelson wrote:
>>
>> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass, it is
>> no longer necessary for the tegra-specific xlate function to do this.
>
>
>> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
>
>
>> @@ -246,7 +246,6 @@ static int tegra_gpio_xlate(struct udevice *dev,
>> struct gpio_desc *desc,
>>         if (ret)
>>                 return ret;
>>         desc->offset = gpio % TEGRA_GPIOS_PER_PORT;
>> -       desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW :
>> 0;
>
>
> I expect that after that, you can also remove the following at the top of
> the file:
>
> #include <dt-bindings/gpio/gpio.h>
>
> I expect that's true of other GPIO drivers too.

But I don't think we want this patch for Tegra since we need to keep
the xlate() method, and with your suggested approach earlier, we won't
call the default xlate() method in the uclass for Tegra.

Regards,
Simon

Thanks for the feedback Simon.

On 04/09/2016 11:34 AM, Simon Glass wrote:
> On 5 April 2016 at 16:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/01/2016 09:47 AM, Eric Nelson wrote:
>>>
>>> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass, it is
>>> no longer necessary for the tegra-specific xlate function to do this.
>>
>>
>>> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
>>
>>
>>> @@ -246,7 +246,6 @@ static int tegra_gpio_xlate(struct udevice *dev,
>>> struct gpio_desc *desc,
>>>         if (ret)
>>>                 return ret;
>>>         desc->offset = gpio % TEGRA_GPIOS_PER_PORT;
>>> -       desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW :
>>> 0;
>>
>>
>> I expect that after that, you can also remove the following at the top of
>> the file:
>>
>> #include <dt-bindings/gpio/gpio.h>
>>
>> I expect that's true of other GPIO drivers too.
> 
> But I don't think we want this patch for Tegra since we need to keep
> the xlate() method, and with your suggested approach earlier, we won't
> call the default xlate() method in the uclass for Tegra.
> 

Based on your comments and Stephens, I can re-work this to have a
__maybe_unused xlate function for the common case of handling
offset+active low, but I think the handling of offset inside of
gpio_find_and_xlate() should be removed at the same time.

It seems weird to have processing for one argument in the common code
when it may not be used (as is the case with tegra).

I see that you've sent some updates, so I'm not sure whether I should
send an update on top of your patch set or without it.

Regards,


Eric

On 04/10/2016 07:45 AM, Eric Nelson wrote:
> On 04/09/2016 11:34 AM, Simon Glass wrote:
>> On 5 April 2016 at 16:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/01/2016 09:47 AM, Eric Nelson wrote:
>>>>
>>>> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass, it is
>>>> no longer necessary for the tegra-specific xlate function to do this.
>>>
>>>> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
>>>
>>>> @@ -246,7 +246,6 @@ static int tegra_gpio_xlate(struct udevice *dev,
>>>> struct gpio_desc *desc,
>>>>         if (ret)
>>>>                 return ret;
>>>>         desc->offset = gpio % TEGRA_GPIOS_PER_PORT;
>>>> -       desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW :
>>>> 0;
>>>
>>>
>>> I expect that after that, you can also remove the following at the top of
>>> the file:
>>>
>>> #include <dt-bindings/gpio/gpio.h>
>>>
>>> I expect that's true of other GPIO drivers too.
>>
>> But I don't think we want this patch for Tegra since we need to keep
>> the xlate() method, and with your suggested approach earlier, we won't
>> call the default xlate() method in the uclass for Tegra.
>>
> 
> Based on your comments and Stephens, I can re-work this to have a
> __maybe_unused xlate function for the common case of handling
> offset+active low, but I think the handling of offset inside of
> gpio_find_and_xlate() should be removed at the same time.
> 
> It seems weird to have processing for one argument in the common code
> when it may not be used (as is the case with tegra).
> 

While reviewing this, I think I found that the sunxi gpio driver
is broken.

The device trees have #gpio-cells=<3>, and gpio declaration like this:
	<&pio 1 8 GPIO_ACTIVE_HIGH>

but there's no custom xlate routine, the offset is currently going to
have what I presume is the bank number.

Hans, can you confirm this?

Please advise,


Eric