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

[Jean-Jacques Hiblot <jjhiblot@traphandler.com>]

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@traphandler.com>
> 
>> 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@prodrive-technologies.com>
>> ---
>>   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
>>
>