Hi Heiko, On Thu, Oct 2, 2025 at 10:26 AM Heiko Schocher <[email protected]> wrote: > > 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 <[email protected]> wrote: > >> > >> Hi Yegor, > >> > >> On Tue, 30 Sept 2025 at 08:48, Yegor Yefremov > >> <[email protected]> wrote: > >>> > >>> Hi Simon, > >>> > >>> On Tue, Sep 30, 2025 at 4:13 PM Simon Glass <[email protected]> wrote: > >>>> > >>>> Hi Yegor, > >>>> > >>>> On Tue, 30 Sept 2025 at 06:35, Yegor Yefremov > >>>> <[email protected]> wrote: > >>>>> > >>>>> On Tue, Sep 30, 2025 at 12:29 PM Yegor Yefremov > >>>>> <[email protected]> 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 <[email protected]> > 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 <[email protected]> > Reviewed-by: Simon Glass <[email protected]> > """ > > So, at that time it was intentional ... yes ... but I ask me, why > we do not use this LED also on read operations...
So, in other words, patches are welcome :-) Regards, Yegor
