[PATCH] board: rpi: Enable capsule updates

[Matthias Brugger]

On 11/09/2024 12:05, Ilias Apalodimas wrote:
> On Wed, 11 Sept 2024 at 12:50, Peter Robinson <pbrobinson at gmail.com> wrote:
>>
>> On Tue, 10 Sept 2024 at 08:29, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>>>
>>> On Tue, 10 Sept 2024 at 12:09, Ilias Apalodimas
>>> <ilias.apalodimas at linaro.org> wrote:
>>>>
>>>> Since RPI works well using EFI and has no size limitations with regards
>>>> to U-Boot, add the needed structures and Kconfig options needed to
>>>> enable capsule updates
>>>> ---
>>>>   board/raspberrypi/rpi/rpi.c | 22 ++++++++++++++++++++++
>>>>   configs/rpi_4_defconfig     |  2 ++
>>>>   2 files changed, 24 insertions(+)
>>>
>>> Tested-by: Sughosh Ganu <sughosh.ganu at linaro.org>
>>>
>>> A couple of nits below.
>>>
>>>>
>>>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>>>> index ab5ea85cf9f8..1f342eee12b2 100644
>>>> --- a/board/raspberrypi/rpi/rpi.c
>>>> +++ b/board/raspberrypi/rpi/rpi.c
>>>> @@ -63,6 +63,28 @@ struct msg_get_clock_rate {
>>>>          u32 end_tag;
>>>>   };
>>>>
>>>> +struct efi_fw_image fw_images[] = {
>>>> +       {
>>>> +               .fw_name = u"RPI_UBOOT",
>>>> +               .image_index = 1,
>>>> +       },
>>>> +};
>>>> +
>>>> +struct efi_capsule_update_info update_info = {
>>>> +       .dfu_string = "mmc 0=u-boot.bin fat 0 1",
>>>> +       .num_images = ARRAY_SIZE(fw_images),
>>>> +       .images = fw_images,
>>>> +};
>>>> +
>>>> +#if IS_ENABLED(CONFIG_SET_DFU_ALT_INFO)
>>>> +void set_dfu_alt_info(char *interface, char *devstr)
>>>> +{
>>>> +       if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT))
>>>> +               env_set("dfu_alt_info", update_info.dfu_string);
>>>> +}
>>>> +#endif
>>>
>>> Is this really needed? We have a weak function in efi_firrmware.c
>>> which is doing exactly this.
>>>
>>>> +
>>>> +
>>>>   #ifdef CONFIG_ARM64
>>>>   #define DTB_DIR "broadcom/"
>>>>   #else
>>>> diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
>>>> index f5fb322aa8fc..c70414e6fcaf 100644
>>>> --- a/configs/rpi_4_defconfig
>>>> +++ b/configs/rpi_4_defconfig
>>>> @@ -65,3 +65,5 @@ CONFIG_SYS_WHITE_ON_BLACK=y
>>>>   CONFIG_VIDEO_BCM2835=y
>>>>   CONFIG_CONSOLE_SCROLL_LINES=10
>>>>   CONFIG_PHYS_TO_BUS=y
>>>> +CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
>>>> +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
>>>
>>> Can we also add the efidebug and efi nvedit commands here. They are
>>> pretty handy especially when it comes to capsule updates. Thanks.
>>
>> Are they pretty handy for capsule updates for general users when
>> things should "just work", as a user applies them from Linux with
>> fwupdmgr and reboots, or for firmwre developers trying to debug
>> things? If it's the later I don't think we should be enabling them by
>> default :)
> 
> nvedit lets you print and see EFI variables, which is pretty basic if
> you want to boot with efi.
> efidebug is supposed to be a debug tool mostly, but unfortunately, we
> havent plugged in EFI HTTP boot into the 'eficonfig' command yet. So
> the only way you can add a boot option for HTTP is via efidebug
> 

I don't really understand why you are mentioning EFI HTTP. From what I can see 
we need efidebug to be able to do capsule updates from U-Boot command line. It 
is also needed for EFI HTTP boot but this needs CONFIG_EFI_HTTP_BOOT which we 
don't enable (and I think is out-of-scope for capsule update).

/me is puzzled :)

Regards,
Matthias