Re: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity
Stefan Wahren <[email protected]>
| Newsgroups | org.kernel.vger.linux-leds,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
Hi, Am 13.08.26 um 11:53 schrieb Lee Jones: > On Thu, 13 Aug 2026, [email protected] wrote: > >> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: >> - [Medium] The `default-intensity` property read from individual monochromatic LED nodes is applied to the multicolor group without rescaling, causing incorrect brightness calculations. > Please review, explain or fix the Sashiko review(s). > > If this is a set, this request includes the other patches too. Sorry, I don't get the problem with "incorrect brightness calculation", I tought the intensity setting is independent from the brightness. Btw the intensity is applied to the monochromatic LED not to the group itself. hallucination? Does mixing PWM and GPIO LED in a RGB multicolor group make sense at all? > >> -- >> >> --- Patch [1]: [PATCH V4] leds: rgb: leds-group-multicolor: Implement default-intensity --- >> commit 13399f1aaceaf09afa7e252e35000d7d61ab1925 >> Author: Stefan Wahren <[email protected]> >> >> leds: rgb: leds-group-multicolor: Implement default-intensity >> >> This commit introduces support for the default-intensity property during >> boot for LED multicolor groups. If the property is missing, the code falls >> back to the previous behavior of configuring the sub-LED intensity to its >> maximum. >> >>> diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c >>> --- a/drivers/leds/rgb/leds-group-multicolor.c >>> +++ b/drivers/leds/rgb/leds-group-multicolor.c >>> @@ -108,8 +108,10 @@ 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; >>> + ret = device_property_read_u32(led_cdev->dev, "default-intensity", >>> + &subled[i].intensity); >>> + if (ret || subled[i].intensity > max_brightness) >>> + subled[i].intensity = max_brightness; >> [Severity: Medium] >> Does this logic properly scale the intensity when the individual LEDs in the >> group have different maximum brightness values? >> >> If a multicolor group consists of LEDs with vastly different maximum brightness >> capabilities (like a GPIO LED maxing at 1 and a PWM LED maxing at 255), the >> overall group max_brightness will evaluate to 255. >> >> When reading the default-intensity from the device tree, the value is typically >> specified on the scale of the individual sub-LED's max brightness. If this >> value is assigned directly to subled[i].intensity without being scaled up to >> the group's max_brightness, the sub-LED might be initialized with only a tiny >> fraction of its intended intensity, often resulting in the LED being completely >> unlit instead of at the requested intensity. >> >>> } >>> >>> /* Initialise the multicolor's LED class device */ >> -- >> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1