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

[Sughosh Ganu]

On Tue, 8 Dec 2020 at 17:50, Tom Rini <trini at konsulko.com> wrote:

> On Tue, Dec 08, 2020 at 10:48:57AM +0530, Sughosh Ganu wrote:
> > On Tue, 8 Dec 2020 at 00:17, Tom Rini <trini at konsulko.com> wrote:
> >
> > > On Sat, Dec 05, 2020 at 11:31:49AM +0100, Heinrich Schuchardt 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>
> > > >
> > > > I can't see anything QEMU specific in this patch. Why is the code
> under
> > > > board/emulation/?
> > > >
> > > > 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
> > >
> > > First, CONFIG_SET_DFU_ALT_INFO is in Kconfig and needs to be enabled
> > > that way.
> >
> >
> > Will change in the next version.
> >
> >
> > > Second, typically we just set the information in the
> > > environment part of the header.  This is especially true if in both
> this
> > > case and the previous patch to add mtdparts, we don't really have this
> > > being dynamic?  Or are we really expecting / supporting arbitrary sized
> > > flash as this is qemu?  Thanks.
> > >
> >
> > I am not sure I understand this. One thing that can be done is to move
> the
> > setting of the mtd partitions to the Kconfig file, on similar lines to
> what
> > is being done for the st platforms, e.g MTDPARTS_NOR0_TEE. I can move the
> > mtd partition definitions to the Kconfig file if that is what you are
> > asking for.
>
> Environment manipulation via C code is possible, but discouraged.  What
> can be set via Kconfig should be set that way, and what can be put in
> the environment part of the header should be done that way.  Does that
> help clarify?
>

Yes, that does clarify. I will move the settings of the partitions used for
mtdparts in the Kconfig, similar to the way it is being done for the st
platforms. Thanks.

-sughosh