Hi Caleb,
On Thu, 10 Apr 2025 at 07:00, Caleb Connolly <caleb.conno...@linaro.org> wrote:
>
> Hi Simon,
>
> On 4/10/25 13:27, Simon Glass wrote:
> > Hi Caleb,
> >
> > On Wed, 9 Apr 2025 at 11:17, Caleb Connolly <caleb.conno...@linaro.org>
> > wrote:
> >>
> >> OF_LIVE offers a variety of benefits, one of them being that the live
> >> tree can be modified without caring about the underlying FDT. This is
> >> particularly valuable for working around U-Boot limitations like lacking
> >> USB superspeed support on Qualcomm platforms, no runtime OTG, or
> >> peripherals like the sdcard being broken (and displaying potentially
> >> worrying error messages).
> >>
> >> Add an event to signal when the live tree has been built so that we can
> >> apply fixups to it directly before devices are bound.
> >>
> >> Signed-off-by: Caleb Connolly <caleb.conno...@linaro.org>
> >> ---
> >> common/event.c | 3 +++
> >> include/event.h | 9 +++++++++
> >> lib/of_live.c | 3 +++
> >> 3 files changed, 15 insertions(+)
> >>
> >> diff --git a/common/event.c b/common/event.c
> >> index
> >> dda569d447851f559a83f98fb7b1f3543156eab5..8d7513eb10b61919e1e784481dfdcc076be14986
> >> 100644
> >> --- a/common/event.c
> >> +++ b/common/event.c
> >> @@ -47,8 +47,11 @@ const char *const type_name[] = {
> >> "ft_fixup",
> >>
> >> /* main loop events */
> >> "main_loop",
> >> +
> >> + /* livetree has been built */
> >> + "of_live_init",
> >> };
> >>
> >> _Static_assert(ARRAY_SIZE(type_name) == EVT_COUNT, "event type_name
> >> size");
> >> #endif
> >> diff --git a/include/event.h b/include/event.h
> >> index
> >> 75141a192a48b0931667632f41be8ff4d6139f7c..3fc673ba635ed45467aae8587705d37bef1c2a3f
> >> 100644
> >> --- a/include/event.h
> >> +++ b/include/event.h
> >> @@ -152,8 +152,17 @@ enum event_t {
> >> * A non-zero return value causes the boot to fail.
> >> */
> >> EVT_MAIN_LOOP,
> >>
> >> + /**
> >> + * @EVT_OF_LIVE_INIT:
> >> + * This event is triggered immediately after the live device tree
> >> has been
> >> + * built. This allows for machine specific fixups to be done to
> >> the live tree
> >> + * (like disabling known-unsupported devices) before DM init
> >> happens. This
> >> + * event is only available if OF_LIVE is enabled and is only used
> >> after relocation.
> >> + */
> >> + EVT_OF_LIVE_INIT,
> >> +
> >> /**
> >> * @EVT_COUNT:
> >> * This constants holds the maximum event number + 1 and is used
> >> when
> >> * looping over all event classes.
> >> diff --git a/lib/of_live.c b/lib/of_live.c
> >> index
> >> 90b9459ede313e492e28c8556c730f3bd8aaa9df..e1962b8f1fb9d8c2c87d04ca4e238a1e4d00376a
> >> 100644
> >> --- a/lib/of_live.c
> >> +++ b/lib/of_live.c
> >> @@ -10,8 +10,9 @@
> >>
> >> #define LOG_CATEGORY LOGC_DT
> >>
> >> #include <abuf.h>
> >> +#include <event.h>
> >> #include <log.h>
> >> #include <linux/libfdt.h>
> >> #include <of_live.h>
> >> #include <malloc.h>
> >> @@ -334,8 +335,10 @@ int of_live_build(const void *fdt_blob, struct
> >> device_node **rootp)
> >> return ret;
> >> }
> >> debug("%s: stop\n", __func__);
> >>
> >> + event_notify_null(EVT_OF_LIVE_INIT);
> >> +
> >
> > This should go in initr_of_live() since the function you are dealing
> > with here is supposed to just do the live build.
>
> Well, we only every call this function from one place right now, but if
> it was called multiple times for some reason then I would want to be
> able to re-apply fixups to the new live tree.... I guess it should
> probably pass in *rootp to the event handler, let me rework that.>
There's no need to change the root, so what you have is find here.
> > Same for the EFI_STUB thing which I just noticed, actually
>
> what EFI_STUB thing?>
Oh, it's your EFI stub patch which isn't in Tom's tree yet. Just for
when you get to it, then.
> > Also please check for error
> >
> > Otherwise this seems OK to me. I do wonder why we can't use
> > EVT_FT_FIXUP though. Could you add mention of that to your comment in
> > event.h?
>
> Because FT_FIXUP is for fixing up the flat tree before starting the OS?
> these are obviously different things imo, im not sure how i could
> clarify this.>
Well, what is the purpose of your code? Are you saying that it is used
within U-Boot, but not passed to the OS?
> >> return ret;
> >> }
> >>
> >> void of_live_free(struct device_node *root)
> >>
> >> --
> >> 2.49.0
> >>
Regards,
Simon
