[U-Boot] [PATCH v3 0/3] spi, sf: add mtdparts feature to spi and sf commands

[U-Boot] [PATCH v3 0/3] spi, sf: add mtdparts feature to spi and sf commands

[Heiko Schocher]

This patchserie add the popssibility to define mtd partitions on
spi nor flash, and use this settings with the sf commands.

steps:

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

  and addapted it to current mainline.

- move common functions to get offset and size from
  cmdline nand command to extract offset and size from
  a mtd partition to common place "drivers/mtd/mtd_uboot.c"
  maybe another place is better?

- add to the sf command the possibility to use offset and size from
  the settings in mtdparts

With this patchset, the sf command looks now:

=> 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
=>

There are the followings checkpatch warnings:

CHECK: Alignment should match open parenthesis
#153: FILE: common/cmd_nand.c:217:
+               if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
+                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {

CHECK: Alignment should match open parenthesis
#179: FILE: common/cmd_nand.c:557:
+                       if (arg_off(argv[3], &dev, &off, &size, &maxsize,
+                           MTD_DEV_TYPE_NAND, nand_info[dev].size))

CHECK: Alignment should match open parenthesis
#193: FILE: common/cmd_nand.c:576:
+                       if (arg_off_size(argc - 3, argv + 3, &dev, &off, &size,
+                           &maxsize, MTD_DEV_TYPE_NAND,

total: 0 errors, 0 warnings, 3 checks, 361 lines checked

NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE USLEEP_RANGE

20140714_ml_mtdparts/0002-mtd-nand-move-common-functions-from-cmd_nand.c-to-co.patch has style problems, please review.

I see not, why this warning pops up ...

- changes for v2:
  - mtd-spi-add-MTD-layer-driver.patch
    - add comment from Daniel Schwierzeck:
      fix compile error from original patch with
      "static inline" rather than "static __maybe_unused"
- changes for v3:
  - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
  - add comments from scott wood:
    - align MTD_DEV_TYPE_NAND correct
    - remove unnecessary inline
    - rework "jffs2 header" problem later

Cc: Scott Wood <scottwood@freescale.com>
Cc: Tom Rini <trini@ti.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

Daniel Schwierzeck (1):
  mtd, spi: add MTD layer driver

Heiko Schocher (2):
  mtd, nand: move common functions from cmd_nand.c to common place
  spi, sf: use offset and size in sf cmd from mtdpartition

 README                        |   3 +
 common/cmd_nand.c             | 140 ++++++++----------------------------------
 common/cmd_onenand.c          |  19 ++----
 common/cmd_sf.c               |  59 +++++++++---------
 drivers/mtd/Makefile          |   4 +-
 drivers/mtd/mtd_uboot.c       | 114 ++++++++++++++++++++++++++++++++++
 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 ++
 include/linux/mtd/mtd.h       |   7 +++
 11 files changed, 310 insertions(+), 159 deletions(-)
 create mode 100644 drivers/mtd/mtd_uboot.c
 create mode 100644 drivers/mtd/spi/sf_mtd.c

-- 
1.8.3.1

[U-Boot] [PATCH v3 1/3] mtd, spi: add MTD layer driver

[Heiko Schocher]

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=bb246819cdc90493dd7089eaa51b9e639765cced

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>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

---
MAKEALL for ar, mips, powerc compiles clean

- changes for v2:
  - add comment from Daniel Schwierzeck:
    fix compile error from original patch with
    "static inline" rather than "static __maybe_unused"
- changes for v3:
  rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
---
 README                        |   3 ++
 common/cmd_sf.c               |   9 ++--
 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(+), 4 deletions(-)
 create mode 100644 drivers/mtd/spi/sf_mtd.c

diff --git a/README b/README
index 0a0f528..e7be54e 100644
--- a/README
+++ b/README
@@ -2965,6 +2965,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 b4ceb71..7653d7e 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -121,16 +121,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 			return -1;
 	}
 
+	if (flash)
+		spi_flash_free(flash);
+
 	new = spi_flash_probe(bus, cs, speed, mode);
+	flash = new;
+
 	if (!new) {
 		printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
 		return 1;
 	}
 
-	if (flash)
-		spi_flash_free(flash);
-	flash = new;
-
 	return 0;
 }
 
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
index 9e18fb4..b15d273 100644
--- a/drivers/mtd/spi/Makefile
+++ b/drivers/mtd/spi/Makefile
@@ -12,6 +12,7 @@ endif
 
 obj-$(CONFIG_CMD_SF) += sf.o
 obj-$(CONFIG_SPI_FLASH) += sf_params.o sf_probe.o sf_ops.o
+obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
 obj-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.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 19d4914..a9f97d1 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -160,4 +160,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
+
 #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 at 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 4d148d1..6c50766 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -385,6 +385,10 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
 	/* Release spi bus */
 	spi_release_bus(spi);
 
+	ret = spi_flash_mtd_register(flash);
+	if (ret)
+		goto err_claim_bus;
+
 	return flash;
 
 err_read_id:
@@ -416,6 +420,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);
 }
-- 
1.8.3.1

[U-Boot] [PATCH v3 1/3] mtd, spi: add MTD layer driver

[Jagan Teki]

On 5 September 2014 11:08, 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=bb246819cdc90493dd7089eaa51b9e639765cced
>
> 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>
> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>
> ---
> MAKEALL for ar, mips, powerc compiles clean
>
> - changes for v2:
>   - add comment from Daniel Schwierzeck:
>     fix compile error from original patch with
>     "static inline" rather than "static __maybe_unused"
> - changes for v3:
>   rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> ---
>  README                        |   3 ++
>  common/cmd_sf.c               |   9 ++--
>  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(+), 4 deletions(-)
>  create mode 100644 drivers/mtd/spi/sf_mtd.c
>
> diff --git a/README b/README
> index 0a0f528..e7be54e 100644
> --- a/README
> +++ b/README
> @@ -2965,6 +2965,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 b4ceb71..7653d7e 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -121,16 +121,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>                         return -1;
>         }
>
> +       if (flash)
> +               spi_flash_free(flash);
> +
>         new = spi_flash_probe(bus, cs, speed, mode);
> +       flash = new;
> +

Why is this change? I guess it's not related to mtd.
And it's better to add new with global flash only it's probed isn't it?

>         if (!new) {
>                 printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
>                 return 1;
>         }
>
> -       if (flash)
> -               spi_flash_free(flash);
> -       flash = new;
> -
>         return 0;
>  }
>
> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
> index 9e18fb4..b15d273 100644
> --- a/drivers/mtd/spi/Makefile
> +++ b/drivers/mtd/spi/Makefile
> @@ -12,6 +12,7 @@ endif
>
>  obj-$(CONFIG_CMD_SF) += sf.o
>  obj-$(CONFIG_SPI_FLASH) += sf_params.o sf_probe.o sf_ops.o
> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>  obj-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.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 19d4914..a9f97d1 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -160,4 +160,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
> +
>  #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 at 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());

This is quite not required for sf - like mtd_number is devnum that is maximum
number of sf's on the particular board.

We're not dealing with this at this point of time, comments?

> +
> +       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 4d148d1..6c50766 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -385,6 +385,10 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
>         /* Release spi bus */
>         spi_release_bus(spi);
>
> +       ret = spi_flash_mtd_register(flash);
> +       if (ret)
> +               goto err_claim_bus;
> +

Be define this code in macro's and place before spi_release_bus()

>         return flash;
>
>  err_read_id:
> @@ -416,6 +420,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();

Same ..

>         spi_free_slave(flash->spi);
>         free(flash);
>  }
> --
> 1.8.3.1
>

Code looks straight forward and be define the necessary macro's on new
code and please
update the test log on doc/SPI. Mean while I will look for more options.

thanks!
-- 
Jagan.

[U-Boot] [PATCH v3 1/3] mtd, spi: add MTD layer driver

[Daniel Schwierzeck]

2014-11-04 21:32 GMT+01:00 Jagan Teki <jagannadh.teki@gmail.com>:
> On 5 September 2014 11:08, 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=bb246819cdc90493dd7089eaa51b9e639765cced
>>
>> 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>
>> Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>
>> ---
>> MAKEALL for ar, mips, powerc compiles clean
>>
>> - changes for v2:
>>   - add comment from Daniel Schwierzeck:
>>     fix compile error from original patch with
>>     "static inline" rather than "static __maybe_unused"
>> - changes for v3:
>>   rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>> ---
>>  README                        |   3 ++
>>  common/cmd_sf.c               |   9 ++--
>>  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(+), 4 deletions(-)
>>  create mode 100644 drivers/mtd/spi/sf_mtd.c
>>
>> diff --git a/README b/README
>> index 0a0f528..e7be54e 100644
>> --- a/README
>> +++ b/README
>> @@ -2965,6 +2965,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 b4ceb71..7653d7e 100644
>> --- a/common/cmd_sf.c
>> +++ b/common/cmd_sf.c
>> @@ -121,16 +121,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>>                         return -1;
>>         }
>>
>> +       if (flash)
>> +               spi_flash_free(flash);
>> +
>>         new = spi_flash_probe(bus, cs, speed, mode);
>> +       flash = new;
>> +
>
> Why is this change? I guess it's not related to mtd.
> And it's better to add new with global flash only it's probed isn't it?
>
>>         if (!new) {
>>                 printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
>>                 return 1;
>>         }
>>
>> -       if (flash)
>> -               spi_flash_free(flash);
>> -       flash = new;
>> -
>>         return 0;
>>  }
>>
>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>> index 9e18fb4..b15d273 100644
>> --- a/drivers/mtd/spi/Makefile
>> +++ b/drivers/mtd/spi/Makefile
>> @@ -12,6 +12,7 @@ endif
>>
>>  obj-$(CONFIG_CMD_SF) += sf.o
>>  obj-$(CONFIG_SPI_FLASH) += sf_params.o sf_probe.o sf_ops.o
>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>  obj-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.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 19d4914..a9f97d1 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -160,4 +160,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
>> +
>>  #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 at 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());
>
> This is quite not required for sf - like mtd_number is devnum that is maximum
> number of sf's on the particular board.
>
> We're not dealing with this at this point of time, comments?

this is needed if you enable CFI-MTD and SF-MTD at the same time
(common case on evaluation boards). Because CFI is always initialized
in board_init_r() (before SF), it occupies MTD numbers 0 to
CONFIG_SYS_MAX_FLASH_BANKS - 1.

>
>> +
>> +       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 4d148d1..6c50766 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -385,6 +385,10 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
>>         /* Release spi bus */
>>         spi_release_bus(spi);
>>
>> +       ret = spi_flash_mtd_register(flash);
>> +       if (ret)
>> +               goto err_claim_bus;
>> +
>
> Be define this code in macro's and place before spi_release_bus()

the defines are already in sf_internal.h. This technique is much more
elegant and cleaner than polluting the code with #ifdef's

+#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


>
>>         return flash;
>>
>>  err_read_id:
>> @@ -416,6 +420,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();
>
> Same ..
>
>>         spi_free_slave(flash->spi);
>>         free(flash);
>>  }
>> --
>> 1.8.3.1
>>
>
> Code looks straight forward and be define the necessary macro's on new
> code and please
> update the test log on doc/SPI. Mean while I will look for more options.
>
> thanks!
> --
> Jagan.



-- 
- Daniel

[U-Boot] [PATCH v3 1/3] mtd, spi: add MTD layer driver

[Heiko Schocher]

Hello Jagan, Daniel,

Am 04.11.2014 22:24, schrieb Daniel Schwierzeck:
> 2014-11-04 21:32 GMT+01:00 Jagan Teki<jagannadh.teki@gmail.com>:
>> On 5 September 2014 11:08, 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=bb246819cdc90493dd7089eaa51b9e639765cced
>>>
>>> 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>
>>> Cc: Jagannadha Sutradharudu Teki<jagannadh.teki@gmail.com>
>>>
>>> ---
>>> MAKEALL for ar, mips, powerc compiles clean
>>>
>>> - changes for v2:
>>>    - add comment from Daniel Schwierzeck:
>>>      fix compile error from original patch with
>>>      "static inline" rather than "static __maybe_unused"
>>> - changes for v3:
>>>    rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>> ---
>>>   README                        |   3 ++
>>>   common/cmd_sf.c               |   9 ++--
>>>   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(+), 4 deletions(-)
>>>   create mode 100644 drivers/mtd/spi/sf_mtd.c
>>>
>>> diff --git a/README b/README
>>> index 0a0f528..e7be54e 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -2965,6 +2965,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 b4ceb71..7653d7e 100644
>>> --- a/common/cmd_sf.c
>>> +++ b/common/cmd_sf.c
>>> @@ -121,16 +121,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>>>                          return -1;
>>>          }
>>>
>>> +       if (flash)
>>> +               spi_flash_free(flash);
>>> +
>>>          new = spi_flash_probe(bus, cs, speed, mode);
>>> +       flash = new;
>>> +
>>
>> Why is this change? I guess it's not related to mtd.
>> And it's better to add new with global flash only it's probed isn't it?

If we have probed a flash, and we probe another flash on maybe another bus/cs,
we should free the old ... or?

And if the new probe failed, we have to update the "flash" pointer in any case ...

>>>          if (!new) {
>>>                  printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
>>>                  return 1;
>>>          }
>>>
>>> -       if (flash)
>>> -               spi_flash_free(flash);
>>> -       flash = new;
>>> -
>>>          return 0;
>>>   }
>>>
>>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>>> index 9e18fb4..b15d273 100644
>>> --- a/drivers/mtd/spi/Makefile
>>> +++ b/drivers/mtd/spi/Makefile
>>> @@ -12,6 +12,7 @@ endif
>>>
>>>   obj-$(CONFIG_CMD_SF) += sf.o
>>>   obj-$(CONFIG_SPI_FLASH) += sf_params.o sf_probe.o sf_ops.o
>>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>>   obj-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.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 19d4914..a9f97d1 100644
>>> --- a/drivers/mtd/spi/sf_internal.h
>>> +++ b/drivers/mtd/spi/sf_internal.h
>>> @@ -160,4 +160,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
>>> +
>>>   #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 at 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());
>>
>> This is quite not required for sf - like mtd_number is devnum that is maximum
>> number of sf's on the particular board.
>>
>> We're not dealing with this at this point of time, comments?
>
> this is needed if you enable CFI-MTD and SF-MTD at the same time
> (common case on evaluation boards). Because CFI is always initialized
> in board_init_r() (before SF), it occupies MTD numbers 0 to
> CONFIG_SYS_MAX_FLASH_BANKS - 1.

exactly! Thanks for clarifying.

>>> +
>>> +       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 4d148d1..6c50766 100644
>>> --- a/drivers/mtd/spi/sf_probe.c
>>> +++ b/drivers/mtd/spi/sf_probe.c
>>> @@ -385,6 +385,10 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
>>>          /* Release spi bus */
>>>          spi_release_bus(spi);
>>>
>>> +       ret = spi_flash_mtd_register(flash);
>>> +       if (ret)
>>> +               goto err_claim_bus;
>>> +
>>
>> Be define this code in macro's and place before spi_release_bus()
>
> the defines are already in sf_internal.h. This technique is much more
> elegant and cleaner than polluting the code with #ifdef's
>
> +#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

Yep.

>>
>>>          return flash;
>>>
>>>   err_read_id:
>>> @@ -416,6 +420,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();
>>
>> Same ..
>>
>>>          spi_free_slave(flash->spi);
>>>          free(flash);
>>>   }
>>> --
>>> 1.8.3.1
>>>
>>
>> Code looks straight forward and be define the necessary macro's on new
>> code and please

I am here on Daniels side ... do we really want to introduce here macros?

>> update the test log on doc/SPI. Mean while I will look for more options.

What do you mean exactly?

Thanks for the review!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v3 2/3] mtd, nand: move common functions from cmd_nand.c to common place

[Heiko Schocher]

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>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Tom Rini <trini@ti.com>

---
- changes for v2:
  none
- changes for v3:
  - add comments from scott wood:
    - align MTD_DEV_TYPE_NAND correct
    - remove unnecessary inline
    - rework "jffs2 header" problem later
  - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
---
 common/cmd_nand.c       | 140 +++++++++---------------------------------------
 common/cmd_onenand.c    |  19 +++----
 drivers/mtd/Makefile    |   4 +-
 drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |   7 +++
 5 files changed, 154 insertions(+), 130 deletions(-)
 create mode 100644 drivers/mtd/mtd_uboot.c

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index f9ced9d..099ba00 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;
 		}
@@ -592,7 +488,10 @@ 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];
@@ -654,7 +553,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;
 
 			if (argc > 4 && !str2long(argv[4], &pagecount)) {
@@ -669,8 +572,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 */
@@ -816,7 +723,10 @@ 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 at 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 1526d07..423c346 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -483,5 +483,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__ */
-- 
1.8.3.1

[U-Boot] [PATCH v3 2/3] mtd, nand: move common functions from cmd_nand.c to common place

[Jagan Teki]

Hi Heiko Schocher,

Nice pick -

On 5 September 2014 11:08, 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>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Tom Rini <trini@ti.com>
>
> ---
> - changes for v2:
>   none
> - changes for v3:
>   - add comments from scott wood:
>     - align MTD_DEV_TYPE_NAND correct
>     - remove unnecessary inline
>     - rework "jffs2 header" problem later
>   - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> ---
>  common/cmd_nand.c       | 140 +++++++++---------------------------------------
>  common/cmd_onenand.c    |  19 +++----
>  drivers/mtd/Makefile    |   4 +-
>  drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/mtd.h |   7 +++
>  5 files changed, 154 insertions(+), 130 deletions(-)
>  create mode 100644 drivers/mtd/mtd_uboot.c
>
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index f9ced9d..099ba00 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;
>                 }
> @@ -592,7 +488,10 @@ 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];
> @@ -654,7 +553,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;
>
>                         if (argc > 4 && !str2long(argv[4], &pagecount)) {
> @@ -669,8 +572,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 */
> @@ -816,7 +723,10 @@ 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)

Is this a new function call arg_off_size_onenand again, i guess it's
common call arg_off_size()
Please check?

>                 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

I'm thinking its better to be this file in common instead of
drivers/mtd because this more reusable code
for command stuff instead of mtd core.

And name something like mtd_common or make sense to appear command's
usage instead of
main mtd stuff, IMHO.

>  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 at 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 1526d07..423c346 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -483,5 +483,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__ */
> --
> 1.8.3.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

thanks!
-- 
Jagan.

[U-Boot] [PATCH v3 2/3] mtd, nand: move common functions from cmd_nand.c to common place

[Heiko Schocher]

Hello Jagan,

Am 04.11.2014 21:55, schrieb Jagan Teki:
> Hi Heiko Schocher,
>
> Nice pick -
>
> On 5 September 2014 11:08, 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>
>> Cc: Scott Wood<scottwood@freescale.com>
>> Cc: Tom Rini<trini@ti.com>
>>
>> ---
>> - changes for v2:
>>    none
>> - changes for v3:
>>    - add comments from scott wood:
>>      - align MTD_DEV_TYPE_NAND correct
>>      - remove unnecessary inline
>>      - rework "jffs2 header" problem later
>>    - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>> ---
>>   common/cmd_nand.c       | 140 +++++++++---------------------------------------
>>   common/cmd_onenand.c    |  19 +++----
>>   drivers/mtd/Makefile    |   4 +-
>>   drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/mtd/mtd.h |   7 +++
>>   5 files changed, 154 insertions(+), 130 deletions(-)
>>   create mode 100644 drivers/mtd/mtd_uboot.c
>>
>> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
>> index f9ced9d..099ba00 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;
>>                  }
>> @@ -592,7 +488,10 @@ 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];
>> @@ -654,7 +553,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;
>>
>>                          if (argc>  4&&  !str2long(argv[4],&pagecount)) {
>> @@ -669,8 +572,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 */
>> @@ -816,7 +723,10 @@ 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)
>
> Is this a new function call arg_off_size_onenand again, i guess it's
> common call arg_off_size()
> Please check?

The arg_off_size() differs from the arg_off_size_onenand(), thats
the reason why I let the "arg_off_size_onenand()" in ./common/cmd_onenand.c

I have no board with onenand availiable for testing, so it
is to risky to me, to change the code in ./common/cmd_onenand.c

This should be done from someone, which can really test it!

>>                  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
>
> I'm thinking its better to be this file in common instead of
> drivers/mtd because this more reusable code
> for command stuff instead of mtd core.
>
> And name something like mtd_common or make sense to appear command's
> usage instead of
> main mtd stuff, IMHO.

Currently this is only possible on mtd partitions ... for example:

	if ((*off + *size) > mtd->size) {
                 printf("total chip size (0x%llx) exceeded!\n", mtd->size);
                 return -1;
         }

         if (*size == mtd->size)
                 puts("whole chip\n");


This works only for mtd devices ... so it should be in
drivers/mtd ...  the name of the file ... Hmm... I have
no real preference ...

"mtd_uboot.c"
"mtd_common.c"
"mtd_uboot_common.c"

What do others think?

[...]

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v3 3/3] spi, sf: use offset and size in sf cmd from mtdpartition

[Heiko Schocher]

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>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

---
- changes for v2:
  none
- changes for v3:
  rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
---
 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 7653d7e..fc306ee 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -10,6 +10,8 @@
 #include <div64.h>
 #include <malloc.h>
 #include <spi_flash.h>
+#include <jffs2/jffs2.h>
+#include <linux/mtd/mtd.h>
 
 #include <asm/io.h>
 
@@ -244,23 +246,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 */
@@ -299,31 +299,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;
@@ -545,13 +545,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"
+	"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
 );
-- 
1.8.3.1

[U-Boot] [PATCH v3 0/3] spi, sf: add mtdparts feature to spi and sf commands

[Heiko Schocher]

Hello Jagannadha Sutradharudu Teki,

Am 05.09.2014 07:38, schrieb Heiko Schocher:
> This patchserie add the popssibility to define mtd partitions on
> spi nor flash, and use this settings with the sf commands.
>
> steps:
>
> - add MTD layer driver for spi, original patch from:
> http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=bb246819cdc90493dd7089eaa51b9e639765cced
>
>    and addapted it to current mainline.
>
> - move common functions to get offset and size from
>    cmdline nand command to extract offset and size from
>    a mtd partition to common place "drivers/mtd/mtd_uboot.c"
>    maybe another place is better?
>
> - add to the sf command the possibility to use offset and size from
>    the settings in mtdparts
>
> With this patchset, the sf command looks now:
>
> =>  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
> =>


>
> There are the followings checkpatch warnings:
>
> CHECK: Alignment should match open parenthesis
> #153: FILE: common/cmd_nand.c:217:
> +               if (arg_off(argv[2],&idx,&addr,&maxsize,&maxsize,
> +                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>
> CHECK: Alignment should match open parenthesis
> #179: FILE: common/cmd_nand.c:557:
> +                       if (arg_off(argv[3],&dev,&off,&size,&maxsize,
> +                           MTD_DEV_TYPE_NAND, nand_info[dev].size))
>
> CHECK: Alignment should match open parenthesis
> #193: FILE: common/cmd_nand.c:576:
> +                       if (arg_off_size(argc - 3, argv + 3,&dev,&off,&size,
> +&maxsize, MTD_DEV_TYPE_NAND,
>
> total: 0 errors, 0 warnings, 3 checks, 361 lines checked
>
> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE USLEEP_RANGE
>
> 20140714_ml_mtdparts/0002-mtd-nand-move-common-functions-from-cmd_nand.c-to-co.patch has style problems, please review.
>
> I see not, why this warning pops up ...
>
> - changes for v2:
>    - mtd-spi-add-MTD-layer-driver.patch
>      - add comment from Daniel Schwierzeck:
>        fix compile error from original patch with
>        "static inline" rather than "static __maybe_unused"
> - changes for v3:
>    - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>    - add comments from scott wood:
>      - align MTD_DEV_TYPE_NAND correct
>      - remove unnecessary inline
>      - rework "jffs2 header" problem later
>
> Cc: Scott Wood<scottwood@freescale.com>
> Cc: Tom Rini<trini@ti.com>
> Cc: Daniel Schwierzeck<daniel.schwierzeck@gmail.com>
> Cc: Jagannadha Sutradharudu Teki<jagannadh.teki@gmail.com>

ping? Do you have any objections against this patchseries, or do you
think it is ok?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v3 0/3] spi, sf: add mtdparts feature to spi and sf commands

[Jagan Teki]

On 8 October 2014 11:01, Heiko Schocher <hs@denx.de> wrote:
> Hello Jagannadha Sutradharudu Teki,
>
> Am 05.09.2014 07:38, schrieb Heiko Schocher:
>
>> This patchserie add the popssibility to define mtd partitions on
>> spi nor flash, and use this settings with the sf commands.
>>
>> steps:
>>
>> - add MTD layer driver for spi, original patch from:
>>
>> http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=bb246819cdc90493dd7089eaa51b9e639765cced
>>
>>    and addapted it to current mainline.
>>
>> - move common functions to get offset and size from
>>    cmdline nand command to extract offset and size from
>>    a mtd partition to common place "drivers/mtd/mtd_uboot.c"
>>    maybe another place is better?
>>
>> - add to the sf command the possibility to use offset and size from
>>    the settings in mtdparts
>>
>> With this patchset, the sf command looks now:
>>
>> =>  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
>> =>
>
>
>
>>
>> There are the followings checkpatch warnings:
>>
>> CHECK: Alignment should match open parenthesis
>> #153: FILE: common/cmd_nand.c:217:
>> +               if (arg_off(argv[2],&idx,&addr,&maxsize,&maxsize,
>> +                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>>
>> CHECK: Alignment should match open parenthesis
>> #179: FILE: common/cmd_nand.c:557:
>> +                       if (arg_off(argv[3],&dev,&off,&size,&maxsize,
>> +                           MTD_DEV_TYPE_NAND, nand_info[dev].size))
>>
>> CHECK: Alignment should match open parenthesis
>> #193: FILE: common/cmd_nand.c:576:
>> +                       if (arg_off_size(argc - 3, argv +
>> 3,&dev,&off,&size,
>> +&maxsize, MTD_DEV_TYPE_NAND,
>>
>> total: 0 errors, 0 warnings, 3 checks, 361 lines checked
>>
>> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
>> MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
>> USLEEP_RANGE
>>
>>
>> 20140714_ml_mtdparts/0002-mtd-nand-move-common-functions-from-cmd_nand.c-to-co.patch
>> has style problems, please review.
>>
>> I see not, why this warning pops up ...
>>
>> - changes for v2:
>>    - mtd-spi-add-MTD-layer-driver.patch
>>      - add comment from Daniel Schwierzeck:
>>        fix compile error from original patch with
>>        "static inline" rather than "static __maybe_unused"
>> - changes for v3:
>>    - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>    - add comments from scott wood:
>>      - align MTD_DEV_TYPE_NAND correct
>>      - remove unnecessary inline
>>      - rework "jffs2 header" problem later
>>
>> Cc: Scott Wood<scottwood@freescale.com>
>> Cc: Tom Rini<trini@ti.com>
>> Cc: Daniel Schwierzeck<daniel.schwierzeck@gmail.com>
>> Cc: Jagannadha Sutradharudu Teki<jagannadh.teki@gmail.com>
>
>
> ping? Do you have any objections against this patchseries, or do you
> think it is ok?

Please wait for dm-spi to be happen first, so-that we sync again this mtd on top
of that.

thanks!
-- 
Jagan.

[U-Boot] [PATCH v3 0/3] spi, sf: add mtdparts feature to spi and sf commands

[Heiko Schocher]

Hello Jagan,

Am 09.10.2014 12:38, schrieb Jagan Teki:
> On 8 October 2014 11:01, Heiko Schocher<hs@denx.de>  wrote:
>> Hello Jagannadha Sutradharudu Teki,
>>
>> Am 05.09.2014 07:38, schrieb Heiko Schocher:
>>
>>> This patchserie add the popssibility to define mtd partitions on
>>> spi nor flash, and use this settings with the sf commands.
>>>
>>> steps:
>>>
>>> - add MTD layer driver for spi, original patch from:
>>>
>>> http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=bb246819cdc90493dd7089eaa51b9e639765cced
>>>
>>>     and addapted it to current mainline.
>>>
>>> - move common functions to get offset and size from
>>>     cmdline nand command to extract offset and size from
>>>     a mtd partition to common place "drivers/mtd/mtd_uboot.c"
>>>     maybe another place is better?
>>>
>>> - add to the sf command the possibility to use offset and size from
>>>     the settings in mtdparts
>>>
>>> With this patchset, the sf command looks now:
>>>
>>> =>   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
>>> =>
>>
>>
>>
>>>
>>> There are the followings checkpatch warnings:
>>>
>>> CHECK: Alignment should match open parenthesis
>>> #153: FILE: common/cmd_nand.c:217:
>>> +               if (arg_off(argv[2],&idx,&addr,&maxsize,&maxsize,
>>> +                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>>>
>>> CHECK: Alignment should match open parenthesis
>>> #179: FILE: common/cmd_nand.c:557:
>>> +                       if (arg_off(argv[3],&dev,&off,&size,&maxsize,
>>> +                           MTD_DEV_TYPE_NAND, nand_info[dev].size))
>>>
>>> CHECK: Alignment should match open parenthesis
>>> #193: FILE: common/cmd_nand.c:576:
>>> +                       if (arg_off_size(argc - 3, argv +
>>> 3,&dev,&off,&size,
>>> +&maxsize, MTD_DEV_TYPE_NAND,
>>>
>>> total: 0 errors, 0 warnings, 3 checks, 361 lines checked
>>>
>>> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
>>> MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
>>> USLEEP_RANGE
>>>
>>>
>>> 20140714_ml_mtdparts/0002-mtd-nand-move-common-functions-from-cmd_nand.c-to-co.patch
>>> has style problems, please review.
>>>
>>> I see not, why this warning pops up ...
>>>
>>> - changes for v2:
>>>     - mtd-spi-add-MTD-layer-driver.patch
>>>       - add comment from Daniel Schwierzeck:
>>>         fix compile error from original patch with
>>>         "static inline" rather than "static __maybe_unused"
>>> - changes for v3:
>>>     - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>>     - add comments from scott wood:
>>>       - align MTD_DEV_TYPE_NAND correct
>>>       - remove unnecessary inline
>>>       - rework "jffs2 header" problem later
>>>
>>> Cc: Scott Wood<scottwood@freescale.com>
>>> Cc: Tom Rini<trini@ti.com>
>>> Cc: Daniel Schwierzeck<daniel.schwierzeck@gmail.com>
>>> Cc: Jagannadha Sutradharudu Teki<jagannadh.teki@gmail.com>
>>
>>
>> ping? Do you have any objections against this patchseries, or do you
>> think it is ok?
>
> Please wait for dm-spi to be happen first, so-that we sync again this mtd on top
> of that.

I did just a rebase for this patchset with current ML ... but I lost
track of the patches, for which we are waiting ... Are this patches
now in ML (so I can resend a next version)?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany