[U-Boot] [PATCH v7 10/13] cmd: mtd: add 'mtd' command

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Jagan,

Jagan Teki <jagan@amarulasolutions.com> wrote on Mon, 3 Sep 2018
13:34:21 +0530:

> On Mon, Sep 3, 2018 at 12:24 PM, Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> > Hi Stefan,
> >
> > Stefan Roese <sr@denx.de> wrote on Sat, 1 Sep 2018 10:59:56 +0200:
> >  
> >> On 31.08.2018 16:57, Miquel Raynal wrote:  
> >> > There should not be a 'nand' command, a 'sf' command and certainly not
> >> > a new 'spi-nand' command. Write a 'mtd' command instead to manage all
> >> > MTD devices/partitions at once. This should be the preferred way to
> >> > access any MTD device.  
> >> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>  
> >> > Acked-by: Jagan Teki <jagan@openedev.com>
> >> > ---
> >> >   cmd/Kconfig          |  10 +-
> >> >   cmd/Makefile         |   1 +
> >> >   cmd/mtd.c            | 517 +++++++++++++++++++++++++++++++++++++++++++
> >> >   drivers/mtd/Makefile |   2 +-
> >> >   include/mtd.h        |   1 +
> >> >   5 files changed, 528 insertions(+), 3 deletions(-)
> >> >   create mode 100644 cmd/mtd.c  
> >> > > diff --git a/cmd/Kconfig b/cmd/Kconfig  
> >> > index ef43ed8dda..0778d2ecff 100644
> >> > --- a/cmd/Kconfig
> >> > +++ b/cmd/Kconfig
> >> > @@ -847,6 +847,12 @@ config CMD_MMC_SWRITE
> >> >       Enable support for the "mmc swrite" command to write Android sparse
> >> >       images to eMMC.  
> >> >   > +config CMD_MTD  
> >> > +   bool "mtd"
> >> > +   select MTD_PARTITIONS
> >> > +   help
> >> > +     MTD commands support.
> >> > +
> >> >   config CMD_NAND
> >> >     bool "nand"
> >> >     default y if NAND_SUNXI
> >> > @@ -1671,14 +1677,14 @@ config CMD_MTDPARTS  
> >> >   >   config MTDIDS_DEFAULT  
> >> >     string "Default MTD IDs"
> >> > -   depends on CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> >> > +   depends on CMD_MTD || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> >> >     help
> >> >       Defines a default MTD IDs list for use with MTD partitions in the
> >> >       Linux MTD command line partitions format.  
> >> >   >   config MTDPARTS_DEFAULT  
> >> >     string "Default MTD partition scheme"
> >> > -   depends on CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> >> > +   depends on CMD_MTD || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> >> >     help
> >> >       Defines a default MTD partitioning scheme in the Linux MTD command
> >> >       line partitions format
> >> > diff --git a/cmd/Makefile b/cmd/Makefile
> >> > index 323f1fd2c7..32fd102189 100644
> >> > --- a/cmd/Makefile
> >> > +++ b/cmd/Makefile
> >> > @@ -90,6 +90,7 @@ obj-$(CONFIG_CMD_MISC) += misc.o
> >> >   obj-$(CONFIG_CMD_MMC) += mmc.o
> >> >   obj-$(CONFIG_CMD_MMC_SPI) += mmc_spi.o
> >> >   obj-$(CONFIG_MP) += mp.o
> >> > +obj-$(CONFIG_CMD_MTD) += mtd.o
> >> >   obj-$(CONFIG_CMD_MTDPARTS) += mtdparts.o
> >> >   obj-$(CONFIG_CMD_NAND) += nand.o
> >> >   obj-$(CONFIG_CMD_NET) += net.o
> >> > diff --git a/cmd/mtd.c b/cmd/mtd.c
> >> > new file mode 100644
> >> > index 0000000000..32295fe86c
> >> > --- /dev/null
> >> > +++ b/cmd/mtd.c
> >> > @@ -0,0 +1,517 @@
> >> > +// SPDX-License-Identifier:  GPL-2.0+
> >> > +/*
> >> > + * mtd.c
> >> > + *
> >> > + * Generic command to handle basic operations on any memory device.
> >> > + *
> >> > + * Copyright: Bootlin, 2018
> >> > + * Author: Miquèl Raynal <miquel.raynal@bootlin.com>
> >> > + */
> >> > +
> >> > +#include <command.h>
> >> > +#include <common.h>
> >> > +#include <console.h>
> >> > +#include <linux/mtd/mtd.h>
> >> > +#include <linux/mtd/partitions.h>
> >> > +#include <malloc.h>
> >> > +#include <mapmem.h>
> >> > +#include <mtd.h>
> >> > +#include <dm/device.h>
> >> > +#include <dm/uclass-internal.h>
> >> > +
> >> > +#define MTD_NAME_MAX_LEN 20
> >> > +
> >> > +static char *old_mtdparts;
> >> > +
> >> > +static void mtd_dump_buf(u8 *buf, uint len, uint offset)
> >> > +{
> >> > +   int i, j;
> >> > +
> >> > +   for (i = 0; i < len; ) {
> >> > +           printf("0x%08x:\t", offset + i);
> >> > +           for (j = 0; j < 8; j++)
> >> > +                   printf("%02x ", buf[i + j]);
> >> > +           printf(" ");
> >> > +           i += 8;
> >> > +           for (j = 0; j < 8; j++)
> >> > +                   printf("%02x ", buf[i + j]);
> >> > +           printf("\n");
> >> > +           i += 8;
> >> > +   }
> >> > +}
> >> > +
> >> > +static void mtd_show_parts(struct mtd_info *mtd, int level)
> >> > +{
> >> > +   struct mtd_info *part;
> >> > +   int i;
> >> > +
> >> > +   if (list_empty(&mtd->partitions))
> >> > +           return;
> >> > +
> >> > +   list_for_each_entry(part, &mtd->partitions, node) {
> >> > +           for (i = 0; i < level; i++)
> >> > +                   printf("\t");
> >> > +           printf("* %s\n", part->name);
> >> > +           for (i = 0; i < level; i++)
> >> > +                   printf("\t");
> >> > +           printf("  > Offset: 0x%llx bytes\n", part->offset);
> >> > +           for (i = 0; i < level; i++)
> >> > +                   printf("\t");
> >> > +           printf("  > Size: 0x%llx bytes\n", part->size);
> >> > +
> >> > +           mtd_show_parts(part, level + 1);
> >> > +   }
> >> > +}
> >> > +
> >> > +static void mtd_show_device(struct mtd_info *mtd)
> >> > +{
> >> > +   /* Device */
> >> > +   printf("* %s", mtd->name);
> >> > +   if (mtd->dev)
> >> > +           printf(" [device: %s] [parent: %s] [driver: %s]",
> >> > +                  mtd->dev->name, mtd->dev->parent->name,
> >> > +                  mtd->dev->driver->name);
> >> > +   printf("\n");
> >> > +
> >> > +   /* MTD device information */
> >> > +   printf("  > type: ");
> >> > +   switch (mtd->type) {
> >> > +   case MTD_RAM:
> >> > +           printf("RAM\n");
> >> > +           break;
> >> > +   case MTD_ROM:
> >> > +           printf("ROM\n");
> >> > +           break;
> >> > +   case MTD_NORFLASH:
> >> > +           printf("NOR flash\n");
> >> > +           break;
> >> > +   case MTD_NANDFLASH:
> >> > +           printf("NAND flash\n");
> >> > +           break;
> >> > +   case MTD_DATAFLASH:
> >> > +           printf("Data flash\n");
> >> > +           break;
> >> > +   case MTD_UBIVOLUME:
> >> > +           printf("UBI volume\n");
> >> > +           break;
> >> > +   case MTD_MLCNANDFLASH:
> >> > +           printf("MLC NAND flash\n");
> >> > +           break;
> >> > +   case MTD_ABSENT:
> >> > +   default:
> >> > +           printf("Unknown\n");
> >> > +           break;
> >> > +   }
> >> > +
> >> > +   printf("  > Size: 0x%llx bytes\n", mtd->size);
> >> > +   printf("  > Block: 0x%x bytes\n", mtd->erasesize);
> >> > +   printf("  > Min I/O: 0x%x bytes\n", mtd->writesize);
> >> > +
> >> > +   if (mtd->oobsize) {
> >> > +           printf("  > OOB size: %u bytes\n", mtd->oobsize);
> >> > +           printf("  > OOB available: %u bytes\n", mtd->oobavail);
> >> > +   }
> >> > +
> >> > +   if (mtd->ecc_strength) {
> >> > +           printf("  > ECC strength: %u bits\n", mtd->ecc_strength);
> >> > +           printf("  > ECC step size: %u bytes\n", mtd->ecc_step_size);
> >> > +           printf("  > Bitflip threshold: %u bits\n",
> >> > +                  mtd->bitflip_threshold);
> >> > +   }
> >> > +
> >> > +   /* MTD partitions, if any */
> >> > +   mtd_show_parts(mtd, 1);
> >> > +}
> >> > +
> >> > +#if IS_ENABLED(CONFIG_MTD)
> >> > +static void mtd_probe_uclass_mtd_devs(void)
> >> > +{
> >> > +   struct udevice *dev;
> >> > +   int idx = 0;
> >> > +
> >> > +   /* Probe devices with DM compliant drivers */
> >> > +   while (!uclass_find_device(UCLASS_MTD, idx, &dev) && dev) {
> >> > +           mtd_probe(dev);
> >> > +           idx++;
> >> > +   }
> >> > +}
> >> > +#else
> >> > +static void mtd_probe_uclass_mtd_devs(void) { }
> >> > +#endif  
> >>
> >> Does it make sense to support this new command without CONFIG_MTD
> >> enabled? Do you have a use case for this?  
> >
> > Maybe it is a bit misleading and I could add a comment. CONFIG_MTD
> > depends on CONFIG_DM. Any MTD device could be managed by the 'mtd'
> > command, but a lot of drivers do not make use of U-Boot DM yet, so
> > Boris and me thought it would help the acceptance if it could work with
> > all the devices (with let's say "reduced" functionalities).  
> 
> I think giving convince way to use mtd cmd for non-dm code may not
> good for long run, because non-dm drivers never convert (or may take
> long time) to dm and also there may be a possibility of adding #ifndef
> CONFIG_MTD in mtd  code for the sake of non-dm to work with dm code.

Indeed, that's what is done here.

Thanks,
Miquèl