2019-09-18 19:57:58

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v3 0/2] leds: tlc591xx: switch to managed LED registration

This subject of this series used to be "leds: tlc591xx: switch to OF and
managed API"

This mini-series updates the tlc591xx driver to use the managed API. The
driver is also modified to pass the initialization data to the LED core
layer. The goal is to be able to the generic led-backlight [0] driver on
top of it.

changes in v3:
- rebased on top of linux-leds/for-next
- use devm_led_classdev_register_ext() instead of the late
devm_of_led_classdev_register()
- let the LED core assign the names of the LEDs

changes in v2:
- fixed LED indexing. Previous version did not allow for holes: if n LEDs
were used, they had to be led(0) to led(n-1)

[0]: https://patchwork.kernel.org/project/dri-devel/list/?series=175709

Jean-Jacques Hiblot (2):
leds: tlc591xx: simplify driver by using the managed led API
leds: tlc591xx: use devm_led_classdev_register_ext()

drivers/leds/leds-tlc591xx.c | 83 ++++++++++--------------------------
1 file changed, 23 insertions(+), 60 deletions(-)

--
2.17.1


2019-09-18 20:25:12

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: [PATCH v3 1/2] leds: tlc591xx: simplify driver by using the managed led API

Use the managed API of the LED class (devm_led_classdev_register()
instead of led_classdev_register()).
This allows us to remove the code used to track-and-destroy the LED devices

Signed-off-by: Jean-Jacques Hiblot <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
---
drivers/leds/leds-tlc591xx.c | 79 +++++++++---------------------------
1 file changed, 20 insertions(+), 59 deletions(-)

diff --git a/drivers/leds/leds-tlc591xx.c b/drivers/leds/leds-tlc591xx.c
index 59ff088c7d75..3d5a4b92f016 100644
--- a/drivers/leds/leds-tlc591xx.c
+++ b/drivers/leds/leds-tlc591xx.c
@@ -128,51 +128,6 @@ tlc591xx_brightness_set(struct led_classdev *led_cdev,
return err;
}

-static void
-tlc591xx_destroy_devices(struct tlc591xx_priv *priv, unsigned int j)
-{
- int i = j;
-
- while (--i >= 0) {
- if (priv->leds[i].active)
- led_classdev_unregister(&priv->leds[i].ldev);
- }
-}
-
-static int
-tlc591xx_configure(struct device *dev,
- struct tlc591xx_priv *priv,
- const struct tlc591xx *tlc591xx)
-{
- unsigned int i;
- int err = 0;
-
- tlc591xx_set_mode(priv->regmap, MODE2_DIM);
- for (i = 0; i < TLC591XX_MAX_LEDS; i++) {
- struct tlc591xx_led *led = &priv->leds[i];
-
- if (!led->active)
- continue;
-
- led->priv = priv;
- led->led_no = i;
- led->ldev.brightness_set_blocking = tlc591xx_brightness_set;
- led->ldev.max_brightness = LED_FULL;
- err = led_classdev_register(dev, &led->ldev);
- if (err < 0) {
- dev_err(dev, "couldn't register LED %s\n",
- led->ldev.name);
- goto exit;
- }
- }
-
- return 0;
-
-exit:
- tlc591xx_destroy_devices(priv, i);
- return err;
-}
-
static const struct regmap_config tlc591xx_regmap = {
.reg_bits = 8,
.val_bits = 8,
@@ -225,7 +180,11 @@ tlc591xx_probe(struct i2c_client *client,

i2c_set_clientdata(client, priv);

+ tlc591xx_set_mode(priv->regmap, MODE2_DIM);
+
for_each_child_of_node(np, child) {
+ struct tlc591xx_led *led;
+
err = of_property_read_u32(child, "reg", &reg);
if (err) {
of_node_put(child);
@@ -236,22 +195,25 @@ tlc591xx_probe(struct i2c_client *client,
of_node_put(child);
return -EINVAL;
}
- priv->leds[reg].active = true;
- priv->leds[reg].ldev.name =
+ led = &priv->leds[reg];
+
+ led->active = true;
+ led->ldev.name =
of_get_property(child, "label", NULL) ? : child->name;
- priv->leds[reg].ldev.default_trigger =
+ led->ldev.default_trigger =
of_get_property(child, "linux,default-trigger", NULL);
- }
- return tlc591xx_configure(dev, priv, tlc591xx);
-}
-
-static int
-tlc591xx_remove(struct i2c_client *client)
-{
- struct tlc591xx_priv *priv = i2c_get_clientdata(client);
-
- tlc591xx_destroy_devices(priv, TLC591XX_MAX_LEDS);

+ led->priv = priv;
+ led->led_no = reg;
+ led->ldev.brightness_set_blocking = tlc591xx_brightness_set;
+ led->ldev.max_brightness = LED_FULL;
+ err = devm_led_classdev_register(dev, &led->ldev);
+ if (err < 0) {
+ dev_err(dev, "couldn't register LED %s\n",
+ led->ldev.name);
+ return err;
+ }
+ }
return 0;
}

@@ -268,7 +230,6 @@ static struct i2c_driver tlc591xx_driver = {
.of_match_table = of_match_ptr(of_tlc591xx_leds_match),
},
.probe = tlc591xx_probe,
- .remove = tlc591xx_remove,
.id_table = tlc591xx_id,
};

--
2.17.1

2019-09-19 22:19:42

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] leds: tlc591xx: simplify driver by using the managed led API

Hi Jean,

Thank you for the updated set.

We will need one more iteration. Please refer below.

On 9/18/19 5:25 PM, Jean-Jacques Hiblot wrote:
> Use the managed API of the LED class (devm_led_classdev_register()
> instead of led_classdev_register()).
> This allows us to remove the code used to track-and-destroy the LED devices
>
> Signed-off-by: Jean-Jacques Hiblot <[email protected]>
> Reviewed-by: Tomi Valkeinen <[email protected]>
> ---
> drivers/leds/leds-tlc591xx.c | 79 +++++++++---------------------------
> 1 file changed, 20 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/leds/leds-tlc591xx.c b/drivers/leds/leds-tlc591xx.c
> index 59ff088c7d75..3d5a4b92f016 100644
> --- a/drivers/leds/leds-tlc591xx.c
> +++ b/drivers/leds/leds-tlc591xx.c
> @@ -128,51 +128,6 @@ tlc591xx_brightness_set(struct led_classdev *led_cdev,
> return err;
> }
>
[...]
> + led->ldev.max_brightness = LED_FULL;

This is redundant since initially zeroed by kzalloc and LED core sets it
to LED_FULL in this case, so we can remove it by this occasion.

We have also one fix to this driver, preceding this patch, so your set
will need a rebase onto for-5.5 branch [0].


> + err = devm_led_classdev_register(dev, &led->ldev);
> + if (err < 0) {
> + dev_err(dev, "couldn't register LED %s\n",
> + led->ldev.name);
> + return err;
> + }
> + }
> return 0;
> }
>
> @@ -268,7 +230,6 @@ static struct i2c_driver tlc591xx_driver = {
> .of_match_table = of_match_ptr(of_tlc591xx_leds_match),
> },
> .probe = tlc591xx_probe,
> - .remove = tlc591xx_remove,
> .id_table = tlc591xx_id,
> };
>
>

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git/log/?h=for-5.5

--
Best regards,
Jacek Anaszewski

2019-09-26 12:17:36

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] leds: tlc591xx: simplify driver by using the managed led API

On Wed 2019-09-18 17:25:55, Jean-Jacques Hiblot wrote:
> Use the managed API of the LED class (devm_led_classdev_register()
> instead of led_classdev_register()).
> This allows us to remove the code used to track-and-destroy the LED devices
>
> Signed-off-by: Jean-Jacques Hiblot <[email protected]>
> Reviewed-by: Tomi Valkeinen <[email protected]>

Acked-by: Pavel Machek <[email protected]>

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (542.00 B)
signature.asc (188.00 B)
Digital signature
Download all attachments