linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
@ 2014-12-03 17:57 Fabio Estevam
  2014-12-03 19:59 ` Fabio Estevam
  2014-12-03 23:17 ` Grant Likely
  0 siblings, 2 replies; 12+ messages in thread
From: Fabio Estevam @ 2014-12-03 17:57 UTC (permalink / raw)
  To: cooloney
  Cc: rpurdie, jean-michel.hautbois, grant.likely, rafael.j.wysocki,
	mika.westerberg, linux-leds, Fabio Estevam

From: Fabio Estevam <fabio.estevam@freescale.com>

Since commit a43f2cbbb009f96 ("leds: leds-gpio: Make use of device property
API") it is no longer possible to register multiple gpio leds without passing
the 'label' property.

According to Documentation/devicetree/bindings/leds/common.txt:

"Optional properties for child nodes:
- label : The label for this LED.  If omitted, the label is
  taken from the node name (excluding the unit address)."

So retrieve the node name when the 'label' property is absent to keep the old
behaviour and fix this regression.

Reported-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Consider ACPI case as suggested by Grant

 drivers/leds/leds-gpio.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index fd53968..8a8ba11 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -170,6 +170,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 	struct fwnode_handle *child;
 	struct gpio_leds_priv *priv;
 	int count, ret;
+	struct device_node *np;
 
 	count = device_get_child_node_count(dev);
 	if (!count)
@@ -189,7 +190,16 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 			goto err;
 		}
 
-		fwnode_property_read_string(child, "label", &led.name);
+		np = of_node(child);
+
+		if (fwnode_property_present(child, "label")) {
+			fwnode_property_read_string(child, "label", &led.name);
+		} else {
+			if (IS_ENABLED(CONFIG_OF) && !led.name && np)
+				led.name = np->name;
+			if (!led.name)
+				return ERR_PTR(-EINVAL);
+		}
 		fwnode_property_read_string(child, "linux,default-trigger",
 					    &led.default_trigger);
 
-- 
1.9.1

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-03 17:57 [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent Fabio Estevam
@ 2014-12-03 19:59 ` Fabio Estevam
  2014-12-03 22:23   ` Bryan Wu
  2014-12-03 23:16   ` Grant Likely
  2014-12-03 23:17 ` Grant Likely
  1 sibling, 2 replies; 12+ messages in thread
From: Fabio Estevam @ 2014-12-03 19:59 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Richard Purdie, Jean-Michel Hautbois, Grant Likely,
	Rafael J. Wysocki, Mika Westerberg, linux-leds, Fabio Estevam

On Wed, Dec 3, 2014 at 3:57 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Since commit a43f2cbbb009f96 ("leds: leds-gpio: Make use of device property
> API") it is no longer possible to register multiple gpio leds without passing
> the 'label' property.
>
> According to Documentation/devicetree/bindings/leds/common.txt:
>
> "Optional properties for child nodes:
> - label : The label for this LED.  If omitted, the label is
>   taken from the node name (excluding the unit address)."
>
> So retrieve the node name when the 'label' property is absent to keep the old
> behaviour and fix this regression.
>
> Reported-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v1:
> - Consider ACPI case as suggested by Grant
>
>  drivers/leds/leds-gpio.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index fd53968..8a8ba11 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -170,6 +170,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>         struct fwnode_handle *child;
>         struct gpio_leds_priv *priv;
>         int count, ret;
> +       struct device_node *np;
>
>         count = device_get_child_node_count(dev);
>         if (!count)
> @@ -189,7 +190,16 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>                         goto err;
>                 }
>
> -               fwnode_property_read_string(child, "label", &led.name);
> +               np = of_node(child);
> +
> +               if (fwnode_property_present(child, "label")) {
> +                       fwnode_property_read_string(child, "label", &led.name);
> +               } else {
> +                       if (IS_ENABLED(CONFIG_OF) && !led.name && np)
> +                               led.name = np->name;
> +                       if (!led.name)
> +                               return ERR_PTR(-EINVAL);
> +               }
>                 fwnode_property_read_string(child, "linux,default-trigger",

Or maybe we should not expose the CONFIG_OF versus CONFIG_ACPI
ifdefery to the driver and do something like this instead:

diff --git a/drivers/base/property.c b/drivers/base/property.c
index c458458..bdeee26 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -399,6 +399,27 @@ struct fwnode_handle
*device_get_next_child_node(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(device_get_next_child_node);

+const char *device_get_node_name(struct device *dev, struct
fwnode_handle *child)
+{
+
+    if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+        struct device_node *node = of_node(child);
+
+        if (node)
+            return node->name;
+    }
+
+    if (IS_ENABLED(CONFIG_ACPI)) {
+        struct acpi_device *node = acpi_node(child);
+
+        if (node)
+            return acpi_dev_name(node);
+    }
+
+    return NULL;
+}
+EXPORT_SYMBOL_GPL(device_get_node_name);
+
 /**
  * fwnode_handle_put - Drop reference to a device node
  * @fwnode: Pointer to the device node to drop the reference to.
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index fd53968..c1c472b 100644

diff --git a/include/linux/property.h b/include/linux/property.h
index a6a3d98..9bec811 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -72,6 +72,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 struct fwnode_handle *device_get_next_child_node(struct device *dev,
                          struct fwnode_handle *child);

+const char *device_get_node_name(struct device *dev, struct
fwnode_handle *child);
+
 #define device_for_each_child_node(dev, child) \
     for (child = device_get_next_child_node(dev, NULL); child; \
          child = device_get_next_child_node(dev, child))

, and then on leds-gpio.c we can simply do:

--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -189,7 +189,10 @@ static struct gpio_leds_priv
*gpio_leds_create(struct platform_device *pdev)
             goto err;
         }

-        fwnode_property_read_string(child, "label", &led.name);
+        if (fwnode_property_present(child, "label"))
+            fwnode_property_read_string(child, "label", &led.name);
+        else
+            led.name = device_get_node_name(dev, child);
         fwnode_property_read_string(child, "linux,default-trigger",
                         &led.default_trigger);

I am not familiar with ACPI so I don't know if the 'return
acpi_dev_name(node)' is the appropriate way to retrieve the equivalent
of a dt node name.

Please advise.

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-03 19:59 ` Fabio Estevam
@ 2014-12-03 22:23   ` Bryan Wu
  2014-12-03 23:16   ` Grant Likely
  1 sibling, 0 replies; 12+ messages in thread
From: Bryan Wu @ 2014-12-03 22:23 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Richard Purdie, Jean-Michel Hautbois, Grant Likely,
	Rafael J. Wysocki, Mika Westerberg, Linux LED Subsystem,
	Fabio Estevam

On Wed, Dec 3, 2014 at 11:59 AM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 3:57 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Since commit a43f2cbbb009f96 ("leds: leds-gpio: Make use of device property
>> API") it is no longer possible to register multiple gpio leds without passing
>> the 'label' property.
>>
>> According to Documentation/devicetree/bindings/leds/common.txt:
>>
>> "Optional properties for child nodes:
>> - label : The label for this LED.  If omitted, the label is
>>   taken from the node name (excluding the unit address)."
>>
>> So retrieve the node name when the 'label' property is absent to keep the old
>> behaviour and fix this regression.
>>
>> Reported-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>> ---
>> Changes since v1:
>> - Consider ACPI case as suggested by Grant
>>
>>  drivers/leds/leds-gpio.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
>> index fd53968..8a8ba11 100644
>> --- a/drivers/leds/leds-gpio.c
>> +++ b/drivers/leds/leds-gpio.c
>> @@ -170,6 +170,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>>         struct fwnode_handle *child;
>>         struct gpio_leds_priv *priv;
>>         int count, ret;
>> +       struct device_node *np;
>>
>>         count = device_get_child_node_count(dev);
>>         if (!count)
>> @@ -189,7 +190,16 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>>                         goto err;
>>                 }
>>
>> -               fwnode_property_read_string(child, "label", &led.name);
>> +               np = of_node(child);
>> +
>> +               if (fwnode_property_present(child, "label")) {
>> +                       fwnode_property_read_string(child, "label", &led.name);
>> +               } else {
>> +                       if (IS_ENABLED(CONFIG_OF) && !led.name && np)
>> +                               led.name = np->name;
>> +                       if (!led.name)
>> +                               return ERR_PTR(-EINVAL);
>> +               }
>>                 fwnode_property_read_string(child, "linux,default-trigger",
>
> Or maybe we should not expose the CONFIG_OF versus CONFIG_ACPI
> ifdefery to the driver and do something like this instead:
>

V2 is better than V1 to me. I'd like to see some comments from Grant
and leds-gpio.c part looks good to me.

> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index c458458..bdeee26 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -399,6 +399,27 @@ struct fwnode_handle
> *device_get_next_child_node(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(device_get_next_child_node);
>
> +const char *device_get_node_name(struct device *dev, struct
> fwnode_handle *child)
> +{
> +
> +    if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> +        struct device_node *node = of_node(child);
> +
> +        if (node)
> +            return node->name;
> +    }
> +
> +    if (IS_ENABLED(CONFIG_ACPI)) {
> +        struct acpi_device *node = acpi_node(child);
> +
> +        if (node)
> +            return acpi_dev_name(node);
> +    }
> +
> +    return NULL;
> +}
> +EXPORT_SYMBOL_GPL(device_get_node_name);
> +
>  /**
>   * fwnode_handle_put - Drop reference to a device node
>   * @fwnode: Pointer to the device node to drop the reference to.
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index fd53968..c1c472b 100644
>
> diff --git a/include/linux/property.h b/include/linux/property.h
> index a6a3d98..9bec811 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -72,6 +72,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
>  struct fwnode_handle *device_get_next_child_node(struct device *dev,
>                           struct fwnode_handle *child);
>
> +const char *device_get_node_name(struct device *dev, struct
> fwnode_handle *child);
> +
>  #define device_for_each_child_node(dev, child) \
>      for (child = device_get_next_child_node(dev, NULL); child; \
>           child = device_get_next_child_node(dev, child))
>
> , and then on leds-gpio.c we can simply do:
>
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -189,7 +189,10 @@ static struct gpio_leds_priv
> *gpio_leds_create(struct platform_device *pdev)
>              goto err;
>          }
>
> -        fwnode_property_read_string(child, "label", &led.name);
> +        if (fwnode_property_present(child, "label"))
> +            fwnode_property_read_string(child, "label", &led.name);
> +        else
> +            led.name = device_get_node_name(dev, child);
>          fwnode_property_read_string(child, "linux,default-trigger",
>                          &led.default_trigger);
>
> I am not familiar with ACPI so I don't know if the 'return
> acpi_dev_name(node)' is the appropriate way to retrieve the equivalent
> of a dt node name.
>
> Please advise.

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-03 19:59 ` Fabio Estevam
  2014-12-03 22:23   ` Bryan Wu
@ 2014-12-03 23:16   ` Grant Likely
  1 sibling, 0 replies; 12+ messages in thread
From: Grant Likely @ 2014-12-03 23:16 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Bryan Wu, Richard Purdie, Jean-Michel Hautbois, Rafael J. Wysocki,
	Mika Westerberg, Linux LED Subsystem, Fabio Estevam

On Wed, Dec 3, 2014 at 7:59 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 3:57 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Since commit a43f2cbbb009f96 ("leds: leds-gpio: Make use of device property
>> API") it is no longer possible to register multiple gpio leds without passing
>> the 'label' property.
>>
>> According to Documentation/devicetree/bindings/leds/common.txt:
>>
>> "Optional properties for child nodes:
>> - label : The label for this LED.  If omitted, the label is
>>   taken from the node name (excluding the unit address)."
>>
>> So retrieve the node name when the 'label' property is absent to keep the old
>> behaviour and fix this regression.
>>
>> Reported-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>> ---
>> Changes since v1:
>> - Consider ACPI case as suggested by Grant
>>
>>  drivers/leds/leds-gpio.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
>> index fd53968..8a8ba11 100644
>> --- a/drivers/leds/leds-gpio.c
>> +++ b/drivers/leds/leds-gpio.c
>> @@ -170,6 +170,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>>         struct fwnode_handle *child;
>>         struct gpio_leds_priv *priv;
>>         int count, ret;
>> +       struct device_node *np;
>>
>>         count = device_get_child_node_count(dev);
>>         if (!count)
>> @@ -189,7 +190,16 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>>                         goto err;
>>                 }
>>
>> -               fwnode_property_read_string(child, "label", &led.name);
>> +               np = of_node(child);
>> +
>> +               if (fwnode_property_present(child, "label")) {
>> +                       fwnode_property_read_string(child, "label", &led.name);
>> +               } else {
>> +                       if (IS_ENABLED(CONFIG_OF) && !led.name && np)
>> +                               led.name = np->name;
>> +                       if (!led.name)
>> +                               return ERR_PTR(-EINVAL);
>> +               }
>>                 fwnode_property_read_string(child, "linux,default-trigger",
>
> Or maybe we should not expose the CONFIG_OF versus CONFIG_ACPI
> ifdefery to the driver and do something like this instead:
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index c458458..bdeee26 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -399,6 +399,27 @@ struct fwnode_handle
> *device_get_next_child_node(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(device_get_next_child_node);
>
> +const char *device_get_node_name(struct device *dev, struct
> fwnode_handle *child)
> +{
> +
> +    if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
> +        struct device_node *node = of_node(child);
> +
> +        if (node)
> +            return node->name;
> +    }
> +
> +    if (IS_ENABLED(CONFIG_ACPI)) {
> +        struct acpi_device *node = acpi_node(child);
> +
> +        if (node)
> +            return acpi_dev_name(node);
> +    }
> +
> +    return NULL;
> +}
> +EXPORT_SYMBOL_GPL(device_get_node_name);
> +

Instead of a device_get_* variant, for getting the node, I would
create a fwnode_get_name() helper as it would be usable in a larger
variety of situations. Besides, you've already got the 'child' value
to pass into it.

>  /**
>   * fwnode_handle_put - Drop reference to a device node
>   * @fwnode: Pointer to the device node to drop the reference to.
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index fd53968..c1c472b 100644
>
> diff --git a/include/linux/property.h b/include/linux/property.h
> index a6a3d98..9bec811 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -72,6 +72,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
>  struct fwnode_handle *device_get_next_child_node(struct device *dev,
>                           struct fwnode_handle *child);
>
> +const char *device_get_node_name(struct device *dev, struct
> fwnode_handle *child);
> +
>  #define device_for_each_child_node(dev, child) \
>      for (child = device_get_next_child_node(dev, NULL); child; \
>           child = device_get_next_child_node(dev, child))
>
> , and then on leds-gpio.c we can simply do:
>
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -189,7 +189,10 @@ static struct gpio_leds_priv
> *gpio_leds_create(struct platform_device *pdev)
>              goto err;
>          }
>
> -        fwnode_property_read_string(child, "label", &led.name);
> +        if (fwnode_property_present(child, "label"))
> +            fwnode_property_read_string(child, "label", &led.name);
> +        else
> +            led.name = device_get_node_name(dev, child);
>          fwnode_property_read_string(child, "linux,default-trigger",
>                          &led.default_trigger);
>
> I am not familiar with ACPI so I don't know if the 'return
> acpi_dev_name(node)' is the appropriate way to retrieve the equivalent
> of a dt node name.

I cannot give good guidance here. I'll leave it to the ACPI folks.

g.

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-03 17:57 [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent Fabio Estevam
  2014-12-03 19:59 ` Fabio Estevam
@ 2014-12-03 23:17 ` Grant Likely
  2014-12-03 23:28   ` Fabio Estevam
  1 sibling, 1 reply; 12+ messages in thread
From: Grant Likely @ 2014-12-03 23:17 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Bryan Wu, Richard Purdie, Jean-Michel Hautbois, Rafael J. Wysocki,
	Mika Westerberg, Linux LED Subsystem, Fabio Estevam

On Wed, Dec 3, 2014 at 5:57 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Since commit a43f2cbbb009f96 ("leds: leds-gpio: Make use of device property
> API") it is no longer possible to register multiple gpio leds without passing
> the 'label' property.
>
> According to Documentation/devicetree/bindings/leds/common.txt:
>
> "Optional properties for child nodes:
> - label : The label for this LED.  If omitted, the label is
>   taken from the node name (excluding the unit address)."
>
> So retrieve the node name when the 'label' property is absent to keep the old
> behaviour and fix this regression.
>
> Reported-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Acked-by: Grant Likely <grant.likely@linaro.org>

(Assuming that creating a fwnode_get_name() function turns out to be a
non-starter.)

g.

> ---
> Changes since v1:
> - Consider ACPI case as suggested by Grant
>
>  drivers/leds/leds-gpio.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
> index fd53968..8a8ba11 100644
> --- a/drivers/leds/leds-gpio.c
> +++ b/drivers/leds/leds-gpio.c
> @@ -170,6 +170,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>         struct fwnode_handle *child;
>         struct gpio_leds_priv *priv;
>         int count, ret;
> +       struct device_node *np;
>
>         count = device_get_child_node_count(dev);
>         if (!count)
> @@ -189,7 +190,16 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
>                         goto err;
>                 }
>
> -               fwnode_property_read_string(child, "label", &led.name);
> +               np = of_node(child);
> +
> +               if (fwnode_property_present(child, "label")) {
> +                       fwnode_property_read_string(child, "label", &led.name);
> +               } else {
> +                       if (IS_ENABLED(CONFIG_OF) && !led.name && np)
> +                               led.name = np->name;
> +                       if (!led.name)
> +                               return ERR_PTR(-EINVAL);
> +               }
>                 fwnode_property_read_string(child, "linux,default-trigger",
>                                             &led.default_trigger);
>
> --
> 1.9.1
>

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-03 23:17 ` Grant Likely
@ 2014-12-03 23:28   ` Fabio Estevam
  2014-12-03 23:59     ` Bryan Wu
  0 siblings, 1 reply; 12+ messages in thread
From: Fabio Estevam @ 2014-12-03 23:28 UTC (permalink / raw)
  To: Grant Likely
  Cc: Bryan Wu, Richard Purdie, Jean-Michel Hautbois, Rafael J. Wysocki,
	Mika Westerberg, Linux LED Subsystem, Fabio Estevam

On Wed, Dec 3, 2014 at 9:17 PM, Grant Likely <grant.likely@linaro.org> wrote:
> On Wed, Dec 3, 2014 at 5:57 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Since commit a43f2cbbb009f96 ("leds: leds-gpio: Make use of device property
>> API") it is no longer possible to register multiple gpio leds without passing
>> the 'label' property.
>>
>> According to Documentation/devicetree/bindings/leds/common.txt:
>>
>> "Optional properties for child nodes:
>> - label : The label for this LED.  If omitted, the label is
>>   taken from the node name (excluding the unit address)."
>>
>> So retrieve the node name when the 'label' property is absent to keep the old
>> behaviour and fix this regression.
>>
>> Reported-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>
> Acked-by: Grant Likely <grant.likely@linaro.org>
>
> (Assuming that creating a fwnode_get_name() function turns out to be a
> non-starter.)

Thanks, Grant.

Maybe we should go with this v2 patch initially to fix the regression
and then we could consider introducing fwnode_get_name() in a future
patch.

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-03 23:28   ` Fabio Estevam
@ 2014-12-03 23:59     ` Bryan Wu
  2014-12-04  0:12       ` Fabio Estevam
  0 siblings, 1 reply; 12+ messages in thread
From: Bryan Wu @ 2014-12-03 23:59 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Grant Likely, Richard Purdie, Jean-Michel Hautbois,
	Rafael J. Wysocki, Mika Westerberg, Linux LED Subsystem,
	Fabio Estevam

On Wed, Dec 3, 2014 at 3:28 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 9:17 PM, Grant Likely <grant.likely@linaro.org> wrote:
>> On Wed, Dec 3, 2014 at 5:57 PM, Fabio Estevam <festevam@gmail.com> wrote:
>>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>>
>>> Since commit a43f2cbbb009f96 ("leds: leds-gpio: Make use of device property
>>> API") it is no longer possible to register multiple gpio leds without passing
>>> the 'label' property.
>>>
>>> According to Documentation/devicetree/bindings/leds/common.txt:
>>>
>>> "Optional properties for child nodes:
>>> - label : The label for this LED.  If omitted, the label is
>>>   taken from the node name (excluding the unit address)."
>>>
>>> So retrieve the node name when the 'label' property is absent to keep the old
>>> behaviour and fix this regression.
>>>
>>> Reported-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
>>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>>
>> (Assuming that creating a fwnode_get_name() function turns out to be a
>> non-starter.)
>
> Thanks, Grant.
>
> Maybe we should go with this v2 patch initially to fix the regression
> and then we could consider introducing fwnode_get_name() in a future
> patch.

I think V1 just touches leds-gpio.c and might be easier to merge as a
good fix. And then you can provide a patch to introduce
fwnode_get_name().

Or you update your V2 patch to use fwnode_get_name() and try to merge
it as a fix.

-Bryan

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-03 23:59     ` Bryan Wu
@ 2014-12-04  0:12       ` Fabio Estevam
  2014-12-04  0:48         ` Bryan Wu
  0 siblings, 1 reply; 12+ messages in thread
From: Fabio Estevam @ 2014-12-04  0:12 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Grant Likely, Richard Purdie, Jean-Michel Hautbois,
	Rafael J. Wysocki, Mika Westerberg, Linux LED Subsystem,
	Fabio Estevam

Hi Bryan,

On Wed, Dec 3, 2014 at 9:59 PM, Bryan Wu <cooloney@gmail.com> wrote:

>> Maybe we should go with this v2 patch initially to fix the regression
>> and then we could consider introducing fwnode_get_name() in a future
>> patch.
>
> I think V1 just touches leds-gpio.c and might be easier to merge as a
> good fix. And then you can provide a patch to introduce
> fwnode_get_name().

Yes, I agree.

Just to make sure we are on the same page: there were two formal
patches that I submitted:

- v1: which did not take ACPI into account as pointed out by Grant
- v2: The original one of this thread that does take ACPI into account

Both of them only touch leds-gpio.c.

The one that touches drivers/base/property.c was just a suggestion
that I sent as a reply to v2. It was not a formal submission. I also
don't know if such suggestion would work for ACPI, as ACPI is
something I am not familiar with.

It seems to me that you are calling my [PATCH v2] as v1 and the
suggested approach that touches drivers/base/property.c as v2, so
that's the confusion ;-). Sorry about that.

Please let me know.

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-04  0:12       ` Fabio Estevam
@ 2014-12-04  0:48         ` Bryan Wu
  2014-12-04  0:50           ` Fabio Estevam
  0 siblings, 1 reply; 12+ messages in thread
From: Bryan Wu @ 2014-12-04  0:48 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Grant Likely, Richard Purdie, Jean-Michel Hautbois,
	Rafael J. Wysocki, Mika Westerberg, Linux LED Subsystem,
	Fabio Estevam

On Wed, Dec 3, 2014 at 4:12 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Bryan,
>
> On Wed, Dec 3, 2014 at 9:59 PM, Bryan Wu <cooloney@gmail.com> wrote:
>
>>> Maybe we should go with this v2 patch initially to fix the regression
>>> and then we could consider introducing fwnode_get_name() in a future
>>> patch.
>>
>> I think V1 just touches leds-gpio.c and might be easier to merge as a
>> good fix. And then you can provide a patch to introduce
>> fwnode_get_name().
>
> Yes, I agree.
>
> Just to make sure we are on the same page: there were two formal
> patches that I submitted:
>
> - v1: which did not take ACPI into account as pointed out by Grant
> - v2: The original one of this thread that does take ACPI into account
>
> Both of them only touch leds-gpio.c.
>
> The one that touches drivers/base/property.c was just a suggestion
> that I sent as a reply to v2. It was not a formal submission. I also
> don't know if such suggestion would work for ACPI, as ACPI is
> something I am not familiar with.
>
> It seems to me that you are calling my [PATCH v2] as v1 and the
> suggested approach that touches drivers/base/property.c as v2, so
> that's the confusion ;-). Sorry about that.
>
> Please let me know.

I got it. Actually we are talking about the same patch in the email [PATCH v2].
So I agree to merge this fix firstly and then introduce a new fwnode_get_name().

If it's OK, I will merge this patch with Grant's Ack.

Thanks,
-Bryan

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-04  0:48         ` Bryan Wu
@ 2014-12-04  0:50           ` Fabio Estevam
  2014-12-04  1:00             ` Bryan Wu
  0 siblings, 1 reply; 12+ messages in thread
From: Fabio Estevam @ 2014-12-04  0:50 UTC (permalink / raw)
  To: Bryan Wu
  Cc: Grant Likely, Richard Purdie, Jean-Michel Hautbois,
	Rafael J. Wysocki, Mika Westerberg, Linux LED Subsystem,
	Fabio Estevam

On Wed, Dec 3, 2014 at 10:48 PM, Bryan Wu <cooloney@gmail.com> wrote:

> I got it. Actually we are talking about the same patch in the email [PATCH v2].
> So I agree to merge this fix firstly and then introduce a new fwnode_get_name().
>
> If it's OK, I will merge this patch with Grant's Ack.

Excellent, thanks!

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-04  0:50           ` Fabio Estevam
@ 2014-12-04  1:00             ` Bryan Wu
  2014-12-04  1:29               ` Rafael J. Wysocki
  0 siblings, 1 reply; 12+ messages in thread
From: Bryan Wu @ 2014-12-04  1:00 UTC (permalink / raw)
  To: Fabio Estevam, Rafael J. Wysocki
  Cc: Grant Likely, Richard Purdie, Jean-Michel Hautbois,
	Mika Westerberg, Linux LED Subsystem, Fabio Estevam

On Wed, Dec 3, 2014 at 4:50 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 10:48 PM, Bryan Wu <cooloney@gmail.com> wrote:
>
>> I got it. Actually we are talking about the same patch in the email [PATCH v2].
>> So I agree to merge this fix firstly and then introduce a new fwnode_get_name().
>>
>> If it's OK, I will merge this patch with Grant's Ack.
>
> Excellent, thanks!

Oh, I just found PATCH "leds: leds-gpio: Make use of device property
API" is not in my tree. So probably Rafael should take it with my Ack
as well:

Acked-by: Bryan Wu <cooloney@gmail.com>

-Bryan

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

* Re: [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent
  2014-12-04  1:00             ` Bryan Wu
@ 2014-12-04  1:29               ` Rafael J. Wysocki
  0 siblings, 0 replies; 12+ messages in thread
From: Rafael J. Wysocki @ 2014-12-04  1:29 UTC (permalink / raw)
  To: Bryan Wu, Fabio Estevam
  Cc: Grant Likely, Richard Purdie, Jean-Michel Hautbois,
	Mika Westerberg, Linux LED Subsystem, Fabio Estevam

On 12/4/2014 2:00 AM, Bryan Wu wrote:
> On Wed, Dec 3, 2014 at 4:50 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> On Wed, Dec 3, 2014 at 10:48 PM, Bryan Wu <cooloney@gmail.com> wrote:
>>
>>> I got it. Actually we are talking about the same patch in the email [PATCH v2].
>>> So I agree to merge this fix firstly and then introduce a new fwnode_get_name().
>>>
>>> If it's OK, I will merge this patch with Grant's Ack.
>> Excellent, thanks!
> Oh, I just found PATCH "leds: leds-gpio: Make use of device property
> API" is not in my tree. So probably Rafael should take it with my Ack
> as well:
>
> Acked-by: Bryan Wu <cooloney@gmail.com>

Applied, thanks!

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

end of thread, other threads:[~2014-12-04  1:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-03 17:57 [PATCH v2] leds: leds-gpio: Fix multiple instances registration when 'label' property is absent Fabio Estevam
2014-12-03 19:59 ` Fabio Estevam
2014-12-03 22:23   ` Bryan Wu
2014-12-03 23:16   ` Grant Likely
2014-12-03 23:17 ` Grant Likely
2014-12-03 23:28   ` Fabio Estevam
2014-12-03 23:59     ` Bryan Wu
2014-12-04  0:12       ` Fabio Estevam
2014-12-04  0:48         ` Bryan Wu
2014-12-04  0:50           ` Fabio Estevam
2014-12-04  1:00             ` Bryan Wu
2014-12-04  1:29               ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).