Re: [PATCH v6 1/2] leds: leds-lp50xx: Handle reg to get correct multi_index

Jacek Anaszewski
2025-06-16T21:29:58Z

Hi Johan,

On 6/16/25 13:25, Johan Adolfsson wrote:
> mc_subled used for multi_index needs well defined array indexes,
> to guarantee the desired result, use reg for that.
>
> If devicetree child nodes is processed in random or reverse order
> you may end up with multi_index "blue green red" instead of the expected
> "red green blue".
> If user space apps uses multi_index to deduce how to control the leds
> they would most likely be broken without this patch if devicetree
> processing is reversed (which it appears to be).
>
> arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-fuji.dts has reg set
> but I don't see how it can have worked without this change.
>
> If reg is not set, an error is returned,
> If reg is out of range, an error is returned.
> reg within led child nodes starts with 0, to map to the iout in each bank.
>
> Signed-off-by: Johan Adolfsson <johan.adolfsson@axis.com>
> ---
> drivers/leds/leds-lp50xx.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
> index 02cb1565a9fb..344791b6c575 100644
> --- a/drivers/leds/leds-lp50xx.c
> +++ b/drivers/leds/leds-lp50xx.c
> @@ -476,6 +476,7 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
> return -ENOMEM;
>
> fwnode_for_each_child_node(child, led_node) {
> + int multi_index = num_colors;

Any specific reason for initializing this to num_colors?

> ret = fwnode_property_read_u32(led_node, "color",
> &color_id);
> if (ret) {
> @@ -483,8 +484,16 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
> dev_err(priv->dev, "Cannot read color\n");
> return ret;
> }
> + ret = fwnode_property_read_u32(led_node, "reg", &multi_index);
> + if (ret != 0) {
> + dev_err(priv->dev, "reg must be set\n");
> + return -EINVAL;
> + } else if (multi_index >= LP50XX_LEDS_PER_MODULE) {
> + dev_err(priv->dev, "reg %i out of range\n", multi_index);
> + return -EINVAL;
> + }
>
> - mc_led_info[num_colors].color_index = color_id;
> + mc_led_info[multi_index].color_index = color_id;
> num_colors++;
> }
>
>

Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

--
Best regards,
Jacek Anaszewski