Hi Heinrich,
[...]
>>> +static const struct udevice_id efi_bootdev_ids[] = {
>>> + { .compatible = "u-boot,bootdev-efi" },
>>> + { }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(efi_bootdev) = {
>>> + .name = "efi_bootdev",
>>> + .id = UCLASS_BOOTDEV,
>>> + .ops = &efi_bootdev_ops,
>>> + .bind = efi_bootdev_bind,
>>> + .of_match = efi_bootdev_ids,
>>> +};
>>> +
>>> +/**
>>> + * efi_bootdev_hunt() - find devices used by EFI boot manager
>>> + *
>>> + * Invoke all hunters that have not been invoked by now.
>>> + * This should only be the network hunter.
>>> + *
>>> + * @info: info structure describing this hunter
>>> + * @show: true to show information from the hunter
>>> + *
>>> + * Return: 0 if device found, -EINVAL otherwise
>>> + */
>>> +static int efi_bootdev_hunt(struct bootdev_hunter *info, bool show)
>>> +{
>>> + return bootdev_hunt(NULL, 0);
>>
>> This recursive calling of hunting is very strange. Since bootmgr seems
>> to need all the hunting to be done, you could move this line to
>> somewhere in bootmgr.
>
> Thank you Simon for reviewing.
>
> I added patch 5/8 to avoid infinite recursion.
>
> Maybe we should move this to efi_init_obj_list() because in the
> eficonfig and and efidebug command we also assume that all boot devices
> have been hunted down. If a device has not been probed you cannot create
> a boot option for it.
I don't think teaching the EFI subsystem internals of a Kconfig option is the
right thing to do.
What happens if someone compiles without that? Can't we have a generic function
somewhere that does
that? And then we can decide when to call it, efi_init_obj_list() of the
commands themselves
explicitly.
Thanks
/Ilias
>
>>
>>> +}
>>> +
>>> +BOOTDEV_HUNTER(efi_bootdev_hunt) = {
>>> + .prio = BOOTDEVP_6_EFI,
>>> + .uclass = UCLASS_EFI_BOOT_MGR,
>>> + .hunt = efi_bootdev_hunt,
>>> + .drv = DM_DRIVER_REF(efi_bootdev),
>>> +};
>>> +
>>> +static int efi_bootdev_create(void)
>>> +{
>>> + int ret;
>>> + struct udevice *dev_mgr;
>>> + struct udevice *dev;
>>> +
>>> + ret = device_bind_driver(gd->dm_root, "efimgr",
>>> + "efimgr", &dev_mgr);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = device_bind_driver(dev_mgr, "efi_bootdev",
>>> + "efimgr.bootdev", &dev);
>>> + return ret;
>>> +}
>>> +
>>> +EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_R, efi_bootdev_create);
>>> +
>>> +U_BOOT_DRIVER(efimgr) = {
>>> + .name = "efimgr",
>>> + .id = UCLASS_EFI_BOOT_MGR,
>>> +};
>>> +
>>> +UCLASS_DRIVER(efimgr) = {
>>> + .id = UCLASS_EFI_BOOT_MGR,
>>> + .name = "efimgr",
>>> +};
>>> --
>>> 2.48.1
>>>
>>
>> I am not seeing any tests in this series. At least we should update
>> the bootflow_efi() test or add something similar.
>
> I have not looked into the tests because I first wanted to have feedback
> if the general direction is ok.
>
> Unfortunately most bootstd tests are checking console output line by
> line instead of functional tests which makes them fail now.
>
> The bootstd tests should better have been written in a way that would
> allow running them on real boards without making assumptions about the
> board.
>
> Best regards
>
> Heinrich
