[PATCH 1/6] event: signal when livetree has been built

[Caleb Connolly]

On 4/10/25 15:07, Simon Glass wrote:
> Hi Caleb,
> 
> On Thu, 10 Apr 2025 at 07:00, Caleb Connolly <caleb.connolly at 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.connolly at 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.connolly at 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.

> 
>>>>           return ret;
>>>>    }
>>>>
>>>>    void of_live_free(struct device_node *root)
>>>>
>>>> --
>>>> 2.49.0
>>>>
> 
> Regards,
> Simon

-- 
Caleb (they/them)