On Fri, Nov 17, 2023 at 04:35:22PM +0100, Heinrich Schuchardt wrote:

On 8/17/23 19:41, Tom Rini wrote:

...

The implementation of this option in lib/fwu_updates/fwu_gpt.c is wholly dependent on DFU, so make that a requirement here.

Signed-off-by: Tom Rini trini@konsulko.com

I'm honestly not sure how to best proceed here as drivers/fwu-mdata/gpt_blk.c doesn't need DFU, but both files are controlled by this single symbol.

Cc: Masami Hiramatsu masami.hiramatsu@linaro.org Cc: Jassi Brar jaswinder.singh@linaro.org Cc: Sughosh Ganu sughosh.ganu@linaro.org Cc: Ilias Apalodimas ilias.apalodimas@linaro.org Cc: Heinrich Schuchardt xypron.glpk@gmx.de

---

drivers/fwu-mdata/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/fwu-mdata/Kconfig b/drivers/fwu-mdata/Kconfig index 42736a5e43b0..0bd5fef4abfa 100644 --- a/drivers/fwu-mdata/Kconfig +++ b/drivers/fwu-mdata/Kconfig @@ -15,7 +15,7 @@ config FWU_MDATA_GPT_BLK bool "FWU Metadata access for GPT partitioned Block devices" select PARTITION_TYPE_GUID select PARTITION_UUIDS

• depends on FWU_MDATA && BLK && EFI_PARTITION

• depends on FWU_MDATA && BLK && EFI_PARTITION && DFU

Hello Tom,

This looks like the wrong level of configuration to add a DFU dependency.

EFI_CAPSULE_FIRMWARE_FIT and EFI_CAPSULE_FIRMWARE_RAW select DFU and EFI_CAPSULE_FIRMWARE.

We probably should let CONFIG_FWU_MDATA depend on EFI_CAPSULE_ON_DISK.

OK, but that doesn't address the dependency? The code in lib/fwu_updates/fwu_gpt.c needs DFU and is under FWU_MDATA_GPT_BLK. It provides fwu_plat_get_alt_num() which is called by fwu_get_image_index() and is under FWU_MULTI_BANK_UPDATE and in turn called by efi_firmware_raw_set_image() which is under EFI_CAPSULE_FIRMWARE_RAW. If both lib/fwu_updates/fwu_gpt.c and drivers/fwu-mdata/gpt_blk.c should be under the same symbol then it needs to depend on DFU, since it's a direct dependency in the code. We could change it up to EFI_CAPSULE_FIRMWARE_RAW I guess, but I'm not sure that's cleaner.

On 8/17/23 19:41, Tom Rini wrote:

Move where we include <dfu.h> to be guarded by a DFU-related Kconfig option.

Signed-off-by: Tom Rini trini@konsulko.com

This becomes an issue with https://patchwork.ozlabs.org/project/uboot/patch/20220620111354.448512-1-jh8... applied as now we can't include <dfu.h> without CONFIG_DFU_NAME_MAX_SIZE set (or we have a dummy value for ifndef). Another option would be to restructure the code to move the dfu code to another file, but I don't know if that will make the rest of the real use cases here easier or harder.

Cc: Michal Simek michal.simek@amd.com

board/xilinx/zynqmp/zynqmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index 309f24a5f43d..367c3c65a9dc 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -8,7 +8,6 @@ #include <command.h> #include <cpu_func.h> #include <debug_uart.h> -#include <dfu.h> #include <env.h> #include <env_internal.h> #include <init.h> @@ -606,6 +605,7 @@ enum env_location env_get_location(enum env_operation op, int prio) }

#if defined(CONFIG_SET_DFU_ALT_INFO) +#include <dfu.h>

#define DFU_ALT_BUF_LEN SZ_1K

Reviewed-by: Michal Simek michal.simek@amd.com

Thanks, Michal