Hi Heiko, On Thu, Oct 2, 2025 at 1:30 PM Heiko Schocher <[email protected]> wrote: > > Hello Yegor, > > On 02.10.25 13:28, Yegor Yefremov wrote: > > 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 :-) > > From my side: Full Ack. > > May we should ask "Christian Marangi <[email protected]>" > if there are some special reasons for only adding it to the write > command.
I have just sent a patch and included Christian Marangi in CC. Yegor
