On Mon, Sep 3, 2018 at 12:24 PM, Miquel Raynal <miquel.ray...@bootlin.com> wrote: > Hi Stefan, > > Stefan Roese <s...@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.ray...@bootlin.com> >> > Acked-by: Jagan Teki <ja...@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.ray...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot