[PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards

[Diederik de Haas]

Hi Naoki,

On Wed Nov 12, 2025 at 12:42 AM CET, FUKAUMI Naoki wrote:
> On 11/12/25 03:32, Quentin Schulz wrote:
>> On 11/11/25 5:14 PM, Dragan Simic wrote:
>>> On Tuesday, November 11, 2025 16:32 CET, FUKAUMI Naoki 
>>> <naoki at radxa.com> wrote:
>>>> On 11/11/25 23:46, Dragan Simic wrote:
>>>>> On Tuesday, November 11, 2025 14:07 CET, "Diederik de Haas" 
>>>>> <diederik at cknow-tech.com> wrote:
>>>>>> On Tue Nov 11, 2025 at 6:41 AM CET, FUKAUMI Naoki wrote:
>>>>>>> Radxa's boards turn all LEDs on at boot(loader), but some boards 
>>>>>>> don't have `default-state` property in Linux kernel tree but
>>>>>>> have it in U-Boot tree instead[1].
>>>>>>>
>>>>>>> This patch adds `default-state = "on"` for (almost) all LEDs (with a
>>>>>>> few exceptions which should be "off" such as RGB LEDs on E25 and LAN/
>>>>>>> WAN LEDs on E20C/E52C).
>>>>>>
>>>>>> I'm missing the *why* these changes would be an improvement.
>>>>>>
>>>>>> Personally, for both 'heartbeat' and 'netdev' triggers, I want them to
>>>>>> be off by default and once it gets a 'heartbeat' or a 'netdev' 
>>>>>> trigger, THEN I want the LED to be on/blinking.
>>>>>
>>>>> That's a good question for Naoki.  My own preference would also
>>>>> be to have the device's power LED turned on by U-Boot as quickly
>>>>> as possible after supplying power to the board or turning it on
>>>>> by pressing the power button.  I'm actually not a big fan of
>>>>> having all the LEDs shining for a couple of seconds or so, which
>>>>> may actually look like some error condition to me.
>>>>>
>>>>> Having all that in mind, I may suggest that just the U-Boot's
>>>>> behavior is changed to turn the power LEDs on only.
>>>>
>>>> I can't quite explain it, but...
>>>>
>>>> - 1st (Power) LED
>>>>
>>>> The 1st (power) LED turns on automatically/immediately without software
>>>> intervention. (On some boards, this LED cannot be controlled by software
>>>> at all.)
>>>>
>>>> In DTS, this should be described using `default-state = "on"`. The use
>>>> of the Linux-specific property `linux,default-trigger = "default-on"` is
>>>> unsuitable for non-Linux environments.
>>>>
>>>> - 2nd (Heartbeat) LED
>>>>
>>>> The 2nd (heartbeat) LED can be controlled by software. It should be lit
>>>> up as quickly as possible to indicate that the very first software
>>>> (e.g., the bootloader) is running.
>>>>
>>>> On Linux, usually this is used as `linux,default-trigger = "heartbeat"`.
>>>> It indicates that kernel is running (regardless of the `default-state`
>>>> setting), and its behavior can be modified in user space.
>>>
>>> As discussed already in the #linux-rockchip IRC channel, [1] perhaps
>>> the best option would be to have the power LEDs turned on as quickly
>>> upon powering on the board as possible, and to have U-Boot pulsate
>>> the heartbeat LEDs using the LED_BOOT feature.  In such a scenario,
>>> no other LEDs would be turned on early, and the LED-related DT parts
>>> specific to U-Boot would be migrated to the kernel DTs.
>>>
>>> [1] https://libera.catirclogs.org/linux-rockchip/2025-11-11#38997824;
>> 
>> The LED_BOOT feature (guarded by the Kconfig symbol of the same name) in 
>> U-Boot only applies if /options/u-boot/boot-led property is set.
>
> For the default state of the heartbeat LED, I'm thinking of using 
> LED_BOOT (/options/u-boot/boot-led), but I'm concerned that this is 
> U-Boot-specific.

If U-Boot wants to use the heartbeat LED to signal the *bootloader* is
running, I guess that's fine. And if you want to make it solid or
blinking, that seems best discussed on the U-Boot ML.

I still consider the bootloader and the kernel stages separate.
And I haven't seen an argument why I should change *my* opinion on the
heartbeat and netdev triggers (default-state) wrt the kernel.

I don't think that what U-Boot does or doesn't do, should determine what
the Linux kernel does or doesn't do.
I have no plans to use another bootloader then U-Boot, but it's possible
that people do, so what the Linux kernel does should be independent from
what the/a specific bootloader does.

And as I said before, *I* want LEDs with netdev and heartbeat triggers,
to be off (at the start, which is indeed the default value).
I use the heartbeat trigger to:
1) See the kernel has started (and has gotten to the point the heartbeat
   'infrastructure' has been set up
2) Wait for the blinking to slow down as that (generally) means it's
   pretty much done with the boot process and the SSH server should
   probably be running then, so I can login
3) When the heartbeat LED is solid, that means the system has crashed
   (f.e. due to overheating ...)

And also, if you're going to change/override other people's choices, a
motivation as to why would be 'nice'.

>> <more discussion about LED functionality in U-Boot ...>
>
> As you know, default "default-state" is "off".
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/common.yaml?h=v6.17#n74
>
> As far as I understand, there should not be any workarounds for specific 
> implementations.
>   https://lore.kernel.org/linux-rockchip/3389401.44csPzL39Z@phil/
>
> So removing `default-state = "off"` is acceptable, right?

I don't see/understand the connection with 'workarounds for specific
implementations' with removing ``default-state = "off"``.

IMO it's perfectly fine to remove ``default-state = "off"``, although
having it explicitly may be useful, especially if the commit that set
that property specified *why* it should be "off".

Relatedly, when a node does not have the 'default-state' property, I
would _assume_ the author wanted/intended it to be "off". Ideally it
would be described in the commit message, but that is optional.
But if that is changed, then it should be motivated *why*.

Cheers,
  Diederik

>
> Best regards,
>
> --
> FUKAUMI Naoki
> Radxa Computer (Shenzhen) Co., Ltd.
>
>> If one wants to detect via an LED the current boot stage (U-Boot 
>> reached, kernel started), then we need to NOT use LED_BOOT feature and 
>> have U-Boot set the "boot" LED the opposite state than the default HW 
>> state, i.e. if the LED is on without any running SW (power applied to 
>> the device, empty boot media), then U-Boot should set it to off. Then 
>> the kernel simply needs to start the heartbeat mode whenever ready. If 
>> the default HW state is off, then U-Boot should set it on. I haven't 
>> looked into the kernel side of things, but there could be a window 
>> during which default-state property is applied before the heartbeat is 
>> actually started.
>> 
>> The logic exposed in the previous paragraph should provide visual cues 
>> on the current boot stage.
>> 
>> Note that LEDs with linux,default-trigger = "pattern" (with default- 
>> state property) will be blinking once auto-configured in U-Boot as well 
>> according to my reading of the led-uclass.c.
>> 
>> Hope this helps.
>> 
>> Cheers,
>> Quentin
>>