diff mbox series

[v3,03/11] leds: led-class: Add __of_led_get() helper

Message ID 20221216113013.126881-4-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series leds: lookup-table support + int3472/media privacy LED support | expand

Commit Message

Hans de Goede Dec. 16, 2022, 11:30 a.m. UTC
Turn of_led_get() into a more generic __of_led_get() helper function,
which can lookup LEDs in devicetree by either name or index.

This is a preparation patch for adding a generic (non devicetree specific)
led_get() function.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/leds/led-class.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

Comments

Andy Shevchenko Dec. 16, 2022, 1:50 p.m. UTC | #1
On Fri, Dec 16, 2022 at 12:30:05PM +0100, Hans de Goede wrote:
> Turn of_led_get() into a more generic __of_led_get() helper function,
> which can lookup LEDs in devicetree by either name or index.

...

> +	/*
> +	 * For named LEDs, first look up the name in the "leds-names" property.
> +	 * If it cannot be found, then of_parse_phandle() will propagate the error.
> +	 */
> +	if (name)
> +		index = of_property_match_string(np, "leds-names", name);

I can't find this property anywhere in the kernel. Is it new?
If so, where is the bindings? And why entire code can't be converted
to use fwnode for this case?
Hans de Goede Dec. 16, 2022, 3:52 p.m. UTC | #2
Hi,

On 12/16/22 14:50, Andy Shevchenko wrote:
> On Fri, Dec 16, 2022 at 12:30:05PM +0100, Hans de Goede wrote:
>> Turn of_led_get() into a more generic __of_led_get() helper function,
>> which can lookup LEDs in devicetree by either name or index.
> 
> ...
> 
>> +	/*
>> +	 * For named LEDs, first look up the name in the "leds-names" property.
>> +	 * If it cannot be found, then of_parse_phandle() will propagate the error.
>> +	 */
>> +	if (name)
>> +		index = of_property_match_string(np, "leds-names", name);
> 
> I can't find this property anywhere in the kernel. Is it new?

Yes and no, adding a foo-names property for foo[] arrays to be
able to get members by name is a standard template for devicetree
bindings. See e.g. the clock bindings:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml

> If so, where is the bindings?

As for why not document this, there are currently no devicetree users
and the devicetree maintainers have repeatedly let me know not to
submit new bindings for fwnode x86 bindings ...

> And why entire code can't be converted
> to use fwnode for this case?

This is a trivial change to allow the new functions to work
with devicetree. Note this series does not introduce any devicetree
users, hence no bindings. But it is good to have compatibility backed in
from day 1.

Converting to fwnode APIs would be more involved and I cannot test
those changes.

Regards,

Hans
Andy Shevchenko Dec. 16, 2022, 4:05 p.m. UTC | #3
On Fri, Dec 16, 2022 at 04:52:33PM +0100, Hans de Goede wrote:
> On 12/16/22 14:50, Andy Shevchenko wrote:
> > On Fri, Dec 16, 2022 at 12:30:05PM +0100, Hans de Goede wrote:

...

> >> +	/*
> >> +	 * For named LEDs, first look up the name in the "leds-names" property.
> >> +	 * If it cannot be found, then of_parse_phandle() will propagate the error.
> >> +	 */
> >> +	if (name)
> >> +		index = of_property_match_string(np, "leds-names", name);
> > 
> > I can't find this property anywhere in the kernel. Is it new?
> 
> Yes and no, adding a foo-names property for foo[] arrays to be
> able to get members by name is a standard template for devicetree
> bindings. See e.g. the clock bindings:
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml
> 
> > If so, where is the bindings?
> 
> As for why not document this, there are currently no devicetree users
> and the devicetree maintainers have repeatedly let me know not to
> submit new bindings for fwnode x86 bindings ...

How above is related to fwnode as you put that? (I do see OF specific code
which is required to have a binding, right?)

> > And why entire code can't be converted
> > to use fwnode for this case?
> 
> This is a trivial change to allow the new functions to work
> with devicetree. Note this series does not introduce any devicetree
> users, hence no bindings. But it is good to have compatibility backed in
> from day 1.

AFAIU the OF properties must be documented from day 1.

> Converting to fwnode APIs would be more involved and I cannot test
> those changes.
diff mbox series

Patch

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 5f40110bd260..1dd421ca56c1 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -232,19 +232,18 @@  static struct led_classdev *__led_get(struct device *led_dev)
 	return led_cdev;
 }
 
-/**
- * of_led_get() - request a LED device via the LED framework
- * @np: device node to get the LED device from
- * @index: the index of the LED
- *
- * Returns the LED device parsed from the phandle specified in the "leds"
- * property of a device tree node or a negative error-code on failure.
- */
-struct led_classdev *of_led_get(struct device_node *np, int index)
+static struct led_classdev *__of_led_get(struct device_node *np, int index,
+					 const char *name)
 {
 	struct device *led_dev;
 	struct device_node *led_node;
 
+	/*
+	 * For named LEDs, first look up the name in the "leds-names" property.
+	 * If it cannot be found, then of_parse_phandle() will propagate the error.
+	 */
+	if (name)
+		index = of_property_match_string(np, "leds-names", name);
 	led_node = of_parse_phandle(np, "leds", index);
 	if (!led_node)
 		return ERR_PTR(-ENOENT);
@@ -254,6 +253,19 @@  struct led_classdev *of_led_get(struct device_node *np, int index)
 
 	return __led_get(led_dev);
 }
+
+/**
+ * of_led_get() - request a LED device via the LED framework
+ * @np: device node to get the LED device from
+ * @index: the index of the LED
+ *
+ * Returns the LED device parsed from the phandle specified in the "leds"
+ * property of a device tree node or a negative error-code on failure.
+ */
+struct led_classdev *of_led_get(struct device_node *np, int index)
+{
+	return __of_led_get(np, index, NULL);
+}
 EXPORT_SYMBOL_GPL(of_led_get);
 
 /**