[PATCH 01/25] leds: class: Improve LED and LED flash class registration API

From: Jacek Anaszewski
Date: Sun Mar 10 2019 - 14:29:06 EST


Replace of_led_classdev_register() with led_classdev_register_ext(), which
accepts easily extendable struct led_init_data, instead of the fixed
struct device_node argument. The latter can be now passed in a fwnode
property of the struct led_init_data.

The modification is driven by the need for passing another argument
to the LED class initialization function - a LED class device name.
Currently it is conveyed in the "name" char pointer property of
struct led_classdev, which is semantically and functionally incorrect
since it is needed only on LED class device registration, but persists
in system memory throughout the whole LED class device lifetime.

Whereas in case of the name strings coming from DT "label" property or
statically initialized ones the cost is only the pointer size, it grows
to LED_MAX_NAME_SIZE (64) with the advent of a new LED class device naming
scheme, where color and function properties come from separate board
firmware properties and the name needs to be constructed in a new buffer.

The change will not break any existing clients since the patch alters
also existing led_classdev{_flash}_register() macro wrappers, that pass
NULL in place of init_data, which leads to using legacy name
initialization path basing on the struct led_classdev's "name" property.

Two existing users of devm_of_led_classdev_registers() are modified
to use devm_led_classdev_register(), which will not impact their
operation since they in fact didn't need to pass struct device_node on
registration from the beginning.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
Tested-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
Acked-by: Pavel Machek <pavel@xxxxxx>
Acked-by: Dan Murphy <dmurphy@xxxxxx>
Cc: Daniel Mack <daniel@xxxxxxxxxx>
Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
Cc: Oleh Kravchenko <oleg@xxxxxxxxxx>
Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
Cc: Simon Shields <simon@xxxxxxxxxxxxx>
---
drivers/leds/led-class-flash.c | 9 +++++----
drivers/leds/led-class.c | 34 ++++++++++++++++++++++------------
drivers/leds/leds-cr0014114.c | 3 +--
drivers/leds/leds-gpio.c | 2 +-
include/linux/led-class-flash.h | 15 ++++++++++-----
include/linux/leds.h | 37 +++++++++++++++++++++++++++----------
6 files changed, 66 insertions(+), 34 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index cf398275a53c..8d1c1177bb9a 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -285,8 +285,9 @@ static void led_flash_init_sysfs_groups(struct led_classdev_flash *fled_cdev)
led_cdev->groups = flash_groups;
}

-int led_classdev_flash_register(struct device *parent,
- struct led_classdev_flash *fled_cdev)
+int led_classdev_flash_register_ext(struct device *parent,
+ struct led_classdev_flash *fled_cdev,
+ struct led_init_data *init_data)
{
struct led_classdev *led_cdev;
const struct led_flash_ops *ops;
@@ -312,13 +313,13 @@ int led_classdev_flash_register(struct device *parent,
}

/* Register led class device */
- ret = led_classdev_register(parent, led_cdev);
+ ret = led_classdev_register_ext(parent, led_cdev, init_data);
if (ret < 0)
return ret;

return 0;
}
-EXPORT_SYMBOL_GPL(led_classdev_flash_register);
+EXPORT_SYMBOL_GPL(led_classdev_flash_register_ext);

void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev)
{
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 3c7e3487b373..a8337e0e02ea 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -17,6 +17,7 @@
#include <linux/leds.h>
#include <linux/list.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/timer.h>
@@ -244,18 +245,25 @@ static int led_classdev_next_name(const char *init_name, char *name,
}

/**
- * of_led_classdev_register - register a new object of led_classdev class.
+ * led_classdev_register_ext - register a new object of led_classdev class
+ * with init data.
*
* @parent: parent of LED device
* @led_cdev: the led_classdev structure for this device.
- * @np: DT node describing this LED
+ * @init_data: LED class device initialization data
*/
-int of_led_classdev_register(struct device *parent, struct device_node *np,
- struct led_classdev *led_cdev)
+int led_classdev_register_ext(struct device *parent,
+ struct led_classdev *led_cdev,
+ struct led_init_data *init_data)
{
char name[LED_MAX_NAME_SIZE];
int ret;

+ if (init_data && init_data->name)
+ led_cdev->name = init_data->name;
+ else
+ dev_info(parent, "init_data not available");
+
ret = led_classdev_next_name(led_cdev->name, name, sizeof(name));
if (ret < 0)
return ret;
@@ -268,7 +276,8 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,
mutex_unlock(&led_cdev->led_access);
return PTR_ERR(led_cdev->dev);
}
- led_cdev->dev->of_node = np;
+ if (init_data && init_data->fwnode)
+ led_cdev->dev->of_node = to_of_node(init_data->fwnode);

if (ret)
dev_warn(parent, "Led %s renamed to %s due to name collision",
@@ -313,7 +322,7 @@ int of_led_classdev_register(struct device *parent, struct device_node *np,

return 0;
}
-EXPORT_SYMBOL_GPL(of_led_classdev_register);
+EXPORT_SYMBOL_GPL(led_classdev_register_ext);

/**
* led_classdev_unregister - unregisters a object of led_properties class.
@@ -358,14 +367,15 @@ static void devm_led_classdev_release(struct device *dev, void *res)
}

/**
- * devm_of_led_classdev_register - resource managed led_classdev_register()
+ * devm_led_classdev_register_ext - resource managed led_classdev_register_ext()
*
* @parent: parent of LED device
* @led_cdev: the led_classdev structure for this device.
+ * @init_data: LED class device initialization data
*/
-int devm_of_led_classdev_register(struct device *parent,
- struct device_node *np,
- struct led_classdev *led_cdev)
+int devm_led_classdev_register_ext(struct device *parent,
+ struct led_classdev *led_cdev,
+ struct led_init_data *init_data)
{
struct led_classdev **dr;
int rc;
@@ -374,7 +384,7 @@ int devm_of_led_classdev_register(struct device *parent,
if (!dr)
return -ENOMEM;

- rc = of_led_classdev_register(parent, np, led_cdev);
+ rc = led_classdev_register_ext(parent, led_cdev, init_data);
if (rc) {
devres_free(dr);
return rc;
@@ -385,7 +395,7 @@ int devm_of_led_classdev_register(struct device *parent,

return 0;
}
-EXPORT_SYMBOL_GPL(devm_of_led_classdev_register);
+EXPORT_SYMBOL_GPL(devm_led_classdev_register_ext);

static int devm_led_classdev_match(struct device *dev, void *res, void *data)
{
diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c
index 0e4262462cb9..1c82152764d2 100644
--- a/drivers/leds/leds-cr0014114.c
+++ b/drivers/leds/leds-cr0014114.c
@@ -207,8 +207,7 @@ static int cr0014114_probe_dt(struct cr0014114 *priv)
led->ldev.max_brightness = CR_MAX_BRIGHTNESS;
led->ldev.brightness_set_blocking = cr0014114_set_sync;

- ret = devm_of_led_classdev_register(priv->dev, np,
- &led->ldev);
+ ret = devm_led_classdev_register(priv->dev, &led->ldev);
if (ret) {
dev_err(priv->dev,
"failed to register LED device %s, err %d",
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 998f2ff6914d..b26cf78993d1 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -112,7 +112,7 @@ static int create_gpio_led(const struct gpio_led *template,
if (ret < 0)
return ret;

- return devm_of_led_classdev_register(parent, np, &led_dat->cdev);
+ return devm_led_classdev_register(parent, &led_dat->cdev);
}

struct gpio_leds_priv {
diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
index 700efaa9e115..ded8d9fa6149 100644
--- a/include/linux/led-class-flash.h
+++ b/include/linux/led-class-flash.h
@@ -90,15 +90,20 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
}

/**
- * led_classdev_flash_register - register a new object of led_classdev class
- * with support for flash LEDs
- * @parent: the flash LED to register
+ * led_classdev_flash_register_ext - register a new object of LED class with
+ * init data and with support for flash LEDs
+ * @parent: LED flash controller device this flash LED is driven by
* @fled_cdev: the led_classdev_flash structure for this device
+ * @init_data: the LED class flash device initialization data
*
* Returns: 0 on success or negative error value on failure
*/
-extern int led_classdev_flash_register(struct device *parent,
- struct led_classdev_flash *fled_cdev);
+extern int led_classdev_flash_register_ext(struct device *parent,
+ struct led_classdev_flash *fled_cdev,
+ struct led_init_data *init_data);
+
+#define led_classdev_flash_register(parent, fled_cdev) \
+ led_classdev_flash_register_ext(parent, fled_cdev, NULL)

/**
* led_classdev_flash_unregister - unregisters an object of led_classdev class
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 5263f87e1d2c..bffb4315fd66 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -20,6 +20,7 @@
#include <linux/spinlock.h>
#include <linux/timer.h>
#include <linux/workqueue.h>
+#include <uapi/linux/uleds.h>

struct device;
struct led_pattern;
@@ -34,6 +35,13 @@ enum led_brightness {
LED_FULL = 255,
};

+struct led_init_data {
+ /* Device fwnode handle */
+ struct fwnode_handle *fwnode;
+ /* Requested LED class device name */
+ char name[LED_MAX_NAME_SIZE];
+};
+
struct led_classdev {
const char *name;
enum led_brightness brightness;
@@ -129,16 +137,25 @@ struct led_classdev {
struct mutex led_access;
};

-extern int of_led_classdev_register(struct device *parent,
- struct device_node *np,
- struct led_classdev *led_cdev);
-#define led_classdev_register(parent, led_cdev) \
- of_led_classdev_register(parent, NULL, led_cdev)
-extern int devm_of_led_classdev_register(struct device *parent,
- struct device_node *np,
- struct led_classdev *led_cdev);
-#define devm_led_classdev_register(parent, led_cdev) \
- devm_of_led_classdev_register(parent, NULL, led_cdev)
+/**
+ * led_classdev_register_ext - register a new object of LED class with
+ * init data
+ * @parent: LED controller device this LED is driven by
+ * @led_cdev: the led_classdev structure for this device
+ * @init_data: the LED class device initialization data
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_classdev_register_ext(struct device *parent,
+ struct led_classdev *led_cdev,
+ struct led_init_data *init_data);
+#define led_classdev_register(parent, led_cdev) \
+ led_classdev_register_ext(parent, led_cdev, NULL)
+extern int devm_led_classdev_register_ext(struct device *parent,
+ struct led_classdev *led_cdev,
+ struct led_init_data *init_data);
+#define devm_led_classdev_register(parent, led_cdev) \
+ devm_led_classdev_register_ext(parent, led_cdev, NULL)
extern void led_classdev_unregister(struct led_classdev *led_cdev);
extern void devm_led_classdev_unregister(struct device *parent,
struct led_classdev *led_cdev);
--
2.11.0