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

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

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).

Thanks,
Miquèl