Re: [PATCH v2] efi_loader: Use sysreset instead of reset command

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 3/16/22 20:23, Simon Glass wrote:
> On Wed, 16 Mar 2022 at 06:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 3/16/22 09:03, Masami Hiramatsu wrote:
>>> Use sysreset_walk_halt() directly from reset-after-capsule-on-disk
>>> feature to reboot (cold reset) machine instead of using reset command
>>> interface, since this is not a command.
>>> Note that this will make CONFIG_EFI_CAPSULE_ON_DISK depending on
>>> the CONFIG_SYSRESET.
>>>
>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>>
>> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>
>>> ---
>>>    Changes in v2:
>>>     - Add CONFIG_SYSRESET dependency.
>>>     - Fix to add #include <sysreset.h>
>>> ---
>>>    lib/efi_loader/Kconfig       |    1 +
>>>    lib/efi_loader/efi_capsule.c |    5 +++--
>>>    2 files changed, 4 insertions(+), 2 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
>
>>>
>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
>>> index e5e35fe51f..33138237cc 100644
>>> --- a/lib/efi_loader/Kconfig
>>> +++ b/lib/efi_loader/Kconfig
>>> @@ -130,6 +130,7 @@ config EFI_RUNTIME_UPDATE_CAPSULE
>>>
>>>    config EFI_CAPSULE_ON_DISK
>>>        bool "Enable capsule-on-disk support"
>>> +     depends on SYSRESET

With this change xilinx_zynq_virt_defconfig fails to build:

arm-linux-gnueabi-ld.bfd: cmd/efidebug.o: in function
`do_efi_capsule_on_disk_update':
/home/zfsdt/workspace/u-boot-build/denx/cmd/efidebug.c:92: undefined
reference to `efi_launch_capsules'

Command 'efi capsule disk-update' should only be enabled if
CONFIG_EFI_CAPSULE_ON_DISK=y.

arm-linux-gnueabi-ld.bfd: lib/efi_loader/efi_setup.o: in function
`efi_init_obj_list':
/home/zfsdt/workspace/u-boot-build/denx/lib/efi_loader/efi_setup.c:272:
undefined reference to `efi_load_capsule_drivers'
make: *** [Makefile:1801: u-boot] Error 1

efi_load_capsule_drivers() must be available even if EFI_CAPSULE_ON_DISK
is not enabled to make CapsuleUpdate() work.

Please, add the missing changes.

Best regards

Heinrich

>>>        select EFI_HAVE_CAPSULE_SUPPORT
>>>        help
>>>          Select this option if you want to use capsule-on-disk feature,
>>> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
>>> index 613b531b82..a3d1d7e95a 100644
>>> --- a/lib/efi_loader/efi_capsule.c
>>> +++ b/lib/efi_loader/efi_capsule.c
>>> @@ -18,6 +18,7 @@
>>>    #include <malloc.h>
>>>    #include <mapmem.h>
>>>    #include <sort.h>
>>> +#include <sysreset.h>
>>>    #include <asm/global_data.h>
>>>
>>>    #include <crypto/pkcs7.h>
>>> @@ -1150,9 +1151,9 @@ efi_status_t efi_launch_capsules(void)
>>>         * UEFI spec requires to reset system after complete processing capsule
>>>         * update on the storage.
>>>         */
>>> -     log_info("Reboot after firmware update");
>>> +     log_info("Reboot after firmware update.\n");
>>>        /* Cold reset is required for loading the new firmware. */
>>> -     do_reset(NULL, 0, 0, NULL);
>>> +     sysreset_walk_halt(SYSRESET_COLD);
>>>        hang();
>>>        /* not reach here */
>>>
>>>
>>