[PATCH v2] arm64: dts: rockchip: Turn all LEDs on at boot for Radxa boards
FUKAUMI Naoki
naoki at radxa.com
Wed Nov 12 03:36:54 PST 2025
Hi all,
My goal is to minimize the DTS fragments
(u-boot/arch/arm/dts/*-u-boot.dtsi) in U-Boot, consolidate them
upstream, and improve clarity/visibility.
On 11/12/25 19:34, Quentin Schulz wrote:
> 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.)
I'm not saying the DTS has anything about LEDs that can't be controlled
by software, nor am I trying to add such a thing to the DTS.
I'm just pointing out that the power LED is always on right after
power-up. This makes it useless for determining if the software is running.
>>>>>> 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.
Each software should be independent, but hardware (state) cannot be
independent.
> 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).
If you are using U-Boot, heartbeat LED is already on by U-Boot,
e.g.
https://source.denx.de/u-boot/u-boot/-/blob/v2025.10/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi#L10-12
But it's not visible in DTS in Linux,
e.g.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts?h=v6.17#n55
I think this situation should be fixed.
Best regards,
--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.
>> 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
>
More information about the Linux-rockchip
mailing list