activity-led usage

[Heiko Schocher]

Hello Yegor,

On 02.10.25 10:22, Yegor Yefremov wrote:
> Hi Simon and Marek,
> 
> On Wed, Oct 1, 2025 at 1:09 AM Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Yegor,
>>
>> On Tue, 30 Sept 2025 at 08:48, Yegor Yefremov
>> <yegorslists at googlemail.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On Tue, Sep 30, 2025 at 4:13 PM Simon Glass <sjg at chromium.org> wrote:
>>>>
>>>> Hi Yegor,
>>>>
>>>> On Tue, 30 Sept 2025 at 06:35, Yegor Yefremov
>>>> <yegorslists at googlemail.com> wrote:
>>>>>
>>>>> On Tue, Sep 30, 2025 at 12:29 PM Yegor Yefremov
>>>>> <yegorslists at googlemail.com> wrote:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> I added the following LED definition to the am335x-baltos.dts:
>>>>>>
>>>>>>          leds {
>>>>>>                  pinctrl-names = "default";
>>>>>>                  pinctrl-0 = <&user_leds>;
>>>>>>
>>>>>>                  compatible = "gpio-leds";
>>>>>>
>>>>>>                  led-power {
>>>>>>                          label = "led-red";
>>>>>>                          gpios = <&gpio3 0 GPIO_ACTIVE_LOW>;
>>>>>>                          default-state = "on";
>>>>>>                  };
>>>>>>          };
>>>>>>          user_leds: user-leds-pins {
>>>>>>                  pinctrl-single,pins = <
>>>>>>                          AM33XX_PADCONF(AM335X_PIN_MII1_COL,
>>>>>> PIN_OUTPUT_PULLDOWN, MUX_MODE7)     /* mii1_col.gpio3_0 PWR LED */
>>>>>>                  >;
>>>>>>          };
>>>>>>
>>>>>> The gpio-leds driver is working and I can toggle the led-red via the
>>>>>> led command.
>>>>>>
>>>>>> Then, I created an am335x-baltos-u-boot.dtsi file with the following content:
>>>>>>
>>>>>> / {
>>>>>>      config {
>>>>>>          u-boot,activity-led = "led-red";
>>>>>>      };
>>>>>> };
>>>>>>
>>>>>> I have also tried "led-power" instead of "led-red".
>>>>>>
>>>>>> In my final DTS files I have the following nodes:
>>>>>>
>>>>>>          leds {
>>>>>>                  pinctrl-names = "default";
>>>>>>                  pinctrl-0 = <0x56>;
>>>>>>                  compatible = "gpio-leds";
>>>>>>
>>>>>>                  led-power {
>>>>>>                          label = "led-red";
>>>>>>                          gpios = <0x57 0x00 0x01>;
>>>>>>                          default-state = "on";
>>>>>>                  };
>>>>>>          };
>>>>>>
>>>>>>          config {
>>>>>>                  u-boot,activity-led = "led-red";
>>>>>>          };
>>>>>>
>>>>>> U-Boot cannot find the activity-led node.
>>>>>> ofnode_options_get_by_phandle("activity-led", &led_node); returns -22,
>>>>>> because ofnode_path("/options/u-boot") fails.
>>>>>>
>>>>>> When driver debug is enabled, I also see the following messages:
>>>>>>
>>>>>> bind node config
>>>>>> Device 'config' has no compatible string
>>>>>>
>>>>>> My am335x-baltos-u-boot.dtsi code is similar to what I could find in
>>>>>> arch/arm/dts for the boot-led usage.
>>>>>>
>>>>>> Where should I debug further?
>>>>
>>>> It is a bit confusing as there is still 'legacy' LED stuff in the
>>>> tree. For your problem I would suggest checking that the LED device is
>>>> configured correctly.
>>>>
>>>> For sandbox in test.dts you can see:
>>>>
>>>>      options {
>>>>          u-boot {
>>>>              compatible = "u-boot,config";
>>>>              boot-led = <&sandbox_led_green>;
>>>>              activity-led = <&sandbox_led_red>;
>>>> ...
>>>>          };
>>>>      };
>>>>
>>>> Check that led_init() is being called in the LED uclass.
>>>
>>> It is working now using your suggestion. Thanks.
>>>
>>>>>
>>>>> Should activity-led be also mentioned in doc/device-tree-bindings/config.txt?
>>>>
>>>> We should be using /options/u-boot for this[1]
>>>
>>> This means, everything that can be found via the following command
>>> won't work and should be removed?
>>>
>>> grep -r --include='*.dts[i]' boot-led arch/arm/dts
>>
>> Yes. I did a series to remove the legacy LED stuff last year but it
>> wasn't applied because people wanted me to do more, probably what you
>> are looking at doing now :-)
> 
> This would be great if this patch series could be merged. Marek has a
> board (arch/arm/dts/stm32mp157a-dhcor-avenger96-u-boot.dtsi) that can
> be reworked too.
> 
> AFAIK, led_activity_blink() is used very seldom. For example, in
> cmd/ubi.c it is only used for writing [1]. mmc is omitted completely.
> Is this intentional or is this functionality just missing?
> 
> [1] https://github.com/u-boot/u-boot/blob/master/cmd/ubi.c#L494

The commit which introduced this says:
"""
commit 990f726ce797c040adda6d1e1bc6a21bd2b28657
Author: Christian Marangi <ansuelsmth at gmail.com>
Date:   Tue Oct 1 14:24:41 2024 +0200

     ubi: implement support for LED activity

     Implement support for LED activity. If the feature is enabled,
     make the defined ACTIVITY LED to signal ubi write operation.

     Signed-off-by: Christian Marangi <ansuelsmth at gmail.com>
     Reviewed-by: Simon Glass <sjg at chromium.org>
"""

So, at that time it was intentional ... yes ... but I ask me, why
we do not use this LED also on read operations...

bye,
Heiko
> 
> Regards,
> Yegor
> 

-- 
Nabla Software Engineering
HRB 40522 Augsburg
Phone: +49 821 45592596
E-Mail: office at nabladev.com
Geschäftsführer : Stefano Babic