Re: [PATCH v2] leds: group-multicolor: Add support for initial value.

[Jean-Jacques Hiblot]

On 14/12/2025 16:21, Lee Jones wrote:
> INTENTIONAL TOP POST: I think the original author should have been on Cc.
Hi,

Sorry, I didn't follow the discussion and I'm not sure what you're 
trying to achieve with this patch. So correct me if I'm wrong.
As I understand it you're trying to capture the current state of the
LEDs at probe() time to set the the relative intensity of the LEDs 
constituting the group.

> Adding: Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx>
>
> > When the driver is loaded, it turned off all LEDs in the group. This
> > patch changes the driver to take over existing LED states and set
> > the brighness and intensity in the group accordingly.
> >
> > Signed-off-by: Martijn de Gouw <martijn.de.gouw@xxxxxxxxxxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/leds/rgb/leds-group-multicolor.c | 14 ++++++++++++--
> >   1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c
> > index 548c7dd63ba1e..a12720d73f188 100644
> > --- a/drivers/leds/rgb/leds-group-multicolor.c
> > +++ b/drivers/leds/rgb/leds-group-multicolor.c
> > @@ -69,6 +69,7 @@ static int leds_gmc_probe(struct platform_device *pdev)
> >   	struct mc_subled *subled;
> >   	struct leds_multicolor *priv;
> >   	unsigned int max_brightness = 0;
> > +	unsigned int default_brightness = 0;
> >   	int i, ret, count = 0, common_flags = 0;
> >   
> >   	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > @@ -96,6 +97,13 @@ static int leds_gmc_probe(struct platform_device *pdev)
> >   
> >   		max_brightness = max(max_brightness, led_cdev->max_brightness);
> >   
> > +		/*
> > +		 * If any LED is on, set brightness to the max brightness.
> > +		 * The actual brightness of the LED is set as intensity value.
> > +		 */
> > +		if (led_cdev->brightness)
> > +			default_brightness = max_brightness;
Storing a brightness here is a bit confusing. What we're interested in 
is whether at least on of the LED is on (and thus the group is 
considered on). I've would rather use a boolean here, like is_on = true
> > +
> >   		count++;
> >   	}
> >   
> > @@ -109,14 +117,16 @@ static int leds_gmc_probe(struct platform_device *pdev)
> >   
> >   		subled[i].color_index = led_cdev->color;
> >   
> > -		/* Configure the LED intensity to its maximum */
> > -		subled[i].intensity = max_brightness;
> > +		/* Configure the LED intensity to its current brightness */
> > +		subled[i].intensity = DIV_ROUND_CLOSEST(led_cdev->brightness * max_brightness,
> > +							led_cdev->max_brightness);
You cannot use this computation in all cases. If the LEDs have not been 
turned on, their intensity will always be 0. Maybe the 'is_on' flag can 
be use to select which computation should be used.
> >   	}
> >   
> >   	/* Initialise the multicolor's LED class device */
> >   	cdev = &priv->mc_cdev.led_cdev;
> >   	cdev->brightness_set_blocking = leds_gmc_set;
> >   	cdev->max_brightness = max_brightness;

> > +	cdev->brightness = default_brightness;
cdev->brightness = is_on ? max_brightness : 0;
> >   	cdev->color = LED_COLOR_ID_MULTI;
> >   	priv->mc_cdev.num_colors = count;
> >   

JJ
> > --
> > 2.39.2