Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API
@ 2019-07-08 10:06 Jean-Jacques Hiblot
  2019-07-08 10:06 ` Jean-Jacques Hiblot
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-08 10:06 UTC (permalink / raw)
  To: jacek.anaszewski, pavel, dmurphy
  Cc: linux-leds, linux-kernel, tomi.valkeinen, Jean-Jacques Hiblot

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

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)

Jean-Jacques Hiblot (2):
  leds: tlc591xx: simplify driver by using the managed led API
  leds: tlc591xx: Use the OF version of the LED registration function

 drivers/leds/leds-tlc591xx.c | 79 +++++++++---------------------------
 1 file changed, 20 insertions(+), 59 deletions(-)

-- 
2.17.1

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

* [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API
  2019-07-08 10:06 [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Jean-Jacques Hiblot
@ 2019-07-08 10:06 ` Jean-Jacques Hiblot
  2019-07-08 10:06 ` [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API Jean-Jacques Hiblot
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-08 10:06 UTC (permalink / raw)
  To: jacek.anaszewski, pavel, dmurphy
  Cc: linux-leds, linux-kernel, tomi.valkeinen, Jean-Jacques Hiblot

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

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)

Jean-Jacques Hiblot (2):
  leds: tlc591xx: simplify driver by using the managed led API
  leds: tlc591xx: Use the OF version of the LED registration function

 drivers/leds/leds-tlc591xx.c | 79 +++++++++---------------------------
 1 file changed, 20 insertions(+), 59 deletions(-)

-- 
2.17.1


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

* [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API
  2019-07-08 10:06 [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Jean-Jacques Hiblot
  2019-07-08 10:06 ` Jean-Jacques Hiblot
@ 2019-07-08 10:06 ` Jean-Jacques Hiblot
  2019-07-08 10:06   ` Jean-Jacques Hiblot
  2019-07-08 18:08   ` Dan Murphy
  2019-07-08 10:06 ` [PATCH v2 2/2] leds: tlc591xx: Use the OF version of the LED registration function Jean-Jacques Hiblot
  2019-07-31 13:14 ` [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Tomi Valkeinen
  3 siblings, 2 replies; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-08 10:06 UTC (permalink / raw)
  To: jacek.anaszewski, pavel, dmurphy
  Cc: linux-leds, linux-kernel, tomi.valkeinen, Jean-Jacques Hiblot

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 <jjhiblot@ti.com>
---
 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

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

* [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API
  2019-07-08 10:06 ` [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API Jean-Jacques Hiblot
@ 2019-07-08 10:06   ` Jean-Jacques Hiblot
  2019-07-08 18:08   ` Dan Murphy
  1 sibling, 0 replies; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-08 10:06 UTC (permalink / raw)
  To: jacek.anaszewski, pavel, dmurphy
  Cc: linux-leds, linux-kernel, tomi.valkeinen, Jean-Jacques Hiblot

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 <jjhiblot@ti.com>
---
 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


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

* [PATCH v2 2/2] leds: tlc591xx: Use the OF version of the LED registration function
  2019-07-08 10:06 [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Jean-Jacques Hiblot
  2019-07-08 10:06 ` Jean-Jacques Hiblot
  2019-07-08 10:06 ` [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API Jean-Jacques Hiblot
@ 2019-07-08 10:06 ` Jean-Jacques Hiblot
  2019-07-08 10:06   ` Jean-Jacques Hiblot
  2019-07-31 13:14 ` [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Tomi Valkeinen
  3 siblings, 1 reply; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-08 10:06 UTC (permalink / raw)
  To: jacek.anaszewski, pavel, dmurphy
  Cc: linux-leds, linux-kernel, tomi.valkeinen, Jean-Jacques Hiblot

The driver parses the device-tree to identify which LED should be handled.
Since the information about the device node is known at this time, we can
provide the LED core with it. It may be useful later.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-tlc591xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-tlc591xx.c b/drivers/leds/leds-tlc591xx.c
index 3d5a4b92f016..10764a62cb71 100644
--- a/drivers/leds/leds-tlc591xx.c
+++ b/drivers/leds/leds-tlc591xx.c
@@ -207,7 +207,7 @@ tlc591xx_probe(struct i2c_client *client,
 		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);
+		err = devm_of_led_classdev_register(dev, child, &led->ldev);
 		if (err < 0) {
 			dev_err(dev, "couldn't register LED %s\n",
 				led->ldev.name);
-- 
2.17.1

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

* [PATCH v2 2/2] leds: tlc591xx: Use the OF version of the LED registration function
  2019-07-08 10:06 ` [PATCH v2 2/2] leds: tlc591xx: Use the OF version of the LED registration function Jean-Jacques Hiblot
@ 2019-07-08 10:06   ` Jean-Jacques Hiblot
  0 siblings, 0 replies; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-08 10:06 UTC (permalink / raw)
  To: jacek.anaszewski, pavel, dmurphy
  Cc: linux-leds, linux-kernel, tomi.valkeinen, Jean-Jacques Hiblot

The driver parses the device-tree to identify which LED should be handled.
Since the information about the device node is known at this time, we can
provide the LED core with it. It may be useful later.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/leds-tlc591xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-tlc591xx.c b/drivers/leds/leds-tlc591xx.c
index 3d5a4b92f016..10764a62cb71 100644
--- a/drivers/leds/leds-tlc591xx.c
+++ b/drivers/leds/leds-tlc591xx.c
@@ -207,7 +207,7 @@ tlc591xx_probe(struct i2c_client *client,
 		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);
+		err = devm_of_led_classdev_register(dev, child, &led->ldev);
 		if (err < 0) {
 			dev_err(dev, "couldn't register LED %s\n",
 				led->ldev.name);
-- 
2.17.1


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

* Re: [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API
  2019-07-08 10:06 ` [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API Jean-Jacques Hiblot
  2019-07-08 10:06   ` Jean-Jacques Hiblot
@ 2019-07-08 18:08   ` Dan Murphy
  2019-07-08 18:08     ` Dan Murphy
  2019-07-10  7:44     ` Jean-Jacques Hiblot
  1 sibling, 2 replies; 11+ messages in thread
From: Dan Murphy @ 2019-07-08 18:08 UTC (permalink / raw)
  To: Jean-Jacques Hiblot, jacek.anaszewski, pavel
  Cc: linux-leds, linux-kernel, tomi.valkeinen

JJ

On 7/8/19 5:06 AM, 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

What changed from v1?

I don't see any changes especially the bounds on the reg property.

Dan


> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>   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,
>   };
>   

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

* Re: [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API
  2019-07-08 18:08   ` Dan Murphy
@ 2019-07-08 18:08     ` Dan Murphy
  2019-07-10  7:44     ` Jean-Jacques Hiblot
  1 sibling, 0 replies; 11+ messages in thread
From: Dan Murphy @ 2019-07-08 18:08 UTC (permalink / raw)
  To: Jean-Jacques Hiblot, jacek.anaszewski, pavel
  Cc: linux-leds, linux-kernel, tomi.valkeinen

JJ

On 7/8/19 5:06 AM, 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

What changed from v1?

I don't see any changes especially the bounds on the reg property.

Dan


> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>   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,
>   };
>   

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

* Re: [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API
  2019-07-08 18:08   ` Dan Murphy
  2019-07-08 18:08     ` Dan Murphy
@ 2019-07-10  7:44     ` Jean-Jacques Hiblot
  2019-07-10  7:44       ` Jean-Jacques Hiblot
  1 sibling, 1 reply; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-10  7:44 UTC (permalink / raw)
  To: Dan Murphy, jacek.anaszewski, pavel
  Cc: linux-leds, linux-kernel, tomi.valkeinen

Hi Dan,

On 08/07/2019 20:08, Dan Murphy wrote:
> JJ
>
> On 7/8/19 5:06 AM, 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
>
> What changed from v1?

What changed is a bug fix in the loop: replaced led = &priv->leds[idx] 
with led = &priv->leds[reg];


>
> I don't see any changes especially the bounds on the reg property.

It is not quite apparent in the diff, but there is such a check. Here is 
what we have in the probe():

     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);
             return err;
         }
         if (reg < 0 || reg >= tlc591xx->max_leds ||
             priv->leds[reg].active) {
             of_node_put(child);
             return -EINVAL;
         }
         led = &priv->leds[reg];

[...]
         }
     }

>
> Dan
>
>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> ---
>>   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,
>>   };

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

* Re: [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API
  2019-07-10  7:44     ` Jean-Jacques Hiblot
@ 2019-07-10  7:44       ` Jean-Jacques Hiblot
  0 siblings, 0 replies; 11+ messages in thread
From: Jean-Jacques Hiblot @ 2019-07-10  7:44 UTC (permalink / raw)
  To: Dan Murphy, jacek.anaszewski, pavel
  Cc: linux-leds, linux-kernel, tomi.valkeinen

Hi Dan,

On 08/07/2019 20:08, Dan Murphy wrote:
> JJ
>
> On 7/8/19 5:06 AM, 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
>
> What changed from v1?

What changed is a bug fix in the loop: replaced led = &priv->leds[idx] 
with led = &priv->leds[reg];


>
> I don't see any changes especially the bounds on the reg property.

It is not quite apparent in the diff, but there is such a check. Here is 
what we have in the probe():

     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);
             return err;
         }
         if (reg < 0 || reg >= tlc591xx->max_leds ||
             priv->leds[reg].active) {
             of_node_put(child);
             return -EINVAL;
         }
         led = &priv->leds[reg];

[...]
         }
     }

>
> Dan
>
>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> ---
>>   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,
>>   };

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

* Re: [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API
  2019-07-08 10:06 [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Jean-Jacques Hiblot
                   ` (2 preceding siblings ...)
  2019-07-08 10:06 ` [PATCH v2 2/2] leds: tlc591xx: Use the OF version of the LED registration function Jean-Jacques Hiblot
@ 2019-07-31 13:14 ` Tomi Valkeinen
  3 siblings, 0 replies; 11+ messages in thread
From: Tomi Valkeinen @ 2019-07-31 13:14 UTC (permalink / raw)
  To: Jean-Jacques Hiblot, jacek.anaszewski, pavel, dmurphy
  Cc: linux-leds, linux-kernel

On 08/07/2019 13:06, Jean-Jacques Hiblot wrote:
> This mini-series updates the tlc591xx driver to use the managed API. The
> driver is also modified to pass the DT node to the LED core layer.
> The goal is to be able to the generic led-backlight [0] driver on top of
> it.
> 
> 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)
> 
> Jean-Jacques Hiblot (2):
>    leds: tlc591xx: simplify driver by using the managed led API
>    leds: tlc591xx: Use the OF version of the LED registration function
> 
>   drivers/leds/leds-tlc591xx.c | 79 +++++++++---------------------------
>   1 file changed, 20 insertions(+), 59 deletions(-)
> 

For the series:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-08 10:06 [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Jean-Jacques Hiblot
2019-07-08 10:06 ` Jean-Jacques Hiblot
2019-07-08 10:06 ` [PATCH v2 1/2] leds: tlc591xx: simplify driver by using the managed led API Jean-Jacques Hiblot
2019-07-08 10:06   ` Jean-Jacques Hiblot
2019-07-08 18:08   ` Dan Murphy
2019-07-08 18:08     ` Dan Murphy
2019-07-10  7:44     ` Jean-Jacques Hiblot
2019-07-10  7:44       ` Jean-Jacques Hiblot
2019-07-08 10:06 ` [PATCH v2 2/2] leds: tlc591xx: Use the OF version of the LED registration function Jean-Jacques Hiblot
2019-07-08 10:06   ` Jean-Jacques Hiblot
2019-07-31 13:14 ` [PATCH v2 0/2] leds: tlc591xx: switch to OF and managed API Tomi Valkeinen

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org linux-leds@archiver.kernel.org
	public-inbox-index linux-leds


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/ public-inbox