[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Device tree parsing of GPIO nodes is currently ignoring flags.

Add support for GPIO_ACTIVE_LOW by checking for the presence
of the flag and setting the desc->flags field to the driver
model constant GPIOD_ACTIVE_LOW.

Signed-off-by: Eric Nelson <eric@nelint.com>
---
 drivers/gpio/gpio-uclass.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index b58d4e6..6d30612 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -118,12 +119,16 @@ static int gpio_find_and_xlate(struct gpio_desc *desc,
 {
 	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
 
+	desc->flags = 0;
 	/* Use the first argument as the offset by default */
-	if (args->args_count > 0)
+	if (args->args_count > 0) {
 		desc->offset = args->args[0];
+		if ((args->args_count > 1) &&
+		    (args->args[1] & GPIO_ACTIVE_LOW))
+			desc->flags = GPIOD_ACTIVE_LOW;
+	}
 	else
 		desc->offset = -1;
-	desc->flags = 0;
 
 	return ops->xlate ? ops->xlate(desc->dev, desc, args) : 0;
 }
-- 
2.6.2

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Peng Fan]

Hi Eric,

On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>Device tree parsing of GPIO nodes is currently ignoring flags.
>
>Add support for GPIO_ACTIVE_LOW by checking for the presence
>of the flag and setting the desc->flags field to the driver
>model constant GPIOD_ACTIVE_LOW.

You may need to try this: https://patchwork.ozlabs.org/patch/597363/

Regards,
Peng.

>
>Signed-off-by: Eric Nelson <eric@nelint.com>
>---
> drivers/gpio/gpio-uclass.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
>index b58d4e6..6d30612 100644
>--- a/drivers/gpio/gpio-uclass.c
>+++ b/drivers/gpio/gpio-uclass.c
>@@ -6,6 +6,7 @@
> 
> #include <common.h>
> #include <dm.h>
>+#include <dt-bindings/gpio/gpio.h>
> #include <errno.h>
> #include <fdtdec.h>
> #include <malloc.h>
>@@ -118,12 +119,16 @@ static int gpio_find_and_xlate(struct gpio_desc *desc,
> {
> 	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
> 
>+	desc->flags = 0;
> 	/* Use the first argument as the offset by default */
>-	if (args->args_count > 0)
>+	if (args->args_count > 0) {
> 		desc->offset = args->args[0];
>+		if ((args->args_count > 1) &&
>+		    (args->args[1] & GPIO_ACTIVE_LOW))
>+			desc->flags = GPIOD_ACTIVE_LOW;
>+	}
> 	else
> 		desc->offset = -1;
>-	desc->flags = 0;
> 
> 	return ops->xlate ? ops->xlate(desc->dev, desc, args) : 0;
> }
>-- 
>2.6.2
>
>_______________________________________________
>U-Boot mailing list
>U-Boot at lists.denx.de
>http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Hi Peng,

On 03/28/2016 09:57 PM, Peng Fan wrote:
> Hi Eric,
> 
> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>
>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>> of the flag and setting the desc->flags field to the driver
>> model constant GPIOD_ACTIVE_LOW.
> 
> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
> 
Thanks for pointing this out.

This patch also works, but it has me confused.

How/why is parsing the ACTIVE_LOW flag specific to MXC?

This is a general-purpose flag in the kernel, not something machine-
specific.

It also appears that there are a bunch of other copies
of this same bit of code in the various mach_xlate() routines:

desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;

If it's done in gpio-uclass, this isn't needed and xlate can
be removed from mxc-gpio and quite a few other architectures.

Please advise,


Eric

[U-Boot] [PATCH 0/7] Handle GPIO_ACTIVE_LOW in gpio-uclass

[Eric Nelson]

As Peng pointed out in [1], GPIO_ACTIVE_LOW is currently being
parsed by driver-specific xlate routines, and an NXP/mxc-specific
patch ([2]) to do the same on those processors is pending.

Upon further inspection, I found that many arch-specific xlate
routines were present only to parse either or both offset and
the GPIO_ACTIVE_LOW flag, both of which can be handled at a global
level.

This series adds global support for GPIO_ACTIVE_LOW and removes
the architecture-specific gpio xlate routine from five drivers.

It also removes the handling of flags in the tegra gpio driver,
though a custom xlate is still needed.

[1] - http://lists.denx.de/pipermail/u-boot/2016-March/thread.html#249946
[2] - https://patchwork.ozlabs.org/patch/597363/

Eric Nelson (7):
  dm: gpio: handle GPIO_ACTIVE_LOW flag in DT
  gpio: intel_broadwell: remove gpio_xlate routine
  gpio: omap: remove gpio_xlate routine
  gpio: pic32: remove gpio_xlate routine
  gpio: rk: remove gpio_xlate routine
  gpio: exynos(s5p): remove gpio_xlate routine
  gpio: tegra: remove flags parsing in xlate routine

 drivers/gpio/gpio-uclass.c          | 12 ++++++++----
 drivers/gpio/intel_broadwell_gpio.c | 10 ----------
 drivers/gpio/omap_gpio.c            | 10 ----------
 drivers/gpio/pic32_gpio.c           |  9 ---------
 drivers/gpio/rk_gpio.c              | 10 ----------
 drivers/gpio/s5p_gpio.c             | 10 ----------
 drivers/gpio/tegra_gpio.c           |  1 -
 7 files changed, 8 insertions(+), 54 deletions(-)

-- 
2.6.2

[U-Boot] [PATCH 1/7] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Device tree parsing of GPIO nodes is currently ignoring flags
and architecture-specific xlate routines are handling the
parsing of GPIO_ACTIVE_LOW.

Since GPIO_ACTIVE_LOW isn't specific to a particular device
type, this patch adds support at a global level and removes
the need for many of the driver-specific xlate routines.

Signed-off-by: Eric Nelson <eric@nelint.com>
---
 drivers/gpio/gpio-uclass.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index b58d4e6..a3cbb83 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -118,12 +119,15 @@ static int gpio_find_and_xlate(struct gpio_desc *desc,
 {
 	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
 
+	desc->offset = -1;
+	desc->flags = 0;
 	/* Use the first argument as the offset by default */
-	if (args->args_count > 0)
+	if (args->args_count > 0) {
 		desc->offset = args->args[0];
-	else
-		desc->offset = -1;
-	desc->flags = 0;
+		if ((args->args_count > 1) &&
+		    (args->args[1] & GPIO_ACTIVE_LOW))
+			desc->flags = GPIOD_ACTIVE_LOW;
+	}
 
 	return ops->xlate ? ops->xlate(desc->dev, desc, args) : 0;
 }
-- 
2.6.2

[U-Boot] [PATCH 2/7] gpio: intel_broadwell: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the intel_broadwell driver doesn't need a custom xlate routine.

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

diff --git a/drivers/gpio/intel_broadwell_gpio.c b/drivers/gpio/intel_broadwell_gpio.c
index 8cf76f9..81ce446 100644
--- a/drivers/gpio/intel_broadwell_gpio.c
+++ b/drivers/gpio/intel_broadwell_gpio.c
@@ -162,15 +162,6 @@ static int broadwell_gpio_ofdata_to_platdata(struct udevice *dev)
 	return 0;
 }
 
-static int broadwell_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-				struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_broadwell_ops = {
 	.request		= broadwell_gpio_request,
 	.direction_input	= broadwell_gpio_direction_input,
@@ -178,7 +169,6 @@ static const struct dm_gpio_ops gpio_broadwell_ops = {
 	.get_value		= broadwell_gpio_get_value,
 	.set_value		= broadwell_gpio_set_value,
 	.get_function		= broadwell_gpio_get_function,
-	.xlate			= broadwell_gpio_xlate,
 };
 
 static const struct udevice_id intel_broadwell_gpio_ids[] = {
-- 
2.6.2

[U-Boot] [PATCH 2/7] gpio: intel_broadwell: remove gpio_xlate routine

[Simon Glass]

On 1 April 2016 at 09:47, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the intel_broadwell driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> ---
>  drivers/gpio/intel_broadwell_gpio.c | 10 ----------
>  1 file changed, 10 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 3/7] gpio: omap: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the omap gpio driver doesn't need a custom xlate routine.

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

diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c
index 93d18e4..5ab53bc 100644
--- a/drivers/gpio/omap_gpio.c
+++ b/drivers/gpio/omap_gpio.c
@@ -277,22 +277,12 @@ static int omap_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_INPUT;
 }
 
-static int omap_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			   struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_omap_ops = {
 	.direction_input	= omap_gpio_direction_input,
 	.direction_output	= omap_gpio_direction_output,
 	.get_value		= omap_gpio_get_value,
 	.set_value		= omap_gpio_set_value,
 	.get_function		= omap_gpio_get_function,
-	.xlate			= omap_gpio_xlate,
 };
 
 static int omap_gpio_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [PATCH 3/7] gpio: omap: remove gpio_xlate routine

[Simon Glass]

On 1 April 2016 at 09:47, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the omap gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> ---
>  drivers/gpio/omap_gpio.c | 10 ----------
>  1 file changed, 10 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 4/7] gpio: pic32: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the pic32 gpio driver doesn't need a custom xlate routine.

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

diff --git a/drivers/gpio/pic32_gpio.c b/drivers/gpio/pic32_gpio.c
index 499b4fa..6999bc0 100644
--- a/drivers/gpio/pic32_gpio.c
+++ b/drivers/gpio/pic32_gpio.c
@@ -99,14 +99,6 @@ static int pic32_gpio_direction_output(struct udevice *dev,
 	return 0;
 }
 
-static int pic32_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			    struct fdtdec_phandle_args *args)
-{
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static int pic32_gpio_get_function(struct udevice *dev, unsigned offset)
 {
 	int ret = GPIOF_UNUSED;
@@ -131,7 +123,6 @@ static const struct dm_gpio_ops gpio_pic32_ops = {
 	.get_value		= pic32_gpio_get_value,
 	.set_value		= pic32_gpio_set_value,
 	.get_function		= pic32_gpio_get_function,
-	.xlate			= pic32_gpio_xlate,
 };
 
 static int pic32_gpio_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [PATCH 4/7] gpio: pic32: remove gpio_xlate routine

[Simon Glass]

On 1 April 2016 at 09:47, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the pic32 gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> ---
>  drivers/gpio/pic32_gpio.c | 9 ---------
>  1 file changed, 9 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 5/7] gpio: rk: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the Rockchip gpio driver doesn't need a custom xlate routine.

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

diff --git a/drivers/gpio/rk_gpio.c b/drivers/gpio/rk_gpio.c
index 40e87bd..253ccec 100644
--- a/drivers/gpio/rk_gpio.c
+++ b/drivers/gpio/rk_gpio.c
@@ -98,15 +98,6 @@ static int rockchip_gpio_get_function(struct udevice *dev, unsigned offset)
 #endif
 }
 
-static int rockchip_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			    struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static int rockchip_gpio_probe(struct udevice *dev)
 {
 	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
@@ -135,7 +126,6 @@ static const struct dm_gpio_ops gpio_rockchip_ops = {
 	.get_value		= rockchip_gpio_get_value,
 	.set_value		= rockchip_gpio_set_value,
 	.get_function		= rockchip_gpio_get_function,
-	.xlate			= rockchip_gpio_xlate,
 };
 
 static const struct udevice_id rockchip_gpio_ids[] = {
-- 
2.6.2

[U-Boot] [PATCH 5/7] gpio: rk: remove gpio_xlate routine

[Simon Glass]

On 1 April 2016 at 09:47, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the Rockchip gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> ---
>  drivers/gpio/rk_gpio.c | 10 ----------
>  1 file changed, 10 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 6/7] gpio: exynos(s5p): remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the Exynos/S5P gpio driver doesn't need a custom xlate routine.

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

diff --git a/drivers/gpio/s5p_gpio.c b/drivers/gpio/s5p_gpio.c
index 0f22b23..591c036 100644
--- a/drivers/gpio/s5p_gpio.c
+++ b/drivers/gpio/s5p_gpio.c
@@ -276,22 +276,12 @@ static int exynos_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_FUNC;
 }
 
-static int exynos_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			     struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_exynos_ops = {
 	.direction_input	= exynos_gpio_direction_input,
 	.direction_output	= exynos_gpio_direction_output,
 	.get_value		= exynos_gpio_get_value,
 	.set_value		= exynos_gpio_set_value,
 	.get_function		= exynos_gpio_get_function,
-	.xlate			= exynos_gpio_xlate,
 };
 
 static int gpio_exynos_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [PATCH 6/7] gpio: exynos(s5p): remove gpio_xlate routine

[Simon Glass]

On 1 April 2016 at 09:47, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the Exynos/S5P gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> ---
>  drivers/gpio/s5p_gpio.c | 10 ----------
>  1 file changed, 10 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

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

[Eric Nelson]

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(-)

diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
index 5a03115..7ae1509 100644
--- 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;
 
 	return 0;
 }
-- 
2.6.2

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

[Stephen Warren]

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.

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

[Simon Glass]

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

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

[Eric Nelson]

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

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

[Eric Nelson]

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

[U-Boot] [PATCH V2 0/6] Handle GPIO_ACTIVE_LOW in gpio-uclass

[Eric Nelson]

As Peng pointed out in [1], GPIO_ACTIVE_LOW is currently being
parsed by driver-specific xlate routines, and an NXP/mxc-specific
patch ([2]) to do the same on those processors is pending.

This patch series takes a different approach and provides a default
routine for xlate that handles the most common case of GPIO 
device tree node parsing:

	<&gpio1 2 GPIO_ACTIVE_LOW>

The first routine adds the default gpio_xlate_offs_flags() routine,
and the remainder of the patches remove the driver-specific versions
from the intel_broadwell, omap, pic32, rk, and s5p drivers.

V2 of this patch set removes parsing of offset from the gpio_find_and_xlate
routine, and only parses the offset and GPIO_ACTIVE_LOW flag when a
driver-specific xlate is unavailable.

V2 also drops the update to the tegra_gpio driver.

Eric Nelson (6):
  dm: gpio: add a default gpio xlate routine
  gpio: intel_broadwell: remove gpio_xlate routine
  gpio: omap: remove gpio_xlate routine
  gpio: pic32: remove gpio_xlate routine
  gpio: rk: remove gpio_xlate routine
  gpio: exynos(s5p): remove gpio_xlate routine

 drivers/gpio/gpio-uclass.c          | 26 +++++++++++++++++++-------
 drivers/gpio/intel_broadwell_gpio.c | 10 ----------
 drivers/gpio/omap_gpio.c            | 11 -----------
 drivers/gpio/pic32_gpio.c           | 10 ----------
 drivers/gpio/rk_gpio.c              | 11 -----------
 drivers/gpio/s5p_gpio.c             | 11 -----------
 include/asm-generic/gpio.h          | 19 ++++++++++++++-----
 7 files changed, 33 insertions(+), 65 deletions(-)

-- 
2.6.2

[U-Boot] [PATCH V2 1/6] dm: gpio: add a default gpio xlate routine

[Eric Nelson]

Many drivers use a common form of offset + flags for device
tree nodes. e.g.:
	<&gpio1 2 GPIO_ACTIVE_LOW>

This patch adds a common implementation of this type of parsing
and calls it when a gpio driver doesn't supply its' own xlate
routine.

This will allow removal of the driver-specific versions in a
handful of drivers and simplify the addition of new drivers.

Signed-off-by: Eric Nelson <eric@nelint.com>
---
 drivers/gpio/gpio-uclass.c | 26 +++++++++++++++++++-------
 include/asm-generic/gpio.h | 19 ++++++++++++++-----
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index b58d4e6..6c24e65 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -113,19 +114,29 @@ int gpio_lookup_name(const char *name, struct udevice **devp,
 	return 0;
 }
 
+int gpio_xlate_offs_flags(struct udevice *dev,
+					 struct gpio_desc *desc,
+					 struct fdtdec_phandle_args *args)
+{
+	int ret = -EINVAL;
+	if (args->args_count > 1) {
+		desc->offset = args->args[0];
+		if (args->args[1] & GPIO_ACTIVE_LOW)
+			desc->flags = GPIOD_ACTIVE_LOW;
+		ret = 0;
+	}
+	return ret;
+}
+
 static int gpio_find_and_xlate(struct gpio_desc *desc,
 			       struct fdtdec_phandle_args *args)
 {
 	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
 
-	/* Use the first argument as the offset by default */
-	if (args->args_count > 0)
-		desc->offset = args->args[0];
+	if (ops->xlate)
+		return ops->xlate(desc->dev, desc, args);
 	else
-		desc->offset = -1;
-	desc->flags = 0;
-
-	return ops->xlate ? ops->xlate(desc->dev, desc, args) : 0;
+		return gpio_xlate_offs_flags(desc->dev, desc, args);
 }
 
 int dm_gpio_request(struct gpio_desc *desc, const char *label)
@@ -605,6 +616,7 @@ static int _gpio_request_by_name_nodev(const void *blob, int node,
 
 	desc->dev = NULL;
 	desc->offset = 0;
+	desc->flags = 0;
 	ret = fdtdec_parse_phandle_with_args(blob, node, list_name,
 					     "#gpio-cells", 0, index, &args);
 	if (ret) {
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 68b5f0b..2500c10 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -207,6 +207,16 @@ int gpio_requestf(unsigned gpio, const char *fmt, ...)
 struct fdtdec_phandle_args;
 
 /**
+ * gpio_xlate_offs_flags() - implementation for common use of dm_gpio_ops.xlate
+ *
+ * This routine sets the offset field to args[0] and the flags field to
+ * GPIOD_ACTIVE_LOW if the GPIO_ACTIVE_LOW flag is present in args[1].
+ *
+ */
+int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
+			  struct fdtdec_phandle_args *args);
+
+/**
  * struct struct dm_gpio_ops - Driver model GPIO operations
  *
  * Refer to functions above for description. These function largely copy
@@ -258,12 +268,11 @@ struct dm_gpio_ops {
 	 *
 	 *   @desc->dev to @dev
 	 *   @desc->flags to 0
-	 *   @desc->offset to the value of the first argument in args, if any,
-	 *		otherwise -1 (which is invalid)
+	 *   @desc->offset to 0
 	 *
-	 * This method is optional so if the above defaults suit it can be
-	 * omitted. Typical behaviour is to set up the GPIOD_ACTIVE_LOW flag
-	 * in desc->flags.
+	 * This method is optional and defaults to gpio_xlate_offs_flags,
+	 * which will parse offset and the GPIO_ACTIVE_LOW flag in the first
+	 * two arguments.
 	 *
 	 * Note that @dev is passed in as a parameter to follow driver model
 	 * uclass conventions, even though it is already available as
-- 
2.6.2

[U-Boot] [PATCH V2 1/6] dm: gpio: add a default gpio xlate routine

[Stephen Warren]

On 04/11/2016 11:00 AM, Eric Nelson wrote:
> Many drivers use a common form of offset + flags for device
> tree nodes. e.g.:
> 	<&gpio1 2 GPIO_ACTIVE_LOW>
>
> This patch adds a common implementation of this type of parsing
> and calls it when a gpio driver doesn't supply its' own xlate
> routine.
>
> This will allow removal of the driver-specific versions in a
> handful of drivers and simplify the addition of new drivers.

The series,
Acked-by: Stephen Warren <swarren@nvidia.com>

Although one nit below.

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

> +int gpio_xlate_offs_flags(struct udevice *dev,
> +					 struct gpio_desc *desc,
> +					 struct fdtdec_phandle_args *args)
> +{
> +	int ret = -EINVAL;
> +	if (args->args_count > 1) {
> +		desc->offset = args->args[0];
> +		if (args->args[1] & GPIO_ACTIVE_LOW)
> +			desc->flags = GPIOD_ACTIVE_LOW;
> +		ret = 0;
> +	}
> +	return ret;
> +}

Why not return the error first, and avoid the need for an extra 
indentation level for the rest of the function, and make it quite a bit 
more readable in my opinion. The default could also be made to cover 
more situations (e.g. bindings that define a single cell for the GPIO 
but not cell at all for any flags):

if (args->args_count < 1)
     return -EINVAL;

desc->offset = args->args[0];

if (args->args_count < 2)
     return 0;

if (args->args[1] & GPIO_ACTIVE_LOW)
     desc->flags = GPIOD_ACTIVE_LOW;

return 0;

[U-Boot] [PATCH V2 1/6] dm: gpio: add a default gpio xlate routine

[Eric Nelson]

Hi Stephen,

On 04/11/2016 10:16 AM, Stephen Warren wrote:
> On 04/11/2016 11:00 AM, Eric Nelson wrote:
>> Many drivers use a common form of offset + flags for device
>> tree nodes. e.g.:
>>     <&gpio1 2 GPIO_ACTIVE_LOW>
>>
>> This patch adds a common implementation of this type of parsing
>> and calls it when a gpio driver doesn't supply its' own xlate
>> routine.
>>
>> This will allow removal of the driver-specific versions in a
>> handful of drivers and simplify the addition of new drivers.
> 
> The series,
> Acked-by: Stephen Warren <swarren@nvidia.com>
> 
> Although one nit below.
> 
>> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> 
>> +int gpio_xlate_offs_flags(struct udevice *dev,
>> +                     struct gpio_desc *desc,
>> +                     struct fdtdec_phandle_args *args)
>> +{
>> +    int ret = -EINVAL;
>> +    if (args->args_count > 1) {
>> +        desc->offset = args->args[0];
>> +        if (args->args[1] & GPIO_ACTIVE_LOW)
>> +            desc->flags = GPIOD_ACTIVE_LOW;
>> +        ret = 0;
>> +    }
>> +    return ret;
>> +}
> 
> Why not return the error first, and avoid the need for an extra
> indentation level for the rest of the function, and make it quite a bit
> more readable in my opinion. The default could also be made to cover
> more situations (e.g. bindings that define a single cell for the GPIO
> but not cell at all for any flags):
> 
> if (args->args_count < 1)
>     return -EINVAL;
> 
> desc->offset = args->args[0];
> 
> if (args->args_count < 2)
>     return 0;
> 
> if (args->args[1] & GPIO_ACTIVE_LOW)
>     desc->flags = GPIOD_ACTIVE_LOW;
> 
> return 0;

I like it. V3 of that one patch forthcoming.

[U-Boot] [PATCH V3 1/6] dm: gpio: add a default gpio xlate routine

[Eric Nelson]

Many drivers use a common form of offset + flags for device
tree nodes. e.g.:
	<&gpio1 2 GPIO_ACTIVE_LOW>

This patch adds a common implementation of this type of parsing
and calls it when a gpio driver doesn't supply its' own xlate
routine.

This will allow removal of the driver-specific versions in a
handful of drivers and simplify the addition of new drivers.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
---
  V2 removes parsing of offset from the gpio_find_and_xlate routine,
  and only parses the offset and GPIO_ACTIVE_LOW flag when a 
  driver-specific xlate is unavailable.

  V3 re-works tests of the argument count as suggested by Stephen Warren
  and will allow parsing of nodes with no flags field:
	<&gpio1 2>

 drivers/gpio/gpio-uclass.c | 30 +++++++++++++++++++++++-------
 include/asm-generic/gpio.h | 19 ++++++++++++++-----
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index b58d4e6..3e83cec 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -113,19 +114,33 @@ int gpio_lookup_name(const char *name, struct udevice **devp,
 	return 0;
 }
 
+int gpio_xlate_offs_flags(struct udevice *dev,
+					 struct gpio_desc *desc,
+					 struct fdtdec_phandle_args *args)
+{
+	if (args->args_count < 1)
+		return -EINVAL;
+
+	desc->offset = args->args[0];
+
+	if (args->args_count < 2)
+		return 0;
+
+	if (args->args[1] & GPIO_ACTIVE_LOW)
+		desc->flags = GPIOD_ACTIVE_LOW;
+
+	return 0;
+}
+
 static int gpio_find_and_xlate(struct gpio_desc *desc,
 			       struct fdtdec_phandle_args *args)
 {
 	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
 
-	/* Use the first argument as the offset by default */
-	if (args->args_count > 0)
-		desc->offset = args->args[0];
+	if (ops->xlate)
+		return ops->xlate(desc->dev, desc, args);
 	else
-		desc->offset = -1;
-	desc->flags = 0;
-
-	return ops->xlate ? ops->xlate(desc->dev, desc, args) : 0;
+		return gpio_xlate_offs_flags(desc->dev, desc, args);
 }
 
 int dm_gpio_request(struct gpio_desc *desc, const char *label)
@@ -605,6 +620,7 @@ static int _gpio_request_by_name_nodev(const void *blob, int node,
 
 	desc->dev = NULL;
 	desc->offset = 0;
+	desc->flags = 0;
 	ret = fdtdec_parse_phandle_with_args(blob, node, list_name,
 					     "#gpio-cells", 0, index, &args);
 	if (ret) {
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 68b5f0b..2500c10 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -207,6 +207,16 @@ int gpio_requestf(unsigned gpio, const char *fmt, ...)
 struct fdtdec_phandle_args;
 
 /**
+ * gpio_xlate_offs_flags() - implementation for common use of dm_gpio_ops.xlate
+ *
+ * This routine sets the offset field to args[0] and the flags field to
+ * GPIOD_ACTIVE_LOW if the GPIO_ACTIVE_LOW flag is present in args[1].
+ *
+ */
+int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
+			  struct fdtdec_phandle_args *args);
+
+/**
  * struct struct dm_gpio_ops - Driver model GPIO operations
  *
  * Refer to functions above for description. These function largely copy
@@ -258,12 +268,11 @@ struct dm_gpio_ops {
 	 *
 	 *   @desc->dev to @dev
 	 *   @desc->flags to 0
-	 *   @desc->offset to the value of the first argument in args, if any,
-	 *		otherwise -1 (which is invalid)
+	 *   @desc->offset to 0
 	 *
-	 * This method is optional so if the above defaults suit it can be
-	 * omitted. Typical behaviour is to set up the GPIOD_ACTIVE_LOW flag
-	 * in desc->flags.
+	 * This method is optional and defaults to gpio_xlate_offs_flags,
+	 * which will parse offset and the GPIO_ACTIVE_LOW flag in the first
+	 * two arguments.
 	 *
 	 * Note that @dev is passed in as a parameter to follow driver model
 	 * uclass conventions, even though it is already available as
-- 
2.6.2

[U-Boot] [PATCH V3 1/6] dm: gpio: add a default gpio xlate routine

[Purna Chandra Mandal]

On 04/11/2016 11:01 PM, Eric Nelson wrote:

> Many drivers use a common form of offset + flags for device
> tree nodes. e.g.:
> 	<&gpio1 2 GPIO_ACTIVE_LOW>
>
> This patch adds a common implementation of this type of parsing
> and calls it when a gpio driver doesn't supply its' own xlate
> routine.
>
> This will allow removal of the driver-specific versions in a
> handful of drivers and simplify the addition of new drivers.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Reviewed-by: Purna Chandra Mandal <purna.mandal@microchip.com>

[U-Boot] [PATCH V2 2/6] gpio: intel_broadwell: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the intel_broadwell driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 Nothing changed in V2.
 drivers/gpio/intel_broadwell_gpio.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpio/intel_broadwell_gpio.c b/drivers/gpio/intel_broadwell_gpio.c
index 8cf76f9..81ce446 100644
--- a/drivers/gpio/intel_broadwell_gpio.c
+++ b/drivers/gpio/intel_broadwell_gpio.c
@@ -162,15 +162,6 @@ static int broadwell_gpio_ofdata_to_platdata(struct udevice *dev)
 	return 0;
 }
 
-static int broadwell_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-				struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_broadwell_ops = {
 	.request		= broadwell_gpio_request,
 	.direction_input	= broadwell_gpio_direction_input,
@@ -178,7 +169,6 @@ static const struct dm_gpio_ops gpio_broadwell_ops = {
 	.get_value		= broadwell_gpio_get_value,
 	.set_value		= broadwell_gpio_set_value,
 	.get_function		= broadwell_gpio_get_function,
-	.xlate			= broadwell_gpio_xlate,
 };
 
 static const struct udevice_id intel_broadwell_gpio_ids[] = {
-- 
2.6.2

[U-Boot] [PATCH V2 3/6] gpio: omap: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the omap gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/omap_gpio.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c
index 93d18e4..cd960dc 100644
--- a/drivers/gpio/omap_gpio.c
+++ b/drivers/gpio/omap_gpio.c
@@ -25,7 +25,6 @@
 #include <asm/io.h>
 #include <asm/errno.h>
 #include <malloc.h>
-#include <dt-bindings/gpio/gpio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -277,22 +276,12 @@ static int omap_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_INPUT;
 }
 
-static int omap_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			   struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_omap_ops = {
 	.direction_input	= omap_gpio_direction_input,
 	.direction_output	= omap_gpio_direction_output,
 	.get_value		= omap_gpio_get_value,
 	.set_value		= omap_gpio_set_value,
 	.get_function		= omap_gpio_get_function,
-	.xlate			= omap_gpio_xlate,
 };
 
 static int omap_gpio_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [PATCH V2 4/6] gpio: pic32: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the pic32 gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/pic32_gpio.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpio/pic32_gpio.c b/drivers/gpio/pic32_gpio.c
index 499b4fa..7a037f3 100644
--- a/drivers/gpio/pic32_gpio.c
+++ b/drivers/gpio/pic32_gpio.c
@@ -12,7 +12,6 @@
 #include <asm/io.h>
 #include <asm/gpio.h>
 #include <linux/compat.h>
-#include <dt-bindings/gpio/gpio.h>
 #include <mach/pic32.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -99,14 +98,6 @@ static int pic32_gpio_direction_output(struct udevice *dev,
 	return 0;
 }
 
-static int pic32_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			    struct fdtdec_phandle_args *args)
-{
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static int pic32_gpio_get_function(struct udevice *dev, unsigned offset)
 {
 	int ret = GPIOF_UNUSED;
@@ -131,7 +122,6 @@ static const struct dm_gpio_ops gpio_pic32_ops = {
 	.get_value		= pic32_gpio_get_value,
 	.set_value		= pic32_gpio_set_value,
 	.get_function		= pic32_gpio_get_function,
-	.xlate			= pic32_gpio_xlate,
 };
 
 static int pic32_gpio_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [PATCH V2 4/6] gpio: pic32: remove gpio_xlate routine

[Purna Chandra Mandal]

On 04/11/2016 10:30 PM, Eric Nelson wrote:

> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the pic32 gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Purna Chandra Mandal <purna.mandal@microchip.com>

[U-Boot] [PATCH V2 5/6] gpio: rk: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the Rockchip gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/rk_gpio.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/rk_gpio.c b/drivers/gpio/rk_gpio.c
index 40e87bd..fefe3ca 100644
--- a/drivers/gpio/rk_gpio.c
+++ b/drivers/gpio/rk_gpio.c
@@ -16,7 +16,6 @@
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <dm/pinctrl.h>
-#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/clock/rk3288-cru.h>
 
 enum {
@@ -98,15 +97,6 @@ static int rockchip_gpio_get_function(struct udevice *dev, unsigned offset)
 #endif
 }
 
-static int rockchip_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			    struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static int rockchip_gpio_probe(struct udevice *dev)
 {
 	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
@@ -135,7 +125,6 @@ static const struct dm_gpio_ops gpio_rockchip_ops = {
 	.get_value		= rockchip_gpio_get_value,
 	.set_value		= rockchip_gpio_set_value,
 	.get_function		= rockchip_gpio_get_function,
-	.xlate			= rockchip_gpio_xlate,
 };
 
 static const struct udevice_id rockchip_gpio_ids[] = {
-- 
2.6.2

[U-Boot] [PATCH V2 6/6] gpio: exynos(s5p): remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the Exynos/S5P gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/s5p_gpio.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/s5p_gpio.c b/drivers/gpio/s5p_gpio.c
index 0f22b23..377fed4 100644
--- a/drivers/gpio/s5p_gpio.c
+++ b/drivers/gpio/s5p_gpio.c
@@ -13,7 +13,6 @@
 #include <asm/io.h>
 #include <asm/gpio.h>
 #include <dm/device-internal.h>
-#include <dt-bindings/gpio/gpio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -276,22 +275,12 @@ static int exynos_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_FUNC;
 }
 
-static int exynos_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			     struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_exynos_ops = {
 	.direction_input	= exynos_gpio_direction_input,
 	.direction_output	= exynos_gpio_direction_output,
 	.get_value		= exynos_gpio_get_value,
 	.set_value		= exynos_gpio_set_value,
 	.get_function		= exynos_gpio_get_function,
-	.xlate			= exynos_gpio_xlate,
 };
 
 static int gpio_exynos_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [PATCH V2 6/6] gpio: exynos(s5p): remove gpio_xlate routine

[Minkyu Kang]

On 12/04/16 02:00, Eric Nelson wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the Exynos/S5P gpio driver doesn't need a custom xlate routine.
> 
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> ---
>  V2 removes the include of <dt-bindings/gpio/gpio.h>
>  drivers/gpio/s5p_gpio.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/gpio/s5p_gpio.c b/drivers/gpio/s5p_gpio.c
> index 0f22b23..377fed4 100644
> --- a/drivers/gpio/s5p_gpio.c
> +++ b/drivers/gpio/s5p_gpio.c
> @@ -13,7 +13,6 @@
>  #include <asm/io.h>
>  #include <asm/gpio.h>
>  #include <dm/device-internal.h>
> -#include <dt-bindings/gpio/gpio.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -276,22 +275,12 @@ static int exynos_gpio_get_function(struct udevice *dev, unsigned offset)
>  		return GPIOF_FUNC;
>  }
>  
> -static int exynos_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
> -			     struct fdtdec_phandle_args *args)
> -{
> -	desc->offset = args->args[0];
> -	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
> -
> -	return 0;
> -}
> -
>  static const struct dm_gpio_ops gpio_exynos_ops = {
>  	.direction_input	= exynos_gpio_direction_input,
>  	.direction_output	= exynos_gpio_direction_output,
>  	.get_value		= exynos_gpio_get_value,
>  	.set_value		= exynos_gpio_set_value,
>  	.get_function		= exynos_gpio_get_function,
> -	.xlate			= exynos_gpio_xlate,
>  };
>  
>  static int gpio_exynos_probe(struct udevice *dev)
> 


Acked-by: Minkyu Kang <mk7.kang@samsung.com>

Thanks,
Minkyu Kang.

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Peng Fan]

Hi Eric,

On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>Hi Peng,
>
>On 03/28/2016 09:57 PM, Peng Fan wrote:
>> Hi Eric,
>> 
>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>
>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>> of the flag and setting the desc->flags field to the driver
>>> model constant GPIOD_ACTIVE_LOW.
>> 
>> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
>> 
>Thanks for pointing this out.
>
>This patch also works, but it has me confused.
>
>How/why is parsing the ACTIVE_LOW flag specific to MXC?
>
>This is a general-purpose flag in the kernel, not something machine-
>specific.
>
>It also appears that there are a bunch of other copies
>of this same bit of code in the various mach_xlate() routines:
>
>desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
>
>If it's done in gpio-uclass, this isn't needed and xlate can
>be removed from mxc-gpio and quite a few other architectures.
>
>Please advise,

I saw you have posted a patch set to convert other gpio drivers.
But actually the translation of gpio property should be done by
each gpio driver. Alought we have gpio-cells=<2> for most gpio
drivers, but if there is one case that gpio-cells=<3>, and it have
different meaning for each cell from other most drivers?

So I suggest we follow the linux way,

434         if (!chip->of_xlate) {                                                  
435                 chip->of_gpio_n_cells = 2;                                      
436                 chip->of_xlate = of_gpio_simple_xlate;                          
437         }

If gpio drivers does not provide xlate function, then use a simple xlate
function. If gpio drivers have their own xlate function, then use their
own way. Also I do no think delete the xlate implementation is not a good idea.

Simon may give more comments on this.

Regards,
Peng.
>
>
>Eric

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Hi Peng,

On 04/01/2016 10:46 PM, Peng Fan wrote:
> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>
>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>> of the flag and setting the desc->flags field to the driver
>>>> model constant GPIOD_ACTIVE_LOW.
>>>
>>> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
>>>
>> Thanks for pointing this out.
>>
>> This patch also works, but it has me confused.
>>
>> How/why is parsing the ACTIVE_LOW flag specific to MXC?
>>
>> This is a general-purpose flag in the kernel, not something machine-
>> specific.
>>
>> It also appears that there are a bunch of other copies
>> of this same bit of code in the various mach_xlate() routines:
>>
>> desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
>>
>> If it's done in gpio-uclass, this isn't needed and xlate can
>> be removed from mxc-gpio and quite a few other architectures.
>>
>> Please advise,
> 
> I saw you have posted a patch set to convert other gpio drivers.
> But actually the translation of gpio property should be done by
> each gpio driver. Alought we have gpio-cells=<2> for most gpio
> drivers, but if there is one case that gpio-cells=<3>, and it have
> different meaning for each cell from other most drivers?
> 

Which case has gpio-cells=<3>?

As far as I can tell, only tegra and sandbox have something other
than parsing of offset and the GPIO_ACTIVE_LOW flag.

Tegra seems to have gpio-cells=<2> and sandbox has either 0 or 1.

> So I suggest we follow the linux way,
> 
> 434         if (!chip->of_xlate) {                                                  
> 435                 chip->of_gpio_n_cells = 2;                                      
> 436                 chip->of_xlate = of_gpio_simple_xlate;                          
> 437         }
> 
> If gpio drivers does not provide xlate function, then use a simple xlate
> function. If gpio drivers have their own xlate function, then use their
> own way.
>
The recommendation in device-tree-bindings/gpio/gpio.txt is to have
the GPIO_ACTIVE_LOW/HIGH flag in the second cell, so parsing that
directly in gpio_find_and_xlate() seems right.

That way, driver-specific parsing only needs to handle additional
flags or needs as is the case with tegra.

> Also I do no think delete the xlate implementation is not a good idea.
> 

Which xlate routine? All of those that my patch set removes?

Since none of them do anything besides parsing the offset and
GPIO_ACTIVE_LOW flag, this seems like code bloat.

Please advise,


Eric

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Stephen Warren]

On 04/02/2016 09:13 AM, Eric Nelson wrote:
> Hi Peng,
>
> On 04/01/2016 10:46 PM, Peng Fan wrote:
>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>
>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>> of the flag and setting the desc->flags field to the driver
>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>
>>>> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
>>>>
>>> Thanks for pointing this out.
>>>
>>> This patch also works, but it has me confused.
>>>
>>> How/why is parsing the ACTIVE_LOW flag specific to MXC?
>>>
>>> This is a general-purpose flag in the kernel, not something machine-
>>> specific.
>>>
>>> It also appears that there are a bunch of other copies
>>> of this same bit of code in the various mach_xlate() routines:
>>>
>>> desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
>>>
>>> If it's done in gpio-uclass, this isn't needed and xlate can
>>> be removed from mxc-gpio and quite a few other architectures.
>>>
>>> Please advise,
>>
>> I saw you have posted a patch set to convert other gpio drivers.
>> But actually the translation of gpio property should be done by
>> each gpio driver. Alought we have gpio-cells=<2> for most gpio
>> drivers, but if there is one case that gpio-cells=<3>, and it have
>> different meaning for each cell from other most drivers?
>
> Which case has gpio-cells=<3>?
>
> As far as I can tell, only tegra and sandbox have something other
> than parsing of offset and the GPIO_ACTIVE_LOW flag.
>
> Tegra seems to have gpio-cells=<2> and sandbox has either 0 or 1.
>
>> So I suggest we follow the linux way,
>>
>> 434         if (!chip->of_xlate) {
>> 435                 chip->of_gpio_n_cells = 2;
>> 436                 chip->of_xlate = of_gpio_simple_xlate;
>> 437         }
>>
>> If gpio drivers does not provide xlate function, then use a simple xlate
>> function. If gpio drivers have their own xlate function, then use their
>> own way.
>
> The recommendation in device-tree-bindings/gpio/gpio.txt is to have
> the GPIO_ACTIVE_LOW/HIGH flag in the second cell, so parsing that
> directly in gpio_find_and_xlate() seems right.

That's a recommendation when designing GPIO controller bindings, not a 
definition of something that is categorically true for all bindings. 
Each binding is free to represent its flags (if any) in whatever way it 
wants, and so as Peng says, each driver has be in full control over its 
own of_xlate functionality. Now, for drivers that happen to use a common 
flag representation, we can plug in a common implementation of the xlate 
function.

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Hi Stephen and Peng,

On 04/02/2016 08:37 PM, Stephen Warren wrote:
> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>
>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>
>>>>> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
>>>>>
>>>> Thanks for pointing this out.
>>>>
>>>> This patch also works, but it has me confused.
>>>>
>>>> How/why is parsing the ACTIVE_LOW flag specific to MXC?
>>>>
>>>> This is a general-purpose flag in the kernel, not something machine-
>>>> specific.
>>>>
>>>> It also appears that there are a bunch of other copies
>>>> of this same bit of code in the various mach_xlate() routines:
>>>>
>>>> desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
>>>>
>>>> If it's done in gpio-uclass, this isn't needed and xlate can
>>>> be removed from mxc-gpio and quite a few other architectures.
>>>>
>>>> Please advise,
>>>
>>> I saw you have posted a patch set to convert other gpio drivers.
>>> But actually the translation of gpio property should be done by
>>> each gpio driver. Alought we have gpio-cells=<2> for most gpio
>>> drivers, but if there is one case that gpio-cells=<3>, and it have
>>> different meaning for each cell from other most drivers?
>>
>> Which case has gpio-cells=<3>?
>>
>> As far as I can tell, only tegra and sandbox have something other
>> than parsing of offset and the GPIO_ACTIVE_LOW flag.
>>
>> Tegra seems to have gpio-cells=<2> and sandbox has either 0 or 1.
>>
>>> So I suggest we follow the linux way,
>>>
>>> 434         if (!chip->of_xlate) {
>>> 435                 chip->of_gpio_n_cells = 2;
>>> 436                 chip->of_xlate = of_gpio_simple_xlate;
>>> 437         }
>>>
>>> If gpio drivers does not provide xlate function, then use a simple xlate
>>> function. If gpio drivers have their own xlate function, then use their
>>> own way.
>>
>> The recommendation in device-tree-bindings/gpio/gpio.txt is to have
>> the GPIO_ACTIVE_LOW/HIGH flag in the second cell, so parsing that
>> directly in gpio_find_and_xlate() seems right.
> 
> That's a recommendation when designing GPIO controller bindings, not a
> definition of something that is categorically true for all bindings.
> Each binding is free to represent its flags (if any) in whatever way it
> wants, and so as Peng says, each driver has be in full control over its
> own of_xlate functionality. Now, for drivers that happen to use a common
> flag representation, we can plug in a common implementation of the xlate
> function.

Isn't that what this patch-set does?
	http://lists.denx.de/pipermail/u-boot/2016-April/250228.html

For the cost of a couple of lines of code in gpio-uclass, we remove
~50 lines from existing implementations, essentially allowing them
to use the common (or default) implementation. Nothing in the patch
prevents completely custom handling by a driver.

If we want to be pedantic about requiring each driver to have function
xlate, then we should get rid of gpio_find_and_xlate entirely from
gpio-uclass.c.

Regards,


Eric

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Stephen Warren]

On 04/03/2016 08:07 AM, Eric Nelson wrote:
> Hi Stephen and Peng,
>
> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>
>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>
>>>>>> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
>>>>>>
>>>>> Thanks for pointing this out.
>>>>>
>>>>> This patch also works, but it has me confused.
>>>>>
>>>>> How/why is parsing the ACTIVE_LOW flag specific to MXC?
>>>>>
>>>>> This is a general-purpose flag in the kernel, not something machine-
>>>>> specific.
>>>>>
>>>>> It also appears that there are a bunch of other copies
>>>>> of this same bit of code in the various mach_xlate() routines:
>>>>>
>>>>> desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
>>>>>
>>>>> If it's done in gpio-uclass, this isn't needed and xlate can
>>>>> be removed from mxc-gpio and quite a few other architectures.
>>>>>
>>>>> Please advise,
>>>>
>>>> I saw you have posted a patch set to convert other gpio drivers.
>>>> But actually the translation of gpio property should be done by
>>>> each gpio driver. Alought we have gpio-cells=<2> for most gpio
>>>> drivers, but if there is one case that gpio-cells=<3>, and it have
>>>> different meaning for each cell from other most drivers?
>>>
>>> Which case has gpio-cells=<3>?
>>>
>>> As far as I can tell, only tegra and sandbox have something other
>>> than parsing of offset and the GPIO_ACTIVE_LOW flag.
>>>
>>> Tegra seems to have gpio-cells=<2> and sandbox has either 0 or 1.
>>>
>>>> So I suggest we follow the linux way,
>>>>
>>>> 434         if (!chip->of_xlate) {
>>>> 435                 chip->of_gpio_n_cells = 2;
>>>> 436                 chip->of_xlate = of_gpio_simple_xlate;
>>>> 437         }
>>>>
>>>> If gpio drivers does not provide xlate function, then use a simple xlate
>>>> function. If gpio drivers have their own xlate function, then use their
>>>> own way.
>>>
>>> The recommendation in device-tree-bindings/gpio/gpio.txt is to have
>>> the GPIO_ACTIVE_LOW/HIGH flag in the second cell, so parsing that
>>> directly in gpio_find_and_xlate() seems right.
>>
>> That's a recommendation when designing GPIO controller bindings, not a
>> definition of something that is categorically true for all bindings.
>> Each binding is free to represent its flags (if any) in whatever way it
>> wants, and so as Peng says, each driver has be in full control over its
>> own of_xlate functionality. Now, for drivers that happen to use a common
>> flag representation, we can plug in a common implementation of the xlate
>> function.
>
> Isn't that what this patch-set does?
> 	http://lists.denx.de/pipermail/u-boot/2016-April/250228.html

No, I don't believe so. Rather, it forcibly decodes the common layout in 
the common code, in such a way that it's always used. Then, the 
driver-specific of_xlate is called, which could fix up (i.e. undo) the 
incorrect results if they weren't appropriate, and then apply the 
correct translation.

Better would be to never decode the results incorrectly in the first 
place, yet allow each driver to use the common decoder function if it's 
appropriate.

gpio_find_and_xlate() should do either exactly:

a)

return ops->xlate(desc->dev, desc, args);

In this case, .xlate could never be NULL, and all drivers would need to 
provide some implementation. We could provide a common implementation 
gpio_xlate_common() that all drivers (that use the common DT specifier 
layout) can plug into their .xlate pointer.

b)

if (ops->xlate)
     return ops->xlate(desc->dev, desc, args);
else
     return gpio_xlate_common(desc->dev, desc, args);

Making that else clause call a separate function allows any custom 
ops->xlate implementation to call it too, assuming the code there is 
valid but simply needs extension rather than completely custom replacement.

> For the cost of a couple of lines of code in gpio-uclass, we remove
> ~50 lines from existing implementations, essentially allowing them
> to use the common (or default) implementation. Nothing in the patch
> prevents completely custom handling by a driver.
>
> If we want to be pedantic about requiring each driver to have function
> xlate, then we should get rid of gpio_find_and_xlate entirely from
> gpio-uclass.c.

The intent of the change is good.

I'm not sure why we need to remove gpio_find_and_xlate(); it provides an 
API for clients so they don't need to know how to access driver 
functionality through the ops pointer, which I think is an 
internal/private implementation detail. Is that detail exposed to 
clients in other places? If so, removing the wrapper seems fine. If not, 
I suspect it's a deliberate abstraction.

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Simon Glass]

Hi,

On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>
>> Hi Stephen and Peng,
>>
>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>
>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>
>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>
>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>
>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>
>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>
>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>
>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>
>>>>>>>
>>>>>>> You may need to try this: https://patchwork.ozlabs.org/patch/597363/
>>>>>>>
>>>>>> Thanks for pointing this out.
>>>>>>
>>>>>> This patch also works, but it has me confused.
>>>>>>
>>>>>> How/why is parsing the ACTIVE_LOW flag specific to MXC?
>>>>>>
>>>>>> This is a general-purpose flag in the kernel, not something machine-
>>>>>> specific.
>>>>>>
>>>>>> It also appears that there are a bunch of other copies
>>>>>> of this same bit of code in the various mach_xlate() routines:
>>>>>>
>>>>>> desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
>>>>>>
>>>>>> If it's done in gpio-uclass, this isn't needed and xlate can
>>>>>> be removed from mxc-gpio and quite a few other architectures.
>>>>>>
>>>>>> Please advise,
>>>>>
>>>>>
>>>>> I saw you have posted a patch set to convert other gpio drivers.
>>>>> But actually the translation of gpio property should be done by
>>>>> each gpio driver. Alought we have gpio-cells=<2> for most gpio
>>>>> drivers, but if there is one case that gpio-cells=<3>, and it have
>>>>> different meaning for each cell from other most drivers?
>>>>
>>>>
>>>> Which case has gpio-cells=<3>?
>>>>
>>>> As far as I can tell, only tegra and sandbox have something other
>>>> than parsing of offset and the GPIO_ACTIVE_LOW flag.
>>>>
>>>> Tegra seems to have gpio-cells=<2> and sandbox has either 0 or 1.
>>>>
>>>>> So I suggest we follow the linux way,
>>>>>
>>>>> 434         if (!chip->of_xlate) {
>>>>> 435                 chip->of_gpio_n_cells = 2;
>>>>> 436                 chip->of_xlate = of_gpio_simple_xlate;
>>>>> 437         }
>>>>>
>>>>> If gpio drivers does not provide xlate function, then use a simple
>>>>> xlate
>>>>> function. If gpio drivers have their own xlate function, then use their
>>>>> own way.
>>>>
>>>>
>>>> The recommendation in device-tree-bindings/gpio/gpio.txt is to have
>>>> the GPIO_ACTIVE_LOW/HIGH flag in the second cell, so parsing that
>>>> directly in gpio_find_and_xlate() seems right.
>>>
>>>
>>> That's a recommendation when designing GPIO controller bindings, not a
>>> definition of something that is categorically true for all bindings.
>>> Each binding is free to represent its flags (if any) in whatever way it
>>> wants, and so as Peng says, each driver has be in full control over its
>>> own of_xlate functionality. Now, for drivers that happen to use a common
>>> flag representation, we can plug in a common implementation of the xlate
>>> function.
>>
>>
>> Isn't that what this patch-set does?
>>         http://lists.denx.de/pipermail/u-boot/2016-April/250228.html
>
>
> No, I don't believe so. Rather, it forcibly decodes the common layout in the
> common code, in such a way that it's always used. Then, the driver-specific
> of_xlate is called, which could fix up (i.e. undo) the incorrect results if
> they weren't appropriate, and then apply the correct translation.
>
> Better would be to never decode the results incorrectly in the first place,
> yet allow each driver to use the common decoder function if it's
> appropriate.
>
> gpio_find_and_xlate() should do either exactly:
>
> a)
>
> return ops->xlate(desc->dev, desc, args);
>
> In this case, .xlate could never be NULL, and all drivers would need to
> provide some implementation. We could provide a common implementation
> gpio_xlate_common() that all drivers (that use the common DT specifier
> layout) can plug into their .xlate pointer.
>
> b)
>
> if (ops->xlate)
>     return ops->xlate(desc->dev, desc, args);
> else
>     return gpio_xlate_common(desc->dev, desc, args);
>
> Making that else clause call a separate function allows any custom
> ops->xlate implementation to call it too, assuming the code there is valid
> but simply needs extension rather than completely custom replacement.
>
>> For the cost of a couple of lines of code in gpio-uclass, we remove
>> ~50 lines from existing implementations, essentially allowing them
>> to use the common (or default) implementation. Nothing in the patch
>> prevents completely custom handling by a driver.
>>
>> If we want to be pedantic about requiring each driver to have function
>> xlate, then we should get rid of gpio_find_and_xlate entirely from
>> gpio-uclass.c.
>
>
> The intent of the change is good.
>
> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
> for clients so they don't need to know how to access driver functionality
> through the ops pointer, which I think is an internal/private implementation
> detail. Is that detail exposed to clients in other places? If so, removing
> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.

This seems a bit pedantic, but since Linux does it this way I think we
should follow along.

Eric you still get to remove the code from all the GPIO drivers - the
difference is just creating a common function to call when no xlate()
method is available.

Can you please take a look at what Stephen suggests?

Regards,
Simon

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Hi Simon,

On 04/09/2016 11:33 AM, Simon Glass wrote:
> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>
>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>
>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>

<snip>

>>
>> The intent of the change is good.
>>
>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>> for clients so they don't need to know how to access driver functionality
>> through the ops pointer, which I think is an internal/private implementation
>> detail. Is that detail exposed to clients in other places? If so, removing
>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
> 
> This seems a bit pedantic, but since Linux does it this way I think we
> should follow along.
> 
> Eric you still get to remove the code from all the GPIO drivers - the
> difference is just creating a common function to call when no xlate()
> method is available.
> 
> Can you please take a look at what Stephen suggests?
> 

Got it. I'm just not sure about where to start (before or after
the patch set you sent) and whether to also remove offset parsing
from gpio_find_and_xlate().

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Simon Glass]

Hi Eric,

On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
> Hi Simon,
>
> On 04/09/2016 11:33 AM, Simon Glass wrote:
>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>
>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>>
>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>
>
> <snip>
>
>>>
>>> The intent of the change is good.
>>>
>>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>>> for clients so they don't need to know how to access driver functionality
>>> through the ops pointer, which I think is an internal/private implementation
>>> detail. Is that detail exposed to clients in other places? If so, removing
>>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
>>
>> This seems a bit pedantic, but since Linux does it this way I think we
>> should follow along.
>>
>> Eric you still get to remove the code from all the GPIO drivers - the
>> difference is just creating a common function to call when no xlate()
>> method is available.
>>
>> Can you please take a look at what Stephen suggests?
>>
>
> Got it. I'm just not sure about where to start (before or after
> the patch set you sent) and whether to also remove offset parsing
> from gpio_find_and_xlate().
>

Which patch did I send? My understanding is:

- Add my review/ack tag to the patches as necessary
- Drop the tegra patch
- Update gpio_find_and_xlate() to call a default function if there is
no xlate() method
- Resend the series

I'm not sure about removing the existing functionality from
gpio_find_and_xlate(), but my guess is that it is best to move it to
your default function, so that gpio_find_and_xlate() doesn't include
any default behaviour in the case where there is a xlate() method.

Regards,
Simon

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Hi Simon,

On 04/11/2016 07:47 AM, Simon Glass wrote:
> Hi Eric,
> 
> On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
>> Hi Simon,
>>
>> On 04/09/2016 11:33 AM, Simon Glass wrote:
>>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>>
>>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>>>
>>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>>
>>
>> <snip>
>>
>>>>
>>>> The intent of the change is good.
>>>>
>>>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>>>> for clients so they don't need to know how to access driver functionality
>>>> through the ops pointer, which I think is an internal/private implementation
>>>> detail. Is that detail exposed to clients in other places? If so, removing
>>>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
>>>
>>> This seems a bit pedantic, but since Linux does it this way I think we
>>> should follow along.
>>>
>>> Eric you still get to remove the code from all the GPIO drivers - the
>>> difference is just creating a common function to call when no xlate()
>>> method is available.
>>>
>>> Can you please take a look at what Stephen suggests?
>>>
>>
>> Got it. I'm just not sure about where to start (before or after
>> the patch set you sent) and whether to also remove offset parsing
>> from gpio_find_and_xlate().
>>
> 
> Which patch did I send? My understanding is:
> 

At the time I sent this, you had just submitted the patch set adding
more driver-model support for block devices.

	http://lists.denx.de/pipermail/u-boot/2016-April/251095.html

> - Add my review/ack tag to the patches as necessary
> - Drop the tegra patch
> - Update gpio_find_and_xlate() to call a default function if there is
> no xlate() method
> - Resend the series
> 
> I'm not sure about removing the existing functionality from
> gpio_find_and_xlate(), but my guess is that it is best to move it to
> your default function, so that gpio_find_and_xlate() doesn't include
> any default behaviour in the case where there is a xlate() method.
> 

Reviewing the use of the offset field did yield some information about
the broken sunxi support and also that Vybrid was also missing
the xlate routine.

Since reviewing your patch sets (driver model updates for blk and also
driver model updates for mmc) will take some time, so I'll base an
updated patch set on master. My guess is that any merge issues will
be trivial.

I'll remove your acks in the updated patch set, since the updates
to the drivers won't drop the xlate field, but will connect them
to the common (__maybe_unused) routine. This will prevent the code
from leaking into machines like Tegra that don't need the common code.

Regards,


Eric

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Simon Glass]

Hi Eric,

On 11 April 2016 at 08:55, Eric Nelson <eric@nelint.com> wrote:
> Hi Simon,
>
> On 04/11/2016 07:47 AM, Simon Glass wrote:
>> Hi Eric,
>>
>> On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
>>> Hi Simon,
>>>
>>> On 04/09/2016 11:33 AM, Simon Glass wrote:
>>>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>>>>
>>>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>>>
>>>
>>> <snip>
>>>
>>>>>
>>>>> The intent of the change is good.
>>>>>
>>>>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>>>>> for clients so they don't need to know how to access driver functionality
>>>>> through the ops pointer, which I think is an internal/private implementation
>>>>> detail. Is that detail exposed to clients in other places? If so, removing
>>>>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
>>>>
>>>> This seems a bit pedantic, but since Linux does it this way I think we
>>>> should follow along.
>>>>
>>>> Eric you still get to remove the code from all the GPIO drivers - the
>>>> difference is just creating a common function to call when no xlate()
>>>> method is available.
>>>>
>>>> Can you please take a look at what Stephen suggests?
>>>>
>>>
>>> Got it. I'm just not sure about where to start (before or after
>>> the patch set you sent) and whether to also remove offset parsing
>>> from gpio_find_and_xlate().
>>>
>>
>> Which patch did I send? My understanding is:
>>
>
> At the time I sent this, you had just submitted the patch set adding
> more driver-model support for block devices.
>
>         http://lists.denx.de/pipermail/u-boot/2016-April/251095.html
>
>> - Add my review/ack tag to the patches as necessary
>> - Drop the tegra patch
>> - Update gpio_find_and_xlate() to call a default function if there is
>> no xlate() method
>> - Resend the series
>>
>> I'm not sure about removing the existing functionality from
>> gpio_find_and_xlate(), but my guess is that it is best to move it to
>> your default function, so that gpio_find_and_xlate() doesn't include
>> any default behaviour in the case where there is a xlate() method.
>>
>
> Reviewing the use of the offset field did yield some information about
> the broken sunxi support and also that Vybrid was also missing
> the xlate routine.
>
> Since reviewing your patch sets (driver model updates for blk and also
> driver model updates for mmc) will take some time, so I'll base an
> updated patch set on master. My guess is that any merge issues will
> be trivial.

Yes, that's right.
>
> I'll remove your acks in the updated patch set, since the updates
> to the drivers won't drop the xlate field, but will connect them
> to the common (__maybe_unused) routine. This will prevent the code
> from leaking into machines like Tegra that don't need the common code.

I'm pretty sure you can drop the xlate() implementations from the
functions, though, and those at the patches I acked.

I don't think you need __maybe_unused

static int gpio_find_and_xlate(...)
{
   get ops...

   if (ops->xlate)
      return ops->xlate(....)
   else
      return gpio_default_xlate()...
}

gpio_default_xlate() (or whatever name you use) should be exported so
drivers can use it.

Regards,
Simon

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Hi Simon,

On 04/11/2016 07:59 AM, Simon Glass wrote:
> On 11 April 2016 at 08:55, Eric Nelson <eric@nelint.com> wrote:
>> On 04/11/2016 07:47 AM, Simon Glass wrote:
>>> On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
>>>> On 04/09/2016 11:33 AM, Simon Glass wrote:
>>>>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>>>>
>>>>
>>>> <snip>
>>>>
>>>>>>
>>>>>> The intent of the change is good.
>>>>>>
>>>>>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>>>>>> for clients so they don't need to know how to access driver functionality
>>>>>> through the ops pointer, which I think is an internal/private implementation
>>>>>> detail. Is that detail exposed to clients in other places? If so, removing
>>>>>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
>>>>>
>>>>> This seems a bit pedantic, but since Linux does it this way I think we
>>>>> should follow along.
>>>>>
>>>>> Eric you still get to remove the code from all the GPIO drivers - the
>>>>> difference is just creating a common function to call when no xlate()
>>>>> method is available.
>>>>>
>>>>> Can you please take a look at what Stephen suggests?
>>>>>
>>>>
>>>> Got it. I'm just not sure about where to start (before or after
>>>> the patch set you sent) and whether to also remove offset parsing
>>>> from gpio_find_and_xlate().
>>>>
>>>
>>> Which patch did I send? My understanding is:
>>>
>>
>> At the time I sent this, you had just submitted the patch set adding
>> more driver-model support for block devices.
>>
>>         http://lists.denx.de/pipermail/u-boot/2016-April/251095.html
>>
>>> - Add my review/ack tag to the patches as necessary
>>> - Drop the tegra patch
>>> - Update gpio_find_and_xlate() to call a default function if there is
>>> no xlate() method
>>> - Resend the series
>>>
>>> I'm not sure about removing the existing functionality from
>>> gpio_find_and_xlate(), but my guess is that it is best to move it to
>>> your default function, so that gpio_find_and_xlate() doesn't include
>>> any default behaviour in the case where there is a xlate() method.
>>>
>>
>> Reviewing the use of the offset field did yield some information about
>> the broken sunxi support and also that Vybrid was also missing
>> the xlate routine.
>>
>> Since reviewing your patch sets (driver model updates for blk and also
>> driver model updates for mmc) will take some time, so I'll base an
>> updated patch set on master. My guess is that any merge issues will
>> be trivial.
> 
> Yes, that's right.
>>
>> I'll remove your acks in the updated patch set, since the updates
>> to the drivers won't drop the xlate field, but will connect them
>> to the common (__maybe_unused) routine. This will prevent the code
>> from leaking into machines like Tegra that don't need the common code.
> 
> I'm pretty sure you can drop the xlate() implementations from the
> functions, though, and those at the patches I acked.
> 
> I don't think you need __maybe_unused
> 
> static int gpio_find_and_xlate(...)
> {
>    get ops...
> 
>    if (ops->xlate)
>       return ops->xlate(....)
>    else
>       return gpio_default_xlate()...
> }
>
> gpio_default_xlate() (or whatever name you use) should be exported so
> drivers can use it.
> 

This will leak gpio_default_xlate (locally named gpio_xlate_offs_flags)
into machines that don't need it.

I can go the route you suggest above, but it will cost the tegra
and sandbox builds ~64 bytes ;)

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Simon Glass]

Hi Eric,

On 11 April 2016 at 09:10, Eric Nelson <eric@nelint.com> wrote:
> Hi Simon,
>
> On 04/11/2016 07:59 AM, Simon Glass wrote:
>> On 11 April 2016 at 08:55, Eric Nelson <eric@nelint.com> wrote:
>>> On 04/11/2016 07:47 AM, Simon Glass wrote:
>>>> On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
>>>>> On 04/09/2016 11:33 AM, Simon Glass wrote:
>>>>>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>>>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>>>>>
>>>>>
>>>>> <snip>
>>>>>
>>>>>>>
>>>>>>> The intent of the change is good.
>>>>>>>
>>>>>>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>>>>>>> for clients so they don't need to know how to access driver functionality
>>>>>>> through the ops pointer, which I think is an internal/private implementation
>>>>>>> detail. Is that detail exposed to clients in other places? If so, removing
>>>>>>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
>>>>>>
>>>>>> This seems a bit pedantic, but since Linux does it this way I think we
>>>>>> should follow along.
>>>>>>
>>>>>> Eric you still get to remove the code from all the GPIO drivers - the
>>>>>> difference is just creating a common function to call when no xlate()
>>>>>> method is available.
>>>>>>
>>>>>> Can you please take a look at what Stephen suggests?
>>>>>>
>>>>>
>>>>> Got it. I'm just not sure about where to start (before or after
>>>>> the patch set you sent) and whether to also remove offset parsing
>>>>> from gpio_find_and_xlate().
>>>>>
>>>>
>>>> Which patch did I send? My understanding is:
>>>>
>>>
>>> At the time I sent this, you had just submitted the patch set adding
>>> more driver-model support for block devices.
>>>
>>>         http://lists.denx.de/pipermail/u-boot/2016-April/251095.html
>>>
>>>> - Add my review/ack tag to the patches as necessary
>>>> - Drop the tegra patch
>>>> - Update gpio_find_and_xlate() to call a default function if there is
>>>> no xlate() method
>>>> - Resend the series
>>>>
>>>> I'm not sure about removing the existing functionality from
>>>> gpio_find_and_xlate(), but my guess is that it is best to move it to
>>>> your default function, so that gpio_find_and_xlate() doesn't include
>>>> any default behaviour in the case where there is a xlate() method.
>>>>
>>>
>>> Reviewing the use of the offset field did yield some information about
>>> the broken sunxi support and also that Vybrid was also missing
>>> the xlate routine.
>>>
>>> Since reviewing your patch sets (driver model updates for blk and also
>>> driver model updates for mmc) will take some time, so I'll base an
>>> updated patch set on master. My guess is that any merge issues will
>>> be trivial.
>>
>> Yes, that's right.
>>>
>>> I'll remove your acks in the updated patch set, since the updates
>>> to the drivers won't drop the xlate field, but will connect them
>>> to the common (__maybe_unused) routine. This will prevent the code
>>> from leaking into machines like Tegra that don't need the common code.
>>
>> I'm pretty sure you can drop the xlate() implementations from the
>> functions, though, and those at the patches I acked.
>>
>> I don't think you need __maybe_unused
>>
>> static int gpio_find_and_xlate(...)
>> {
>>    get ops...
>>
>>    if (ops->xlate)
>>       return ops->xlate(....)
>>    else
>>       return gpio_default_xlate()...
>> }
>>
>> gpio_default_xlate() (or whatever name you use) should be exported so
>> drivers can use it.
>>
>
> This will leak gpio_default_xlate (locally named gpio_xlate_offs_flags)
> into machines that don't need it.
>
> I can go the route you suggest above, but it will cost the tegra
> and sandbox builds ~64 bytes ;)
>

Sure, but we can live with that.

Regards,
Simon

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Stephen Warren]

On 04/11/2016 09:12 AM, Simon Glass wrote:
> Hi Eric,
>
> On 11 April 2016 at 09:10, Eric Nelson <eric@nelint.com> wrote:
>> Hi Simon,
>>
>> On 04/11/2016 07:59 AM, Simon Glass wrote:
>>> On 11 April 2016 at 08:55, Eric Nelson <eric@nelint.com> wrote:
>>>> On 04/11/2016 07:47 AM, Simon Glass wrote:
>>>>> On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
>>>>>> On 04/09/2016 11:33 AM, Simon Glass wrote:
>>>>>>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>>>>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>>>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring flags.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>>>>>>
>>>>>>
>>>>>> <snip>
>>>>>>
>>>>>>>>
>>>>>>>> The intent of the change is good.
>>>>>>>>
>>>>>>>> I'm not sure why we need to remove gpio_find_and_xlate(); it provides an API
>>>>>>>> for clients so they don't need to know how to access driver functionality
>>>>>>>> through the ops pointer, which I think is an internal/private implementation
>>>>>>>> detail. Is that detail exposed to clients in other places? If so, removing
>>>>>>>> the wrapper seems fine. If not, I suspect it's a deliberate abstraction.
>>>>>>>
>>>>>>> This seems a bit pedantic, but since Linux does it this way I think we
>>>>>>> should follow along.
>>>>>>>
>>>>>>> Eric you still get to remove the code from all the GPIO drivers - the
>>>>>>> difference is just creating a common function to call when no xlate()
>>>>>>> method is available.
>>>>>>>
>>>>>>> Can you please take a look at what Stephen suggests?
>>>>>>>
>>>>>>
>>>>>> Got it. I'm just not sure about where to start (before or after
>>>>>> the patch set you sent) and whether to also remove offset parsing
>>>>>> from gpio_find_and_xlate().
>>>>>>
>>>>>
>>>>> Which patch did I send? My understanding is:
>>>>>
>>>>
>>>> At the time I sent this, you had just submitted the patch set adding
>>>> more driver-model support for block devices.
>>>>
>>>>          http://lists.denx.de/pipermail/u-boot/2016-April/251095.html
>>>>
>>>>> - Add my review/ack tag to the patches as necessary
>>>>> - Drop the tegra patch
>>>>> - Update gpio_find_and_xlate() to call a default function if there is
>>>>> no xlate() method
>>>>> - Resend the series
>>>>>
>>>>> I'm not sure about removing the existing functionality from
>>>>> gpio_find_and_xlate(), but my guess is that it is best to move it to
>>>>> your default function, so that gpio_find_and_xlate() doesn't include
>>>>> any default behaviour in the case where there is a xlate() method.
>>>>>
>>>>
>>>> Reviewing the use of the offset field did yield some information about
>>>> the broken sunxi support and also that Vybrid was also missing
>>>> the xlate routine.
>>>>
>>>> Since reviewing your patch sets (driver model updates for blk and also
>>>> driver model updates for mmc) will take some time, so I'll base an
>>>> updated patch set on master. My guess is that any merge issues will
>>>> be trivial.
>>>
>>> Yes, that's right.
>>>>
>>>> I'll remove your acks in the updated patch set, since the updates
>>>> to the drivers won't drop the xlate field, but will connect them
>>>> to the common (__maybe_unused) routine. This will prevent the code
>>>> from leaking into machines like Tegra that don't need the common code.
>>>
>>> I'm pretty sure you can drop the xlate() implementations from the
>>> functions, though, and those at the patches I acked.
>>>
>>> I don't think you need __maybe_unused
>>>
>>> static int gpio_find_and_xlate(...)
>>> {
>>>     get ops...
>>>
>>>     if (ops->xlate)
>>>        return ops->xlate(....)
>>>     else
>>>        return gpio_default_xlate()...
>>> }
>>>
>>> gpio_default_xlate() (or whatever name you use) should be exported so
>>> drivers can use it.
>>>
>>
>> This will leak gpio_default_xlate (locally named gpio_xlate_offs_flags)
>> into machines that don't need it.
>>
>> I can go the route you suggest above, but it will cost the tegra
>> and sandbox builds ~64 bytes ;)
>>
>
> Sure, but we can live with that.

You can avoid that by requiring that ops->xlate always be non-NULL, and 
simply referencing the default function from drivers that want to use 
it. Not a big deal either way though.

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Simon Glass]

Hi,

On 11 April 2016 at 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/11/2016 09:12 AM, Simon Glass wrote:
>>
>> Hi Eric,
>>
>> On 11 April 2016 at 09:10, Eric Nelson <eric@nelint.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On 04/11/2016 07:59 AM, Simon Glass wrote:
>>>>
>>>> On 11 April 2016 at 08:55, Eric Nelson <eric@nelint.com> wrote:
>>>>>
>>>>> On 04/11/2016 07:47 AM, Simon Glass wrote:
>>>>>>
>>>>>> On 10 April 2016 at 08:48, Eric Nelson <eric@nelint.com> wrote:
>>>>>>>
>>>>>>> On 04/09/2016 11:33 AM, Simon Glass wrote:
>>>>>>>>
>>>>>>>> On 4 April 2016 at 11:50, Stephen Warren <swarren@wwwdotorg.org>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On 04/03/2016 08:07 AM, Eric Nelson wrote:
>>>>>>>>>>
>>>>>>>>>> On 04/02/2016 08:37 PM, Stephen Warren wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 04/02/2016 09:13 AM, Eric Nelson wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On 04/01/2016 10:46 PM, Peng Fan wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, Mar 31, 2016 at 01:41:04PM -0700, Eric Nelson wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 03/28/2016 09:57 PM, Peng Fan wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Fri, Mar 25, 2016 at 01:12:11PM -0700, Eric Nelson wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Device tree parsing of GPIO nodes is currently ignoring
>>>>>>>>>>>>>>>> flags.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Add support for GPIO_ACTIVE_LOW by checking for the presence
>>>>>>>>>>>>>>>> of the flag and setting the desc->flags field to the driver
>>>>>>>>>>>>>>>> model constant GPIOD_ACTIVE_LOW.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>
>>>>>>> <snip>
>>>>>>>
>>>>>>>>>
>>>>>>>>> The intent of the change is good.
>>>>>>>>>
>>>>>>>>> I'm not sure why we need to remove gpio_find_and_xlate(); it
>>>>>>>>> provides an API
>>>>>>>>> for clients so they don't need to know how to access driver
>>>>>>>>> functionality
>>>>>>>>> through the ops pointer, which I think is an internal/private
>>>>>>>>> implementation
>>>>>>>>> detail. Is that detail exposed to clients in other places? If so,
>>>>>>>>> removing
>>>>>>>>> the wrapper seems fine. If not, I suspect it's a deliberate
>>>>>>>>> abstraction.
>>>>>>>>
>>>>>>>>
>>>>>>>> This seems a bit pedantic, but since Linux does it this way I think
>>>>>>>> we
>>>>>>>> should follow along.
>>>>>>>>
>>>>>>>> Eric you still get to remove the code from all the GPIO drivers -
>>>>>>>> the
>>>>>>>> difference is just creating a common function to call when no
>>>>>>>> xlate()
>>>>>>>> method is available.
>>>>>>>>
>>>>>>>> Can you please take a look at what Stephen suggests?
>>>>>>>>
>>>>>>>
>>>>>>> Got it. I'm just not sure about where to start (before or after
>>>>>>> the patch set you sent) and whether to also remove offset parsing
>>>>>>> from gpio_find_and_xlate().
>>>>>>>
>>>>>>
>>>>>> Which patch did I send? My understanding is:
>>>>>>
>>>>>
>>>>> At the time I sent this, you had just submitted the patch set adding
>>>>> more driver-model support for block devices.
>>>>>
>>>>>          http://lists.denx.de/pipermail/u-boot/2016-April/251095.html
>>>>>
>>>>>> - Add my review/ack tag to the patches as necessary
>>>>>> - Drop the tegra patch
>>>>>> - Update gpio_find_and_xlate() to call a default function if there is
>>>>>> no xlate() method
>>>>>> - Resend the series
>>>>>>
>>>>>> I'm not sure about removing the existing functionality from
>>>>>> gpio_find_and_xlate(), but my guess is that it is best to move it to
>>>>>> your default function, so that gpio_find_and_xlate() doesn't include
>>>>>> any default behaviour in the case where there is a xlate() method.
>>>>>>
>>>>>
>>>>> Reviewing the use of the offset field did yield some information about
>>>>> the broken sunxi support and also that Vybrid was also missing
>>>>> the xlate routine.
>>>>>
>>>>> Since reviewing your patch sets (driver model updates for blk and also
>>>>> driver model updates for mmc) will take some time, so I'll base an
>>>>> updated patch set on master. My guess is that any merge issues will
>>>>> be trivial.
>>>>
>>>>
>>>> Yes, that's right.
>>>>>
>>>>>
>>>>> I'll remove your acks in the updated patch set, since the updates
>>>>> to the drivers won't drop the xlate field, but will connect them
>>>>> to the common (__maybe_unused) routine. This will prevent the code
>>>>> from leaking into machines like Tegra that don't need the common code.
>>>>
>>>>
>>>> I'm pretty sure you can drop the xlate() implementations from the
>>>> functions, though, and those at the patches I acked.
>>>>
>>>> I don't think you need __maybe_unused
>>>>
>>>> static int gpio_find_and_xlate(...)
>>>> {
>>>>     get ops...
>>>>
>>>>     if (ops->xlate)
>>>>        return ops->xlate(....)
>>>>     else
>>>>        return gpio_default_xlate()...
>>>> }
>>>>
>>>> gpio_default_xlate() (or whatever name you use) should be exported so
>>>> drivers can use it.
>>>>
>>>
>>> This will leak gpio_default_xlate (locally named gpio_xlate_offs_flags)
>>> into machines that don't need it.
>>>
>>> I can go the route you suggest above, but it will cost the tegra
>>> and sandbox builds ~64 bytes ;)
>>>
>>
>> Sure, but we can live with that.
>
>
> You can avoid that by requiring that ops->xlate always be non-NULL, and
> simply referencing the default function from drivers that want to use it.
> Not a big deal either way though.

I'd prefer not to do that. It just adds cruft, the removal of which is
the purpose of Eric's series.

Regards,
Simon

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

On 04/11/2016 09:53 AM, Simon Glass wrote:
> Hi,
> 
> On 11 April 2016 at 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/11/2016 09:12 AM, Simon Glass wrote:
>>>
>>> Hi Eric,
>>>
>>> On 11 April 2016 at 09:10, Eric Nelson <eric@nelint.com> wrote:
>>>>

<snip>

>>>>> I don't think you need __maybe_unused
>>>>>
>>>>> static int gpio_find_and_xlate(...)
>>>>> {
>>>>>     get ops...
>>>>>
>>>>>     if (ops->xlate)
>>>>>        return ops->xlate(....)
>>>>>     else
>>>>>        return gpio_default_xlate()...
>>>>> }
>>>>>
>>>>> gpio_default_xlate() (or whatever name you use) should be exported so
>>>>> drivers can use it.
>>>>>
>>>>
>>>> This will leak gpio_default_xlate (locally named gpio_xlate_offs_flags)
>>>> into machines that don't need it.
>>>>
>>>> I can go the route you suggest above, but it will cost the tegra
>>>> and sandbox builds ~64 bytes ;)
>>>>
>>>
>>> Sure, but we can live with that.
>>
>>
>> You can avoid that by requiring that ops->xlate always be non-NULL, and
>> simply referencing the default function from drivers that want to use it.
>> Not a big deal either way though.
> 
> I'd prefer not to do that. It just adds cruft, the removal of which is
> the purpose of Eric's series.
> 

We must be close to the goal now, since we're picking apart stuff like
this!

Since I've done it both ways locally, I'll try to recap.

Requiring an xlate puts a greater demand on the drivers, and requires
an extra patch to get Vybrid working (adding xlate to vybrid_gpio.c)
but does make it clearer which drivers need updates to handle DT
parsing.

There are a lot of them:
	altera_pio
	at91_gpio
	atmel_pio4
	axp_gpio
	bcm2835_gpio
	dwapb_gpio
	gpio-uniphier
	hi6220_gpio
	intel_ich6_gpio
	lpc32xx_gpio
	msm_gpio
	mvebu_gpio
	pm8916_gpio

None of these have dts files in either U-Boot or the kernel, so this
doesn't appear to be a problem.

Calling gpio_xlate_offs_flags as done in the V2 I just sent adds 64
bytes of code to the output for all machines, but transparently adds
support for machines like vybrid and mxc.

Regards,


Eric

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Simon Glass]

Hi Eric,

On 11 April 2016 at 11:17, Eric Nelson <eric@nelint.com> wrote:
> On 04/11/2016 09:53 AM, Simon Glass wrote:
>> Hi,
>>
>> On 11 April 2016 at 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/11/2016 09:12 AM, Simon Glass wrote:
>>>>
>>>> Hi Eric,
>>>>
>>>> On 11 April 2016 at 09:10, Eric Nelson <eric@nelint.com> wrote:
>>>>>
>
> <snip>
>
>>>>>> I don't think you need __maybe_unused
>>>>>>
>>>>>> static int gpio_find_and_xlate(...)
>>>>>> {
>>>>>>     get ops...
>>>>>>
>>>>>>     if (ops->xlate)
>>>>>>        return ops->xlate(....)
>>>>>>     else
>>>>>>        return gpio_default_xlate()...
>>>>>> }
>>>>>>
>>>>>> gpio_default_xlate() (or whatever name you use) should be exported so
>>>>>> drivers can use it.
>>>>>>
>>>>>
>>>>> This will leak gpio_default_xlate (locally named gpio_xlate_offs_flags)
>>>>> into machines that don't need it.
>>>>>
>>>>> I can go the route you suggest above, but it will cost the tegra
>>>>> and sandbox builds ~64 bytes ;)
>>>>>
>>>>
>>>> Sure, but we can live with that.
>>>
>>>
>>> You can avoid that by requiring that ops->xlate always be non-NULL, and
>>> simply referencing the default function from drivers that want to use it.
>>> Not a big deal either way though.
>>
>> I'd prefer not to do that. It just adds cruft, the removal of which is
>> the purpose of Eric's series.
>>
>
> We must be close to the goal now, since we're picking apart stuff like
> this!
>
> Since I've done it both ways locally, I'll try to recap.
>
> Requiring an xlate puts a greater demand on the drivers, and requires
> an extra patch to get Vybrid working (adding xlate to vybrid_gpio.c)
> but does make it clearer which drivers need updates to handle DT
> parsing.
>
> There are a lot of them:
>         altera_pio
>         at91_gpio
>         atmel_pio4
>         axp_gpio
>         bcm2835_gpio
>         dwapb_gpio
>         gpio-uniphier
>         hi6220_gpio
>         intel_ich6_gpio
>         lpc32xx_gpio
>         msm_gpio
>         mvebu_gpio
>         pm8916_gpio
>
> None of these have dts files in either U-Boot or the kernel, so this
> doesn't appear to be a problem.
>
> Calling gpio_xlate_offs_flags as done in the V2 I just sent adds 64
> bytes of code to the output for all machines, but transparently adds
> support for machines like vybrid and mxc.

Yes that seems OK to me. Can you please send a new version of the series?

Regards,
Simon

[U-Boot] [PATCH] dm: gpio: handle GPIO_ACTIVE_LOW flag in DT

[Eric Nelson]

Hi Simon,

On 04/20/2016 07:40 AM, Simon Glass wrote:
> Hi Eric,
> 
> On 11 April 2016 at 11:17, Eric Nelson <eric@nelint.com> wrote:
>> On 04/11/2016 09:53 AM, Simon Glass wrote:
>>> On 11 April 2016 at 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 04/11/2016 09:12 AM, Simon Glass wrote:
>>>>> On 11 April 2016 at 09:10, Eric Nelson <eric@nelint.com> wrote:
>>>>>>
>>

<snip>

>> None of these have dts files in either U-Boot or the kernel, so this
>> doesn't appear to be a problem.
>>
>> Calling gpio_xlate_offs_flags as done in the V2 I just sent adds 64
>> bytes of code to the output for all machines, but transparently adds
>> support for machines like vybrid and mxc.
> 
> Yes that seems OK to me. Can you please send a new version of the series?
> 

Sure. I'll re-send shortly.

[U-Boot] [RESEND PATCH 0/6] Handle GPIO_ACTIVE_LOW in gpio-uclass

[Eric Nelson]

As Peng pointed out in [1], GPIO_ACTIVE_LOW is currently being
parsed by driver-specific xlate routines, and an NXP/mxc-specific
patch ([2]) to do the same on those processors is pending.

Upon further inspection, I found that many arch-specific xlate
routines were present only to parse either or both offset and
the GPIO_ACTIVE_LOW flag, both of which can be handled at a global
level.

This series adds global support for GPIO_ACTIVE_LOW and removes
the architecture-specific gpio xlate routine from five drivers.

It also removes the handling of flags in the tegra gpio driver,
though a custom xlate is still needed.

[1] - http://lists.denx.de/pipermail/u-boot/2016-March/thread.html#249946
[2] - https://patchwork.ozlabs.org/patch/597363/

Eric Nelson (7):
  dm: gpio: handle GPIO_ACTIVE_LOW flag in DT
  gpio: intel_broadwell: remove gpio_xlate routine
  gpio: omap: remove gpio_xlate routine
  gpio: pic32: remove gpio_xlate routine
  gpio: rk: remove gpio_xlate routine
  gpio: exynos(s5p): remove gpio_xlate routine
  gpio: tegra: remove flags parsing in xlate routine

 drivers/gpio/gpio-uclass.c          | 12 ++++++++----
 drivers/gpio/intel_broadwell_gpio.c | 10 ----------
 drivers/gpio/omap_gpio.c            | 10 ----------
 drivers/gpio/pic32_gpio.c           |  9 ---------
 drivers/gpio/rk_gpio.c              | 10 ----------
 drivers/gpio/s5p_gpio.c             | 10 ----------
 drivers/gpio/tegra_gpio.c           |  1 -
 7 files changed, 8 insertions(+), 54 deletions(-)

-- 
2.6.2

[U-Boot] [RESEND PATCH V3 1/6] dm: gpio: add a default gpio xlate routine

[Eric Nelson]

Many drivers use a common form of offset + flags for device
tree nodes. e.g.:
	<&gpio1 2 GPIO_ACTIVE_LOW>

This patch adds a common implementation of this type of parsing
and calls it when a gpio driver doesn't supply its' own xlate
routine.

This will allow removal of the driver-specific versions in a
handful of drivers and simplify the addition of new drivers.

Signed-off-by: Eric Nelson <eric@nelint.com>
---
  V2 removes parsing of offset from the gpio_find_and_xlate routine,
  and only parses the offset and GPIO_ACTIVE_LOW flag when a 
  driver-specific xlate is unavailable.

  V3 re-works tests of the argument count as suggested by Stephen Warren
  and will allow parsing of nodes with no flags field:
	<&gpio1 2>

 drivers/gpio/gpio-uclass.c | 30 +++++++++++++++++++++++-------
 include/asm-generic/gpio.h | 19 ++++++++++++++-----
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index b58d4e6..3e83cec 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -113,19 +114,33 @@ int gpio_lookup_name(const char *name, struct udevice **devp,
 	return 0;
 }
 
+int gpio_xlate_offs_flags(struct udevice *dev,
+					 struct gpio_desc *desc,
+					 struct fdtdec_phandle_args *args)
+{
+	if (args->args_count < 1)
+		return -EINVAL;
+
+	desc->offset = args->args[0];
+
+        if (args->args_count < 2)
+		return 0;
+
+	if (args->args[1] & GPIO_ACTIVE_LOW)
+		desc->flags = GPIOD_ACTIVE_LOW;
+
+	return 0;
+}
+
 static int gpio_find_and_xlate(struct gpio_desc *desc,
 			       struct fdtdec_phandle_args *args)
 {
 	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
 
-	/* Use the first argument as the offset by default */
-	if (args->args_count > 0)
-		desc->offset = args->args[0];
+	if (ops->xlate)
+		return ops->xlate(desc->dev, desc, args);
 	else
-		desc->offset = -1;
-	desc->flags = 0;
-
-	return ops->xlate ? ops->xlate(desc->dev, desc, args) : 0;
+		return gpio_xlate_offs_flags(desc->dev, desc, args);
 }
 
 int dm_gpio_request(struct gpio_desc *desc, const char *label)
@@ -605,6 +620,7 @@ static int _gpio_request_by_name_nodev(const void *blob, int node,
 
 	desc->dev = NULL;
 	desc->offset = 0;
+	desc->flags = 0;
 	ret = fdtdec_parse_phandle_with_args(blob, node, list_name,
 					     "#gpio-cells", 0, index, &args);
 	if (ret) {
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 68b5f0b..2500c10 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -207,6 +207,16 @@ int gpio_requestf(unsigned gpio, const char *fmt, ...)
 struct fdtdec_phandle_args;
 
 /**
+ * gpio_xlate_offs_flags() - implementation for common use of dm_gpio_ops.xlate
+ *
+ * This routine sets the offset field to args[0] and the flags field to
+ * GPIOD_ACTIVE_LOW if the GPIO_ACTIVE_LOW flag is present in args[1].
+ *
+ */
+int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
+			  struct fdtdec_phandle_args *args);
+
+/**
  * struct struct dm_gpio_ops - Driver model GPIO operations
  *
  * Refer to functions above for description. These function largely copy
@@ -258,12 +268,11 @@ struct dm_gpio_ops {
 	 *
 	 *   @desc->dev to @dev
 	 *   @desc->flags to 0
-	 *   @desc->offset to the value of the first argument in args, if any,
-	 *		otherwise -1 (which is invalid)
+	 *   @desc->offset to 0
 	 *
-	 * This method is optional so if the above defaults suit it can be
-	 * omitted. Typical behaviour is to set up the GPIOD_ACTIVE_LOW flag
-	 * in desc->flags.
+	 * This method is optional and defaults to gpio_xlate_offs_flags,
+	 * which will parse offset and the GPIO_ACTIVE_LOW flag in the first
+	 * two arguments.
 	 *
 	 * Note that @dev is passed in as a parameter to follow driver model
 	 * uclass conventions, even though it is already available as
-- 
2.6.2

[U-Boot] [RESEND PATCH V3 1/6] dm: gpio: add a default gpio xlate routine

[Stephen Warren]

On 04/20/2016 09:37 AM, Eric Nelson wrote:
> Many drivers use a common form of offset + flags for device
> tree nodes. e.g.:
> 	<&gpio1 2 GPIO_ACTIVE_LOW>
>
> This patch adds a common implementation of this type of parsing
> and calls it when a gpio driver doesn't supply its' own xlate
> routine.
>
> This will allow removal of the driver-specific versions in a
> handful of drivers and simplify the addition of new drivers.

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

> +int gpio_xlate_offs_flags(struct udevice *dev,
> +					 struct gpio_desc *desc,
> +					 struct fdtdec_phandle_args *args)
> +{
> +	if (args->args_count < 1)
> +		return -EINVAL;
> +
> +	desc->offset = args->args[0];
> +
> +        if (args->args_count < 2)
> +		return 0;

Nit: There's an indentation inconsistency there.

Aside from that, the series,
Reviewed-by: Stephen Warren <swarren@nvidia.com>

[U-Boot] [RESEND PATCH V3 1/6] dm: gpio: add a default gpio xlate routine

[Eric Nelson]

Thanks Stephen,

On 04/21/2016 10:03 AM, Stephen Warren wrote:
> On 04/20/2016 09:37 AM, Eric Nelson wrote:
>> Many drivers use a common form of offset + flags for device
>> tree nodes. e.g.:
>>     <&gpio1 2 GPIO_ACTIVE_LOW>
>>
>> This patch adds a common implementation of this type of parsing
>> and calls it when a gpio driver doesn't supply its' own xlate
>> routine.
>>
>> This will allow removal of the driver-specific versions in a
>> handful of drivers and simplify the addition of new drivers.
> 
>> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> 
>> +int gpio_xlate_offs_flags(struct udevice *dev,
>> +                     struct gpio_desc *desc,
>> +                     struct fdtdec_phandle_args *args)
>> +{
>> +    if (args->args_count < 1)
>> +        return -EINVAL;
>> +
>> +    desc->offset = args->args[0];
>> +
>> +        if (args->args_count < 2)
>> +        return 0;
> 
> Nit: There's an indentation inconsistency there.
> 
I seem to have fat fingered this.

V4 on its way.

[U-Boot] [PATCH V4, 1/6] dm: gpio: add a default gpio xlate routine

[Eric Nelson]

Many drivers use a common form of offset + flags for device
tree nodes. e.g.:
	<&gpio1 2 GPIO_ACTIVE_LOW>

This patch adds a common implementation of this type of parsing
and calls it when a gpio driver doesn't supply its' own xlate
routine.

This will allow removal of the driver-specific versions in a
handful of drivers and simplify the addition of new drivers.

Signed-off-by: Eric Nelson <eric@nelint.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
  V2 removes parsing of offset from the gpio_find_and_xlate routine,
  and only parses the offset and GPIO_ACTIVE_LOW flag when a 
  driver-specific xlate is unavailable.

  V3 re-works tests of the argument count as suggested by Stephen Warren
  and will allow parsing of nodes with no flags field:
	<&gpio1 2>

  V4 fixes an indent with spaces instead of tabs

 drivers/gpio/gpio-uclass.c | 30 +++++++++++++++++++++++-------
 include/asm-generic/gpio.h | 19 ++++++++++++++-----
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index b58d4e6..732b6c2 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <errno.h>
 #include <fdtdec.h>
 #include <malloc.h>
@@ -113,19 +114,33 @@ int gpio_lookup_name(const char *name, struct udevice **devp,
 	return 0;
 }
 
+int gpio_xlate_offs_flags(struct udevice *dev,
+					 struct gpio_desc *desc,
+					 struct fdtdec_phandle_args *args)
+{
+	if (args->args_count < 1)
+		return -EINVAL;
+
+	desc->offset = args->args[0];
+
+	if (args->args_count < 2)
+		return 0;
+
+	if (args->args[1] & GPIO_ACTIVE_LOW)
+		desc->flags = GPIOD_ACTIVE_LOW;
+
+	return 0;
+}
+
 static int gpio_find_and_xlate(struct gpio_desc *desc,
 			       struct fdtdec_phandle_args *args)
 {
 	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
 
-	/* Use the first argument as the offset by default */
-	if (args->args_count > 0)
-		desc->offset = args->args[0];
+	if (ops->xlate)
+		return ops->xlate(desc->dev, desc, args);
 	else
-		desc->offset = -1;
-	desc->flags = 0;
-
-	return ops->xlate ? ops->xlate(desc->dev, desc, args) : 0;
+		return gpio_xlate_offs_flags(desc->dev, desc, args);
 }
 
 int dm_gpio_request(struct gpio_desc *desc, const char *label)
@@ -605,6 +620,7 @@ static int _gpio_request_by_name_nodev(const void *blob, int node,
 
 	desc->dev = NULL;
 	desc->offset = 0;
+	desc->flags = 0;
 	ret = fdtdec_parse_phandle_with_args(blob, node, list_name,
 					     "#gpio-cells", 0, index, &args);
 	if (ret) {
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 68b5f0b..2500c10 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -207,6 +207,16 @@ int gpio_requestf(unsigned gpio, const char *fmt, ...)
 struct fdtdec_phandle_args;
 
 /**
+ * gpio_xlate_offs_flags() - implementation for common use of dm_gpio_ops.xlate
+ *
+ * This routine sets the offset field to args[0] and the flags field to
+ * GPIOD_ACTIVE_LOW if the GPIO_ACTIVE_LOW flag is present in args[1].
+ *
+ */
+int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
+			  struct fdtdec_phandle_args *args);
+
+/**
  * struct struct dm_gpio_ops - Driver model GPIO operations
  *
  * Refer to functions above for description. These function largely copy
@@ -258,12 +268,11 @@ struct dm_gpio_ops {
 	 *
 	 *   @desc->dev to @dev
 	 *   @desc->flags to 0
-	 *   @desc->offset to the value of the first argument in args, if any,
-	 *		otherwise -1 (which is invalid)
+	 *   @desc->offset to 0
 	 *
-	 * This method is optional so if the above defaults suit it can be
-	 * omitted. Typical behaviour is to set up the GPIOD_ACTIVE_LOW flag
-	 * in desc->flags.
+	 * This method is optional and defaults to gpio_xlate_offs_flags,
+	 * which will parse offset and the GPIO_ACTIVE_LOW flag in the first
+	 * two arguments.
 	 *
 	 * Note that @dev is passed in as a parameter to follow driver model
 	 * uclass conventions, even though it is already available as
-- 
2.6.2

[U-Boot] [PATCH V4, 1/6] dm: gpio: add a default gpio xlate routine

[Simon Glass]

On 24 April 2016 at 17:32, Eric Nelson <eric@nelint.com> wrote:
> Many drivers use a common form of offset + flags for device
> tree nodes. e.g.:
>         <&gpio1 2 GPIO_ACTIVE_LOW>
>
> This patch adds a common implementation of this type of parsing
> and calls it when a gpio driver doesn't supply its' own xlate
> routine.
>
> This will allow removal of the driver-specific versions in a
> handful of drivers and simplify the addition of new drivers.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> ---
>   V2 removes parsing of offset from the gpio_find_and_xlate routine,
>   and only parses the offset and GPIO_ACTIVE_LOW flag when a
>   driver-specific xlate is unavailable.
>
>   V3 re-works tests of the argument count as suggested by Stephen Warren
>   and will allow parsing of nodes with no flags field:
>         <&gpio1 2>
>
>   V4 fixes an indent with spaces instead of tabs
>
>  drivers/gpio/gpio-uclass.c | 30 +++++++++++++++++++++++-------
>  include/asm-generic/gpio.h | 19 ++++++++++++++-----
>  2 files changed, 37 insertions(+), 12 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH V4, 1/6] dm: gpio: add a default gpio xlate routine

[Simon Glass]

On 27 April 2016 at 09:12, Simon Glass <sjg@chromium.org> wrote:
> On 24 April 2016 at 17:32, Eric Nelson <eric@nelint.com> wrote:
>> Many drivers use a common form of offset + flags for device
>> tree nodes. e.g.:
>>         <&gpio1 2 GPIO_ACTIVE_LOW>
>>
>> This patch adds a common implementation of this type of parsing
>> and calls it when a gpio driver doesn't supply its' own xlate
>> routine.
>>
>> This will allow removal of the driver-specific versions in a
>> handful of drivers and simplify the addition of new drivers.
>>
>> Signed-off-by: Eric Nelson <eric@nelint.com>
>> Reviewed-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>   V2 removes parsing of offset from the gpio_find_and_xlate routine,
>>   and only parses the offset and GPIO_ACTIVE_LOW flag when a
>>   driver-specific xlate is unavailable.
>>
>>   V3 re-works tests of the argument count as suggested by Stephen Warren
>>   and will allow parsing of nodes with no flags field:
>>         <&gpio1 2>
>>
>>   V4 fixes an indent with spaces instead of tabs
>>
>>  drivers/gpio/gpio-uclass.c | 30 +++++++++++++++++++++++-------
>>  include/asm-generic/gpio.h | 19 ++++++++++++++-----
>>  2 files changed, 37 insertions(+), 12 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH V2 2/6] gpio: intel_broadwell: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the intel_broadwell driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 Nothing changed in V2.
 drivers/gpio/intel_broadwell_gpio.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpio/intel_broadwell_gpio.c b/drivers/gpio/intel_broadwell_gpio.c
index 8cf76f9..81ce446 100644
--- a/drivers/gpio/intel_broadwell_gpio.c
+++ b/drivers/gpio/intel_broadwell_gpio.c
@@ -162,15 +162,6 @@ static int broadwell_gpio_ofdata_to_platdata(struct udevice *dev)
 	return 0;
 }
 
-static int broadwell_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-				struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_broadwell_ops = {
 	.request		= broadwell_gpio_request,
 	.direction_input	= broadwell_gpio_direction_input,
@@ -178,7 +169,6 @@ static const struct dm_gpio_ops gpio_broadwell_ops = {
 	.get_value		= broadwell_gpio_get_value,
 	.set_value		= broadwell_gpio_set_value,
 	.get_function		= broadwell_gpio_get_function,
-	.xlate			= broadwell_gpio_xlate,
 };
 
 static const struct udevice_id intel_broadwell_gpio_ids[] = {
-- 
2.6.2

[U-Boot] [RESEND PATCH V2 2/6] gpio: intel_broadwell: remove gpio_xlate routine

[Simon Glass]

On 20 April 2016 at 09:37, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the intel_broadwell driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> ---
>  Nothing changed in V2.
>  drivers/gpio/intel_broadwell_gpio.c | 10 ----------
>  1 file changed, 10 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH V2 3/6] gpio: omap: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the omap gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/omap_gpio.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c
index 93d18e4..cd960dc 100644
--- a/drivers/gpio/omap_gpio.c
+++ b/drivers/gpio/omap_gpio.c
@@ -25,7 +25,6 @@
 #include <asm/io.h>
 #include <asm/errno.h>
 #include <malloc.h>
-#include <dt-bindings/gpio/gpio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -277,22 +276,12 @@ static int omap_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_INPUT;
 }
 
-static int omap_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			   struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_omap_ops = {
 	.direction_input	= omap_gpio_direction_input,
 	.direction_output	= omap_gpio_direction_output,
 	.get_value		= omap_gpio_get_value,
 	.set_value		= omap_gpio_set_value,
 	.get_function		= omap_gpio_get_function,
-	.xlate			= omap_gpio_xlate,
 };
 
 static int omap_gpio_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [RESEND PATCH V2 3/6] gpio: omap: remove gpio_xlate routine

[Simon Glass]

On 20 April 2016 at 09:37, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the omap gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> ---
>  V2 removes the include of <dt-bindings/gpio/gpio.h>
>  drivers/gpio/omap_gpio.c | 11 -----------
>  1 file changed, 11 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH V2 4/6] gpio: pic32: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the pic32 gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Purna Chandra Mandal <purna.mandal@microchip.com>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/pic32_gpio.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpio/pic32_gpio.c b/drivers/gpio/pic32_gpio.c
index 499b4fa..7a037f3 100644
--- a/drivers/gpio/pic32_gpio.c
+++ b/drivers/gpio/pic32_gpio.c
@@ -12,7 +12,6 @@
 #include <asm/io.h>
 #include <asm/gpio.h>
 #include <linux/compat.h>
-#include <dt-bindings/gpio/gpio.h>
 #include <mach/pic32.h>
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -99,14 +98,6 @@ static int pic32_gpio_direction_output(struct udevice *dev,
 	return 0;
 }
 
-static int pic32_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			    struct fdtdec_phandle_args *args)
-{
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static int pic32_gpio_get_function(struct udevice *dev, unsigned offset)
 {
 	int ret = GPIOF_UNUSED;
@@ -131,7 +122,6 @@ static const struct dm_gpio_ops gpio_pic32_ops = {
 	.get_value		= pic32_gpio_get_value,
 	.set_value		= pic32_gpio_set_value,
 	.get_function		= pic32_gpio_get_function,
-	.xlate			= pic32_gpio_xlate,
 };
 
 static int pic32_gpio_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [RESEND PATCH V2 4/6] gpio: pic32: remove gpio_xlate routine

[Simon Glass]

On 20 April 2016 at 09:37, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the pic32 gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Purna Chandra Mandal <purna.mandal@microchip.com>
> ---
>  V2 removes the include of <dt-bindings/gpio/gpio.h>
>  drivers/gpio/pic32_gpio.c | 10 ----------
>  1 file changed, 10 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH V2 5/6] gpio: rk: remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the Rockchip gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/rk_gpio.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/rk_gpio.c b/drivers/gpio/rk_gpio.c
index 40e87bd..fefe3ca 100644
--- a/drivers/gpio/rk_gpio.c
+++ b/drivers/gpio/rk_gpio.c
@@ -16,7 +16,6 @@
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <dm/pinctrl.h>
-#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/clock/rk3288-cru.h>
 
 enum {
@@ -98,15 +97,6 @@ static int rockchip_gpio_get_function(struct udevice *dev, unsigned offset)
 #endif
 }
 
-static int rockchip_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			    struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static int rockchip_gpio_probe(struct udevice *dev)
 {
 	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
@@ -135,7 +125,6 @@ static const struct dm_gpio_ops gpio_rockchip_ops = {
 	.get_value		= rockchip_gpio_get_value,
 	.set_value		= rockchip_gpio_set_value,
 	.get_function		= rockchip_gpio_get_function,
-	.xlate			= rockchip_gpio_xlate,
 };
 
 static const struct udevice_id rockchip_gpio_ids[] = {
-- 
2.6.2

[U-Boot] [RESEND PATCH V2 5/6] gpio: rk: remove gpio_xlate routine

[Simon Glass]

On 20 April 2016 at 09:37, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the Rockchip gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> ---
>  V2 removes the include of <dt-bindings/gpio/gpio.h>
>  drivers/gpio/rk_gpio.c | 11 -----------
>  1 file changed, 11 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [RESEND PATCH V2 6/6] gpio: exynos(s5p): remove gpio_xlate routine

[Eric Nelson]

With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
the Exynos/S5P gpio driver doesn't need a custom xlate routine.

Signed-off-by: Eric Nelson <eric@nelint.com>
Acked-by: Simon Glass <sjg@chromium.org>
Acked-by: Minkyu Kang <mk7.kang@samsung.com>
---
 V2 removes the include of <dt-bindings/gpio/gpio.h>
 drivers/gpio/s5p_gpio.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/s5p_gpio.c b/drivers/gpio/s5p_gpio.c
index 0f22b23..377fed4 100644
--- a/drivers/gpio/s5p_gpio.c
+++ b/drivers/gpio/s5p_gpio.c
@@ -13,7 +13,6 @@
 #include <asm/io.h>
 #include <asm/gpio.h>
 #include <dm/device-internal.h>
-#include <dt-bindings/gpio/gpio.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -276,22 +275,12 @@ static int exynos_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_FUNC;
 }
 
-static int exynos_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
-			     struct fdtdec_phandle_args *args)
-{
-	desc->offset = args->args[0];
-	desc->flags = args->args[1] & GPIO_ACTIVE_LOW ? GPIOD_ACTIVE_LOW : 0;
-
-	return 0;
-}
-
 static const struct dm_gpio_ops gpio_exynos_ops = {
 	.direction_input	= exynos_gpio_direction_input,
 	.direction_output	= exynos_gpio_direction_output,
 	.get_value		= exynos_gpio_get_value,
 	.set_value		= exynos_gpio_set_value,
 	.get_function		= exynos_gpio_get_function,
-	.xlate			= exynos_gpio_xlate,
 };
 
 static int gpio_exynos_probe(struct udevice *dev)
-- 
2.6.2

[U-Boot] [RESEND PATCH V2 6/6] gpio: exynos(s5p): remove gpio_xlate routine

[Simon Glass]

On 20 April 2016 at 09:37, Eric Nelson <eric@nelint.com> wrote:
> With the addition of GPIO_ACTIVE_LOW parsing in gpio-uclass,
> the Exynos/S5P gpio driver doesn't need a custom xlate routine.
>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> Acked-by: Minkyu Kang <mk7.kang@samsung.com>
> ---
>  V2 removes the include of <dt-bindings/gpio/gpio.h>
>  drivers/gpio/s5p_gpio.c | 11 -----------
>  1 file changed, 11 deletions(-)

Applied to u-boot-dm, thanks!