On 12/7/20 6:42 AM, Sughosh Ganu wrote:
On Sat, 5 Dec 2020 at 16:01, Heinrich Schuchardt <xypron.g...@gmx.de
<mailto:xypron.g...@gmx.de>> wrote:
On 11/26/20 7:41 PM, Sughosh Ganu wrote:
> The dfu framework uses the dfu_alt_info environment variable to get
> information that is needed for performing the firmware update.
Set the
> dfu_alt_info for the platform to reflect the two mtd partitions
> created for the u-boot env and the firmware image.
>
> Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org
<mailto:sughosh.g...@linaro.org>>
I can't see anything QEMU specific in this patch. Why is the code under
board/emulation/?
The device that the board wants to use to set the dfu_alt_info to would
be very much board specific. For example, the dfu_alt_info is being set
for the qemu platform for an mtd as the interface, with nor0 as the
device. Different platforms might have different requirements, like the
setting of the variable done for st
platforms(board/st/common/stm32mp_dfu.c). I think this information is
very much platform specific.
In the function below you simply collect MTD partitions. Isn't this
something that could be reused for other boards?
On other boards (e.g. board/samsung/common/misc.c)
CONFIG_SET_DFU_ALT_INFO controls if set_dfu_alt_info() exists. Should
the same be done for QEMU ARM?
Reading through patch 14/14 it is unclear to me how you control to which
partition you write TF-A, SPL, U-Boot depending on which of these exists
on the MTD device.
Wouldn't it make more sense to encode in the capsule to where it will be
written?
Best regards
Heinrich
-sughosh
Best regards
Heinrich
> ---
> board/emulation/qemu-arm/qemu-arm.c | 55
+++++++++++++++++++++++++++++
> include/configs/qemu-arm.h | 1 +
> 2 files changed, 56 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c
b/board/emulation/qemu-arm/qemu-arm.c
> index d5ed3eebaf..8cad54c76f 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -200,8 +200,63 @@ void flash_write32(u32 value, void *addr)
>
> #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
>
> +#include <memalign.h>
> #include <mtd.h>
>
> +#define MTDPARTS_LEN 256
> +#define MTDIDS_LEN 128
> +
> +#define DFU_ALT_BUF_LEN SZ_1K
> +
> +static void board_get_alt_info(struct mtd_info *mtd, char *buf)
> +{
> + struct mtd_info *part;
> + bool first = true;
> + const char *name;
> + int len, partnum = 0;
> +
> + name = mtd->name;
> + len = strlen(buf);
> +
> + if (buf[0] != '\0')
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, "&");
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> + "mtd %s=", name);
> +
> + list_for_each_entry(part, &mtd->partitions, node) {
> + partnum++;
> + if (!first)
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN
- len, ";");
> + first = false;
> +
> + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
> + "%s part %d",
> + part->name, partnum);
> + }
> +}
> +
> +void set_dfu_alt_info(char *interface, char *devstr)
> +{
> + struct mtd_info *mtd;
> +
> + ALLOC_CACHE_ALIGN_BUFFER(char, buf, DFU_ALT_BUF_LEN);
> +
> + if (env_get("dfu_alt_info"))
> + return;
> +
> + memset(buf, 0, sizeof(buf));
> +
> + /* probe all MTD devices */
> + mtd_probe_devices();
> +
> + mtd = get_mtd_device_nm("nor0");
> + if (!IS_ERR_OR_NULL(mtd))
> + board_get_alt_info(mtd, buf);
> +
> + env_set("dfu_alt_info", buf);
> + printf("dfu_alt_info set\n");
> +}
> +
> static void board_get_mtdparts(const char *dev, const char
*partition,
> char *mtdids, char *mtdparts)
> {
> diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> index 69ff329434..726f985d35 100644
> --- a/include/configs/qemu-arm.h
> +++ b/include/configs/qemu-arm.h
> @@ -33,6 +33,7 @@
> #include <config_distro_bootcmd.h>
>
> #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_MANAGEMENT)
> +#define CONFIG_SET_DFU_ALT_INFO
> #define CONFIG_SYS_MTDPARTS_RUNTIME
> #endif
>
>
