[PATCH 06/14] qemu: arm64: Set dfu_alt_info variable for the platform

[Heinrich Schuchardt]

On 12/7/20 6:42 AM, Sughosh Ganu wrote:
>
> On Sat, 5 Dec 2020 at 16:01, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at 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.ganu at linaro.org
>     <mailto:sughosh.ganu at 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
>      >
>      >
>