Hi On Sun, Aug 18, 2024 at 6:24 PM Christian Marangi <ansuels...@gmail.com> wrote: > > On Wed, Aug 14, 2024 at 10:17:18AM +0200, Michael Nazzareno Trimarchi wrote: > > Hi all > > > > On Wed, Aug 14, 2024 at 6:34 AM Heiko Schocher <h...@denx.de> wrote: > > > > > > Hello Christian, > > > > > > On 12.08.24 12:32, Christian Marangi wrote: > > > > 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 <ansuels...@gmail.com> > > > > --- > > > > cmd/ubi.c | 17 +++++++++++++++-- > > > > 1 file changed, 15 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/cmd/ubi.c b/cmd/ubi.c > > > > index 0e62e449327..6f679eae9c3 100644 > > > > --- a/cmd/ubi.c > > > > +++ b/cmd/ubi.c > > > > @@ -14,6 +14,7 @@ > > > > #include <command.h> > > > > #include <env.h> > > > > #include <exports.h> > > > > +#include <led.h> > > > > #include <malloc.h> > > > > #include <memalign.h> > > > > #include <mtd.h> > > > > @@ -488,10 +489,22 @@ exit: > > > > > > > > int ubi_volume_write(char *volume, void *buf, loff_t offset, size_t > > > > size) > > > > { > > > > + int ret; > > > > + > > > > +#ifdef CONFIG_LED_ACTIVITY_ENABLE > > > > + led_activity_blink(); > > > > +#endif > > > > > > Do we really need ifdef? May it is possible to declare an empty function > > > when CONFIG_LED_ACTIVITY_ENABLE is not set? May this applies for the whole > > > series? > > > > > > > + > > > > if (!offset) > > > > - return ubi_volume_begin_write(volume, buf, size, size); > > > > + ret = ubi_volume_begin_write(volume, buf, size, size); > > > > + else > > > > + ret = ubi_volume_offset_write(volume, buf, offset, size); > > > > > > > > - return ubi_volume_offset_write(volume, buf, offset, size); > > > > +#ifdef CONFIG_LED_ACTIVITY_ENABLE > > > > + led_activity_off(); > > > > +#endif > > > > + > > > > + return ret; > > > > } > > > > > > > > int ubi_volume_read(char *volume, char *buf, loff_t offset, size_t > > > > size) > > > > > > > > > I rather prefer to have some registration of events that need to be > > executed for > > a particular i/o activity and then a subscription process from led > > subsystem if that > > particular event is connected to the dts or just on a board file > > > > My concern is that it might become too complex just for the sake of > putting a LED intro a state. Do we have other case where such event > subsystem might be useful?
I was thinking of reusing the cyclic subsystem that allows you to subscribe to functions that are executed periodically. I mean it's not exciting to have function call everywhere, and anyway I think that #if defined(CONFIG_FOO) foo_activity #else foo_activity() { }; #endif This is my preference to not have it ENABLED everywhere. As I mentioned I even not have experience about having such needs in in code. Most can be implemented in a script except blinking like: led on; ext4load <> ; led off. We can definitely script most of it. The only exception can be led blink; ext4load <>; led off. > > Uboot is not really multi thread so we don't expect that much thing to > happen at the same time. Do we have case where an i/o might happen in > multiple place? Example transfering data and writing them at the same > time? The common practice is to first transfer and then handle. > Michael > > > > > > > bye, > > > Heiko > > > -- > > > DENX Software Engineering GmbH, Managing Director: Erika Unter > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > > > Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de > > > > > > > > -- > > Michael Nazzareno Trimarchi > > Co-Founder & Chief Executive Officer > > M. +39 347 913 2170 > > mich...@amarulasolutions.com > > __________________________________ > > > > Amarula Solutions BV > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > T. +31 (0)85 111 9172 > > i...@amarulasolutions.com > > www.amarulasolutions.com > > -- > Ansuel -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __________________________________ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com