Re: [PATCH v5 03/11] led: implement LED boot API

Simon Glass
2024-11-05T15:54:21Z

Hi Christian,

On Mon, 4 Nov 2024 at 16:13, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Sun, Nov 03, 2024 at 07:46:19AM -0700, Simon Glass wrote:
> > Hi Christian,
> >
> > On Sat, 2 Nov 2024 at 13:52, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > >
> > > On Sat, Nov 02, 2024 at 01:50:13PM -0600, Simon Glass wrote:
> > > > Hi Christian,
> > > >
> > > > On Sat, 2 Nov 2024 at 13:36, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > > > >
> > > > > On Sat, Nov 02, 2024 at 01:33:10PM -0600, Simon Glass wrote:
> > > > > > Hi Christian,
> > > > > >
> > > > > > On Wed, 2 Oct 2024 at 16:55, Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > On Tue, 1 Oct 2024 at 06:25, Christian Marangi <ansuelsmth@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Implement LED boot API to signal correct boot of the system.
> > > > > > > >
> > > > > > > > led_boot_on/off/blink() are introduced to turn ON, OFF and BLINK the
> > > > > > > > designated boot LED.
> > > > > > > >
> > > > > > > > New Kconfig is introduced, CONFIG_LED_BOOT to enable the feature.
> > > > > > > > This makes use of the /options/u-boot property "boot-led" to the
> > > > > > > > define the boot LED.
> > > > > > > > It's also introduced a new /options/u-boot property "boot-led-period"
> > > > > > > > to define the default period when the LED is set to blink mode.
> > > > > > >
> > > > > > > BTW could you please send a schema update for this so that Linux accepts it?
> > > > > >
> > > > > > I'm just checking that you did this?
> > > > > >
> > > > >
> > > > > Yep [0]. Also tagged Rob 2 hours ago. (eh the coincidence)
> > > > >
> > > > > Tell me if I should send it with usual way with a dedicated mail. (any
> > > > > hint on the mailing-list address?)
> > > >
> > > > OK great! Yes, once it lands, please send a patch to bring it into U-Boot
> > > >
> > > > Regards,
> > > > Simon
> > > >
> > > > > [0] https://github.com/devicetree-org/dt-schema/pull/144
> > >
> > > I'm confused what kind of patch I should send to U-Boot?
> >
> > Something to add the binding to U-Boot also. See my patch at [1]
> >
> > Regards,
> > Simon
> >
> > [1] https://patchwork.ozlabs.org/project/uboot/patch/20241103003322.626036-26-sjg@chromium.org/
>
> Ok thanks for the hint.
>
> Anyway Rob checked the patch (good news) but he request some changes
> (bad news)
>
> I created this PR [1] waiting for CI to confirm everything is ok.
>
> Would be good if you can check the changes while CI tests.

A few things:
- Better to return -ENOENT or -EINVAL if something is missing, since
we use -ENODEV in driver model to mean there is no device
- 'nodep' instead of 'node' for the parameter, so it is clear that it
is returned

Otherwise it looks good to me (with the proviso that's I'm just
looking at an emphemal github link!)

Regards,
Simon

>
> [1] https://github.com/u-boot/u-boot/pull/686
>
> --
> Ansuel