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

[Christian Marangi <ansuelsmth@gmail.com>]

On Tue, Nov 05, 2024 at 08:12:47AM -0700, Simon Glass wrote:
> 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!)
>

Thanks a lot. Fighting a bit with the tests but thanks for the feedback.

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

-- 
	Ansuel