[U-Boot] [PATCH v6 29/31] cmd: mtd: add 'mtd' command
Boris Brezillon
boris.brezillon at bootlin.com
Thu Aug 16 19:50:58 UTC 2018
On Thu, 16 Aug 2018 17:30:27 +0200
Miquel Raynal <miquel.raynal at bootlin.com> 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 | 7 +
> cmd/Makefile | 1 +
> cmd/mtd.c | 513 ++++++++++++++++++++++++++++++++++++++++
> drivers/mtd/Makefile | 2 +-
> include/linux/mtd/mtd.h | 1 +
> 5 files changed, 523 insertions(+), 1 deletion(-)
> create mode 100644 cmd/mtd.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index ef43ed8dda..81351920ab 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -847,6 +847,13 @@ config CMD_MMC_SWRITE
> Enable support for the "mmc swrite" command to write Android sparse
> images to eMMC.
>
> +config CMD_MTD
> + bool "mtd"
> + depends on CMD_MTDPARTS
> + select MTD_PARTITIONS
> + help
> + MTD commands support.
> +
> config CMD_NAND
> bool "nand"
> default y if NAND_SUNXI
> 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..b9d58725f0
> --- /dev/null
> +++ b/cmd/mtd.c
> @@ -0,0 +1,513 @@
> +// 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 <dm/device.h>
> +#include <dm/uclass-internal.h>
> +
> +#define MTD_NAME_MAX_LEN 20
> +
> +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);
> +}
> +
> +/*
> + * Ensure the MTD device partition list has been initialized already (needed for
> + * non DM-compliant devices).
> + */
> +static void mtd_quirk_init_non_dm_device_lists(void)
> +{
> + struct mtd_info *mtd;
> +
> + mtd_for_each_device(mtd) {
> + if (!mtd_is_partition(mtd) && !mtd->partitions.next) {
> + INIT_LIST_HEAD(&mtd->partitions);
> + INIT_LIST_HEAD(&mtd->node);
> + }
> + }
> +}
> +
> +#if IS_ENABLED(CONFIG_DM)
> +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
> +
> +int mtd_probe_devices(void)
> +{
> + const char *mtdparts = env_get("mtdparts");
> +
> + mtd_quirk_init_non_dm_device_lists();
> +
> + mtd_probe_uclass_mtd_devs();
> +
> + /* Create the partitions defined in mtdparts, here the fun begins */
> +
> + /* Ignore the extra 'mtdparts=' prefix if any */
> + if (strstr(mtdparts, "mtdparts="))
> + mtdparts += 9;
> +
> + /* For each MTD device in mtdparts */
> + while (mtdparts[0] != '\0') {
Hm, so you're unconditionally re-parsing mtdparts, even if nothing
changed since the last time you parsed it? Might be expensing if mtd
read/write is called iteratively on small MTD portions.
> + struct mtd_partition *parts;
> + struct list_head *iterator;
> + bool part_in_use = false;
> + struct mtd_info *parent;
> + char mtdname[MTD_NAME_MAX_LEN];
> + int mtdname_len, len, ret;
> +
> + mtdname_len = strchr(mtdparts, ':') - mtdparts;
What if mtdparts is malformed? strchr() might return NULL if it didn't
find ':', and you probably want to stop parsing mtdparts in that case.
> + strncpy(mtdname, mtdparts, mtdname_len);
> + mtdname[mtdname_len] = '\0';
> + /* Move the pointer forward (including the ':') */
> + mtdparts += mtdname_len + 1;
> + parent = get_mtd_device_nm(mtdname);
> + if (IS_ERR_OR_NULL(parent)) {
> + char altname[MTD_NAME_MAX_LEN];
> +
> + /*
> + * The MTD device named "mtdname" does not exist. Try to
> + * find a correspondance with an MTD device having the
> + * same type and number as defined in the mtdids.
> + */
> + printf("No device named %s\n", mtdname);
> +
> + ret = mtdparts_init();
> + if (ret) {
> + printf("Could not initialize mtdparts\n");
> + return ret;
> + }
> +
> + ret = mtdids_search_alternate_name(mtdname, altname,
> + MTD_NAME_MAX_LEN);
> + if (ret) {
> + printf("Did not found an equivalent in mtdids\n");
> + return ret;
> + }
> +
> + parent = get_mtd_device_nm(altname);
> + if (IS_ERR_OR_NULL(parent)) {
> + printf("No device named %s\n", altname);
> + return -EINVAL;
> + }
> + }
> + put_mtd_device(parent);
> +
> + /*
> + * Parse the MTD device partitions. It will update the mtdparts
> + * pointer, create an array of parts (that must be freed), and
> + * return the number of partition structures in the array.
> + */
> + ret = mtdparts_parse_part(parent, &mtdparts, &parts, &len);
> + if (ret) {
> + printf("Could not parse device %s\n", parent->name);
> + return -EINVAL;
> + }
> +
> + if (!len)
> + continue;
> +
> + /* Check that no partition is actually in use */
> + list_for_each(iterator, &parent->partitions) {
> + struct mtd_info *p = mtd_part_node_to_info(iterator);
> +
> + if (p->usecount) {
> + printf("MTD partition \"%s\" already in use.\n",
> + p->name);
> + part_in_use = true;
> + }
> + }
> +
> + if (part_in_use) {
> + printf("Not reconstructing partition list for MTD device %s\n",
> + parent->name);
> + continue;
> + }
> +
> + /* Free the existing partitions */
> + del_mtd_partitions(parent);
> +
> + /* Create the new MTD partitions */
> + add_mtd_partitions(parent, parts, len);
> + free(parts);
> + }
> +
> + return 0;
> +}
> +
> +static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
> +{
> + do_div(len, mtd->writesize);
> +
> + return len;
> +}
> +
> +static bool mtd_is_aligned_with_min_io_size(struct mtd_info *mtd, u64 size)
> +{
> + return do_div(size, mtd->writesize);
> +}
> +
> +static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
> +{
> + return do_div(size, mtd->erasesize);
> +}
> +
> +static int do_mtd_list(void)
> +{
> + struct mtd_info *mtd;
> + int dev_nb = 0;
> +
> + /* Ensure all devices (and their partitions) are probed */
> + mtd_probe_devices();
> +
> + printf("List of MTD devices:\n");
> + mtd_for_each_device(mtd) {
> + if (!mtd_is_partition(mtd))
> + mtd_show_device(mtd);
I would print partitions too, and if possible, put them under the
master device with an extra level of indentation.
> +
> + dev_nb++;
> + }
> +
> + if (!dev_nb) {
> + printf("No MTD device found\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + return 0;
> +}
> +
> +static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct mtd_info *mtd;
> + const char *cmd;
> + char *mtdname;
> + int ret;
> +
> + /* All MTD commands need at least two arguments */
> + if (argc < 2)
> + return CMD_RET_USAGE;
> +
> + /* Parse the command name and its optional suffixes */
> + cmd = argv[1];
> +
> + /* List the MTD devices if that is what the user wants */
> + if (strcmp(cmd, "list") == 0)
> + return do_mtd_list();
> +
> + /*
> + * The remaining commands require also at least a device ID.
> + * Check the selected device is valid. Ensure it is probed.
> + */
> + if (argc < 3)
> + return CMD_RET_USAGE;
> +
> + mtdname = argv[2];
> + mtd = get_mtd_device_nm(mtdname);
> + if (IS_ERR_OR_NULL(mtd)) {
> + mtd_probe_devices();
Actually this should be done unconditionally, just in case the object
returned by get_mtd_device_nm() is a partition. mtdparts env var might
have changed between the 2 mtd xxx calls, and the partition we've been
given might no longer be valid. The solution would be to avoid
re-parsing things when mtdparts didn't change (you can check that
in mtd_probe_devices()).
> + mtd = get_mtd_device_nm(mtdname);
> + if (IS_ERR_OR_NULL(mtd)) {
> + printf("MTD device %s not found, ret %ld\n",
> + mtdname, PTR_ERR(mtd));
> + return 1;
return CMD_RET_FAILURE;
> + }
> + }
> + put_mtd_device(mtd);
> +
Why do you call put_mtd_device() here? You should retain the reference
until you're done interacting with the device.
More information about the U-Boot
mailing list