All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gpio: of: Remove unneeded dummy function
@ 2014-04-23 15:28 Thierry Reding
  2014-04-23 15:28 ` [PATCH 2/2] gpio: of: Allow -gpio suffix for property names Thierry Reding
  2014-04-24 12:43 ` [PATCH 1/2] gpio: of: Remove unneeded dummy function Linus Walleij
  0 siblings, 2 replies; 14+ messages in thread
From: Thierry Reding @ 2014-04-23 15:28 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot; +Cc: linux-gpio, linux-kernel

From: Thierry Reding <treding@nvidia.com>

of_find_gpio() is always called under an IS_ENABLED(CONFIG_OF), so the
dummy implementation provided for !OF configurations is not needed.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpio/gpiolib.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index ee1819fdcf35..7a0b97076374 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2590,7 +2590,6 @@ void gpiod_add_lookup_table(struct gpiod_lookup_table *table)
 	mutex_unlock(&gpio_lookup_lock);
 }
 
-#ifdef CONFIG_OF
 static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 				      unsigned int idx,
 				      enum gpio_lookup_flags *flags)
@@ -2615,14 +2614,6 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 
 	return desc;
 }
-#else
-static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
-				      unsigned int idx,
-				      enum gpio_lookup_flags *flags)
-{
-	return ERR_PTR(-ENODEV);
-}
-#endif
 
 static struct gpio_desc *acpi_find_gpio(struct device *dev, const char *con_id,
 					unsigned int idx,
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-23 15:28 [PATCH 1/2] gpio: of: Remove unneeded dummy function Thierry Reding
@ 2014-04-23 15:28 ` Thierry Reding
  2014-04-24 12:47   ` Linus Walleij
  2014-04-25  7:52   ` Linus Walleij
  2014-04-24 12:43 ` [PATCH 1/2] gpio: of: Remove unneeded dummy function Linus Walleij
  1 sibling, 2 replies; 14+ messages in thread
From: Thierry Reding @ 2014-04-23 15:28 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot; +Cc: linux-gpio, linux-kernel

From: Thierry Reding <treding@nvidia.com>

Many bindings use the -gpio suffix in property names. Support this in
addition to the -gpios suffix when requesting GPIOs using the new
descriptor-based API.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpio/gpiolib.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 7a0b97076374..b991462c22fb 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2594,17 +2594,23 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 				      unsigned int idx,
 				      enum gpio_lookup_flags *flags)
 {
+	static const char *suffixes[] = { "gpios", "gpio" };
 	char prop_name[32]; /* 32 is max size of property name */
 	enum of_gpio_flags of_flags;
 	struct gpio_desc *desc;
+	unsigned int i;
 
-	if (con_id)
-		snprintf(prop_name, 32, "%s-gpios", con_id);
-	else
-		snprintf(prop_name, 32, "gpios");
+	for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
+		if (con_id)
+			snprintf(prop_name, 32, "%s-%s", con_id, suffixes[i]);
+		else
+			snprintf(prop_name, 32, "%s", suffixes[i]);
 
-	desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
-					&of_flags);
+		desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
+						&of_flags);
+		if (!IS_ERR(desc))
+			break;
+	}
 
 	if (IS_ERR(desc))
 		return desc;
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] gpio: of: Remove unneeded dummy function
  2014-04-23 15:28 [PATCH 1/2] gpio: of: Remove unneeded dummy function Thierry Reding
  2014-04-23 15:28 ` [PATCH 2/2] gpio: of: Allow -gpio suffix for property names Thierry Reding
@ 2014-04-24 12:43 ` Linus Walleij
  1 sibling, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2014-04-24 12:43 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> of_find_gpio() is always called under an IS_ENABLED(CONFIG_OF), so the
> dummy implementation provided for !OF configurations is not needed.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-23 15:28 ` [PATCH 2/2] gpio: of: Allow -gpio suffix for property names Thierry Reding
@ 2014-04-24 12:47   ` Linus Walleij
  2014-04-24 14:06     ` Rob Herring
  2014-04-25  7:52   ` Linus Walleij
  1 sibling, 1 reply; 14+ messages in thread
From: Linus Walleij @ 2014-04-24 12:47 UTC (permalink / raw)
  To: Thierry Reding, devicetree; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> Many bindings use the -gpio suffix in property names. Support this in
> addition to the -gpios suffix when requesting GPIOs using the new
> descriptor-based API.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Are the DT bindings really full of such ambiguity between
singular and plural? Examples?

What happens in affected drivers today? It just doesn't work?

Does that mean these bindings are not actively used by any
drivers yet so we could augment the bindings instead, or are
they already deployed so we must implement this?

Would like a word from the DT people here...

>  drivers/gpio/gpiolib.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 7a0b97076374..b991462c22fb 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2594,17 +2594,23 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
>                                       unsigned int idx,
>                                       enum gpio_lookup_flags *flags)
>  {
> +       static const char *suffixes[] = { "gpios", "gpio" };
>         char prop_name[32]; /* 32 is max size of property name */
>         enum of_gpio_flags of_flags;
>         struct gpio_desc *desc;
> +       unsigned int i;
>
> -       if (con_id)
> -               snprintf(prop_name, 32, "%s-gpios", con_id);
> -       else
> -               snprintf(prop_name, 32, "gpios");
> +       for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
> +               if (con_id)
> +                       snprintf(prop_name, 32, "%s-%s", con_id, suffixes[i]);
> +               else
> +                       snprintf(prop_name, 32, "%s", suffixes[i]);
>
> -       desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
> -                                       &of_flags);
> +               desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
> +                                               &of_flags);
> +               if (!IS_ERR(desc))
> +                       break;
> +       }

Code snippet left for reference on devicetree ML.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-24 12:47   ` Linus Walleij
@ 2014-04-24 14:06     ` Rob Herring
  2014-04-24 18:22       ` Thierry Reding
  2014-04-25  7:38       ` Alexandre Courbot
  0 siblings, 2 replies; 14+ messages in thread
From: Rob Herring @ 2014-04-24 14:06 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Thierry Reding, devicetree, Alexandre Courbot, linux-gpio, linux-kernel

On Thu, Apr 24, 2014 at 7:47 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
>
>> From: Thierry Reding <treding@nvidia.com>
>>
>> Many bindings use the -gpio suffix in property names. Support this in
>> addition to the -gpios suffix when requesting GPIOs using the new
>> descriptor-based API.
>>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>
> Are the DT bindings really full of such ambiguity between
> singular and plural? Examples?
>
> What happens in affected drivers today? It just doesn't work?

They mostly seem to use of_get_named_gpio.

>
> Does that mean these bindings are not actively used by any
> drivers yet so we could augment the bindings instead, or are
> they already deployed so we must implement this?
>
> Would like a word from the DT people here...

Interestingly, there is not a single occurrence of '-gpio ' in
powerpc, but a bunch in ARM. In hindsight, we should have perhaps
enforced using -gpios only, but that doesn't really matter now. We
have -gpio in use, so we need to support it. That doesn't necessarily
mean this function has to support it. For example, this function could
a legacy method and some other function should be used instead
(of_get_named_gpio perhaps).

>>  drivers/gpio/gpiolib.c | 18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index 7a0b97076374..b991462c22fb 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -2594,17 +2594,23 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
>>                                       unsigned int idx,
>>                                       enum gpio_lookup_flags *flags)
>>  {
>> +       static const char *suffixes[] = { "gpios", "gpio" };
>>         char prop_name[32]; /* 32 is max size of property name */
>>         enum of_gpio_flags of_flags;
>>         struct gpio_desc *desc;
>> +       unsigned int i;
>>
>> -       if (con_id)
>> -               snprintf(prop_name, 32, "%s-gpios", con_id);
>> -       else
>> -               snprintf(prop_name, 32, "gpios");
>> +       for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
>> +               if (con_id)
>> +                       snprintf(prop_name, 32, "%s-%s", con_id, suffixes[i]);
>> +               else
>> +                       snprintf(prop_name, 32, "%s", suffixes[i]);

This has the side effect of searching for "gpio" as property name
which I don't think we want to allow. It also allows a DT with either
suffix to work. While I don't necessarily think the kernel's job
should be DT validation, we don't have any other validation today and
I don't see how this change simplifies the code. Between stricter DT
checking (in the kernel) and simpler code, I'd pick the latter.

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-24 14:06     ` Rob Herring
@ 2014-04-24 18:22       ` Thierry Reding
  2014-04-25 15:24         ` Stephen Warren
  2014-04-25  7:38       ` Alexandre Courbot
  1 sibling, 1 reply; 14+ messages in thread
From: Thierry Reding @ 2014-04-24 18:22 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, devicetree, Alexandre Courbot, linux-gpio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4249 bytes --]

On Thu, Apr 24, 2014 at 09:06:24AM -0500, Rob Herring wrote:
> On Thu, Apr 24, 2014 at 7:47 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:
> >
> >> From: Thierry Reding <treding@nvidia.com>
> >>
> >> Many bindings use the -gpio suffix in property names. Support this in
> >> addition to the -gpios suffix when requesting GPIOs using the new
> >> descriptor-based API.
> >>
> >> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >
> > Are the DT bindings really full of such ambiguity between
> > singular and plural? Examples?
> >
> > What happens in affected drivers today? It just doesn't work?
> 
> They mostly seem to use of_get_named_gpio.

Indeed. That has the downside of requiring manual parsing and handling
of the GPIO polarity, though.

> > Does that mean these bindings are not actively used by any
> > drivers yet so we could augment the bindings instead, or are
> > they already deployed so we must implement this?
> >
> > Would like a word from the DT people here...
> 
> Interestingly, there is not a single occurrence of '-gpio ' in
> powerpc, but a bunch in ARM. In hindsight, we should have perhaps
> enforced using -gpios only, but that doesn't really matter now. We
> have -gpio in use, so we need to support it.

I think I also saw a proposal only recently to add support for a
gpios/gpio-names type of binding

> That doesn't necessarily mean this function has to support it. For
> example, this function could a legacy method and some other function
> should be used instead (of_get_named_gpio perhaps).

The reason why I posted this is precisely because I wanted to convert
over some drivers to use the new helpers (because they make things like
polarity handling much easier). My first attempt was to fix the binding
because I was under the impression that -gpio usage was discouraged, but
people didn't like that because, you know, DT bindings being a stable
ABI and all that.

The downside of not allowing the gpiod API to support the -gpio suffix
is that we'll never be able to convert drivers that use such a binding
and will forever have a hodgepodge of GPIO APIs that we need to support.

> >>  drivers/gpio/gpiolib.c | 18 ++++++++++++------
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> >> index 7a0b97076374..b991462c22fb 100644
> >> --- a/drivers/gpio/gpiolib.c
> >> +++ b/drivers/gpio/gpiolib.c
> >> @@ -2594,17 +2594,23 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
> >>                                       unsigned int idx,
> >>                                       enum gpio_lookup_flags *flags)
> >>  {
> >> +       static const char *suffixes[] = { "gpios", "gpio" };
> >>         char prop_name[32]; /* 32 is max size of property name */
> >>         enum of_gpio_flags of_flags;
> >>         struct gpio_desc *desc;
> >> +       unsigned int i;
> >>
> >> -       if (con_id)
> >> -               snprintf(prop_name, 32, "%s-gpios", con_id);
> >> -       else
> >> -               snprintf(prop_name, 32, "gpios");
> >> +       for (i = 0; i < ARRAY_SIZE(suffixes); i++) {
> >> +               if (con_id)
> >> +                       snprintf(prop_name, 32, "%s-%s", con_id, suffixes[i]);
> >> +               else
> >> +                       snprintf(prop_name, 32, "%s", suffixes[i]);
> 
> This has the side effect of searching for "gpio" as property name
> which I don't think we want to allow.

Why don't we want to allow a "gpio" property when we already allow
"gpios"?

> It also allows a DT with either suffix to work. While I don't
> necessarily think the kernel's job should be DT validation, we don't
> have any other validation today and I don't see how this change
> simplifies the code. Between stricter DT checking (in the kernel) and
> simpler code, I'd pick the latter.

I had briefly considered adding more validation here as well, such as
refusing to hand out any GPIO with idx > 0 for the -gpio suffix, but
then opted not to do that in favour of code simplicity.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-24 14:06     ` Rob Herring
  2014-04-24 18:22       ` Thierry Reding
@ 2014-04-25  7:38       ` Alexandre Courbot
  1 sibling, 0 replies; 14+ messages in thread
From: Alexandre Courbot @ 2014-04-25  7:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Thierry Reding, devicetree, linux-gpio, linux-kernel

On Thu, Apr 24, 2014 at 11:06 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Apr 24, 2014 at 7:47 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
>> <thierry.reding@gmail.com> wrote:
>>
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Many bindings use the -gpio suffix in property names. Support this in
>>> addition to the -gpios suffix when requesting GPIOs using the new
>>> descriptor-based API.
>>>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>
>> Are the DT bindings really full of such ambiguity between
>> singular and plural? Examples?
>>
>> What happens in affected drivers today? It just doesn't work?
>
> They mostly seem to use of_get_named_gpio.

In an idea world of_get_named_gpio() would be gpiolib-private so
people cannot come with their own custom-named DT GPIO properties.
Given its broad usage this is not possible, but maybe we can at least
do it for of_get_named_gpiod().

>
>>
>> Does that mean these bindings are not actively used by any
>> drivers yet so we could augment the bindings instead, or are
>> they already deployed so we must implement this?
>>
>> Would like a word from the DT people here...
>
> Interestingly, there is not a single occurrence of '-gpio ' in
> powerpc, but a bunch in ARM. In hindsight, we should have perhaps
> enforced using -gpios only, but that doesn't really matter now. We
> have -gpio in use, so we need to support it. That doesn't necessarily
> mean this function has to support it. For example, this function could
> a legacy method and some other function should be used instead
> (of_get_named_gpio perhaps).

It seems like we have to support that use-case indeed (many instances
in arch/arm). The incentive for handling this in that function vs.
user code is that having support here would allow drivers to directly
use gpiod_get() and having it automatically handle GPIO properties
like active-low instead of requiring user code to handle it by itself
every time.

Without this many drivers for devices using "-gpio" properties could
not switch to the new gpiod interface.

So as far as I'm concerned this code makes GPIO user-code easier. This
is not to say that we should allow that "-gpio" suffix for new
bindings.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-23 15:28 ` [PATCH 2/2] gpio: of: Allow -gpio suffix for property names Thierry Reding
  2014-04-24 12:47   ` Linus Walleij
@ 2014-04-25  7:52   ` Linus Walleij
  2014-06-02 23:04     ` Tony Lindgren
  1 sibling, 1 reply; 14+ messages in thread
From: Linus Walleij @ 2014-04-25  7:52 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Alexandre Courbot, linux-gpio, linux-kernel

On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> Many bindings use the -gpio suffix in property names. Support this in
> addition to the -gpios suffix when requesting GPIOs using the new
> descriptor-based API.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

It appears this can save quite a lot of code in drivers, work that
I trust Thierry to persue based on this to some extent so patch is
tentatively applied unless something comes up.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-24 18:22       ` Thierry Reding
@ 2014-04-25 15:24         ` Stephen Warren
  2014-05-02 22:22           ` Linus Walleij
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Warren @ 2014-04-25 15:24 UTC (permalink / raw)
  To: Thierry Reding, Rob Herring
  Cc: Linus Walleij, devicetree, Alexandre Courbot, linux-gpio, linux-kernel

On 04/24/2014 12:22 PM, Thierry Reding wrote:
...
> The downside of not allowing the gpiod API to support the -gpio suffix
> is that we'll never be able to convert drivers that use such a binding
> and will forever have a hodgepodge of GPIO APIs that we need to support.

Perhaps rather than making the existing gpiod API automatically search
for both -gpios and -gpio, we could make a new API for the other suffix,
so that driver indicate explicitly which property name they want. That
way, someone can't accidentally write -gpio in the DT and have it still
work. Or, add a parameter to the existing API, but that's probably a lot
more churn.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-25 15:24         ` Stephen Warren
@ 2014-05-02 22:22           ` Linus Walleij
  0 siblings, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2014-05-02 22:22 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Thierry Reding, Rob Herring, devicetree, Alexandre Courbot,
	linux-gpio, linux-kernel

On Fri, Apr 25, 2014 at 8:24 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/24/2014 12:22 PM, Thierry Reding wrote:
> ...
>> The downside of not allowing the gpiod API to support the -gpio suffix
>> is that we'll never be able to convert drivers that use such a binding
>> and will forever have a hodgepodge of GPIO APIs that we need to support.
>
> Perhaps rather than making the existing gpiod API automatically search
> for both -gpios and -gpio, we could make a new API for the other suffix,
> so that driver indicate explicitly which property name they want. That
> way, someone can't accidentally write -gpio in the DT and have it still
> work. Or, add a parameter to the existing API, but that's probably a lot
> more churn.

Hm, that is possible, I just worry that this will lead the DT and ACPI
semantics to diverge even more, and the present patch make things
more coherent from the framework side of things instead of even more
elaborate per-HW-info-method :-/

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-04-25  7:52   ` Linus Walleij
@ 2014-06-02 23:04     ` Tony Lindgren
  2014-06-02 23:14       ` Tony Lindgren
  0 siblings, 1 reply; 14+ messages in thread
From: Tony Lindgren @ 2014-06-02 23:04 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Thierry Reding, Alexandre Courbot, linux-gpio, linux-kernel, linux-omap

* Linus Walleij <linus.walleij@linaro.org> [140425 00:53]:
> On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> 
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Many bindings use the -gpio suffix in property names. Support this in
> > addition to the -gpios suffix when requesting GPIOs using the new
> > descriptor-based API.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> It appears this can save quite a lot of code in drivers, work that
> I trust Thierry to persue based on this to some extent so patch is
> tentatively applied unless something comes up.

Looks like this patch causes a regression where GPIOs on I2C will
no longer return -EPROBE_DEFER but seem to return -ENOENT instead.

This breaks drivers using things like devm_gpiod_get_index()
on a GPIO that's on a I2C bus not probed yet.

Reverting commit dd34c37aa3e (gpio: of: Allow -gpio suffix for
property names) fixes things.

Regards,

Tony

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-06-02 23:04     ` Tony Lindgren
@ 2014-06-02 23:14       ` Tony Lindgren
  2014-06-04 13:08         ` Thierry Reding
  2014-06-12  8:18         ` Linus Walleij
  0 siblings, 2 replies; 14+ messages in thread
From: Tony Lindgren @ 2014-06-02 23:14 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Thierry Reding, Alexandre Courbot, linux-gpio, linux-kernel, linux-omap

* Tony Lindgren <tony@atomide.com> [140602 16:06]:
> * Linus Walleij <linus.walleij@linaro.org> [140425 00:53]:
> > On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
> > <thierry.reding@gmail.com> wrote:
> > 
> > > From: Thierry Reding <treding@nvidia.com>
> > >
> > > Many bindings use the -gpio suffix in property names. Support this in
> > > addition to the -gpios suffix when requesting GPIOs using the new
> > > descriptor-based API.
> > >
> > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > 
> > It appears this can save quite a lot of code in drivers, work that
> > I trust Thierry to persue based on this to some extent so patch is
> > tentatively applied unless something comes up.
> 
> Looks like this patch causes a regression where GPIOs on I2C will
> no longer return -EPROBE_DEFER but seem to return -ENOENT instead.
> 
> This breaks drivers using things like devm_gpiod_get_index()
> on a GPIO that's on a I2C bus not probed yet.
> 
> Reverting commit dd34c37aa3e (gpio: of: Allow -gpio suffix for
> property names) fixes things.

Looks like something like below fixes the issue.

Regards,

Tony

8< -----------------------
From: Tony Lindgren <tony@atomide.com>
Date: Mon, 2 Jun 2014 16:13:46 -0700
Subject: [PATCH] gpio: of: Fix handling for deferred probe for -gpio suffix

Commit dd34c37aa3e (gpio: of: Allow -gpio suffix for property names)
added parsing for both -gpio and -gpios suffix but also changed
the handling for deferred probe unintentionally. Because of the
looping the second name will now return -ENOENT instead of
-EPROBE_DEFER. Fix the issue by breaking out of the loop if
-EPROBE_DEFER is encountered.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2614,7 +2614,7 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 
 		desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
 						&of_flags);
-		if (!IS_ERR(desc))
+		if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
 			break;
 	}
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-06-02 23:14       ` Tony Lindgren
@ 2014-06-04 13:08         ` Thierry Reding
  2014-06-12  8:18         ` Linus Walleij
  1 sibling, 0 replies; 14+ messages in thread
From: Thierry Reding @ 2014-06-04 13:08 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Linus Walleij, Alexandre Courbot, linux-gpio, linux-kernel, linux-omap

[-- Attachment #1: Type: text/plain, Size: 2344 bytes --]

On Mon, Jun 02, 2014 at 04:14:23PM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [140602 16:06]:
> > * Linus Walleij <linus.walleij@linaro.org> [140425 00:53]:
> > > On Wed, Apr 23, 2014 at 5:28 PM, Thierry Reding
> > > <thierry.reding@gmail.com> wrote:
> > > 
> > > > From: Thierry Reding <treding@nvidia.com>
> > > >
> > > > Many bindings use the -gpio suffix in property names. Support this in
> > > > addition to the -gpios suffix when requesting GPIOs using the new
> > > > descriptor-based API.
> > > >
> > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > 
> > > It appears this can save quite a lot of code in drivers, work that
> > > I trust Thierry to persue based on this to some extent so patch is
> > > tentatively applied unless something comes up.
> > 
> > Looks like this patch causes a regression where GPIOs on I2C will
> > no longer return -EPROBE_DEFER but seem to return -ENOENT instead.
> > 
> > This breaks drivers using things like devm_gpiod_get_index()
> > on a GPIO that's on a I2C bus not probed yet.
> > 
> > Reverting commit dd34c37aa3e (gpio: of: Allow -gpio suffix for
> > property names) fixes things.
> 
> Looks like something like below fixes the issue.
> 
> Regards,
> 
> Tony
> 
> 8< -----------------------
> From: Tony Lindgren <tony@atomide.com>
> Date: Mon, 2 Jun 2014 16:13:46 -0700
> Subject: [PATCH] gpio: of: Fix handling for deferred probe for -gpio suffix
> 
> Commit dd34c37aa3e (gpio: of: Allow -gpio suffix for property names)
> added parsing for both -gpio and -gpios suffix but also changed
> the handling for deferred probe unintentionally. Because of the
> looping the second name will now return -ENOENT instead of
> -EPROBE_DEFER. Fix the issue by breaking out of the loop if
> -EPROBE_DEFER is encountered.
> 
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2614,7 +2614,7 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
>  
>  		desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx,
>  						&of_flags);
> -		if (!IS_ERR(desc))
> +		if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER))
>  			break;
>  	}

This looks good to me:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] gpio: of: Allow -gpio suffix for property names
  2014-06-02 23:14       ` Tony Lindgren
  2014-06-04 13:08         ` Thierry Reding
@ 2014-06-12  8:18         ` Linus Walleij
  1 sibling, 0 replies; 14+ messages in thread
From: Linus Walleij @ 2014-06-12  8:18 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Thierry Reding, Alexandre Courbot, linux-gpio, linux-kernel, Linux-OMAP

On Tue, Jun 3, 2014 at 1:14 AM, Tony Lindgren <tony@atomide.com> wrote:

> Looks like something like below fixes the issue.
>
> Regards,
>
> Tony
>
> 8< -----------------------
> From: Tony Lindgren <tony@atomide.com>
> Date: Mon, 2 Jun 2014 16:13:46 -0700
> Subject: [PATCH] gpio: of: Fix handling for deferred probe for -gpio suffix
>
> Commit dd34c37aa3e (gpio: of: Allow -gpio suffix for property names)
> added parsing for both -gpio and -gpios suffix but also changed
> the handling for deferred probe unintentionally. Because of the
> looping the second name will now return -ENOENT instead of
> -EPROBE_DEFER. Fix the issue by breaking out of the loop if
> -EPROBE_DEFER is encountered.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Patch applied for fixes with Thierry's Reveiwed-by tag.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-06-12  8:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-23 15:28 [PATCH 1/2] gpio: of: Remove unneeded dummy function Thierry Reding
2014-04-23 15:28 ` [PATCH 2/2] gpio: of: Allow -gpio suffix for property names Thierry Reding
2014-04-24 12:47   ` Linus Walleij
2014-04-24 14:06     ` Rob Herring
2014-04-24 18:22       ` Thierry Reding
2014-04-25 15:24         ` Stephen Warren
2014-05-02 22:22           ` Linus Walleij
2014-04-25  7:38       ` Alexandre Courbot
2014-04-25  7:52   ` Linus Walleij
2014-06-02 23:04     ` Tony Lindgren
2014-06-02 23:14       ` Tony Lindgren
2014-06-04 13:08         ` Thierry Reding
2014-06-12  8:18         ` Linus Walleij
2014-04-24 12:43 ` [PATCH 1/2] gpio: of: Remove unneeded dummy function Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.