linux-kernel - Re: [PATCH RFC] leds: leds-lp50xx: Handle reg to get correct multi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <PAWPR02MB928130924556174EA494C97D9B90A@PAWPR02MB9281.eurprd02.prod.outlook.com>
Date: Thu, 15 May 2025 07:36:08 +0000
From: Johan Adolfsson <Johan.Adolfsson@...s.com>
To: Jacek Anaszewski <jacek.anaszewski@...il.com>, Lee Jones <lee@...nel.org>,
	Pavel Machek <pavel@...nel.org>
CC: "linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Kernel
	<Kernel@...s.com>, Johan Adolfsson <Johan.Adolfsson@...s.com>
Subject: Re: [PATCH RFC] leds: leds-lp50xx: Handle reg to get correct
 multi_index

Hi Jacek,

>From: Jacek Anaszewski <jacek.anaszewski@...il.com>
>Sent: Wednesday, May 14, 2025 21:55
>To: Johan Adolfsson; Lee Jones; Pavel Machek
>Cc: linux-leds@...r.kernel.org; linux-kernel@...r.kernel.org; Kernel
>Subject: Re: [PATCH RFC] leds: leds-lp50xx: Handle reg to get correct multi_index
>
>Hi Johan,
>
>On 5/14/25 16:34, Johan Adolfsson wrote:
> From: Jacek Anaszewski <jacek.anaszewski@...il.com>
> Sent: Tuesday, May 13, 2025 21:52
> To: Johan Adolfsson; Lee Jones; Pavel Machek
> Cc: linux-leds@...r.kernel.org; linux-kernel@...r.kernel.org; Kernel
> Subject: Re: [PATCH RFC] leds: leds-lp50xx: Handle reg to get correct multi_index
> ..
>>>                        }
>>> +                     ret = fwnode_property_read_u32(led_node, "reg", &multi_index);
>>> +                     if (ret) {
>>> +                             multi_index = num_colors;
>>
>> Why not to fail if 'reg' parsing fails?
>> It is marked required in DT bindings [0].
>
>> I didn't want to start failing if reg is missing since it has never been handled until now, despite what the doc says since 2022...
>
>There is one in-tree user [0], and we will need to patch it as well,
>because it uses reg 0,1,2 for each RGB LED module, instead of iout
>numbers as it will be after your change.

Not sure i follow you here. What works for me is starting with reg = <0> for the childs of each bank (multi-led).
The register (maps to iout I guess) is calculated based on led_number * 3 + i where i is the offset as well as the color_index in subled_info (reg).
So I don't think additional changes to that dts is needed.

>
>We will need to also state clearly in the bindings that 'reg' property
>maps to iouts for the non-banked RGB LED modules.

Not sure it actually does, haven't really tested that, or at least not gotten it to work as I expected. 
For the non-rgb led in the product I requested those to be on the first pin on each bank (wasting 2 pins per bank),
since I couldn't figure out how to configure it in another way when prototyping.

Since the 3 outputs per bank share a single brightness setting, it could complicate things as well.
(And we didn't need all the pins anyway)

>For banked RGB LED modules it is more tricky, because there is one
>LED multicolor class device created for them. Probably to be correct
>we would need make the 'reg' properties in the subnodes also arrays
>reflecting iouts that will be governed by BANK_A_Color, BANK_B_Color,
>and BANK_C_Color registers respectively. And DT parser in the driver
>would need to enforce proper iout definition for the banked modules

>E.g. the multi-led@3 node from the example should look like below:
>
>             multi-led@3 {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 reg = <0x3>, <0x4>, <0x5>;
>                 color = <LED_COLOR_ID_RGB>;
>                 function = LED_FUNCTION_STANDBY;
>
>                 led@9 {
>                     reg = <0x9>, <0xc>, <0xf>;
>                     color = <LED_COLOR_ID_RED>;
>                 };
>
>                 led@a {
>                     reg = <0xa>, <0xd>, <0x10>;
>                     color = <LED_COLOR_ID_GREEN>;
>                 };
>
>                 led@b {
>                     reg = <0xb>, <0xe>, <0x11>;
>                     color = <LED_COLOR_ID_BLUE>;
>                 };
>
>
>[0] arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-fuji.dts

Not sure about that, I dont' think we handle an array of reg values within the led node.

>--
>Best regards,
>Jacek Anaszewski

Best regards
/Johan