Hi Caleb,
On Thu, 10 Apr 2025 at 08:04, Caleb Connolly <caleb.conno...@linaro.org> wrote:
>
>
>
> On 4/10/25 15:07, Simon Glass wrote:
> > 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?
>
> Yes, please read the cover letter and commit messages. FT_FIXUP allows
> for the FDT that is about to be passed to the OS to be fixed up,
> OF_LIVE_INIT signifies that U-Boot has finished building it's livetree.
> The livetree is obviously not used outside of U-Boot, being a totally
> custom in-memory representation of the DT.
Oh, I wondered what '(which is not passed on to further boot stages)' meant.
Sorry, no, we can't do that. We are moving towards using livetree in
U-Boot - there is already the fixup thing as mentioned. We just don't
yet have the flattening at the end before boot. If you have time you
could look at that.
Your event to modify the livetree is fine, but any modifications
should be passed on to the OS. If you don't want that, then your fixup
would need to remove them, I suppose. Does the OS actually care,
though?
Regards,
Simon
