From: Christian Hitz christian.hitz@bbv.ch
led_banks contains LED module number(s) that should be grouped into the module bank. led_banks is 0-initialized. By checking the led_banks entries for 0, un-set entries are detected. But a 0-entry also indicates that LED module 0 should be grouped into the module bank.
By only iterating over the available entries no check for unused entries is required and LED module 0 can be added to bank.
Signed-off-by: Christian Hitz christian.hitz@bbv.ch Cc: stable@vger.kernel.org --- drivers/leds/leds-lp50xx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c index 94f8ef6b482c..d50c7f3e8f99 100644 --- a/drivers/leds/leds-lp50xx.c +++ b/drivers/leds/leds-lp50xx.c @@ -341,17 +341,15 @@ static int lp50xx_brightness_set(struct led_classdev *cdev, return ret; }
-static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[]) +static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[], int num_leds) { u8 led_config_lo, led_config_hi; u32 bank_enable_mask = 0; int ret; int i;
- for (i = 0; i < priv->chip_info->max_modules; i++) { - if (led_banks[i]) - bank_enable_mask |= (1 << led_banks[i]); - } + for (i = 0; i < num_leds; i++) + bank_enable_mask |= (1 << led_banks[i]);
led_config_lo = bank_enable_mask; led_config_hi = bank_enable_mask >> 8; @@ -405,7 +403,7 @@ static int lp50xx_probe_leds(struct fwnode_handle *child, struct lp50xx *priv, return ret; }
- ret = lp50xx_set_banks(priv, led_banks); + ret = lp50xx_set_banks(priv, led_banks, num_leds); if (ret) { dev_err(priv->dev, "Cannot setup banked LEDs\n"); return ret;
Hi Christian,
On 10/8/25 14:32, Christian Hitz wrote:
From: Christian Hitz christian.hitz@bbv.ch
led_banks contains LED module number(s) that should be grouped into the module bank. led_banks is 0-initialized. By checking the led_banks entries for 0, un-set entries are detected. But a 0-entry also indicates that LED module 0 should be grouped into the module bank.
By only iterating over the available entries no check for unused entries is required and LED module 0 can be added to bank.
Signed-off-by: Christian Hitz christian.hitz@bbv.ch Cc: stable@vger.kernel.org
drivers/leds/leds-lp50xx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c index 94f8ef6b482c..d50c7f3e8f99 100644 --- a/drivers/leds/leds-lp50xx.c +++ b/drivers/leds/leds-lp50xx.c @@ -341,17 +341,15 @@ static int lp50xx_brightness_set(struct led_classdev *cdev, return ret; } -static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[]) +static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[], int num_leds) { u8 led_config_lo, led_config_hi; u32 bank_enable_mask = 0; int ret; int i;
- for (i = 0; i < priv->chip_info->max_modules; i++) {
if (led_banks[i])bank_enable_mask |= (1 << led_banks[i]);- }
- for (i = 0; i < num_leds; i++)
bank_enable_mask |= (1 << led_banks[i]);
Probably the first idea was to have a bitmask indicating which bank to enable, but it ended up in having array of bank ids in DT with no related adjustment in the driver.
This patch deserves Fixes tag.
led_config_lo = bank_enable_mask; led_config_hi = bank_enable_mask >> 8; @@ -405,7 +403,7 @@ static int lp50xx_probe_leds(struct fwnode_handle *child, struct lp50xx *priv, return ret; }
ret = lp50xx_set_banks(priv, led_banks);
ret = lp50xx_set_banks(priv, led_banks, num_leds);
Reviewed-by: Jacek Anaszewski jacek.anaszewski@gmail.com
On Sat, 11 Oct 2025 14:16:16 +0200 Jacek Anaszewski jacek.anaszewski@gmail.com wrote:
Hi Christian,
On 10/8/25 14:32, Christian Hitz wrote:
From: Christian Hitz christian.hitz@bbv.ch
led_banks contains LED module number(s) that should be grouped into the module bank. led_banks is 0-initialized. By checking the led_banks entries for 0, un-set entries are detected. But a 0-entry also indicates that LED module 0 should be grouped into the module bank.
By only iterating over the available entries no check for unused entries is required and LED module 0 can be added to bank.
Signed-off-by: Christian Hitz christian.hitz@bbv.ch Cc: stable@vger.kernel.org
drivers/leds/leds-lp50xx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c index 94f8ef6b482c..d50c7f3e8f99 100644 --- a/drivers/leds/leds-lp50xx.c +++ b/drivers/leds/leds-lp50xx.c @@ -341,17 +341,15 @@ static int lp50xx_brightness_set(struct led_classdev *cdev, return ret; } -static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[]) +static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[], int num_leds) { u8 led_config_lo, led_config_hi; u32 bank_enable_mask = 0; int ret; int i;
- for (i = 0; i < priv->chip_info->max_modules; i++) {
if (led_banks[i])bank_enable_mask |= (1 << led_banks[i]);- }
- for (i = 0; i < num_leds; i++)
bank_enable_mask |= (1 << led_banks[i]);Probably the first idea was to have a bitmask indicating which bank to enable, but it ended up in having array of bank ids in DT with no related adjustment in the driver.
This patch deserves Fixes tag.
This code has not changed since the inital introduction of this driver.
Fixes: 242b81170fb8 ("leds: lp50xx: Add the LP50XX family of the RGB LED driver")
led_config_lo = bank_enable_mask; led_config_hi = bank_enable_mask >> 8; @@ -405,7 +403,7 @@ static int lp50xx_probe_leds(struct fwnode_handle *child, struct lp50xx *priv, return ret; }
ret = lp50xx_set_banks(priv, led_banks);
ret = lp50xx_set_banks(priv, led_banks, num_leds);Reviewed-by: Jacek Anaszewski jacek.anaszewski@gmail.com
-- Best regards, Jacek Anaszewski
Sent using hkml (https://github.com/sjp38/hackermail)
On 10/13/25 10:54, Christian Hitz wrote:
On Sat, 11 Oct 2025 14:16:16 +0200 Jacek Anaszewski jacek.anaszewski@gmail.com wrote:
Hi Christian,
On 10/8/25 14:32, Christian Hitz wrote:
From: Christian Hitz christian.hitz@bbv.ch
led_banks contains LED module number(s) that should be grouped into the module bank. led_banks is 0-initialized. By checking the led_banks entries for 0, un-set entries are detected. But a 0-entry also indicates that LED module 0 should be grouped into the module bank.
By only iterating over the available entries no check for unused entries is required and LED module 0 can be added to bank.
Signed-off-by: Christian Hitz christian.hitz@bbv.ch Cc: stable@vger.kernel.org
drivers/leds/leds-lp50xx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c index 94f8ef6b482c..d50c7f3e8f99 100644 --- a/drivers/leds/leds-lp50xx.c +++ b/drivers/leds/leds-lp50xx.c @@ -341,17 +341,15 @@ static int lp50xx_brightness_set(struct led_classdev *cdev, return ret; } -static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[]) +static int lp50xx_set_banks(struct lp50xx *priv, u32 led_banks[], int num_leds) { u8 led_config_lo, led_config_hi; u32 bank_enable_mask = 0; int ret; int i;
- for (i = 0; i < priv->chip_info->max_modules; i++) {
if (led_banks[i])bank_enable_mask |= (1 << led_banks[i]);- }
- for (i = 0; i < num_leds; i++)
bank_enable_mask |= (1 << led_banks[i]);Probably the first idea was to have a bitmask indicating which bank to enable, but it ended up in having array of bank ids in DT with no related adjustment in the driver.
This patch deserves Fixes tag.
This code has not changed since the inital introduction of this driver.
Yeah, I had on mind design approach changes between subsequent versions of the patch that was adding the driver.
Fixes: 242b81170fb8 ("leds: lp50xx: Add the LP50XX family of the RGB LED driver")
On Wed, 08 Oct 2025 14:32:21 +0200, Christian Hitz wrote:
led_banks contains LED module number(s) that should be grouped into the module bank. led_banks is 0-initialized. By checking the led_banks entries for 0, un-set entries are detected. But a 0-entry also indicates that LED module 0 should be grouped into the module bank.
By only iterating over the available entries no check for unused entries is required and LED module 0 can be added to bank.
[...]
Applied, thanks!
[1/1] leds: leds-lp50xx: allow LED 0 to be added to module bank commit: 26fe74d598c32e7bc6f150edfc4aa43e1bee55db
-- Lee Jones [李琼斯]
linux-stable-mirror@lists.linaro.org
-
Christian Hitz -
Jacek Anaszewski -
Lee Jones