[PATCH v1 0/2] leds: lm3533: cleanups and support getting LED sources from DT

[PATCH v1 0/2] leds: lm3533: cleanups and support getting LED sources from DT

[Svyatoslav Ryhel]

Refactor probe and switch do managed helpers, add support for configuring
LED sources from DT.

This patchset was derived from
https://lore.kernel.org/lkml/20261112081941.41363-1-clamor95@gmail.com/

Svyatoslav Ryhel (2):
  leds: lm3533: Clean up probe and use managed helpers
  leds: lm3533: Support getting LED sources from DT

 drivers/leds/leds-lm3533.c | 105 +++++++++++++++++++++++++------------
 1 file changed, 71 insertions(+), 34 deletions(-)

-- 
2.53.0

[PATCH v1 1/2] leds: lm3533: Clean up probe and use managed helpers

[Svyatoslav Ryhel]

Clean up the lm3533 LED driver probe and remove routines by adopting
managed device helpers and modernizing error handling:

- Introduce a local `struct device *dev` variable to clean up repeated
  `&pdev->dev` references.
- Use `dev_err_probe()` for cleaner error handling on invalid IDs and
  LED class registration failures.
- Switch to `devm_led_classdev_register()` to automatically handle LED
  class device unregistration, removing the need for manual cleanup in
  error paths and the `remove` hook.
- Remove redundant `dev_dbg()` statements and cleanup unused cleanup
  labels.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/leds/leds-lm3533.c | 48 +++++++++++---------------------------
 1 file changed, 14 insertions(+), 34 deletions(-)

diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index d5997cbd88dfc..7db1105bee5c2 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -633,22 +633,19 @@ static int lm3533_led_setup(struct lm3533_led *led)
 
 static int lm3533_led_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct lm3533 *lm3533;
 	struct lm3533_led *led;
 	int ret;
 
-	dev_dbg(&pdev->dev, "%s\n", __func__);
-
-	lm3533 = dev_get_drvdata(pdev->dev.parent);
+	lm3533 = dev_get_drvdata(dev->parent);
 	if (!lm3533)
 		return -EINVAL;
 
-	if (pdev->id < 0 || pdev->id >= LM3533_LVCTRLBANK_COUNT) {
-		dev_err(&pdev->dev, "illegal LED id %d\n", pdev->id);
-		return -EINVAL;
-	}
+	if (pdev->id < 0 || pdev->id >= LM3533_LVCTRLBANK_COUNT)
+		return dev_err_probe(dev, -EINVAL, "Illegal LED id %d\n", pdev->id);
 
-	led = devm_kzalloc_obj(&pdev->dev, *led);
+	led = devm_kzalloc_obj(dev, *led);
 	if (!led)
 		return -ENOMEM;
 
@@ -662,14 +659,12 @@ static int lm3533_led_probe(struct platform_device *pdev)
 	led->cdev.groups = lm3533_led_attribute_groups;
 	led->id = pdev->id;
 
-	led->cdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
-					pdev->name, led->id);
+	led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s-%d", pdev->name, led->id);
 	if (!led->cdev.name)
 		return -ENOMEM;
 
 	led->cdev.default_trigger = "none";
-	device_property_read_string(&pdev->dev, "linux,default-trigger",
-				    &led->cdev.default_trigger);
+	device_property_read_string(dev, "linux,default-trigger", &led->cdev.default_trigger);
 
 	mutex_init(&led->mutex);
 
@@ -683,45 +678,30 @@ static int lm3533_led_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, led);
 
-	ret = led_classdev_register(&pdev->dev, &led->cdev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to register LED %d\n", pdev->id);
-		return ret;
-	}
+	ret = devm_led_classdev_register(dev, &led->cdev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register LED %d\n", pdev->id);
 
 	led->cb.dev = led->cdev.dev;
 
-	device_property_read_u32(&pdev->dev, "led-max-microamp",
-				 &led->max_current);
+	device_property_read_u32(dev, "led-max-microamp", &led->max_current);
 	led->max_current = clamp(led->max_current, LM3533_MAX_CURRENT_MIN,
 				 LM3533_MAX_CURRENT_MAX);
 
-	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
+	device_property_read_u32(dev, "ti,pwm-config-mask", &led->pwm);
 
 	ret = lm3533_led_setup(led);
 	if (ret)
-		goto err_deregister;
-
-	ret = lm3533_ctrlbank_enable(&led->cb);
-	if (ret)
-		goto err_deregister;
-
-	return 0;
-
-err_deregister:
-	led_classdev_unregister(&led->cdev);
+		return ret;
 
-	return ret;
+	return lm3533_ctrlbank_enable(&led->cb);
 }
 
 static void lm3533_led_remove(struct platform_device *pdev)
 {
 	struct lm3533_led *led = platform_get_drvdata(pdev);
 
-	dev_dbg(&pdev->dev, "%s\n", __func__);
-
 	lm3533_ctrlbank_disable(&led->cb);
-	led_classdev_unregister(&led->cdev);
 }
 
 static void lm3533_led_shutdown(struct platform_device *pdev)
-- 
2.53.0

Re: [PATCH v1 1/2] leds: lm3533: Clean up probe and use managed helpers

[sashiko-bot]

> Clean up the lm3533 LED driver probe and remove routines by adopting
> managed device helpers and modernizing error handling:
> 
> - Introduce a local `struct device *dev` variable to clean up repeated
>   `&pdev->dev` references.
> - Use `dev_err_probe()` for cleaner error handling on invalid IDs and
>   LED class registration failures.
> - Switch to `devm_led_classdev_register()` to automatically handle LED
>   class device unregistration, removing the need for manual cleanup in
>   error paths and the `remove` hook.
> - Remove redundant `dev_dbg()` statements and cleanup unused cleanup
>   labels.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115084347.70592-1-clamor95@gmail.com?part=1

[PATCH v1 2/2] leds: lm3533: Support getting LED sources from DT

[Svyatoslav Ryhel]

Add support for the standard `led-sources` device tree property to
configure the mapping between control banks and physical low-voltage LED
(LVLED) outputs.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/leds/leds-lm3533.c | 57 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index 7db1105bee5c2..ad8bc3751e3e5 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -26,6 +26,14 @@
 #define LM3533_ALS_CHANNEL_LV_MIN	1
 #define LM3533_ALS_CHANNEL_LV_MAX	2
 
+#define LM3533_REG_OUTPUT_CONF1			0x10
+#define   OUTPUT_CONF1_SHIFT			2
+#define   OUTPUT_LVLED_MASK			0x3
+#define   OUTPUT_CONF1_PREP(x)			(((x) << OUTPUT_CONF1_SHIFT) & 0xff)
+#define LM3533_REG_OUTPUT_CONF2			0x11
+#define   OUTPUT_CONF2_SHIFT			6
+#define   OUTPUT_CONF2_PREP(x)			(((x) >> OUTPUT_CONF2_SHIFT) & 0xff)
+
 #define LM3533_REG_CTRLBANK_BCONF_BASE		0x1b
 #define LM3533_REG_PATTERN_ENABLE		0x28
 #define LM3533_REG_PATTERN_LOW_TIME_BASE	0x71
@@ -54,6 +62,9 @@ struct lm3533_led {
 	u32 max_current;
 	u32 pwm;
 
+	int num_leds;
+	u32 leds[LM3533_LVCTRLBANK_MAX];
+
 	bool have_als;
 };
 
@@ -622,8 +633,36 @@ static const struct attribute_group *lm3533_led_attribute_groups[] = {
 
 static int lm3533_led_setup(struct lm3533_led *led)
 {
+	u32 output_cfg_shift = 0;
+	u32 output_cfg_val = 0;
+	u32 output_cfg_mask = 0;
 	int ret;
 
+	for (int i = 0; i < led->num_leds; i++) {
+		if (led->leds[i] >= LM3533_LVCTRLBANK_MAX)
+			continue;
+
+		output_cfg_shift = led->leds[i] * 2;
+		output_cfg_val |= led->id << output_cfg_shift;
+		output_cfg_mask |= OUTPUT_LVLED_MASK << output_cfg_shift;
+	}
+
+	if (led->num_leds) {
+		/* LVLED1, LVLED2 and LVLED3 */
+		ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF1,
+					 OUTPUT_CONF1_PREP(output_cfg_mask),
+					 OUTPUT_CONF1_PREP(output_cfg_val));
+		if (ret)
+			return ret;
+
+		/* LVLED4 and LVLED5 */
+		ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF2,
+					 OUTPUT_CONF2_PREP(output_cfg_mask),
+					 OUTPUT_CONF2_PREP(output_cfg_val));
+		if (ret)
+			return ret;
+	}
+
 	ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
 	if (ret)
 		return ret;
@@ -690,6 +729,24 @@ static int lm3533_led_probe(struct platform_device *pdev)
 
 	device_property_read_u32(dev, "ti,pwm-config-mask", &led->pwm);
 
+	/*
+	 * If led-sources property is not set then either this Control Bank uses
+	 * its default LVLED or is not linked to any LVLED at all.
+	 */
+	led->num_leds = device_property_count_u32(dev, "led-sources");
+	if (led->num_leds > LM3533_LVCTRLBANK_MAX)
+		return dev_err_probe(dev, -EINVAL, "Number of LED sources exceeds max %d: %d\n",
+				     LM3533_LVCTRLBANK_MAX, led->num_leds);
+
+	if (led->num_leds < 0)
+		led->num_leds = 0;
+
+	if (led->num_leds > 0) {
+		ret = device_property_read_u32_array(dev, "led-sources", led->leds, led->num_leds);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to get led-sources\n");
+	}
+
 	ret = lm3533_led_setup(led);
 	if (ret)
 		return ret;
-- 
2.53.0

Re: [PATCH v1 2/2] leds: lm3533: Support getting LED sources from DT

[sashiko-bot]

> Add support for the standard `led-sources` device tree property to
> configure the mapping between control banks and physical low-voltage LED
> (LVLED) outputs.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115084347.70592-1-clamor95@gmail.com?part=2