Hello Jagan,

Am 22.04.2015 12:52, schrieb Jagan Teki:

On 20 April 2015 at 11:17, Heiko Schocher hs@denx.de wrote:

...

From: Daniel Schwierzeck daniel.schwierzeck@gmail.com

add MTD layer driver for spi, original patch from: http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=bb246819cdc90493...

changes from Heiko Schocher against this patch:

• remove compile error if not defining CONFIG_SPI_FLASH_MTD:

  LD drivers/mtd/spi/built-in.o

drivers/mtd/spi/sf_probe.o: In function `spi_flash_mtd_unregister': /home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: multiple definition of `spi_flash_mtd_unregister' drivers/mtd/spi/sf_params.o:/home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: first defined here drivers/mtd/spi/sf_ops.o: In function `spi_flash_mtd_unregister': /home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: multiple definition of `spi_flash_mtd_unregister' drivers/mtd/spi/sf_params.o:/home/hs/abb/imx6/u-boot/drivers/mtd/spi/sf_internal.h:168: first defined here make[1]: *** [drivers/mtd/spi/built-in.o] Fehler 1 make: *** [drivers/mtd/spi] Fehler 2

• add a README entry.
• add correct writebufsize, to fit with Linux v3.14 MTD, UBI/UBIFS sync.

Signed-off-by: Daniel Schwierzeck daniel.schwierzeck@gmail.com Signed-off-by: Heiko Schocher hs@denx.de

---

Changes in v2:

• add comment from Daniel Schwierzeck: fix compile error from original patch with "static inline" rather than "static __maybe_unused"

Series-changes: 3

• rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6

Series-changes: 4

• rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a

Series-changes: 5

• no changes

  README | 3 ++ common/cmd_sf.c | 7 ++- drivers/mtd/spi/Makefile | 1 + drivers/mtd/spi/sf_internal.h | 13 ++++++ drivers/mtd/spi/sf_mtd.c | 104 ++++++++++++++++++++++++++++++++++++++++++ drivers/mtd/spi/sf_probe.c | 5 ++ 6 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 drivers/mtd/spi/sf_mtd.c

diff --git a/README b/README index 9b748cc..56dcfd2 100644 --- a/README +++ b/README @@ -3077,6 +3077,9 @@ CBFS (Coreboot Filesystem) support operation will not execute. The only way to exit this hardware-protected mode is to drive W#/VPP HIGH.

•           CONFIG_SPI_FLASH_MTD
  

•           add  MTD translation layer driver.
  

• • SystemACE Support: CONFIG_SYSTEMACE

diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 5c788e9..25a59e5 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -132,14 +132,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])

     flash = new->uclass_priv;

#else

•   if (flash)
  

•           spi_flash_free(flash);
  

•    new = spi_flash_probe(bus, cs, speed, mode);
  

•   flash = new;
  

This could be really a another patch - as it's not related to mtd.

Yes, done.

...

     if (!new) {
             printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
             return 1;
     }

•   if (flash)
  

•           spi_flash_free(flash);
     flash = new;
  

  #endif

diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile index c61b784e..f8580cd 100644 --- a/drivers/mtd/spi/Makefile +++ b/drivers/mtd/spi/Makefile @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o #endif obj-$(CONFIG_CMD_SF) += sf.o obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index 785f7a9..dc75f43 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -221,4 +221,17 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd, int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, size_t len, void *data);

+#ifdef CONFIG_SPI_FLASH_MTD +int spi_flash_mtd_register(struct spi_flash *flash); +void spi_flash_mtd_unregister(void); +#else +static inline int spi_flash_mtd_register(struct spi_flash *flash) +{

•   return 0;
  

+} +static inline void spi_flash_mtd_unregister(void) +{ +} +#endif

...

From the previous comments to reply - https://patchwork.ozlabs.org/patch/386102/

Adding #def isn't a polluting thing in code size contains software's like u-boot and did the same for many of my code.

So please do as below

#ifdef CONFIG_SPI_FLASH_MTD int spi_flash_mtd_register(struct spi_flash *flash); void spi_flash_mtd_unregister(void); #endif

void spi_flash_free(struct spi_flash *flash) { #ifdef CONFIG_SPI_FLASH_MTD spi_flash_mtd_unregister(); #endif }

#ifdef CONFIG_SPI_FLASH_MTD ret = spi_flash_mtd_register(flash); if (ret) goto err_claim_bus; #endif

really? ... I thought we want to prevent #ifdefs in Code ... Changed...

...

• #endif /* _SF_INTERNAL_H_ */

diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c new file mode 100644 index 0000000..0b9cb62 --- /dev/null +++ b/drivers/mtd/spi/sf_mtd.c @@ -0,0 +1,104 @@ +/*

• • Copyright (C) 2012-2014 Daniel Schwierzeck, daniel.schwierzeck@gmail.com
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <malloc.h> +#include <asm/errno.h> +#include <linux/mtd/mtd.h> +#include <spi_flash.h>

+static struct mtd_info sf_mtd_info; +static char sf_mtd_name[8];

+static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr) +{

•   struct spi_flash *flash = mtd->priv;
  

•   int err;
  

•   instr->state = MTD_ERASING;
  

•   err = spi_flash_erase(flash, instr->addr, instr->len);
  

•   if (err) {
  

•           instr->state = MTD_ERASE_FAILED;
  

•           instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
  

•           return -EIO;
  

•   }
  

•   instr->state = MTD_ERASE_DONE;
  

•   mtd_erase_callback(instr);
  

•   return 0;
  

+}

+static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,

•   size_t *retlen, u_char *buf)
  

+{

•   struct spi_flash *flash = mtd->priv;
  

•   int err;
  

•   err = spi_flash_read(flash, from, len, buf);
  

•   if (!err)
  

•           *retlen = len;
  

•   return err;
  

+}

+static int spi_flash_mtd_write(struct mtd_info *mtd, loff_t to, size_t len,

•   size_t *retlen, const u_char *buf)
  

+{

•   struct spi_flash *flash = mtd->priv;
  

•   int err;
  

•   err = spi_flash_write(flash, to, len, buf);
  

•   if (!err)
  

•           *retlen = len;
  

•   return err;
  

+}

+static void spi_flash_mtd_sync(struct mtd_info *mtd) +{ +}

+static int spi_flash_mtd_number(void) +{ +#ifdef CONFIG_SYS_MAX_FLASH_BANKS

•   return CONFIG_SYS_MAX_FLASH_BANKS;
  

+#else

•   return 0;
  

+#endif +}

+int spi_flash_mtd_register(struct spi_flash *flash) +{

•   memset(&sf_mtd_info, 0, sizeof(sf_mtd_info));
  

•   sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number());
  

•   sf_mtd_info.name = sf_mtd_name;
  

•   sf_mtd_info.type = MTD_NORFLASH;
  

•   sf_mtd_info.flags = MTD_CAP_NORFLASH;
  

•   sf_mtd_info.writesize = 1;
  

•   sf_mtd_info.writebufsize = flash->page_size;
  

•   sf_mtd_info._erase = spi_flash_mtd_erase;
  

•   sf_mtd_info._read = spi_flash_mtd_read;
  

•   sf_mtd_info._write = spi_flash_mtd_write;
  

•   sf_mtd_info._sync = spi_flash_mtd_sync;
  

•   sf_mtd_info.size = flash->size;
  

•   sf_mtd_info.priv = flash;
  

•   /* Only uniform flash devices for now */
  

•   sf_mtd_info.numeraseregions = 0;
  

•   sf_mtd_info.erasesize = flash->sector_size;
  

•   return add_mtd_device(&sf_mtd_info);
  

+}

+void spi_flash_mtd_unregister(void) +{

•   del_mtd_device(&sf_mtd_info);
  

+} diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index 4103723..93209fe 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -399,6 +399,10 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash) /* Release spi bus */ spi_release_bus(spi);

•   ret = spi_flash_mtd_register(flash);
  

•   if (ret)
  

•           return ret;
  

mtd register call is before the spi_release_bus isn't it?

Yep, fixed.

...

•    return 0;
  
  

  err_read_id:

@@ -449,6 +453,7 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,

void spi_flash_free(struct spi_flash *flash) {

•   spi_flash_mtd_unregister();
     spi_free_slave(flash->spi);
     free(flash);
  

  }

-- 2.1.0

thanks!

Thanks for the review

bye, Heiko

On 20 April 2015 at 11:17, Heiko Schocher hs@denx.de wrote:

move common functions from cmd_nand.c (for calculating offset and size from cmdline paramter) to common place, so they could used from other commands which use mtd partitions.

For onenand the arg_off_size() is left in common/cmd_onenand.c. It should use now the common arg_off() function, but as I could not test onenand I let it there ...

Signed-off-by: Heiko Schocher hs@denx.de

Reviewed-by: Jagannadha Sutradharudu Teki jagannadh.teki@gmail.com

---

Changes in v2:

• none

Series-changes: 3

• add comments from scott wood:
  • align MTD_DEV_TYPE_NAND correct
  • remove unnecessary inline
  • rework "jffs2 header" problem later
• rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6

Series-changes: 4

• rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a

Series-changes: 5

• add comment from Scott Wood: keep the continuation line aligned with the arguments

common/cmd_nand.c | 142 +++++++++--------------------------------------- common/cmd_onenand.c | 19 ++----- drivers/mtd/Makefile | 4 +- drivers/mtd/mtd_uboot.c | 114 ++++++++++++++++++++++++++++++++++++++ include/linux/mtd/mtd.h | 7 +++ 5 files changed, 156 insertions(+), 130 deletions(-) create mode 100644 drivers/mtd/mtd_uboot.c

diff --git a/common/cmd_nand.c b/common/cmd_nand.c index 17fa7ea..184335d 100644 --- a/common/cmd_nand.c +++ b/common/cmd_nand.c @@ -133,115 +133,6 @@ static int set_dev(int dev) return 0; }

-static inline int str2off(const char *p, loff_t *num) -{

•   char *endptr;
  

•   *num = simple_strtoull(p, &endptr, 16);
  

•   return *p != '\0' && *endptr == '\0';
  

-}

-static inline int str2long(const char *p, ulong *num) -{

•   char *endptr;
  

•   *num = simple_strtoul(p, &endptr, 16);
  

•   return *p != '\0' && *endptr == '\0';
  

-}

-static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,

•           loff_t *maxsize)
  

-{ -#ifdef CONFIG_CMD_MTDPARTS

•   struct mtd_device *dev;
  

•   struct part_info *part;
  

•   u8 pnum;
  

•   int ret;
  

•   ret = mtdparts_init();
  

•   if (ret)
  

•           return ret;
  

•   ret = find_dev_and_part(partname, &dev, &pnum, &part);
  

•   if (ret)
  

•           return ret;
  

•   if (dev->id->type != MTD_DEV_TYPE_NAND) {
  

•           puts("not a NAND device\n");
  

•           return -1;
  

•   }
  

•   *off = part->offset;
  

•   *size = part->size;
  

•   *maxsize = part->size;
  

•   *idx = dev->id->num;
  

•   ret = set_dev(*idx);
  

•   if (ret)
  

•           return ret;
  

•   return 0;
  

-#else

•   puts("offset is not a number\n");
  

•   return -1;
  

-#endif -}

-static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,

•           loff_t *maxsize)
  

-{

•   if (!str2off(arg, off))
  

•           return get_part(arg, idx, off, size, maxsize);
  

•   if (*off >= nand_info[*idx].size) {
  

•           puts("Offset exceeds device limit\n");
  

•           return -1;
  

•   }
  

•   *maxsize = nand_info[*idx].size - *off;
  

•   *size = *maxsize;
  

•   return 0;
  

-}

-static int arg_off_size(int argc, char *const argv[], int *idx,

•                   loff_t *off, loff_t *size, loff_t *maxsize)
  

-{

•   int ret;
  

•   if (argc == 0) {
  

•           *off = 0;
  

•           *size = nand_info[*idx].size;
  

•           *maxsize = *size;
  

•           goto print;
  

•   }
  

•   ret = arg_off(argv[0], idx, off, size, maxsize);
  

•   if (ret)
  

•           return ret;
  

•   if (argc == 1)
  

•           goto print;
  

•   if (!str2off(argv[1], size)) {
  

•           printf("'%s' is not a number\n", argv[1]);
  

•           return -1;
  

•   }
  

•   if (*size > *maxsize) {
  

•           puts("Size exceeds partition or device limit\n");
  

•           return -1;
  

•   }
  

-print:

•   printf("device %d ", *idx);
  

•   if (*size == nand_info[*idx].size)
  

•           puts("whole chip\n");
  

•   else
  

•           printf("offset 0x%llx, size 0x%llx\n",
  

•                  (unsigned long long)*off, (unsigned long long)*size);
  

•   return 0;
  

-}

#ifdef CONFIG_CMD_NAND_LOCK_UNLOCK static void print_status(ulong start, ulong end, ulong erasesize, int status) { @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[]) goto usage;

            /* We don't care about size, or maxsize. */

•           if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
  

•           if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
  

•                       MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
  

•                           puts("Offset or partition name expected\n");
  

•                           return 1;
  

•           }
  

•           if (set_dev(idx)) {
                    puts("Offset or partition name expected\n");
                    return 1;
            }
  

@@ -595,7 +491,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) printf("\nNAND %s: ", cmd); /* skip first two or three arguments, look for offset and size */ if (arg_off_size(argc - o, argv + o, &dev, &off, &size,

•                            &maxsize) != 0)
  

•                            &maxsize, MTD_DEV_TYPE_NAND,
  

•                            nand_info[dev].size) != 0)
  

•                   return 1;
  

•           if (set_dev(dev))
                    return 1;
  
            nand = &nand_info[dev];
  

@@ -655,7 +555,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (s && !strcmp(s, ".raw")) { raw = 1;

•                   if (arg_off(argv[3], &dev, &off, &size, &maxsize))
  

•                   if (arg_off(argv[3], &dev, &off, &size, &maxsize,
  

•                               MTD_DEV_TYPE_NAND, nand_info[dev].size))
  

•                           return 1;
  

•                   if (set_dev(dev))
                            return 1;
  
                    nand = &nand_info[dev];
  

@@ -672,8 +576,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

                    rwsize = pagecount * (nand->writesize + nand->oobsize);
            } else {

•                   if (arg_off_size(argc - 3, argv + 3, &dev,
  

•                                           &off, &size, &maxsize) != 0)
  

•                   if (arg_off_size(argc - 3, argv + 3, &dev, &off, &size,
  

•                                    &maxsize, MTD_DEV_TYPE_NAND,
  

•                                    nand_info[dev].size) != 0)
  

•                           return 1;
  

•                   if (set_dev(dev))
                            return 1;
  
                    /* size is unspecified */
  

@@ -812,7 +720,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) allexcept = 1;

            if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size,

•                            &maxsize) < 0)
  

•                            &maxsize, MTD_DEV_TYPE_NAND,
  

•                            nand_info[dev].size) < 0)
  

•                   return 1;
  

•           if (set_dev(dev))
                    return 1;
  
            if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
  

diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 06cc140..feab01a 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -24,15 +24,8 @@ static struct mtd_info *mtd; static loff_t next_ofs; static loff_t skip_ofs;

-static inline int str2long(char *p, ulong *num) -{

•   char *endptr;
  

•   *num = simple_strtoul(p, &endptr, 16);
  

•   return (*p != '\0' && *endptr == '\0') ? 1 : 0;
  

-}

-static int arg_off_size(int argc, char * const argv[], ulong *off, size_t *size) +static int arg_off_size_onenand(int argc, char * const argv[], ulong *off,

•                           size_t *size)
  

{ if (argc >= 1) { if (!(str2long(argv[0], off))) { @@ -399,7 +392,7 @@ static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char * const a addr = (ulong)simple_strtoul(argv[1], NULL, 16);

    printf("\nOneNAND read: ");

•   if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
  

•   if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
            return 1;
  
    ret = onenand_block_read(ofs, len, &retlen, (u8 *)addr, oob);
  

@@ -425,7 +418,7 @@ static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char * const addr = (ulong)simple_strtoul(argv[1], NULL, 16);

    printf("\nOneNAND write: ");

•   if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
  

•   if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
            return 1;
  
    ret = onenand_block_write(ofs, len, &retlen, (u8 *)addr, withoob);
  

@@ -461,7 +454,7 @@ static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char * const printf("\nOneNAND erase: ");

    /* skip first two or three arguments, look for offset and size */

•   if (arg_off_size(argc, argv, &ofs, &len) != 0)
  

•   if (arg_off_size_onenand(argc, argv, &ofs, &len) != 0)
            return 1;
  
    ret = onenand_block_erase(ofs, len, force);
  

@@ -486,7 +479,7 @@ static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char * const a printf("\nOneNAND test: ");

    /* skip first two or three arguments, look for offset and size */

•   if (arg_off_size(argc - 1, argv + 1, &ofs, &len) != 0)
  

•   if (arg_off_size_onenand(argc - 1, argv + 1, &ofs, &len) != 0)
            return 1;
  
    ret = onenand_block_test(ofs, len);
  

diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile index 5467a951..a623f4c 100644 --- a/drivers/mtd/Makefile +++ b/drivers/mtd/Makefile @@ -5,8 +5,8 @@ # SPDX-License-Identifier: GPL-2.0+ #

-ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND))) -obj-y += mtdcore.o +ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF))) +obj-y += mtdcore.o mtd_uboot.o endif obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c new file mode 100644 index 0000000..a70d40a --- /dev/null +++ b/drivers/mtd/mtd_uboot.c @@ -0,0 +1,114 @@ +/*

• • (C) Copyright 2014
• • Heiko Schocher, DENX Software Engineering, hs@denx.de.
• • SPDX-License-Identifier: GPL-2.0+
• */

+#include <common.h> +#include <linux/mtd/mtd.h> +#include <jffs2/jffs2.h>

+int str2off(const char *p, loff_t *num) +{

•   char *endptr;
  

•   *num = simple_strtoull(p, &endptr, 16);
  

•   return *p != '\0' && *endptr == '\0';
  

+}

+int str2long(const char *p, ulong *num) +{

•   char *endptr;
  

•   *num = simple_strtoul(p, &endptr, 16);
  

•   return *p != '\0' && *endptr == '\0';
  

+}

+static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,

•           loff_t *maxsize, int devtype)
  

+{ +#ifdef CONFIG_CMD_MTDPARTS

•   struct mtd_device *dev;
  

•   struct part_info *part;
  

•   u8 pnum;
  

•   int ret;
  

•   ret = mtdparts_init();
  

•   if (ret)
  

•           return ret;
  

•   ret = find_dev_and_part(partname, &dev, &pnum, &part);
  

•   if (ret)
  

•           return ret;
  

•   if (dev->id->type != devtype) {
  

•           printf("not same typ %d != %d\n", dev->id->type, devtype);
  

•           return -1;
  

•   }
  

•   *off = part->offset;
  

•   *size = part->size;
  

•   *maxsize = part->size;
  

•   *idx = dev->id->num;
  

•   return 0;
  

+#else

•   puts("offset is not a number\n");
  

•   return -1;
  

+#endif +}

+int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,

•           loff_t *maxsize, int devtype, int chipsize)
  

+{

•   if (!str2off(arg, off))
  

•           return get_part(arg, idx, off, size, maxsize, devtype);
  

•   if (*off >= chipsize) {
  

•           puts("Offset exceeds device limit\n");
  

•           return -1;
  

•   }
  

•   *maxsize = chipsize - *off;
  

•   *size = *maxsize;
  

•   return 0;
  

+}

+int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,

•            loff_t *size, loff_t *maxsize, int devtype, int chipsize)
  

+{

•   int ret;
  

•   if (argc == 0) {
  

•           *off = 0;
  

•           *size = chipsize;
  

•           *maxsize = *size;
  

•           goto print;
  

•   }
  

•   ret = arg_off(argv[0], idx, off, size, maxsize, devtype, chipsize);
  

•   if (ret)
  

•           return ret;
  

•   if (argc == 1)
  

•           goto print;
  

•   if (!str2off(argv[1], size)) {
  

•           printf("'%s' is not a number\n", argv[1]);
  

•           return -1;
  

•   }
  

•   if (*size > *maxsize) {
  

•           puts("Size exceeds partition or device limit\n");
  

•           return -1;
  

•   }
  

+print:

•   printf("device %d ", *idx);
  

•   if (*size == chipsize)
  

•           puts("whole chip\n");
  

•   else
  

•           printf("offset 0x%llx, size 0x%llx\n",
  

•                  (unsigned long long)*off, (unsigned long long)*size);
  

•   return 0;
  

+} diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 8666413..2861af5 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd); int del_mtd_device(struct mtd_info *mtd); int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int); int del_mtd_partitions(struct mtd_info *);

+int str2off(const char *p, loff_t *num); +int str2long(const char *p, ulong *num); +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,

•           loff_t *maxsize, int devtype, int chipsize);
  

+int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,

•            loff_t *size, loff_t *maxsize, int devtype, int chipsize);
  

#endif

#endif /* __MTD_MTD_H__ */

2.1.0

thanks!

Hello Scott,

Am 23.04.2015 00:47, schrieb Scott Wood:

On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:

...

@@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[]) goto usage;

/* We don't care about size, or maxsize. */

• if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
  

• if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
  

• 	    MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
  

• 		puts("Offset or partition name expected\n");
  

• 		return 1;
  

• }
  

Use only one tab per indentation level.

You mean the line "MTD_DEV_TYPE_NAND, nand_info[idx].size)) {" ?

I have to move MTD_xx to the opening bracket to avoid checkpatch.pl errors ...

If you mean the "puts ..." line ... fixed.

...

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 8666413..2861af5 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd); int del_mtd_device(struct mtd_info *mtd); int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int); int del_mtd_partitions(struct mtd_info *);

+int str2off(const char *p, loff_t *num); +int str2long(const char *p, ulong *num);

These should be moved somewhere more generic, especially if they're no longer file-local.

Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe we add a "mtd_" prefix to them? I think these functions are mtd specific ...

...

+int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,

• loff_t *maxsize, int devtype, int chipsize);
  

+int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,

•  loff_t *size, loff_t *maxsize, int devtype, int chipsize);
  

Add an mtd prefix.

Done.

Thanks for your time!

bye, Heiko

Hello Scott,

Am 23.04.2015 08:55, schrieb Scott Wood:

On Thu, 2015-04-23 at 07:57 +0200, Heiko Schocher wrote:

...

Hello Scott,

Am 23.04.2015 00:47, schrieb Scott Wood:

...

On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:

...

@@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[]) goto usage;

	/* We don't care about size, or maxsize. */

• if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
  

• if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
  

• 	    MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
  

• 		puts("Offset or partition name expected\n");
  

• 		return 1;
  

• }
  

Use only one tab per indentation level.

You mean the line "MTD_DEV_TYPE_NAND, nand_info[idx].size)) {" ?

I have to move MTD_xx to the opening bracket to avoid checkpatch.pl errors ...

If you mean the "puts ..." line ... fixed.

I meant the puts and return lines.

...

...

...

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 8666413..2861af5 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd); int del_mtd_device(struct mtd_info *mtd); int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int); int del_mtd_partitions(struct mtd_info *);

+int str2off(const char *p, loff_t *num); +int str2long(const char *p, ulong *num);

These should be moved somewhere more generic, especially if they're no longer file-local.

Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe we add a "mtd_" prefix to them? I think these functions are mtd specific ...

What is mtd-specific about them?

Hmm... I thought:

return *p != '\0' && *endptr == '\0';

is more or less mtd specific ... but you are right, it is not really mtd specific ... so I move them to "./lib/vsprintf.c" ... Ok?

bye, Heiko

Hello Jagan,

Am 22.04.2015 12:58, schrieb Jagan Teki:

On 20 April 2015 at 11:17, Heiko Schocher hs@denx.de wrote:

...

with this patch, it is possible to get the offset and size information from the mtdpartiton setting in "mtdparts", similiar to the "nand" commandos.

=> sf sf - SPI flash sub-system

Usage: sf probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus and chip select sf read addr offset|partition len - read `len' bytes starting at `offset' to memory at `addr' sf write addr offset|partition len - write `len' bytes from memory at `addr' to flash at `offset' sf erase offset|partition [+]len - erase `len' bytes from `offset' `+len' round up `len' to block size sf update addr offset|partition len - erase and write `len' bytes from memory at `addr' to flash at `offset' => for example "env" is defined in mtdparts:

=> sf read 13000000 env device 0 offset 0xd0000, size 0x10000 SF: 65536 bytes @ 0xd0000 Read: OK =>

Signed-off-by: Heiko Schocher hs@denx.de

---

Changes in v2:

• none

Series-changes: 3

• rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6

Series-changes: 4

• rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a

Series-changes: 5

• no changes

  common/cmd_sf.c | 50 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c index 25a59e5..1932ac9 100644 --- a/common/cmd_sf.c +++ b/common/cmd_sf.c @@ -12,6 +12,8 @@ #include <malloc.h> #include <spi.h> #include <spi_flash.h> +#include <jffs2/jffs2.h> +#include <linux/mtd/mtd.h>

#include <asm/io.h> #include <dm/device-internal.h> @@ -258,23 +260,21 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset, static int do_spi_flash_read_write(int argc, char * const argv[]) { unsigned long addr;

•   unsigned long offset;
  

•   unsigned long len;
     void *buf;
     char *endp;
     int ret = 1;
  

•   int dev = 0;
  

•   loff_t offset, len, maxsize;
  
  

•   if (argc < 4)
  

•   if (argc < 3)
             return -1;
  
     addr = simple_strtoul(argv[1], &endp, 16);
     if (*argv[1] == 0 || *endp != 0)
             return -1;
  

•   offset = simple_strtoul(argv[2], &endp, 16);
  

•   if (*argv[2] == 0 || *endp != 0)
  

•           return -1;
  

•   len = simple_strtoul(argv[3], &endp, 16);
  

•   if (*argv[3] == 0 || *endp != 0)
  

•   if (arg_off_size(argc - 2, &argv[2], &dev, &offset, &len, &maxsize,
  

•       MTD_DEV_TYPE_NOR, flash->size))
             return -1;
  
     /* Consistency checking */
  

@@ -313,31 +313,31 @@ static int do_spi_flash_read_write(int argc, char * const argv[])

static int do_spi_flash_erase(int argc, char * const argv[]) {

•   unsigned long offset;
  

•   unsigned long len;
  

•   char *endp;
     int ret;
  

•   int dev = 0;
  

•   loff_t offset, len, maxsize;
  

•   ulong size;
  
     if (argc < 3)
             return -1;
  
  

•   offset = simple_strtoul(argv[1], &endp, 16);
  

•   if (*argv[1] == 0 || *endp != 0)
  

•   if (arg_off(argv[1], &dev, &offset, &len, &maxsize,
  

•       MTD_DEV_TYPE_NOR, flash->size))
             return -1;
  
  

•   ret = sf_parse_len_arg(argv[2], &len);
  

•   ret = sf_parse_len_arg(argv[2], &size);
     if (ret != 1)
             return -1;
  
     /* Consistency checking */
  

•   if (offset + len > flash->size) {
  

•   if (offset + size > flash->size) {
             printf("ERROR: attempting %s past flash size (%#x)\n",
                    argv[0], flash->size);
             return 1;
     }
  
  

•   ret = spi_flash_erase(flash, offset, len);
  

•   printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)len, (u32)offset,
  

•   ret = spi_flash_erase(flash, offset, size);
  

•   printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)size, (u32)offset,
            ret ? "ERROR" : "OK");
  
     return ret == 0 ? 0 : 1;
  

@@ -562,13 +562,13 @@ U_BOOT_CMD( "SPI flash sub-system", "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" " and chip select\n"

•   "sf read addr offset len        - read `len' bytes starting at\n"
  

•   "                                 `offset' to memory at `addr'\n"
  

•   "sf write addr offset len       - write `len' bytes from memory\n"
  

•   "                                 at `addr' to flash at `offset'\n"
  

•   "sf erase offset [+]len         - erase `len' bytes from `offset'\n"
  

•   "                                 `+len' round up `len' to block size\n"
  

•   "sf update addr offset len      - erase and write `len' bytes from memory\n"
  

•   "                                 at `addr' to flash at `offset'"
  

•   "sf read addr offset|partition len      - read `len' bytes starting at\n"
  

•   "                                         `offset' to memory at `addr'\n"
  

Append partition on help text as well more readable.

Added.

...

•   "sf write addr offset|partition len     - write `len' bytes from memory\n"
  

•   "                                         at `addr' to flash at `offset'\n"
  

•   "sf erase offset|partition [+]len       - erase `len' bytes from `offset'\n"
  

•   "                                        `+len' round up `len' to block size\n"
  

•   "sf update addr offset|partition len    - erase and write `len' bytes from memory\n"
  

•   "                                         at `addr' to flash at `offset'"
     SF_TEST_HELP
  

  );

-- 2.1.0

Reviewed-by: Jagannadha Sutradharudu Teki jagannadh.teki@gmail.com

thanks!

Thanks for your time!

bye, Heiko