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

[Quentin Schulz]

Hi all,

On 11/12/25 10:40 AM, Diederik de Haas wrote:
> [You don't often get email from diederik at cknow-tech.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> 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.
>>>>>

I think the wording in the binding can be understood two ways.

The binding says the following about the default-state property:

"""
       The initial state of the LED. If the LED is already on or off and the
       default-state property is set the to same value, then no glitch 
should be
       produced where the LED momentarily turns off (or on). The "keep" 
setting
       will keep the LED at whatever its current state is, without 
producing a
       glitch.
"""

I think the issue here is around the meaning of "initial state". I 
believe Naoki is probably thinking about the **HW** initial state of the 
LED, which is whatever is the state of the LED without SW control. I 
think Diederik is thinking about this being the state of the LED right 
when the SW takes over and configures the LED before the trigger is setup.

In the first interpretation, there's no need for an "improvement" for 
the patches as they would just fix correctness of the DT wrt HW state at 
boot.

In the second interpretation, a change of this value must be justified 
as people will simply disagree forever and we could end up with people 
reverting other people's patches after each release. If it's just a 
matter of taste, I believe the typical answer is keeping the status quo.

We should find a way to make this binding not up to interpretation.

Additionally, if the LED cannot be controlled on some boards, I don't 
think it should be part of the DT.

>>>>> - 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.
>>>>>

My understanding is Naoki wants to use default-state = on, for the 
bootloader to turn it on as soon as it takes over control of the LEDs.

>>>>> 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.
> 

The solution may still involve configuring the Device Tree, and we're 
trying to have U-Boot-specific changes to the Device Tree in U-Boot 
source tree to a minimum.

> I still consider the bootloader and the kernel stages separate.

They do however share most of their Device Tree (for Rockchip at least) 
and the least (ideally no) changes we can have in U-Boot the better.

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

Device Tree is not kernel specific as you said already.

> I don't think that what U-Boot does or doesn't do, should determine what
> the Linux kernel does or doesn't do.

It shouldn't, but (most of) the Device Tree is shared, so you cannot 
just dismiss U-Boot behavior when talking about Linux behavior based on 
Device Tree interpretation. We may have a need for a bootloader-specific 
property. We have a Linux-specific one after all 
(linux,default-trigger). Though... that does seem to be on the edge of 
what the DT is made for (description of the HW, not logic/policy).

> 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.
> 

Barebox also uses upstream DT as far as I know and supports some Radxa 
products (Rock 5B/5T/..., CM3, Rock (RK3188), Rock 3A from the 
arch/arm/boards/radxa-* directories). Zephyr has support for RK3568, 
RK3588, and other SoCs, and uses upstream DT as well.

Again, we're talking about modifications of the Device Tree here, so 
typically I would expect all consumers of that DT to be interpreting the 
properties the same way, except if you have OS-specific properties/nodes 
(think u-boot,config-compatible nodes, linux, prefixed properties, 
bootph- properties, ...).

> 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 ...)
> 

If the *HW* default state of the LED is off and the default-state 
property is off, then you won't be able to tell apart a completely 
bricked board and one that is stuck somewhere between U-Boot proper and 
the Linux kernel taking over that LED.

> 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".
> 

The status property defaults to okay, and we do not want them to be 
listed explicitly. Not sure if there's consensus on applying this to all 
properties which have defaults, across all subsystems.

> 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.

The lack of a property doesn't necessarily mean it was forgotten, agreed.

> But if that is changed, then it should be motivated *why*.
> 

Agreed.

Cheers,
Quentin