* [PATCH v1] video: leds: backlight: lm3533: Support getting LED sources from DT
@ 2026-11-01 14:24 Svyatoslav Ryhel
2026-11-13 14:53 ` Lee Jones
0 siblings, 1 reply; 4+ messages in thread
From: Svyatoslav Ryhel @ 2026-11-01 14:24 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Griffin Kroah-Hartman, Daniel Thompson,
Jingoo Han, Helge Deller
Cc: linux-leds, linux-kernel, dri-devel, linux-fbdev
Add Control Bank to HVLED/LVLED muxing support based on the led-sources
defined in the device tree.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
---
This was part of the original LM3533 controller DT/OF conversion patchset,
but it appears to have been accidentally separated during the final iteration.
Therefore, I am sending it as a follow-up.
---
drivers/leds/leds-lm3533.c | 60 +++++++++++++++++++++++++++++
drivers/video/backlight/lm3533_bl.c | 45 ++++++++++++++++++++++
2 files changed, 105 insertions(+)
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index 7259097bd6a71..a1aeff70a90fc 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -26,6 +26,11 @@
#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 LM3533_REG_OUTPUT_CONF2 0x11
+#define OUTPUT_CONF2_SHIFT 6
#define LM3533_REG_CTRLBANK_BCONF_BASE 0x1b
#define LM3533_REG_PATTERN_ENABLE 0x28
#define LM3533_REG_PATTERN_LOW_TIME_BASE 0x71
@@ -54,6 +59,9 @@ struct lm3533_led {
u32 max_current;
u32 pwm;
+ int num_leds;
+ u32 leds[LM3533_LVCTRLBANK_MAX];
+
bool have_als;
};
@@ -622,8 +630,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;
+ if (led->num_leds) {
+ 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;
+ }
+
+ /* LVLED1, LVLED2 and LVLED3 */
+ ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF1,
+ output_cfg_mask << OUTPUT_CONF1_SHIFT,
+ output_cfg_val << OUTPUT_CONF1_SHIFT);
+ if (ret)
+ return ret;
+
+ /* LVLED4 and LVLED5 */
+ ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF2,
+ output_cfg_mask >> OUTPUT_CONF2_SHIFT,
+ output_cfg_val >> OUTPUT_CONF2_SHIFT);
+ if (ret)
+ return ret;
+ }
+
ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
if (ret)
return ret;
@@ -698,6 +734,30 @@ static int lm3533_led_probe(struct platform_device *pdev)
device_property_read_u32(&pdev->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(&pdev->dev, "led-sources");
+ if (led->num_leds > LM3533_LVCTRLBANK_MAX) {
+ dev_err(&pdev->dev, "num of LED sources exceeds max %d: %d\n",
+ LM3533_LVCTRLBANK_MAX, led->num_leds);
+ ret = -EINVAL;
+ goto err_deregister;
+ }
+
+ if (led->num_leds < 0)
+ led->num_leds = 0;
+
+ if (led->num_leds > 0) {
+ ret = device_property_read_u32_array(&pdev->dev, "led-sources",
+ led->leds, led->num_leds);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to get led-sources\n");
+ goto err_deregister;
+ }
+ }
+
ret = lm3533_led_setup(led);
if (ret)
goto err_deregister;
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index adcd6ac7cf229..e8164c8528577 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -7,6 +7,7 @@
* Author: Johan Hovold <jhovold@gmail.com>
*/
+#include <linux/bits.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/platform_device.h>
@@ -21,6 +22,7 @@
#define LM3533_HVCTRLBANK_COUNT 2
#define LM3533_BL_MAX_BRIGHTNESS 255
+#define LM3533_REG_OUTPUT_CONF1 0x10
#define LM3533_REG_CTRLBANK_AB_BCONF 0x1a
#define CTRLBANK_AB_BCONF_ALS(n) BIT(2 * (n))
#define CTRLBANK_AB_BCONF_MODE(n) BIT(2 * (n) + 1)
@@ -35,6 +37,9 @@ struct lm3533_bl {
u32 max_current;
u32 pwm;
+ int num_leds;
+ u32 led_strings[LM3533_HVCTRLBANK_COUNT];
+
bool have_als;
bool linear;
};
@@ -236,6 +241,8 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
static int lm3533_bl_setup(struct lm3533_bl *bl)
{
int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
+ u32 output_cfg_val = 0;
+ u32 output_cfg_mask = 0;
int ret;
ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
@@ -243,6 +250,21 @@ static int lm3533_bl_setup(struct lm3533_bl *bl)
if (ret)
return ret;
+ if (bl->num_leds) {
+ for (int i = 0; i < bl->num_leds; i++) {
+ if (bl->led_strings[i] >= LM3533_HVCTRLBANK_COUNT)
+ continue;
+
+ output_cfg_val |= ctrlbank << bl->led_strings[i];
+ output_cfg_mask |= BIT(bl->led_strings[i]);
+ }
+
+ ret = regmap_update_bits(bl->regmap, LM3533_REG_OUTPUT_CONF1,
+ output_cfg_mask, output_cfg_val);
+ if (ret)
+ return ret;
+ }
+
ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
if (ret)
return ret;
@@ -320,6 +342,29 @@ static int lm3533_bl_probe(struct platform_device *pdev)
device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
+ /*
+ * If led-sources property is not set then either this Control Bank uses
+ * its default HVLED or is not linked to any HVLED at all.
+ */
+ bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
+ if (bl->num_leds > LM3533_HVCTRLBANK_COUNT) {
+ dev_err(&pdev->dev, "num of LED sources %d exceeds max %d\n",
+ bl->num_leds, LM3533_HVCTRLBANK_COUNT);
+ return -EINVAL;
+ }
+
+ if (bl->num_leds < 0)
+ bl->num_leds = 0;
+
+ if (bl->num_leds > 0) {
+ ret = device_property_read_u32_array(&pdev->dev, "led-sources",
+ bl->led_strings,
+ bl->num_leds);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to get led-sources\n");
+ }
+
ret = lm3533_bl_setup(bl);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] video: leds: backlight: lm3533: Support getting LED sources from DT
2026-11-01 14:24 [PATCH v1] video: leds: backlight: lm3533: Support getting LED sources from DT Svyatoslav Ryhel
@ 2026-11-13 14:53 ` Lee Jones
2026-11-13 15:41 ` Svyatoslav Ryhel
0 siblings, 1 reply; 4+ messages in thread
From: Lee Jones @ 2026-11-13 14:53 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Pavel Machek, Griffin Kroah-Hartman, Daniel Thompson, Jingoo Han,
Helge Deller, linux-leds, linux-kernel, dri-devel, linux-fbdev
Drop the 'video: ' part of the subject line.
Please review, explain or fix the Sashiko review(s).
> Add Control Bank to HVLED/LVLED muxing support based on the led-sources
> defined in the device tree.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
> ---
> This was part of the original LM3533 controller DT/OF conversion patchset,
> but it appears to have been accidentally separated during the final iteration.
> Therefore, I am sending it as a follow-up.
> ---
> drivers/leds/leds-lm3533.c | 60 +++++++++++++++++++++++++++++
> drivers/video/backlight/lm3533_bl.c | 45 ++++++++++++++++++++++
Why are these being bundled up?
What is the dependency? If none, please separate.
> 2 files changed, 105 insertions(+)
>
> diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> index 7259097bd6a71..a1aeff70a90fc 100644
> --- a/drivers/leds/leds-lm3533.c
> +++ b/drivers/leds/leds-lm3533.c
> @@ -26,6 +26,11 @@
> #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 LM3533_REG_OUTPUT_CONF2 0x11
> +#define OUTPUT_CONF2_SHIFT 6
> #define LM3533_REG_CTRLBANK_BCONF_BASE 0x1b
> #define LM3533_REG_PATTERN_ENABLE 0x28
> #define LM3533_REG_PATTERN_LOW_TIME_BASE 0x71
> @@ -54,6 +59,9 @@ struct lm3533_led {
> u32 max_current;
> u32 pwm;
>
> + int num_leds;
> + u32 leds[LM3533_LVCTRLBANK_MAX];
> +
> bool have_als;
> };
>
> @@ -622,8 +630,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;
>
> + if (led->num_leds) {
> + 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;
> + }
> +
> + /* LVLED1, LVLED2 and LVLED3 */
> + ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF1,
> + output_cfg_mask << OUTPUT_CONF1_SHIFT,
> + output_cfg_val << OUTPUT_CONF1_SHIFT);
> + if (ret)
> + return ret;
> +
> + /* LVLED4 and LVLED5 */
> + ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF2,
> + output_cfg_mask >> OUTPUT_CONF2_SHIFT,
> + output_cfg_val >> OUTPUT_CONF2_SHIFT);
> + if (ret)
> + return ret;
> + }
> +
> ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
> if (ret)
> return ret;
> @@ -698,6 +734,30 @@ static int lm3533_led_probe(struct platform_device *pdev)
>
> device_property_read_u32(&pdev->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(&pdev->dev, "led-sources");
> + if (led->num_leds > LM3533_LVCTRLBANK_MAX) {
> + dev_err(&pdev->dev, "num of LED sources exceeds max %d: %d\n",
"Number "
> + LM3533_LVCTRLBANK_MAX, led->num_leds);
> + ret = -EINVAL;
> + goto err_deregister;
> + }
Could we move the 'led-sources' parsing above led_classdev_register(),
or convert the driver to devm_led_classdev_register() and use 'return
dev_err_probe()' here?
> + if (led->num_leds < 0)
> + led->num_leds = 0;
> +
> + if (led->num_leds > 0) {
> + ret = device_property_read_u32_array(&pdev->dev, "led-sources",
> + led->leds, led->num_leds);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to get led-sources\n");
> + goto err_deregister;
> + }
dev_err_probe()
> + }
> +
> ret = lm3533_led_setup(led);
> if (ret)
> goto err_deregister;
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index adcd6ac7cf229..e8164c8528577 100644
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c
> @@ -7,6 +7,7 @@
> * Author: Johan Hovold <jhovold@gmail.com>
> */
>
> +#include <linux/bits.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/platform_device.h>
> @@ -21,6 +22,7 @@
> #define LM3533_HVCTRLBANK_COUNT 2
> #define LM3533_BL_MAX_BRIGHTNESS 255
>
> +#define LM3533_REG_OUTPUT_CONF1 0x10
> #define LM3533_REG_CTRLBANK_AB_BCONF 0x1a
> #define CTRLBANK_AB_BCONF_ALS(n) BIT(2 * (n))
> #define CTRLBANK_AB_BCONF_MODE(n) BIT(2 * (n) + 1)
> @@ -35,6 +37,9 @@ struct lm3533_bl {
> u32 max_current;
> u32 pwm;
>
> + int num_leds;
> + u32 led_strings[LM3533_HVCTRLBANK_COUNT];
> +
> bool have_als;
> bool linear;
> };
> @@ -236,6 +241,8 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
> static int lm3533_bl_setup(struct lm3533_bl *bl)
> {
> int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
> + u32 output_cfg_val = 0;
> + u32 output_cfg_mask = 0;
> int ret;
>
> ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> @@ -243,6 +250,21 @@ static int lm3533_bl_setup(struct lm3533_bl *bl)
> if (ret)
> return ret;
>
> + if (bl->num_leds) {
> + for (int i = 0; i < bl->num_leds; i++) {
> + if (bl->led_strings[i] >= LM3533_HVCTRLBANK_COUNT)
> + continue;
> +
> + output_cfg_val |= ctrlbank << bl->led_strings[i];
> + output_cfg_mask |= BIT(bl->led_strings[i]);
> + }
This for loop doesn't need to be in the if statement.
> + ret = regmap_update_bits(bl->regmap, LM3533_REG_OUTPUT_CONF1,
> + output_cfg_mask, output_cfg_val);
> + if (ret)
> + return ret;
> + }
> +
> ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
> if (ret)
> return ret;
> @@ -320,6 +342,29 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>
> device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
>
> + /*
> + * If led-sources property is not set then either this Control Bank uses
> + * its default HVLED or is not linked to any HVLED at all.
> + */
> + bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
> + if (bl->num_leds > LM3533_HVCTRLBANK_COUNT) {
> + dev_err(&pdev->dev, "num of LED sources %d exceeds max %d\n",
> + bl->num_leds, LM3533_HVCTRLBANK_COUNT);
> + return -EINVAL;
> + }
dev_err_probe()
> +
> + if (bl->num_leds < 0)
> + bl->num_leds = 0;
> +
> + if (bl->num_leds > 0) {
> + ret = device_property_read_u32_array(&pdev->dev, "led-sources",
> + bl->led_strings,
> + bl->num_leds);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to get led-sources\n");
> + }
> +
> ret = lm3533_bl_setup(bl);
> if (ret)
> return ret;
> --
> 2.53.0
>
--
Lee Jones
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] video: leds: backlight: lm3533: Support getting LED sources from DT
2026-11-13 14:53 ` Lee Jones
@ 2026-11-13 15:41 ` Svyatoslav Ryhel
2026-11-13 19:06 ` Lee Jones
0 siblings, 1 reply; 4+ messages in thread
From: Svyatoslav Ryhel @ 2026-11-13 15:41 UTC (permalink / raw)
To: Lee Jones
Cc: Pavel Machek, Griffin Kroah-Hartman, Daniel Thompson, Jingoo Han,
Helge Deller, linux-leds, linux-kernel, dri-devel, linux-fbdev
пт, 13 лист. 2026 р. о 16:53 Lee Jones <lee@kernel.org> пише:
>
> Drop the 'video: ' part of the subject line.
>
> Please review, explain or fix the Sashiko review(s).
>
> > Add Control Bank to HVLED/LVLED muxing support based on the led-sources
> > defined in the device tree.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
> > ---
> > This was part of the original LM3533 controller DT/OF conversion patchset,
> > but it appears to have been accidentally separated during the final iteration.
> > Therefore, I am sending it as a follow-up.
> > ---
> > drivers/leds/leds-lm3533.c | 60 +++++++++++++++++++++++++++++
> > drivers/video/backlight/lm3533_bl.c | 45 ++++++++++++++++++++++
>
> Why are these being bundled up?
>
> What is the dependency? If none, please separate.
>
> > 2 files changed, 105 insertions(+)
> >
> > diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> > index 7259097bd6a71..a1aeff70a90fc 100644
> > --- a/drivers/leds/leds-lm3533.c
> > +++ b/drivers/leds/leds-lm3533.c
> > @@ -26,6 +26,11 @@
> > #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 LM3533_REG_OUTPUT_CONF2 0x11
> > +#define OUTPUT_CONF2_SHIFT 6
> > #define LM3533_REG_CTRLBANK_BCONF_BASE 0x1b
> > #define LM3533_REG_PATTERN_ENABLE 0x28
> > #define LM3533_REG_PATTERN_LOW_TIME_BASE 0x71
> > @@ -54,6 +59,9 @@ struct lm3533_led {
> > u32 max_current;
> > u32 pwm;
> >
> > + int num_leds;
> > + u32 leds[LM3533_LVCTRLBANK_MAX];
> > +
> > bool have_als;
> > };
> >
> > @@ -622,8 +630,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;
> >
> > + if (led->num_leds) {
> > + 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;
> > + }
> > +
> > + /* LVLED1, LVLED2 and LVLED3 */
> > + ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF1,
> > + output_cfg_mask << OUTPUT_CONF1_SHIFT,
> > + output_cfg_val << OUTPUT_CONF1_SHIFT);
> > + if (ret)
> > + return ret;
> > +
> > + /* LVLED4 and LVLED5 */
> > + ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF2,
> > + output_cfg_mask >> OUTPUT_CONF2_SHIFT,
> > + output_cfg_val >> OUTPUT_CONF2_SHIFT);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
> > if (ret)
> > return ret;
> > @@ -698,6 +734,30 @@ static int lm3533_led_probe(struct platform_device *pdev)
> >
> > device_property_read_u32(&pdev->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(&pdev->dev, "led-sources");
> > + if (led->num_leds > LM3533_LVCTRLBANK_MAX) {
> > + dev_err(&pdev->dev, "num of LED sources exceeds max %d: %d\n",
>
> "Number "
>
> > + LM3533_LVCTRLBANK_MAX, led->num_leds);
> > + ret = -EINVAL;
> > + goto err_deregister;
> > + }
>
> Could we move the 'led-sources' parsing above led_classdev_register(),
> or convert the driver to devm_led_classdev_register() and use 'return
> dev_err_probe()' here?
>
I will convert to devm_led_classdev_register() as precursor to this patch.
> > + if (led->num_leds < 0)
> > + led->num_leds = 0;
> > +
> > + if (led->num_leds > 0) {
> > + ret = device_property_read_u32_array(&pdev->dev, "led-sources",
> > + led->leds, led->num_leds);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to get led-sources\n");
> > + goto err_deregister;
> > + }
>
> dev_err_probe()
>
> > + }
> > +
> > ret = lm3533_led_setup(led);
> > if (ret)
> > goto err_deregister;
> > diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> > index adcd6ac7cf229..e8164c8528577 100644
> > --- a/drivers/video/backlight/lm3533_bl.c
> > +++ b/drivers/video/backlight/lm3533_bl.c
> > @@ -7,6 +7,7 @@
> > * Author: Johan Hovold <jhovold@gmail.com>
> > */
> >
> > +#include <linux/bits.h>
> > #include <linux/module.h>
> > #include <linux/init.h>
> > #include <linux/platform_device.h>
> > @@ -21,6 +22,7 @@
> > #define LM3533_HVCTRLBANK_COUNT 2
> > #define LM3533_BL_MAX_BRIGHTNESS 255
> >
> > +#define LM3533_REG_OUTPUT_CONF1 0x10
> > #define LM3533_REG_CTRLBANK_AB_BCONF 0x1a
> > #define CTRLBANK_AB_BCONF_ALS(n) BIT(2 * (n))
> > #define CTRLBANK_AB_BCONF_MODE(n) BIT(2 * (n) + 1)
> > @@ -35,6 +37,9 @@ struct lm3533_bl {
> > u32 max_current;
> > u32 pwm;
> >
> > + int num_leds;
> > + u32 led_strings[LM3533_HVCTRLBANK_COUNT];
> > +
> > bool have_als;
> > bool linear;
> > };
> > @@ -236,6 +241,8 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
> > static int lm3533_bl_setup(struct lm3533_bl *bl)
> > {
> > int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
> > + u32 output_cfg_val = 0;
> > + u32 output_cfg_mask = 0;
> > int ret;
> >
> > ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> > @@ -243,6 +250,21 @@ static int lm3533_bl_setup(struct lm3533_bl *bl)
> > if (ret)
> > return ret;
> >
> > + if (bl->num_leds) {
> > + for (int i = 0; i < bl->num_leds; i++) {
> > + if (bl->led_strings[i] >= LM3533_HVCTRLBANK_COUNT)
> > + continue;
> > +
> > + output_cfg_val |= ctrlbank << bl->led_strings[i];
> > + output_cfg_mask |= BIT(bl->led_strings[i]);
> > + }
>
> This for loop doesn't need to be in the if statement.
>
It is in if statement because if node has no "led-sources"
LM3533_REG_OUTPUT_CONF1 should not be touched and must be left in its
default state.
> > + ret = regmap_update_bits(bl->regmap, LM3533_REG_OUTPUT_CONF1,
> > + output_cfg_mask, output_cfg_val);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
> > if (ret)
> > return ret;
> > @@ -320,6 +342,29 @@ static int lm3533_bl_probe(struct platform_device *pdev)
> >
> > device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
> >
> > + /*
> > + * If led-sources property is not set then either this Control Bank uses
> > + * its default HVLED or is not linked to any HVLED at all.
> > + */
> > + bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
> > + if (bl->num_leds > LM3533_HVCTRLBANK_COUNT) {
> > + dev_err(&pdev->dev, "num of LED sources %d exceeds max %d\n",
> > + bl->num_leds, LM3533_HVCTRLBANK_COUNT);
> > + return -EINVAL;
> > + }
>
> dev_err_probe()
>
> > +
> > + if (bl->num_leds < 0)
> > + bl->num_leds = 0;
> > +
> > + if (bl->num_leds > 0) {
> > + ret = device_property_read_u32_array(&pdev->dev, "led-sources",
> > + bl->led_strings,
> > + bl->num_leds);
> > + if (ret)
> > + return dev_err_probe(&pdev->dev, ret,
> > + "failed to get led-sources\n");
> > + }
> > +
> > ret = lm3533_bl_setup(bl);
> > if (ret)
> > return ret;
> > --
> > 2.53.0
> >
>
Other suggestions are valid, I will apply them in v3
> --
> Lee Jones
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] video: leds: backlight: lm3533: Support getting LED sources from DT
2026-11-13 15:41 ` Svyatoslav Ryhel
@ 2026-11-13 19:06 ` Lee Jones
0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2026-11-13 19:06 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Pavel Machek, Griffin Kroah-Hartman, Daniel Thompson, Jingoo Han,
Helge Deller, linux-leds, linux-kernel, dri-devel, linux-fbdev
On Fri, 13 Nov 2026, Svyatoslav Ryhel wrote:
> пт, 13 лист. 2026 р. о 16:53 Lee Jones <lee@kernel.org> пише:
> >
> > Drop the 'video: ' part of the subject line.
> >
> > Please review, explain or fix the Sashiko review(s).
> >
> > > Add Control Bank to HVLED/LVLED muxing support based on the led-sources
> > > defined in the device tree.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
> > > ---
> > > This was part of the original LM3533 controller DT/OF conversion patchset,
> > > but it appears to have been accidentally separated during the final iteration.
> > > Therefore, I am sending it as a follow-up.
> > > ---
> > > drivers/leds/leds-lm3533.c | 60 +++++++++++++++++++++++++++++
> > > drivers/video/backlight/lm3533_bl.c | 45 ++++++++++++++++++++++
[...]
> > > + if (bl->num_leds) {
> > > + for (int i = 0; i < bl->num_leds; i++) {
> > > + if (bl->led_strings[i] >= LM3533_HVCTRLBANK_COUNT)
> > > + continue;
> > > +
> > > + output_cfg_val |= ctrlbank << bl->led_strings[i];
> > > + output_cfg_mask |= BIT(bl->led_strings[i]);
> > > + }
> >
> > This for loop doesn't need to be in the if statement.
> >
>
> It is in if statement because if node has no "led-sources"
> LM3533_REG_OUTPUT_CONF1 should not be touched and must be left in its
> default state.
But it won't iterate if (!bl->num_leds), so it's being indented for no
reason.
--
Lee Jones
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-13 19:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-01 14:24 [PATCH v1] video: leds: backlight: lm3533: Support getting LED sources from DT Svyatoslav Ryhel
2026-11-13 14:53 ` Lee Jones
2026-11-13 15:41 ` Svyatoslav Ryhel
2026-11-13 19:06 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®