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

[Tom Rini]

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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201208/f6a6772a/attachment.sig>