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

Stefan sr at denx.de
Mon Sep 3 07:43:36 UTC 2018


On 03.09.2018 08:54, Miquel Raynal wrote:
> Hi Stefan,
> 
> Stefan Roese <sr at 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 at bootlin.com>
>>> Acked-by: Jagan Teki <jagan at 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 at 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).

Makes perfect sense this way, thank. I see no need to add a comment
in a potential new patch version.

Thanks,
Stefan


More information about the U-Boot mailing list