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

[U-Boot] [PATCH v6 0/4] 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
+               if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
+                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {

CHECK: Alignment should match open parenthesis
+                       if (arg_off(argv[3], &dev, &off, &size, &maxsize,
+                           MTD_DEV_TYPE_NAND, nand_info[dev].size))

CHECK: Alignment should match open parenthesis
+                       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 ...

resend rebased version of this series, as v3 is pending since
September 2014...

Changes in v6:
- add comments from Jagan Teki:
  new patch in this patchserie, extract this piece
  of code into a new patch.

Changes in v2:
- add comment from Daniel Schwierzeck:
  fix compile error from original patch with
  "static inline" rather than "static __maybe_unused"
Changes in v3:
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
- add comments from scott wood:
  - align MTD_DEV_TYPE_NAND correct
  - remove unnecessary inline
  - rework "jffs2 header" problem later
Changes in v4:
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Changes in v5:
- add comment from Scott Wood:
  keep the continuation line aligned with the arguments

Changes in v6:
- add comments from Jagan Teki:
  move code, which checks if flash pointer is used
  into a new patch.
  - use #ifdef in Code
  - call mtd register before the spi_release_bus
- add Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
- fix Tom Rinis mail addr
- add comment from Scott Wood:
  - fix indentation level
  - add mtd_ prefix
  - move str2off and str2long into common place, as they are no
    mtd specific functions and change return value from int to bool
- add comment from Jagan Teki:
  - append help text
  - add Reviewed-by from Jagannadha Sutradharudu Teki

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

Heiko Schocher (3):
  mtd, nand: move common functions from cmd_nand.c to common place
  spi, sf: use offset and size in sf cmd from mtdpartition
  mtd, spi: check if flash pointer is used

 README                        |   3 +
 common/cmd_nand.c             | 148 +++++++++---------------------------------
 common/cmd_onenand.c          |  19 ++----
 common/cmd_sf.c               |  61 +++++++++--------
 common/cmd_test.c             |  12 +---
 drivers/mtd/Makefile          |   4 +-
 drivers/mtd/mtd_uboot.c       |  99 ++++++++++++++++++++++++++++
 drivers/mtd/spi/Makefile      |   1 +
 drivers/mtd/spi/sf_internal.h |   5 ++
 drivers/mtd/spi/sf_mtd.c      | 104 +++++++++++++++++++++++++++++
 drivers/mtd/spi/sf_probe.c    |  10 +--
 include/linux/mtd/mtd.h       |   5 ++
 include/vsprintf.h            |   2 +
 lib/vsprintf.c                |  16 +++++
 14 files changed, 317 insertions(+), 172 deletions(-)
 create mode 100644 drivers/mtd/mtd_uboot.c
 create mode 100644 drivers/mtd/spi/sf_mtd.c

-- 
2.1.0

[U-Boot] [PATCH v6 1/4] 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>

---

Changes in v6: None
Changes in v2:
- add comment from Daniel Schwierzeck:
  fix compile error from original patch with
  "static inline" rather than "static __maybe_unused"
Series-changes: 3
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
Series-changes: 4
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Series-changes: 5
- no changes
Series-changes: 6
- add comments from Jagan Teki:
  move code, which checks if flash pointer is used
  into a new patch.
  - use #ifdef in Code
  - call mtd register before the spi_release_bus

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

diff --git a/README b/README
index fc1fd52..36f6fc9 100644
--- a/README
+++ b/README
@@ -3097,6 +3097,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 6aabf39..0250011 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 		return 1;
 	}
 
-	if (flash)
-		spi_flash_free(flash);
 	flash = new;
 #endif
 
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
index c61b784e..f8580cd 100644
--- a/drivers/mtd/spi/Makefile
+++ b/drivers/mtd/spi/Makefile
@@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
 #endif
 obj-$(CONFIG_CMD_SF) += sf.o
 obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
+obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
 obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
 obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 785f7a9..8a2eb6e 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -221,4 +221,9 @@ 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);
+#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 d19138d..2342972 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
 	if (spi_enable_wp_pin(flash))
 		puts("Enable WP pin failed\n");
 
-	/* Release spi bus */
-	spi_release_bus(spi);
-
-	return 0;
+#ifdef CONFIG_SPI_FLASH_MTD
+	ret = spi_flash_mtd_register(flash);
+#endif
 
 err_read_id:
 	spi_release_bus(spi);
@@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,
 
 void spi_flash_free(struct spi_flash *flash)
 {
+#ifdef CONFIG_SPI_FLASH_MTD
+	spi_flash_mtd_unregister();
+#endif
 	spi_free_slave(flash->spi);
 	free(flash);
 }
-- 
2.1.0

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

[Jagan Teki]

Hi Heiko,

I have tested this sf-mtd stuff, please see below and enabled prints
in all the func calls.

zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
mtdparts variable not set, see 'help mtdparts'
zynq-uboot> mtdparts

device nor0 <zynq-sf.0>, # parts = 1
 #: name                size            offset          mask_flags
 0: env                 0x00010000      0x00000000      0

active partition: nor0,0 - (env) 0x00010000 @ 0x00000000

defaults:
mtdids  : nor0=zynq-sf.0
mtdparts: none
zynq-uboot> sf erase env 0x10000
spi_flash_erase
spi_flash_cmd_erase_ops
SF: erase d8  0  0  0 (0)
SF: 65536 bytes @ 0x0 Erased: OK
zynq-uboot> mw.b 0x100 0x44 0x10000
zynq-uboot> sf write 0x100 env
device 0 offset 0x0, size 0x10000
spi_flash_cmd_write_ops
SF: 65536 bytes @ 0x0 Written: OK
zynq-uboot> sf read 0x40000 env
device 0 offset 0x0, size 0x10000
spi_flash_cmd_read_ops
off = 0x10000, len = 0
SF: 65536 bytes @ 0x0 Read: OK
zynq-uboot> cmp.b 0x100 0x40000 0x10000
Total of 65536 byte(s) were the same

I wonder none of the sf_mtd_info._* getting called, why?

On 27 April 2015 at 11:12, 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>
>
> ---
>
> Changes in v6: None
> Changes in v2:
> - add comment from Daniel Schwierzeck:
>   fix compile error from original patch with
>   "static inline" rather than "static __maybe_unused"
> Series-changes: 3
> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> Series-changes: 4
> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
> Series-changes: 5
> - no changes
> Series-changes: 6
> - add comments from Jagan Teki:
>   move code, which checks if flash pointer is used
>   into a new patch.
>   - use #ifdef in Code
>   - call mtd register before the spi_release_bus
>
>  README                        |   3 ++
>  common/cmd_sf.c               |   2 -
>  drivers/mtd/spi/Makefile      |   1 +
>  drivers/mtd/spi/sf_internal.h |   5 ++
>  drivers/mtd/spi/sf_mtd.c      | 104 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/mtd/spi/sf_probe.c    |  10 ++--
>  6 files changed, 119 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/mtd/spi/sf_mtd.c
>
> diff --git a/README b/README
> index fc1fd52..36f6fc9 100644
> --- a/README
> +++ b/README
> @@ -3097,6 +3097,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 6aabf39..0250011 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>                 return 1;
>         }
>
> -       if (flash)
> -               spi_flash_free(flash);
>         flash = new;
>  #endif
>
> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
> index c61b784e..f8580cd 100644
> --- a/drivers/mtd/spi/Makefile
> +++ b/drivers/mtd/spi/Makefile
> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>  #endif
>  obj-$(CONFIG_CMD_SF) += sf.o
>  obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>  obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>  obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 785f7a9..8a2eb6e 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -221,4 +221,9 @@ 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);
> +#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;

Even if I remove this every thing as usual, like "sf erase" will call
from cmd_sf
there it calls mtd_arg_off_size, to detected off and size from
partition and after
sf_flash will call erase ops from sf_ops.c.

As it's a mtd call, I thought sf_mtd_into._erase will intern calls
erase ops from sf_ops.c
What is this behavior could you please help me?

> +
> +       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 d19138d..2342972 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
>         if (spi_enable_wp_pin(flash))
>                 puts("Enable WP pin failed\n");
>
> -       /* Release spi bus */
> -       spi_release_bus(spi);
> -
> -       return 0;
> +#ifdef CONFIG_SPI_FLASH_MTD
> +       ret = spi_flash_mtd_register(flash);
> +#endif
>
>  err_read_id:
>         spi_release_bus(spi);
> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,
>
>  void spi_flash_free(struct spi_flash *flash)
>  {
> +#ifdef CONFIG_SPI_FLASH_MTD
> +       spi_flash_mtd_unregister();
> +#endif
>         spi_free_slave(flash->spi);
>         free(flash);
>  }
> --
> 2.1.0
>

thanks!
-- 
Jagan Teki,
Openedev.

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

[Heiko Schocher]

Hello Jagan,

Am 19.05.2015 22:09, schrieb Jagan Teki:
> Hi Heiko,
>
> I have tested this sf-mtd stuff, please see below and enabled prints
> in all the func calls.

Thanks for testing!

> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
> mtdparts variable not set, see 'help mtdparts'
> zynq-uboot> mtdparts
>
> device nor0 <zynq-sf.0>, # parts = 1
>   #: name                size            offset          mask_flags
>   0: env                 0x00010000      0x00000000      0
>
> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>
> defaults:
> mtdids  : nor0=zynq-sf.0
> mtdparts: none
> zynq-uboot> sf erase env 0x10000
> spi_flash_erase
> spi_flash_cmd_erase_ops
> SF: erase d8  0  0  0 (0)
> SF: 65536 bytes @ 0x0 Erased: OK
> zynq-uboot> mw.b 0x100 0x44 0x10000
> zynq-uboot> sf write 0x100 env
> device 0 offset 0x0, size 0x10000
> spi_flash_cmd_write_ops
> SF: 65536 bytes @ 0x0 Written: OK
> zynq-uboot> sf read 0x40000 env
> device 0 offset 0x0, size 0x10000
> spi_flash_cmd_read_ops
> off = 0x10000, len = 0
> SF: 65536 bytes @ 0x0 Read: OK
> zynq-uboot> cmp.b 0x100 0x40000 0x10000
> Total of 65536 byte(s) were the same

Looks good ...

> I wonder none of the sf_mtd_info._* getting called, why?

Hmm.. good question .. you use the "sf ..." commands, they do not
use the mtd interface, right?

I tested this functions with using UBI on the SPI NOR on the
aristainetos and aristianetos2 boards... I added for example in

diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index 0b9cb62..6063ed7 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -39,6 +40,7 @@ static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
         struct spi_flash *flash = mtd->priv;
         int err;

+printf("%s ****\n", __func__);
         err = spi_flash_read(flash, from, len, buf);
         if (!err)
                 *retlen = len;

and see:

=> sf probe
SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB, total 16 MiB
=> mtdparts

device nor0 <spi3.1>, # parts = 4
  #: name                size            offset          mask_flags
  0: u-boot              0x000d0000      0x00000000      0
  1: env                 0x00010000      0x000d0000      0
  2: env-red             0x00010000      0x000e0000      0
  3: rescue-system       0x00f10000      0x000f0000      0

device nand0 <gpmi-nand>, # parts = 1
  #: name                size            offset          mask_flags
  0: ubi                 0x40000000      0x00000000      0

active partition: nor0,0 - (u-boot) 0x000d0000 @ 0x00000000

defaults:
mtdids  : none
mtdparts: none
=> ubi part rescue-system
UBI: default fastmap pool size: 10
UBI: default fastmap WL pool size: 25
UBI: attaching mtd2 to ubi0
UBI DBG gen (pid 1): sizeof(struct ubi_ainf_peb) 48
UBI DBG gen (pid 1): sizeof(struct ubi_wl_entry) 20
UBI DBG gen (pid 1): min_io_size      1
UBI DBG gen (pid 1): max_write_size   256
UBI DBG gen (pid 1): hdrs_min_io_size 1
UBI DBG gen (pid 1): ec_hdr_alsize    64
UBI DBG gen (pid 1): vid_hdr_alsize   64
UBI DBG gen (pid 1): vid_hdr_offset   64
UBI DBG gen (pid 1): vid_hdr_aloffset 64
UBI DBG gen (pid 1): vid_hdr_shift    0
UBI DBG gen (pid 1): leb_start        128
UBI DBG gen (pid 1): max_erroneous    24
UBI DBG gen (pid 1): process PEB 0
UBI DBG bld (pid 1): scan PEB 0
UBI DBG io (pid 1): read EC header from PEB 0
UBI DBG io (pid 1): read 64 bytes from PEB 0:0
spi_flash_mtd_read ****
UBI DBG io (pid 1): read VID header from PEB 0
UBI DBG io (pid 1): read 64 bytes from PEB 0:64
spi_flash_mtd_read ****
[...]

UBI uses the MTD layer ... the sf command not ...
Hope this helps?

bye,
Heiko

> On 27 April 2015 at 11:12, 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>
>>
>> ---
>>
>> Changes in v6: None
>> Changes in v2:
>> - add comment from Daniel Schwierzeck:
>>    fix compile error from original patch with
>>    "static inline" rather than "static __maybe_unused"
>> Series-changes: 3
>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>> Series-changes: 4
>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>> Series-changes: 5
>> - no changes
>> Series-changes: 6
>> - add comments from Jagan Teki:
>>    move code, which checks if flash pointer is used
>>    into a new patch.
>>    - use #ifdef in Code
>>    - call mtd register before the spi_release_bus
>>
>>   README                        |   3 ++
>>   common/cmd_sf.c               |   2 -
>>   drivers/mtd/spi/Makefile      |   1 +
>>   drivers/mtd/spi/sf_internal.h |   5 ++
>>   drivers/mtd/spi/sf_mtd.c      | 104 ++++++++++++++++++++++++++++++++++++++++++
>>   drivers/mtd/spi/sf_probe.c    |  10 ++--
>>   6 files changed, 119 insertions(+), 6 deletions(-)
>>   create mode 100644 drivers/mtd/spi/sf_mtd.c
>>
>> diff --git a/README b/README
>> index fc1fd52..36f6fc9 100644
>> --- a/README
>> +++ b/README
>> @@ -3097,6 +3097,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 6aabf39..0250011 100644
>> --- a/common/cmd_sf.c
>> +++ b/common/cmd_sf.c
>> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>>                  return 1;
>>          }
>>
>> -       if (flash)
>> -               spi_flash_free(flash);
>>          flash = new;
>>   #endif
>>
>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>> index c61b784e..f8580cd 100644
>> --- a/drivers/mtd/spi/Makefile
>> +++ b/drivers/mtd/spi/Makefile
>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>>   #endif
>>   obj-$(CONFIG_CMD_SF) += sf.o
>>   obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>   obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>>   obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index 785f7a9..8a2eb6e 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -221,4 +221,9 @@ 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);
>> +#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;
>
> Even if I remove this every thing as usual, like "sf erase" will call
> from cmd_sf
> there it calls mtd_arg_off_size, to detected off and size from
> partition and after
> sf_flash will call erase ops from sf_ops.c.
>
> As it's a mtd call, I thought sf_mtd_into._erase will intern calls
> erase ops from sf_ops.c
> What is this behavior could you please help me?
>
>> +
>> +       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 d19138d..2342972 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
>>          if (spi_enable_wp_pin(flash))
>>                  puts("Enable WP pin failed\n");
>>
>> -       /* Release spi bus */
>> -       spi_release_bus(spi);
>> -
>> -       return 0;
>> +#ifdef CONFIG_SPI_FLASH_MTD
>> +       ret = spi_flash_mtd_register(flash);
>> +#endif
>>
>>   err_read_id:
>>          spi_release_bus(spi);
>> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,
>>
>>   void spi_flash_free(struct spi_flash *flash)
>>   {
>> +#ifdef CONFIG_SPI_FLASH_MTD
>> +       spi_flash_mtd_unregister();
>> +#endif
>>          spi_free_slave(flash->spi);
>>          free(flash);
>>   }
>> --
>> 2.1.0
>>
>
> thanks!
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Jagan Teki]

Hi Heiko,

On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
> Hello Jagan,
>
> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>
>> Hi Heiko,
>>
>> I have tested this sf-mtd stuff, please see below and enabled prints
>> in all the func calls.
>
>
> Thanks for testing!
>
>
>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>> mtdparts variable not set, see 'help mtdparts'
>> zynq-uboot> mtdparts
>>
>> device nor0 <zynq-sf.0>, # parts = 1
>>   #: name                size            offset          mask_flags
>>   0: env                 0x00010000      0x00000000      0
>>
>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>
>> defaults:
>> mtdids  : nor0=zynq-sf.0
>> mtdparts: none
>> zynq-uboot> sf erase env 0x10000
>> spi_flash_erase
>> spi_flash_cmd_erase_ops
>> SF: erase d8  0  0  0 (0)
>> SF: 65536 bytes @ 0x0 Erased: OK
>> zynq-uboot> mw.b 0x100 0x44 0x10000
>> zynq-uboot> sf write 0x100 env
>> device 0 offset 0x0, size 0x10000
>> spi_flash_cmd_write_ops
>> SF: 65536 bytes @ 0x0 Written: OK
>> zynq-uboot> sf read 0x40000 env
>> device 0 offset 0x0, size 0x10000
>> spi_flash_cmd_read_ops
>> off = 0x10000, len = 0
>> SF: 65536 bytes @ 0x0 Read: OK
>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>> Total of 65536 byte(s) were the same
>
>
> Looks good ...
>
>> I wonder none of the sf_mtd_info._* getting called, why?
>
>
> Hmm.. good question .. you use the "sf ..." commands, they do not
> use the mtd interface, right?

I'm fine with the testing, but mtd code in sf seems used only for in UBI only.
I wouldn't see this is a better approach where mtd code is considered as to
be unknown thing for sf.

>
> I tested this functions with using UBI on the SPI NOR on the
> aristainetos and aristianetos2 boards... I added for example in
>
> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
> index 0b9cb62..6063ed7 100644
> --- a/drivers/mtd/spi/sf_mtd.c
> +++ b/drivers/mtd/spi/sf_mtd.c
> @@ -39,6 +40,7 @@ static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t
> from, size_t len,
>         struct spi_flash *flash = mtd->priv;
>         int err;
>
> +printf("%s ****\n", __func__);
>         err = spi_flash_read(flash, from, len, buf);
>         if (!err)
>                 *retlen = len;
>
> and see:
>
> => sf probe
> SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB, total 16
> MiB
> => mtdparts
>
> device nor0 <spi3.1>, # parts = 4
>  #: name                size            offset          mask_flags
>  0: u-boot              0x000d0000      0x00000000      0
>  1: env                 0x00010000      0x000d0000      0
>  2: env-red             0x00010000      0x000e0000      0
>  3: rescue-system       0x00f10000      0x000f0000      0
>
> device nand0 <gpmi-nand>, # parts = 1
>  #: name                size            offset          mask_flags
>  0: ubi                 0x40000000      0x00000000      0
>
> active partition: nor0,0 - (u-boot) 0x000d0000 @ 0x00000000
>
> defaults:
> mtdids  : none
> mtdparts: none
> => ubi part rescue-system
> UBI: default fastmap pool size: 10
> UBI: default fastmap WL pool size: 25
> UBI: attaching mtd2 to ubi0
> UBI DBG gen (pid 1): sizeof(struct ubi_ainf_peb) 48
> UBI DBG gen (pid 1): sizeof(struct ubi_wl_entry) 20
> UBI DBG gen (pid 1): min_io_size      1
> UBI DBG gen (pid 1): max_write_size   256
> UBI DBG gen (pid 1): hdrs_min_io_size 1
> UBI DBG gen (pid 1): ec_hdr_alsize    64
> UBI DBG gen (pid 1): vid_hdr_alsize   64
> UBI DBG gen (pid 1): vid_hdr_offset   64
> UBI DBG gen (pid 1): vid_hdr_aloffset 64
> UBI DBG gen (pid 1): vid_hdr_shift    0
> UBI DBG gen (pid 1): leb_start        128
> UBI DBG gen (pid 1): max_erroneous    24
> UBI DBG gen (pid 1): process PEB 0
> UBI DBG bld (pid 1): scan PEB 0
> UBI DBG io (pid 1): read EC header from PEB 0
> UBI DBG io (pid 1): read 64 bytes from PEB 0:0
> spi_flash_mtd_read ****
> UBI DBG io (pid 1): read VID header from PEB 0
> UBI DBG io (pid 1): read 64 bytes from PEB 0:64
> spi_flash_mtd_read ****
> [...]
>
> UBI uses the MTD layer ... the sf command not ...
> Hope this helps?
>
> bye,
> Heiko
>
>
>> On 27 April 2015 at 11:12, 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>
>>>
>>> ---
>>>
>>> Changes in v6: None
>>> Changes in v2:
>>> - add comment from Daniel Schwierzeck:
>>>    fix compile error from original patch with
>>>    "static inline" rather than "static __maybe_unused"
>>> Series-changes: 3
>>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>> Series-changes: 4
>>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>>> Series-changes: 5
>>> - no changes
>>> Series-changes: 6
>>> - add comments from Jagan Teki:
>>>    move code, which checks if flash pointer is used
>>>    into a new patch.
>>>    - use #ifdef in Code
>>>    - call mtd register before the spi_release_bus
>>>
>>>   README                        |   3 ++
>>>   common/cmd_sf.c               |   2 -
>>>   drivers/mtd/spi/Makefile      |   1 +
>>>   drivers/mtd/spi/sf_internal.h |   5 ++
>>>   drivers/mtd/spi/sf_mtd.c      | 104
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>   drivers/mtd/spi/sf_probe.c    |  10 ++--
>>>   6 files changed, 119 insertions(+), 6 deletions(-)
>>>   create mode 100644 drivers/mtd/spi/sf_mtd.c
>>>
>>> diff --git a/README b/README
>>> index fc1fd52..36f6fc9 100644
>>> --- a/README
>>> +++ b/README
>>> @@ -3097,6 +3097,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 6aabf39..0250011 100644
>>> --- a/common/cmd_sf.c
>>> +++ b/common/cmd_sf.c
>>> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char * const
>>> argv[])
>>>                  return 1;
>>>          }
>>>
>>> -       if (flash)
>>> -               spi_flash_free(flash);
>>>          flash = new;
>>>   #endif
>>>
>>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>>> index c61b784e..f8580cd 100644
>>> --- a/drivers/mtd/spi/Makefile
>>> +++ b/drivers/mtd/spi/Makefile
>>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>>>   #endif
>>>   obj-$(CONFIG_CMD_SF) += sf.o
>>>   obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
>>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>>   obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>>>   obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
>>> diff --git a/drivers/mtd/spi/sf_internal.h
>>> b/drivers/mtd/spi/sf_internal.h
>>> index 785f7a9..8a2eb6e 100644
>>> --- a/drivers/mtd/spi/sf_internal.h
>>> +++ b/drivers/mtd/spi/sf_internal.h
>>> @@ -221,4 +221,9 @@ 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);
>>> +#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;
>>
>>
>> Even if I remove this every thing as usual, like "sf erase" will call
>> from cmd_sf
>> there it calls mtd_arg_off_size, to detected off and size from
>> partition and after
>> sf_flash will call erase ops from sf_ops.c.
>>
>> As it's a mtd call, I thought sf_mtd_into._erase will intern calls
>> erase ops from sf_ops.c
>> What is this behavior could you please help me?
>>
>>> +
>>> +       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 d19138d..2342972 100644
>>> --- a/drivers/mtd/spi/sf_probe.c
>>> +++ b/drivers/mtd/spi/sf_probe.c
>>> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi,
>>> struct spi_flash *flash)
>>>          if (spi_enable_wp_pin(flash))
>>>                  puts("Enable WP pin failed\n");
>>>
>>> -       /* Release spi bus */
>>> -       spi_release_bus(spi);
>>> -
>>> -       return 0;
>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>> +       ret = spi_flash_mtd_register(flash);
>>> +#endif
>>>
>>>   err_read_id:
>>>          spi_release_bus(spi);
>>> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void
>>> *blob, int slave_node,
>>>
>>>   void spi_flash_free(struct spi_flash *flash)
>>>   {
>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>> +       spi_flash_mtd_unregister();
>>> +#endif
>>>          spi_free_slave(flash->spi);
>>>          free(flash);
>>>   }
>>> --
>>> 2.1.0

thanks!
-- 
Jagan | Openedev.

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

[Heiko Schocher denx]

Hello Jagan,

Am 16.06.2015 um 10:04 schrieb Jagan Teki:
> Hi Heiko,
>
> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>> Hello Jagan,
>>
>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>
>>> Hi Heiko,
>>>
>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>> in all the func calls.
>>
>>
>> Thanks for testing!
>>
>>
>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>> mtdparts variable not set, see 'help mtdparts'
>>> zynq-uboot> mtdparts
>>>
>>> device nor0 <zynq-sf.0>, # parts = 1
>>>    #: name                size            offset          mask_flags
>>>    0: env                 0x00010000      0x00000000      0
>>>
>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>
>>> defaults:
>>> mtdids  : nor0=zynq-sf.0
>>> mtdparts: none
>>> zynq-uboot> sf erase env 0x10000
>>> spi_flash_erase
>>> spi_flash_cmd_erase_ops
>>> SF: erase d8  0  0  0 (0)
>>> SF: 65536 bytes @ 0x0 Erased: OK
>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>> zynq-uboot> sf write 0x100 env
>>> device 0 offset 0x0, size 0x10000
>>> spi_flash_cmd_write_ops
>>> SF: 65536 bytes @ 0x0 Written: OK
>>> zynq-uboot> sf read 0x40000 env
>>> device 0 offset 0x0, size 0x10000
>>> spi_flash_cmd_read_ops
>>> off = 0x10000, len = 0
>>> SF: 65536 bytes @ 0x0 Read: OK
>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>> Total of 65536 byte(s) were the same
>>
>>
>> Looks good ...
>>
>>> I wonder none of the sf_mtd_info._* getting called, why?
>>
>>
>> Hmm.. good question .. you use the "sf ..." commands, they do not
>> use the mtd interface, right?
>
> I'm fine with the testing, but mtd code in sf seems used only for in UBI only.

a fast "grep mtd_read" in the u-boot source shows, that yaffs2
also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
be used also with spi flashes now ... if this is wise, I don;t know ...

I tested with UBI on a spi flash, and that works ... in the same
fashion it currently does on nand for example ...

> I wouldn't see this is a better approach where mtd code is considered as to
> be unknown thing for sf.

I do not understand you here complete ...

drivers/mtd/spi/sf_mtd.c

adds just spi flash specific functions to integrate spi flashes
into mtd ... and mtd users can then read/write/erase
with the mtd_* functions ...

Maybe someone has time to convert common/sf.c to use them?

So, the final question is, can this patches go into mainline?

thanks!

bye,
Heiko
>> I tested this functions with using UBI on the SPI NOR on the
>> aristainetos and aristianetos2 boards... I added for example in
>>
>> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
>> index 0b9cb62..6063ed7 100644
>> --- a/drivers/mtd/spi/sf_mtd.c
>> +++ b/drivers/mtd/spi/sf_mtd.c
>> @@ -39,6 +40,7 @@ static int spi_flash_mtd_read(struct mtd_info *mtd, loff_t
>> from, size_t len,
>>          struct spi_flash *flash = mtd->priv;
>>          int err;
>>
>> +printf("%s ****\n", __func__);
>>          err = spi_flash_read(flash, from, len, buf);
>>          if (!err)
>>                  *retlen = len;
>>
>> and see:
>>
>> => sf probe
>> SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB, total 16
>> MiB
>> => mtdparts
>>
>> device nor0 <spi3.1>, # parts = 4
>>   #: name                size            offset          mask_flags
>>   0: u-boot              0x000d0000      0x00000000      0
>>   1: env                 0x00010000      0x000d0000      0
>>   2: env-red             0x00010000      0x000e0000      0
>>   3: rescue-system       0x00f10000      0x000f0000      0
>>
>> device nand0 <gpmi-nand>, # parts = 1
>>   #: name                size            offset          mask_flags
>>   0: ubi                 0x40000000      0x00000000      0
>>
>> active partition: nor0,0 - (u-boot) 0x000d0000 @ 0x00000000
>>
>> defaults:
>> mtdids  : none
>> mtdparts: none
>> => ubi part rescue-system
>> UBI: default fastmap pool size: 10
>> UBI: default fastmap WL pool size: 25
>> UBI: attaching mtd2 to ubi0
>> UBI DBG gen (pid 1): sizeof(struct ubi_ainf_peb) 48
>> UBI DBG gen (pid 1): sizeof(struct ubi_wl_entry) 20
>> UBI DBG gen (pid 1): min_io_size      1
>> UBI DBG gen (pid 1): max_write_size   256
>> UBI DBG gen (pid 1): hdrs_min_io_size 1
>> UBI DBG gen (pid 1): ec_hdr_alsize    64
>> UBI DBG gen (pid 1): vid_hdr_alsize   64
>> UBI DBG gen (pid 1): vid_hdr_offset   64
>> UBI DBG gen (pid 1): vid_hdr_aloffset 64
>> UBI DBG gen (pid 1): vid_hdr_shift    0
>> UBI DBG gen (pid 1): leb_start        128
>> UBI DBG gen (pid 1): max_erroneous    24
>> UBI DBG gen (pid 1): process PEB 0
>> UBI DBG bld (pid 1): scan PEB 0
>> UBI DBG io (pid 1): read EC header from PEB 0
>> UBI DBG io (pid 1): read 64 bytes from PEB 0:0
>> spi_flash_mtd_read ****
>> UBI DBG io (pid 1): read VID header from PEB 0
>> UBI DBG io (pid 1): read 64 bytes from PEB 0:64
>> spi_flash_mtd_read ****
>> [...]
>>
>> UBI uses the MTD layer ... the sf command not ...
>> Hope this helps?
>>
>> bye,
>> Heiko
>>
>>
>>> On 27 April 2015 at 11:12, 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>
>>>>
>>>> ---
>>>>
>>>> Changes in v6: None
>>>> Changes in v2:
>>>> - add comment from Daniel Schwierzeck:
>>>>     fix compile error from original patch with
>>>>     "static inline" rather than "static __maybe_unused"
>>>> Series-changes: 3
>>>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>>> Series-changes: 4
>>>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>>>> Series-changes: 5
>>>> - no changes
>>>> Series-changes: 6
>>>> - add comments from Jagan Teki:
>>>>     move code, which checks if flash pointer is used
>>>>     into a new patch.
>>>>     - use #ifdef in Code
>>>>     - call mtd register before the spi_release_bus
>>>>
>>>>    README                        |   3 ++
>>>>    common/cmd_sf.c               |   2 -
>>>>    drivers/mtd/spi/Makefile      |   1 +
>>>>    drivers/mtd/spi/sf_internal.h |   5 ++
>>>>    drivers/mtd/spi/sf_mtd.c      | 104
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>    drivers/mtd/spi/sf_probe.c    |  10 ++--
>>>>    6 files changed, 119 insertions(+), 6 deletions(-)
>>>>    create mode 100644 drivers/mtd/spi/sf_mtd.c
>>>>
>>>> diff --git a/README b/README
>>>> index fc1fd52..36f6fc9 100644
>>>> --- a/README
>>>> +++ b/README
>>>> @@ -3097,6 +3097,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 6aabf39..0250011 100644
>>>> --- a/common/cmd_sf.c
>>>> +++ b/common/cmd_sf.c
>>>> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char * const
>>>> argv[])
>>>>                   return 1;
>>>>           }
>>>>
>>>> -       if (flash)
>>>> -               spi_flash_free(flash);
>>>>           flash = new;
>>>>    #endif
>>>>
>>>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>>>> index c61b784e..f8580cd 100644
>>>> --- a/drivers/mtd/spi/Makefile
>>>> +++ b/drivers/mtd/spi/Makefile
>>>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>>>>    #endif
>>>>    obj-$(CONFIG_CMD_SF) += sf.o
>>>>    obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
>>>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>>>    obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>>>>    obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
>>>> diff --git a/drivers/mtd/spi/sf_internal.h
>>>> b/drivers/mtd/spi/sf_internal.h
>>>> index 785f7a9..8a2eb6e 100644
>>>> --- a/drivers/mtd/spi/sf_internal.h
>>>> +++ b/drivers/mtd/spi/sf_internal.h
>>>> @@ -221,4 +221,9 @@ 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);
>>>> +#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;
>>>
>>>
>>> Even if I remove this every thing as usual, like "sf erase" will call
>>> from cmd_sf
>>> there it calls mtd_arg_off_size, to detected off and size from
>>> partition and after
>>> sf_flash will call erase ops from sf_ops.c.
>>>
>>> As it's a mtd call, I thought sf_mtd_into._erase will intern calls
>>> erase ops from sf_ops.c
>>> What is this behavior could you please help me?
>>>
>>>> +
>>>> +       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 d19138d..2342972 100644
>>>> --- a/drivers/mtd/spi/sf_probe.c
>>>> +++ b/drivers/mtd/spi/sf_probe.c
>>>> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi,
>>>> struct spi_flash *flash)
>>>>           if (spi_enable_wp_pin(flash))
>>>>                   puts("Enable WP pin failed\n");
>>>>
>>>> -       /* Release spi bus */
>>>> -       spi_release_bus(spi);
>>>> -
>>>> -       return 0;
>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>> +       ret = spi_flash_mtd_register(flash);
>>>> +#endif
>>>>
>>>>    err_read_id:
>>>>           spi_release_bus(spi);
>>>> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void
>>>> *blob, int slave_node,
>>>>
>>>>    void spi_flash_free(struct spi_flash *flash)
>>>>    {
>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>> +       spi_flash_mtd_unregister();
>>>> +#endif
>>>>           spi_free_slave(flash->spi);
>>>>           free(flash);
>>>>    }
>>>> --
>>>> 2.1.0
>
> thanks!
>

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

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

[Jagan Teki]

Hi Heiko,

On 16 June 2015 at 14:13, Heiko Schocher denx <hs@denx.de> wrote:
> Hello Jagan,
>
>
> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>
>> Hi Heiko,
>>
>> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>>>
>>> Hello Jagan,
>>>
>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>
>>>>
>>>> Hi Heiko,
>>>>
>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>> in all the func calls.
>>>
>>>
>>>
>>> Thanks for testing!
>>>
>>>
>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>> mtdparts variable not set, see 'help mtdparts'
>>>> zynq-uboot> mtdparts
>>>>
>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>    #: name                size            offset          mask_flags
>>>>    0: env                 0x00010000      0x00000000      0
>>>>
>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>
>>>> defaults:
>>>> mtdids  : nor0=zynq-sf.0
>>>> mtdparts: none
>>>> zynq-uboot> sf erase env 0x10000
>>>> spi_flash_erase
>>>> spi_flash_cmd_erase_ops
>>>> SF: erase d8  0  0  0 (0)
>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>> zynq-uboot> sf write 0x100 env
>>>> device 0 offset 0x0, size 0x10000
>>>> spi_flash_cmd_write_ops
>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>> zynq-uboot> sf read 0x40000 env
>>>> device 0 offset 0x0, size 0x10000
>>>> spi_flash_cmd_read_ops
>>>> off = 0x10000, len = 0
>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>> Total of 65536 byte(s) were the same
>>>
>>>
>>>
>>> Looks good ...
>>>
>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>
>>>
>>>
>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>> use the mtd interface, right?
>>
>>
>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>> only.
>
>
> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
> be used also with spi flashes now ... if this is wise, I don;t know ...
>
> I tested with UBI on a spi flash, and that works ... in the same
> fashion it currently does on nand for example ...
>
>> I wouldn't see this is a better approach where mtd code is considered as
>> to
>> be unknown thing for sf.
>
>
> I do not understand you here complete ...
>
> drivers/mtd/spi/sf_mtd.c
>
> adds just spi flash specific functions to integrate spi flashes
> into mtd ... and mtd users can then read/write/erase
> with the mtd_* functions ...
>
> Maybe someone has time to convert common/sf.c to use them?
>
> So, the final question is, can this patches go into mainline?

The only point I'm concerned here is If I need to use sf with mtd support
without using ubifs or any flash filesystem, the code in sf_mtd.c ops becomes
unused.

This seems to be a code size increasing factor which is obviously not a good
point for bootloader atleast for u-boot, what do you think?

I agreed your concerned for someone may add support to common/cmd_sf.c
in future, but I'm bit worried to go this.

>
>>> I tested this functions with using UBI on the SPI NOR on the
>>> aristainetos and aristianetos2 boards... I added for example in
>>>
>>> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
>>> index 0b9cb62..6063ed7 100644
>>> --- a/drivers/mtd/spi/sf_mtd.c
>>> +++ b/drivers/mtd/spi/sf_mtd.c
>>> @@ -39,6 +40,7 @@ static int spi_flash_mtd_read(struct mtd_info *mtd,
>>> loff_t
>>> from, size_t len,
>>>          struct spi_flash *flash = mtd->priv;
>>>          int err;
>>>
>>> +printf("%s ****\n", __func__);
>>>          err = spi_flash_read(flash, from, len, buf);
>>>          if (!err)
>>>                  *retlen = len;
>>>
>>> and see:
>>>
>>> => sf probe
>>> SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB, total
>>> 16
>>> MiB
>>> => mtdparts
>>>
>>> device nor0 <spi3.1>, # parts = 4
>>>   #: name                size            offset          mask_flags
>>>   0: u-boot              0x000d0000      0x00000000      0
>>>   1: env                 0x00010000      0x000d0000      0
>>>   2: env-red             0x00010000      0x000e0000      0
>>>   3: rescue-system       0x00f10000      0x000f0000      0
>>>
>>> device nand0 <gpmi-nand>, # parts = 1
>>>   #: name                size            offset          mask_flags
>>>   0: ubi                 0x40000000      0x00000000      0
>>>
>>> active partition: nor0,0 - (u-boot) 0x000d0000 @ 0x00000000
>>>
>>> defaults:
>>> mtdids  : none
>>> mtdparts: none
>>> => ubi part rescue-system
>>> UBI: default fastmap pool size: 10
>>> UBI: default fastmap WL pool size: 25
>>> UBI: attaching mtd2 to ubi0
>>> UBI DBG gen (pid 1): sizeof(struct ubi_ainf_peb) 48
>>> UBI DBG gen (pid 1): sizeof(struct ubi_wl_entry) 20
>>> UBI DBG gen (pid 1): min_io_size      1
>>> UBI DBG gen (pid 1): max_write_size   256
>>> UBI DBG gen (pid 1): hdrs_min_io_size 1
>>> UBI DBG gen (pid 1): ec_hdr_alsize    64
>>> UBI DBG gen (pid 1): vid_hdr_alsize   64
>>> UBI DBG gen (pid 1): vid_hdr_offset   64
>>> UBI DBG gen (pid 1): vid_hdr_aloffset 64
>>> UBI DBG gen (pid 1): vid_hdr_shift    0
>>> UBI DBG gen (pid 1): leb_start        128
>>> UBI DBG gen (pid 1): max_erroneous    24
>>> UBI DBG gen (pid 1): process PEB 0
>>> UBI DBG bld (pid 1): scan PEB 0
>>> UBI DBG io (pid 1): read EC header from PEB 0
>>> UBI DBG io (pid 1): read 64 bytes from PEB 0:0
>>> spi_flash_mtd_read ****
>>> UBI DBG io (pid 1): read VID header from PEB 0
>>> UBI DBG io (pid 1): read 64 bytes from PEB 0:64
>>> spi_flash_mtd_read ****
>>> [...]
>>>
>>> UBI uses the MTD layer ... the sf command not ...
>>> Hope this helps?
>>>
>>> bye,
>>> Heiko
>>>
>>>
>>>> On 27 April 2015 at 11:12, 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>
>>>>>
>>>>> ---
>>>>>
>>>>> Changes in v6: None
>>>>> Changes in v2:
>>>>> - add comment from Daniel Schwierzeck:
>>>>>     fix compile error from original patch with
>>>>>     "static inline" rather than "static __maybe_unused"
>>>>> Series-changes: 3
>>>>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>>>> Series-changes: 4
>>>>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>>>>> Series-changes: 5
>>>>> - no changes
>>>>> Series-changes: 6
>>>>> - add comments from Jagan Teki:
>>>>>     move code, which checks if flash pointer is used
>>>>>     into a new patch.
>>>>>     - use #ifdef in Code
>>>>>     - call mtd register before the spi_release_bus
>>>>>
>>>>>    README                        |   3 ++
>>>>>    common/cmd_sf.c               |   2 -
>>>>>    drivers/mtd/spi/Makefile      |   1 +
>>>>>    drivers/mtd/spi/sf_internal.h |   5 ++
>>>>>    drivers/mtd/spi/sf_mtd.c      | 104
>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>    drivers/mtd/spi/sf_probe.c    |  10 ++--
>>>>>    6 files changed, 119 insertions(+), 6 deletions(-)
>>>>>    create mode 100644 drivers/mtd/spi/sf_mtd.c
>>>>>
>>>>> diff --git a/README b/README
>>>>> index fc1fd52..36f6fc9 100644
>>>>> --- a/README
>>>>> +++ b/README
>>>>> @@ -3097,6 +3097,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 6aabf39..0250011 100644
>>>>> --- a/common/cmd_sf.c
>>>>> +++ b/common/cmd_sf.c
>>>>> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char *
>>>>> const
>>>>> argv[])
>>>>>                   return 1;
>>>>>           }
>>>>>
>>>>> -       if (flash)
>>>>> -               spi_flash_free(flash);
>>>>>           flash = new;
>>>>>    #endif
>>>>>
>>>>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>>>>> index c61b784e..f8580cd 100644
>>>>> --- a/drivers/mtd/spi/Makefile
>>>>> +++ b/drivers/mtd/spi/Makefile
>>>>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>>>>>    #endif
>>>>>    obj-$(CONFIG_CMD_SF) += sf.o
>>>>>    obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
>>>>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>>>>    obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>>>>>    obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
>>>>> diff --git a/drivers/mtd/spi/sf_internal.h
>>>>> b/drivers/mtd/spi/sf_internal.h
>>>>> index 785f7a9..8a2eb6e 100644
>>>>> --- a/drivers/mtd/spi/sf_internal.h
>>>>> +++ b/drivers/mtd/spi/sf_internal.h
>>>>> @@ -221,4 +221,9 @@ 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);
>>>>> +#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;
>>>>
>>>>
>>>>
>>>> Even if I remove this every thing as usual, like "sf erase" will call
>>>> from cmd_sf
>>>> there it calls mtd_arg_off_size, to detected off and size from
>>>> partition and after
>>>> sf_flash will call erase ops from sf_ops.c.
>>>>
>>>> As it's a mtd call, I thought sf_mtd_into._erase will intern calls
>>>> erase ops from sf_ops.c
>>>> What is this behavior could you please help me?
>>>>
>>>>> +
>>>>> +       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 d19138d..2342972 100644
>>>>> --- a/drivers/mtd/spi/sf_probe.c
>>>>> +++ b/drivers/mtd/spi/sf_probe.c
>>>>> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi,
>>>>> struct spi_flash *flash)
>>>>>           if (spi_enable_wp_pin(flash))
>>>>>                   puts("Enable WP pin failed\n");
>>>>>
>>>>> -       /* Release spi bus */
>>>>> -       spi_release_bus(spi);
>>>>> -
>>>>> -       return 0;
>>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>>> +       ret = spi_flash_mtd_register(flash);
>>>>> +#endif
>>>>>
>>>>>    err_read_id:
>>>>>           spi_release_bus(spi);
>>>>> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void
>>>>> *blob, int slave_node,
>>>>>
>>>>>    void spi_flash_free(struct spi_flash *flash)
>>>>>    {
>>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>>> +       spi_flash_mtd_unregister();
>>>>> +#endif
>>>>>           spi_free_slave(flash->spi);
>>>>>           free(flash);
>>>>>    }
>>>>> --
>>>>> 2.1.0

thanks!
-- 
Jagan | openedev.

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

[Heiko Schocher denx]

Hello Jagan,

Am 16.06.2015 um 10:52 schrieb Jagan Teki:
> Hi Heiko,
>
> On 16 June 2015 at 14:13, Heiko Schocher denx <hs@denx.de> wrote:
>> Hello Jagan,
>>
>>
>> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>>
>>> Hi Heiko,
>>>
>>> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>>>>
>>>> Hello Jagan,
>>>>
>>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>>
>>>>>
>>>>> Hi Heiko,
>>>>>
>>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>>> in all the func calls.
>>>>
>>>>
>>>>
>>>> Thanks for testing!
>>>>
>>>>
>>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>>> mtdparts variable not set, see 'help mtdparts'
>>>>> zynq-uboot> mtdparts
>>>>>
>>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>>     #: name                size            offset          mask_flags
>>>>>     0: env                 0x00010000      0x00000000      0
>>>>>
>>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>>
>>>>> defaults:
>>>>> mtdids  : nor0=zynq-sf.0
>>>>> mtdparts: none
>>>>> zynq-uboot> sf erase env 0x10000
>>>>> spi_flash_erase
>>>>> spi_flash_cmd_erase_ops
>>>>> SF: erase d8  0  0  0 (0)
>>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>>> zynq-uboot> sf write 0x100 env
>>>>> device 0 offset 0x0, size 0x10000
>>>>> spi_flash_cmd_write_ops
>>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>>> zynq-uboot> sf read 0x40000 env
>>>>> device 0 offset 0x0, size 0x10000
>>>>> spi_flash_cmd_read_ops
>>>>> off = 0x10000, len = 0
>>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>>> Total of 65536 byte(s) were the same
>>>>
>>>>
>>>>
>>>> Looks good ...
>>>>
>>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>>
>>>>
>>>>
>>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>>> use the mtd interface, right?
>>>
>>>
>>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>>> only.
>>
>>
>> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
>> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
>> be used also with spi flashes now ... if this is wise, I don;t know ...
>>
>> I tested with UBI on a spi flash, and that works ... in the same
>> fashion it currently does on nand for example ...
>>
>>> I wouldn't see this is a better approach where mtd code is considered as
>>> to
>>> be unknown thing for sf.
>>
>>
>> I do not understand you here complete ...
>>
>> drivers/mtd/spi/sf_mtd.c
>>
>> adds just spi flash specific functions to integrate spi flashes
>> into mtd ... and mtd users can then read/write/erase
>> with the mtd_* functions ...
>>
>> Maybe someone has time to convert common/sf.c to use them?
>>
>> So, the final question is, can this patches go into mainline?
>
> The only point I'm concerned here is If I need to use sf with mtd support
> without using ubifs or any flash filesystem, the code in sf_mtd.c ops becomes
> unused.

Why you want to enable mtd support for sf, if you not use it?
do not define CONFIG_SPI_FLASH_MTD in this case?

> This seems to be a code size increasing factor which is obviously not a good
> point for bootloader atleast for u-boot, what do you think?
>
> I agreed your concerned for someone may add support to common/cmd_sf.c
> in future, but I'm bit worried to go this.

Why? Its the same state as it is in the nand subsystem ...

bye,
Heiko
>
>>
>>>> I tested this functions with using UBI on the SPI NOR on the
>>>> aristainetos and aristianetos2 boards... I added for example in
>>>>
>>>> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
>>>> index 0b9cb62..6063ed7 100644
>>>> --- a/drivers/mtd/spi/sf_mtd.c
>>>> +++ b/drivers/mtd/spi/sf_mtd.c
>>>> @@ -39,6 +40,7 @@ static int spi_flash_mtd_read(struct mtd_info *mtd,
>>>> loff_t
>>>> from, size_t len,
>>>>           struct spi_flash *flash = mtd->priv;
>>>>           int err;
>>>>
>>>> +printf("%s ****\n", __func__);
>>>>           err = spi_flash_read(flash, from, len, buf);
>>>>           if (!err)
>>>>                   *retlen = len;
>>>>
>>>> and see:
>>>>
>>>> => sf probe
>>>> SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB, total
>>>> 16
>>>> MiB
>>>> => mtdparts
>>>>
>>>> device nor0 <spi3.1>, # parts = 4
>>>>    #: name                size            offset          mask_flags
>>>>    0: u-boot              0x000d0000      0x00000000      0
>>>>    1: env                 0x00010000      0x000d0000      0
>>>>    2: env-red             0x00010000      0x000e0000      0
>>>>    3: rescue-system       0x00f10000      0x000f0000      0
>>>>
>>>> device nand0 <gpmi-nand>, # parts = 1
>>>>    #: name                size            offset          mask_flags
>>>>    0: ubi                 0x40000000      0x00000000      0
>>>>
>>>> active partition: nor0,0 - (u-boot) 0x000d0000 @ 0x00000000
>>>>
>>>> defaults:
>>>> mtdids  : none
>>>> mtdparts: none
>>>> => ubi part rescue-system
>>>> UBI: default fastmap pool size: 10
>>>> UBI: default fastmap WL pool size: 25
>>>> UBI: attaching mtd2 to ubi0
>>>> UBI DBG gen (pid 1): sizeof(struct ubi_ainf_peb) 48
>>>> UBI DBG gen (pid 1): sizeof(struct ubi_wl_entry) 20
>>>> UBI DBG gen (pid 1): min_io_size      1
>>>> UBI DBG gen (pid 1): max_write_size   256
>>>> UBI DBG gen (pid 1): hdrs_min_io_size 1
>>>> UBI DBG gen (pid 1): ec_hdr_alsize    64
>>>> UBI DBG gen (pid 1): vid_hdr_alsize   64
>>>> UBI DBG gen (pid 1): vid_hdr_offset   64
>>>> UBI DBG gen (pid 1): vid_hdr_aloffset 64
>>>> UBI DBG gen (pid 1): vid_hdr_shift    0
>>>> UBI DBG gen (pid 1): leb_start        128
>>>> UBI DBG gen (pid 1): max_erroneous    24
>>>> UBI DBG gen (pid 1): process PEB 0
>>>> UBI DBG bld (pid 1): scan PEB 0
>>>> UBI DBG io (pid 1): read EC header from PEB 0
>>>> UBI DBG io (pid 1): read 64 bytes from PEB 0:0
>>>> spi_flash_mtd_read ****
>>>> UBI DBG io (pid 1): read VID header from PEB 0
>>>> UBI DBG io (pid 1): read 64 bytes from PEB 0:64
>>>> spi_flash_mtd_read ****
>>>> [...]
>>>>
>>>> UBI uses the MTD layer ... the sf command not ...
>>>> Hope this helps?
>>>>
>>>> bye,
>>>> Heiko
>>>>
>>>>
>>>>> On 27 April 2015 at 11:12, 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>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Changes in v6: None
>>>>>> Changes in v2:
>>>>>> - add comment from Daniel Schwierzeck:
>>>>>>      fix compile error from original patch with
>>>>>>      "static inline" rather than "static __maybe_unused"
>>>>>> Series-changes: 3
>>>>>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>>>>> Series-changes: 4
>>>>>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>>>>>> Series-changes: 5
>>>>>> - no changes
>>>>>> Series-changes: 6
>>>>>> - add comments from Jagan Teki:
>>>>>>      move code, which checks if flash pointer is used
>>>>>>      into a new patch.
>>>>>>      - use #ifdef in Code
>>>>>>      - call mtd register before the spi_release_bus
>>>>>>
>>>>>>     README                        |   3 ++
>>>>>>     common/cmd_sf.c               |   2 -
>>>>>>     drivers/mtd/spi/Makefile      |   1 +
>>>>>>     drivers/mtd/spi/sf_internal.h |   5 ++
>>>>>>     drivers/mtd/spi/sf_mtd.c      | 104
>>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>>     drivers/mtd/spi/sf_probe.c    |  10 ++--
>>>>>>     6 files changed, 119 insertions(+), 6 deletions(-)
>>>>>>     create mode 100644 drivers/mtd/spi/sf_mtd.c
>>>>>>
>>>>>> diff --git a/README b/README
>>>>>> index fc1fd52..36f6fc9 100644
>>>>>> --- a/README
>>>>>> +++ b/README
>>>>>> @@ -3097,6 +3097,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 6aabf39..0250011 100644
>>>>>> --- a/common/cmd_sf.c
>>>>>> +++ b/common/cmd_sf.c
>>>>>> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char *
>>>>>> const
>>>>>> argv[])
>>>>>>                    return 1;
>>>>>>            }
>>>>>>
>>>>>> -       if (flash)
>>>>>> -               spi_flash_free(flash);
>>>>>>            flash = new;
>>>>>>     #endif
>>>>>>
>>>>>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>>>>>> index c61b784e..f8580cd 100644
>>>>>> --- a/drivers/mtd/spi/Makefile
>>>>>> +++ b/drivers/mtd/spi/Makefile
>>>>>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>>>>>>     #endif
>>>>>>     obj-$(CONFIG_CMD_SF) += sf.o
>>>>>>     obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
>>>>>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>>>>>     obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>>>>>>     obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
>>>>>> diff --git a/drivers/mtd/spi/sf_internal.h
>>>>>> b/drivers/mtd/spi/sf_internal.h
>>>>>> index 785f7a9..8a2eb6e 100644
>>>>>> --- a/drivers/mtd/spi/sf_internal.h
>>>>>> +++ b/drivers/mtd/spi/sf_internal.h
>>>>>> @@ -221,4 +221,9 @@ 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);
>>>>>> +#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;
>>>>>
>>>>>
>>>>>
>>>>> Even if I remove this every thing as usual, like "sf erase" will call
>>>>> from cmd_sf
>>>>> there it calls mtd_arg_off_size, to detected off and size from
>>>>> partition and after
>>>>> sf_flash will call erase ops from sf_ops.c.
>>>>>
>>>>> As it's a mtd call, I thought sf_mtd_into._erase will intern calls
>>>>> erase ops from sf_ops.c
>>>>> What is this behavior could you please help me?
>>>>>
>>>>>> +
>>>>>> +       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 d19138d..2342972 100644
>>>>>> --- a/drivers/mtd/spi/sf_probe.c
>>>>>> +++ b/drivers/mtd/spi/sf_probe.c
>>>>>> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi,
>>>>>> struct spi_flash *flash)
>>>>>>            if (spi_enable_wp_pin(flash))
>>>>>>                    puts("Enable WP pin failed\n");
>>>>>>
>>>>>> -       /* Release spi bus */
>>>>>> -       spi_release_bus(spi);
>>>>>> -
>>>>>> -       return 0;
>>>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>>>> +       ret = spi_flash_mtd_register(flash);
>>>>>> +#endif
>>>>>>
>>>>>>     err_read_id:
>>>>>>            spi_release_bus(spi);
>>>>>> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void
>>>>>> *blob, int slave_node,
>>>>>>
>>>>>>     void spi_flash_free(struct spi_flash *flash)
>>>>>>     {
>>>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>>>> +       spi_flash_mtd_unregister();
>>>>>> +#endif
>>>>>>            spi_free_slave(flash->spi);
>>>>>>            free(flash);
>>>>>>     }
>>>>>> --
>>>>>> 2.1.0
>
> thanks!
>

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

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

[Jagan Teki]

On 16 June 2015 at 14:48, Heiko Schocher denx <hs@denx.de> wrote:
> Hello Jagan,
>
>
> Am 16.06.2015 um 10:52 schrieb Jagan Teki:
>>
>> Hi Heiko,
>>
>> On 16 June 2015 at 14:13, Heiko Schocher denx <hs@denx.de> wrote:
>>>
>>> Hello Jagan,
>>>
>>>
>>> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>>>
>>>>
>>>> Hi Heiko,
>>>>
>>>> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>>>>>
>>>>>
>>>>> Hello Jagan,
>>>>>
>>>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hi Heiko,
>>>>>>
>>>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>>>> in all the func calls.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Thanks for testing!
>>>>>
>>>>>
>>>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>>>> mtdparts variable not set, see 'help mtdparts'
>>>>>> zynq-uboot> mtdparts
>>>>>>
>>>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>>>     #: name                size            offset          mask_flags
>>>>>>     0: env                 0x00010000      0x00000000      0
>>>>>>
>>>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>>>
>>>>>> defaults:
>>>>>> mtdids  : nor0=zynq-sf.0
>>>>>> mtdparts: none
>>>>>> zynq-uboot> sf erase env 0x10000
>>>>>> spi_flash_erase
>>>>>> spi_flash_cmd_erase_ops
>>>>>> SF: erase d8  0  0  0 (0)
>>>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>>>> zynq-uboot> sf write 0x100 env
>>>>>> device 0 offset 0x0, size 0x10000
>>>>>> spi_flash_cmd_write_ops
>>>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>>>> zynq-uboot> sf read 0x40000 env
>>>>>> device 0 offset 0x0, size 0x10000
>>>>>> spi_flash_cmd_read_ops
>>>>>> off = 0x10000, len = 0
>>>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>>>> Total of 65536 byte(s) were the same
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Looks good ...
>>>>>
>>>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>>>> use the mtd interface, right?
>>>>
>>>>
>>>>
>>>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>>>> only.
>>>
>>>
>>>
>>> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
>>> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
>>> be used also with spi flashes now ... if this is wise, I don;t know ...
>>>
>>> I tested with UBI on a spi flash, and that works ... in the same
>>> fashion it currently does on nand for example ...
>>>
>>>> I wouldn't see this is a better approach where mtd code is considered as
>>>> to
>>>> be unknown thing for sf.
>>>
>>>
>>>
>>> I do not understand you here complete ...
>>>
>>> drivers/mtd/spi/sf_mtd.c
>>>
>>> adds just spi flash specific functions to integrate spi flashes
>>> into mtd ... and mtd users can then read/write/erase
>>> with the mtd_* functions ...
>>>
>>> Maybe someone has time to convert common/sf.c to use them?
>>>
>>> So, the final question is, can this patches go into mainline?
>>
>>
>> The only point I'm concerned here is If I need to use sf with mtd support
>> without using ubifs or any flash filesystem, the code in sf_mtd.c ops
>> becomes
>> unused.
>
>
> Why you want to enable mtd support for sf, if you not use it?
> do not define CONFIG_SPI_FLASH_MTD in this case?

Suppose I need sf-mtd and enabled CONFIG_SPI_FLASH_MTD so
when I did it from sf_cmd the code in sf-mtd.c is never been used for
erase/read/write. Is that true right?

>
>> This seems to be a code size increasing factor which is obviously not a
>> good
>> point for bootloader atleast for u-boot, what do you think?
>>
>> I agreed your concerned for someone may add support to common/cmd_sf.c
>> in future, but I'm bit worried to go this.
>
>
> Why? Its the same state as it is in the nand subsystem ...

Agreed but until unless if you have filesystem link to cmd_sf to partitioning.

>
>>
>>>
>>>>> I tested this functions with using UBI on the SPI NOR on the
>>>>> aristainetos and aristianetos2 boards... I added for example in
>>>>>
>>>>> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
>>>>> index 0b9cb62..6063ed7 100644
>>>>> --- a/drivers/mtd/spi/sf_mtd.c
>>>>> +++ b/drivers/mtd/spi/sf_mtd.c
>>>>> @@ -39,6 +40,7 @@ static int spi_flash_mtd_read(struct mtd_info *mtd,
>>>>> loff_t
>>>>> from, size_t len,
>>>>>           struct spi_flash *flash = mtd->priv;
>>>>>           int err;
>>>>>
>>>>> +printf("%s ****\n", __func__);
>>>>>           err = spi_flash_read(flash, from, len, buf);
>>>>>           if (!err)
>>>>>                   *retlen = len;
>>>>>
>>>>> and see:
>>>>>
>>>>> => sf probe
>>>>> SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB,
>>>>> total
>>>>> 16
>>>>> MiB
>>>>> => mtdparts
>>>>>
>>>>> device nor0 <spi3.1>, # parts = 4
>>>>>    #: name                size            offset          mask_flags
>>>>>    0: u-boot              0x000d0000      0x00000000      0
>>>>>    1: env                 0x00010000      0x000d0000      0
>>>>>    2: env-red             0x00010000      0x000e0000      0
>>>>>    3: rescue-system       0x00f10000      0x000f0000      0
>>>>>
>>>>> device nand0 <gpmi-nand>, # parts = 1
>>>>>    #: name                size            offset          mask_flags
>>>>>    0: ubi                 0x40000000      0x00000000      0
>>>>>
>>>>> active partition: nor0,0 - (u-boot) 0x000d0000 @ 0x00000000
>>>>>
>>>>> defaults:
>>>>> mtdids  : none
>>>>> mtdparts: none
>>>>> => ubi part rescue-system
>>>>> UBI: default fastmap pool size: 10
>>>>> UBI: default fastmap WL pool size: 25
>>>>> UBI: attaching mtd2 to ubi0
>>>>> UBI DBG gen (pid 1): sizeof(struct ubi_ainf_peb) 48
>>>>> UBI DBG gen (pid 1): sizeof(struct ubi_wl_entry) 20
>>>>> UBI DBG gen (pid 1): min_io_size      1
>>>>> UBI DBG gen (pid 1): max_write_size   256
>>>>> UBI DBG gen (pid 1): hdrs_min_io_size 1
>>>>> UBI DBG gen (pid 1): ec_hdr_alsize    64
>>>>> UBI DBG gen (pid 1): vid_hdr_alsize   64
>>>>> UBI DBG gen (pid 1): vid_hdr_offset   64
>>>>> UBI DBG gen (pid 1): vid_hdr_aloffset 64
>>>>> UBI DBG gen (pid 1): vid_hdr_shift    0
>>>>> UBI DBG gen (pid 1): leb_start        128
>>>>> UBI DBG gen (pid 1): max_erroneous    24
>>>>> UBI DBG gen (pid 1): process PEB 0
>>>>> UBI DBG bld (pid 1): scan PEB 0
>>>>> UBI DBG io (pid 1): read EC header from PEB 0
>>>>> UBI DBG io (pid 1): read 64 bytes from PEB 0:0
>>>>> spi_flash_mtd_read ****
>>>>> UBI DBG io (pid 1): read VID header from PEB 0
>>>>> UBI DBG io (pid 1): read 64 bytes from PEB 0:64
>>>>> spi_flash_mtd_read ****
>>>>> [...]
>>>>>
>>>>> UBI uses the MTD layer ... the sf command not ...
>>>>> Hope this helps?
>>>>>
>>>>> bye,
>>>>> Heiko
>>>>>
>>>>>
>>>>>> On 27 April 2015 at 11:12, 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>
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v6: None
>>>>>>> Changes in v2:
>>>>>>> - add comment from Daniel Schwierzeck:
>>>>>>>      fix compile error from original patch with
>>>>>>>      "static inline" rather than "static __maybe_unused"
>>>>>>> Series-changes: 3
>>>>>>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>>>>>> Series-changes: 4
>>>>>>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>>>>>>> Series-changes: 5
>>>>>>> - no changes
>>>>>>> Series-changes: 6
>>>>>>> - add comments from Jagan Teki:
>>>>>>>      move code, which checks if flash pointer is used
>>>>>>>      into a new patch.
>>>>>>>      - use #ifdef in Code
>>>>>>>      - call mtd register before the spi_release_bus
>>>>>>>
>>>>>>>     README                        |   3 ++
>>>>>>>     common/cmd_sf.c               |   2 -
>>>>>>>     drivers/mtd/spi/Makefile      |   1 +
>>>>>>>     drivers/mtd/spi/sf_internal.h |   5 ++
>>>>>>>     drivers/mtd/spi/sf_mtd.c      | 104
>>>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>>>     drivers/mtd/spi/sf_probe.c    |  10 ++--
>>>>>>>     6 files changed, 119 insertions(+), 6 deletions(-)
>>>>>>>     create mode 100644 drivers/mtd/spi/sf_mtd.c
>>>>>>>
>>>>>>> diff --git a/README b/README
>>>>>>> index fc1fd52..36f6fc9 100644
>>>>>>> --- a/README
>>>>>>> +++ b/README
>>>>>>> @@ -3097,6 +3097,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 6aabf39..0250011 100644
>>>>>>> --- a/common/cmd_sf.c
>>>>>>> +++ b/common/cmd_sf.c
>>>>>>> @@ -139,8 +139,6 @@ static int do_spi_flash_probe(int argc, char *
>>>>>>> const
>>>>>>> argv[])
>>>>>>>                    return 1;
>>>>>>>            }
>>>>>>>
>>>>>>> -       if (flash)
>>>>>>> -               spi_flash_free(flash);
>>>>>>>            flash = new;
>>>>>>>     #endif
>>>>>>>
>>>>>>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>>>>>>> index c61b784e..f8580cd 100644
>>>>>>> --- a/drivers/mtd/spi/Makefile
>>>>>>> +++ b/drivers/mtd/spi/Makefile
>>>>>>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>>>>>>>     #endif
>>>>>>>     obj-$(CONFIG_CMD_SF) += sf.o
>>>>>>>     obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
>>>>>>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>>>>>>>     obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>>>>>>>     obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
>>>>>>> diff --git a/drivers/mtd/spi/sf_internal.h
>>>>>>> b/drivers/mtd/spi/sf_internal.h
>>>>>>> index 785f7a9..8a2eb6e 100644
>>>>>>> --- a/drivers/mtd/spi/sf_internal.h
>>>>>>> +++ b/drivers/mtd/spi/sf_internal.h
>>>>>>> @@ -221,4 +221,9 @@ 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);
>>>>>>> +#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;
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Even if I remove this every thing as usual, like "sf erase" will call
>>>>>> from cmd_sf
>>>>>> there it calls mtd_arg_off_size, to detected off and size from
>>>>>> partition and after
>>>>>> sf_flash will call erase ops from sf_ops.c.
>>>>>>
>>>>>> As it's a mtd call, I thought sf_mtd_into._erase will intern calls
>>>>>> erase ops from sf_ops.c
>>>>>> What is this behavior could you please help me?
>>>>>>
>>>>>>> +
>>>>>>> +       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 d19138d..2342972 100644
>>>>>>> --- a/drivers/mtd/spi/sf_probe.c
>>>>>>> +++ b/drivers/mtd/spi/sf_probe.c
>>>>>>> @@ -397,10 +397,9 @@ int spi_flash_probe_slave(struct spi_slave *spi,
>>>>>>> struct spi_flash *flash)
>>>>>>>            if (spi_enable_wp_pin(flash))
>>>>>>>                    puts("Enable WP pin failed\n");
>>>>>>>
>>>>>>> -       /* Release spi bus */
>>>>>>> -       spi_release_bus(spi);
>>>>>>> -
>>>>>>> -       return 0;
>>>>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>>>>> +       ret = spi_flash_mtd_register(flash);
>>>>>>> +#endif
>>>>>>>
>>>>>>>     err_read_id:
>>>>>>>            spi_release_bus(spi);
>>>>>>> @@ -450,6 +449,9 @@ struct spi_flash *spi_flash_probe_fdt(const void
>>>>>>> *blob, int slave_node,
>>>>>>>
>>>>>>>     void spi_flash_free(struct spi_flash *flash)
>>>>>>>     {
>>>>>>> +#ifdef CONFIG_SPI_FLASH_MTD
>>>>>>> +       spi_flash_mtd_unregister();
>>>>>>> +#endif
>>>>>>>            spi_free_slave(flash->spi);
>>>>>>>            free(flash);
>>>>>>>     }
>>>>>>> --
>>>>>>> 2.1.0

thanks!
-- 
Jagan | openedev.

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

[Daniel Schwierzeck]

2015-06-16 11:36 GMT+02:00 Jagan Teki <jteki@openedev.com>:
> On 16 June 2015 at 14:48, Heiko Schocher denx <hs@denx.de> wrote:
>> Hello Jagan,
>>
>>
>> Am 16.06.2015 um 10:52 schrieb Jagan Teki:
>>>
>>> Hi Heiko,
>>>
>>> On 16 June 2015 at 14:13, Heiko Schocher denx <hs@denx.de> wrote:
>>>>
>>>> Hello Jagan,
>>>>
>>>>
>>>> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>>>>
>>>>>
>>>>> Hi Heiko,
>>>>>
>>>>> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>>>>>>
>>>>>>
>>>>>> Hello Jagan,
>>>>>>
>>>>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hi Heiko,
>>>>>>>
>>>>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>>>>> in all the func calls.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks for testing!
>>>>>>
>>>>>>
>>>>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>>>>> mtdparts variable not set, see 'help mtdparts'
>>>>>>> zynq-uboot> mtdparts
>>>>>>>
>>>>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>>>>     #: name                size            offset          mask_flags
>>>>>>>     0: env                 0x00010000      0x00000000      0
>>>>>>>
>>>>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>>>>
>>>>>>> defaults:
>>>>>>> mtdids  : nor0=zynq-sf.0
>>>>>>> mtdparts: none
>>>>>>> zynq-uboot> sf erase env 0x10000
>>>>>>> spi_flash_erase
>>>>>>> spi_flash_cmd_erase_ops
>>>>>>> SF: erase d8  0  0  0 (0)
>>>>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>>>>> zynq-uboot> sf write 0x100 env
>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>> spi_flash_cmd_write_ops
>>>>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>>>>> zynq-uboot> sf read 0x40000 env
>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>> spi_flash_cmd_read_ops
>>>>>>> off = 0x10000, len = 0
>>>>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>>>>> Total of 65536 byte(s) were the same
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Looks good ...
>>>>>>
>>>>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>>>>> use the mtd interface, right?
>>>>>
>>>>>
>>>>>
>>>>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>>>>> only.
>>>>
>>>>
>>>>
>>>> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
>>>> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
>>>> be used also with spi flashes now ... if this is wise, I don;t know ...
>>>>
>>>> I tested with UBI on a spi flash, and that works ... in the same
>>>> fashion it currently does on nand for example ...
>>>>
>>>>> I wouldn't see this is a better approach where mtd code is considered as
>>>>> to
>>>>> be unknown thing for sf.
>>>>
>>>>
>>>>
>>>> I do not understand you here complete ...
>>>>
>>>> drivers/mtd/spi/sf_mtd.c
>>>>
>>>> adds just spi flash specific functions to integrate spi flashes
>>>> into mtd ... and mtd users can then read/write/erase
>>>> with the mtd_* functions ...
>>>>
>>>> Maybe someone has time to convert common/sf.c to use them?
>>>>
>>>> So, the final question is, can this patches go into mainline?
>>>
>>>
>>> The only point I'm concerned here is If I need to use sf with mtd support
>>> without using ubifs or any flash filesystem, the code in sf_mtd.c ops
>>> becomes
>>> unused.
>>
>>
>> Why you want to enable mtd support for sf, if you not use it?
>> do not define CONFIG_SPI_FLASH_MTD in this case?
>
> Suppose I need sf-mtd and enabled CONFIG_SPI_FLASH_MTD so
> when I did it from sf_cmd the code in sf-mtd.c is never been used for
> erase/read/write. Is that true right?

you obviously do not understand the intention of sf_mtd. It is only an
adapter for translating mtd_read/mtd_write commands into
spi_flash_read/spi_flash_write commands. It is not intended to use it
within sf_cmd or the SPI flash subsystem. Such an adapter is needed
for subsystems like UBI which can only operate on top of the MTD
layer. Thus if you want to use UBI on SPI flash, you will need sf_mtd.
And using UBI on SPI flash is a legitimate use-case for many users.

BTW: this is the same approach as for CFI NOR flash. The CFI driver
regularly works without MTD support, but you can enable the optional
cfi_mtd driver if you need it.

>
>>
>>> This seems to be a code size increasing factor which is obviously not a
>>> good
>>> point for bootloader atleast for u-boot, what do you think?
>>>
>>> I agreed your concerned for someone may add support to common/cmd_sf.c
>>> in future, but I'm bit worried to go this.
>>
>>
>> Why? Its the same state as it is in the nand subsystem ...
>
> Agreed but until unless if you have filesystem link to cmd_sf to partitioning.

Again sf_mtd is an optional driver for users who need MTD support on
SPI flash. All others do not need to enable sf_mtd. Thus there is no
increasing of code size.

-- 
- Daniel

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

[Jagan Teki]

On 16 June 2015 at 15:36, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
> 2015-06-16 11:36 GMT+02:00 Jagan Teki <jteki@openedev.com>:
>> On 16 June 2015 at 14:48, Heiko Schocher denx <hs@denx.de> wrote:
>>> Hello Jagan,
>>>
>>>
>>> Am 16.06.2015 um 10:52 schrieb Jagan Teki:
>>>>
>>>> Hi Heiko,
>>>>
>>>> On 16 June 2015 at 14:13, Heiko Schocher denx <hs@denx.de> wrote:
>>>>>
>>>>> Hello Jagan,
>>>>>
>>>>>
>>>>> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>>>>>
>>>>>>
>>>>>> Hi Heiko,
>>>>>>
>>>>>> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hello Jagan,
>>>>>>>
>>>>>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Heiko,
>>>>>>>>
>>>>>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>>>>>> in all the func calls.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks for testing!
>>>>>>>
>>>>>>>
>>>>>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>>>>>> mtdparts variable not set, see 'help mtdparts'
>>>>>>>> zynq-uboot> mtdparts
>>>>>>>>
>>>>>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>>>>>     #: name                size            offset          mask_flags
>>>>>>>>     0: env                 0x00010000      0x00000000      0
>>>>>>>>
>>>>>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>>>>>
>>>>>>>> defaults:
>>>>>>>> mtdids  : nor0=zynq-sf.0
>>>>>>>> mtdparts: none
>>>>>>>> zynq-uboot> sf erase env 0x10000
>>>>>>>> spi_flash_erase
>>>>>>>> spi_flash_cmd_erase_ops
>>>>>>>> SF: erase d8  0  0  0 (0)
>>>>>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>>>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>>>>>> zynq-uboot> sf write 0x100 env
>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>> spi_flash_cmd_write_ops
>>>>>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>>>>>> zynq-uboot> sf read 0x40000 env
>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>> spi_flash_cmd_read_ops
>>>>>>>> off = 0x10000, len = 0
>>>>>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>>>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>>>>>> Total of 65536 byte(s) were the same
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Looks good ...
>>>>>>>
>>>>>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>>>>>> use the mtd interface, right?
>>>>>>
>>>>>>
>>>>>>
>>>>>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>>>>>> only.
>>>>>
>>>>>
>>>>>
>>>>> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
>>>>> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
>>>>> be used also with spi flashes now ... if this is wise, I don;t know ...
>>>>>
>>>>> I tested with UBI on a spi flash, and that works ... in the same
>>>>> fashion it currently does on nand for example ...
>>>>>
>>>>>> I wouldn't see this is a better approach where mtd code is considered as
>>>>>> to
>>>>>> be unknown thing for sf.
>>>>>
>>>>>
>>>>>
>>>>> I do not understand you here complete ...
>>>>>
>>>>> drivers/mtd/spi/sf_mtd.c
>>>>>
>>>>> adds just spi flash specific functions to integrate spi flashes
>>>>> into mtd ... and mtd users can then read/write/erase
>>>>> with the mtd_* functions ...
>>>>>
>>>>> Maybe someone has time to convert common/sf.c to use them?
>>>>>
>>>>> So, the final question is, can this patches go into mainline?
>>>>
>>>>
>>>> The only point I'm concerned here is If I need to use sf with mtd support
>>>> without using ubifs or any flash filesystem, the code in sf_mtd.c ops
>>>> becomes
>>>> unused.
>>>
>>>
>>> Why you want to enable mtd support for sf, if you not use it?
>>> do not define CONFIG_SPI_FLASH_MTD in this case?
>>
>> Suppose I need sf-mtd and enabled CONFIG_SPI_FLASH_MTD so
>> when I did it from sf_cmd the code in sf-mtd.c is never been used for
>> erase/read/write. Is that true right?
>
> you obviously do not understand the intention of sf_mtd. It is only an
> adapter for translating mtd_read/mtd_write commands into
> spi_flash_read/spi_flash_write commands. It is not intended to use it
> within sf_cmd or the SPI flash subsystem. Such an adapter is needed
> for subsystems like UBI which can only operate on top of the MTD
> layer. Thus if you want to use UBI on SPI flash, you will need sf_mtd.
> And using UBI on SPI flash is a legitimate use-case for many users.

Say, for example I'm trying to do raw mtd partition without use of any flash
filesystem say UBI. for this also I must t register the spi flash to mtd right?

So I will enable CONFIG_SPI_FLASH_MTD and add the mtd partitions

zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
mtdparts variable not set, see 'help mtdparts'

and then I will able to do partition read/write using sf_cmd.

From your point above "It is not intended to use it within sf_cmd or
the SPI flash subsystem."

If I didn't enable or register with mtd, how come the device gets register
with mtd and how could I add the partitions?

zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
mtdparts variable not set, see 'help mtdparts'
Device nor0 not found!

So my point was if I do a raw mtd partition with spi flash I must use sf_mtd
but there is no usage of sf_mtd_info ops code.

I'm just concentrated on raw mtd usage on spi flash, think on that area and
let me know your inputs.

>
> BTW: this is the same approach as for CFI NOR flash. The CFI driver
> regularly works without MTD support, but you can enable the optional
> cfi_mtd driver if you need it.
>
>>
>>>
>>>> This seems to be a code size increasing factor which is obviously not a
>>>> good
>>>> point for bootloader atleast for u-boot, what do you think?
>>>>
>>>> I agreed your concerned for someone may add support to common/cmd_sf.c
>>>> in future, but I'm bit worried to go this.
>>>
>>>
>>> Why? Its the same state as it is in the nand subsystem ...
>>
>> Agreed but until unless if you have filesystem link to cmd_sf to partitioning.
>
> Again sf_mtd is an optional driver for users who need MTD support on
> SPI flash. All others do not need to enable sf_mtd. Thus there is no
> increasing of code size.

thanks!
-- 
Jagan | openedev.

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

[Daniel Schwierzeck]

2015-06-22 8:43 GMT+02:00 Jagan Teki <jteki@openedev.com>:
> On 16 June 2015 at 15:36, Daniel Schwierzeck
> <daniel.schwierzeck@gmail.com> wrote:
>> 2015-06-16 11:36 GMT+02:00 Jagan Teki <jteki@openedev.com>:
>>> On 16 June 2015 at 14:48, Heiko Schocher denx <hs@denx.de> wrote:
>>>> Hello Jagan,
>>>>
>>>>
>>>> Am 16.06.2015 um 10:52 schrieb Jagan Teki:
>>>>>
>>>>> Hi Heiko,
>>>>>
>>>>> On 16 June 2015 at 14:13, Heiko Schocher denx <hs@denx.de> wrote:
>>>>>>
>>>>>> Hello Jagan,
>>>>>>
>>>>>>
>>>>>> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>>>>>>
>>>>>>>
>>>>>>> Hi Heiko,
>>>>>>>
>>>>>>> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Hello Jagan,
>>>>>>>>
>>>>>>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Heiko,
>>>>>>>>>
>>>>>>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>>>>>>> in all the func calls.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for testing!
>>>>>>>>
>>>>>>>>
>>>>>>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>>>>>>> mtdparts variable not set, see 'help mtdparts'
>>>>>>>>> zynq-uboot> mtdparts
>>>>>>>>>
>>>>>>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>>>>>>     #: name                size            offset          mask_flags
>>>>>>>>>     0: env                 0x00010000      0x00000000      0
>>>>>>>>>
>>>>>>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>>>>>>
>>>>>>>>> defaults:
>>>>>>>>> mtdids  : nor0=zynq-sf.0
>>>>>>>>> mtdparts: none
>>>>>>>>> zynq-uboot> sf erase env 0x10000
>>>>>>>>> spi_flash_erase
>>>>>>>>> spi_flash_cmd_erase_ops
>>>>>>>>> SF: erase d8  0  0  0 (0)
>>>>>>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>>>>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>>>>>>> zynq-uboot> sf write 0x100 env
>>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>>> spi_flash_cmd_write_ops
>>>>>>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>>>>>>> zynq-uboot> sf read 0x40000 env
>>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>>> spi_flash_cmd_read_ops
>>>>>>>>> off = 0x10000, len = 0
>>>>>>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>>>>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>>>>>>> Total of 65536 byte(s) were the same
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Looks good ...
>>>>>>>>
>>>>>>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>>>>>>> use the mtd interface, right?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>>>>>>> only.
>>>>>>
>>>>>>
>>>>>>
>>>>>> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
>>>>>> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
>>>>>> be used also with spi flashes now ... if this is wise, I don;t know ...
>>>>>>
>>>>>> I tested with UBI on a spi flash, and that works ... in the same
>>>>>> fashion it currently does on nand for example ...
>>>>>>
>>>>>>> I wouldn't see this is a better approach where mtd code is considered as
>>>>>>> to
>>>>>>> be unknown thing for sf.
>>>>>>
>>>>>>
>>>>>>
>>>>>> I do not understand you here complete ...
>>>>>>
>>>>>> drivers/mtd/spi/sf_mtd.c
>>>>>>
>>>>>> adds just spi flash specific functions to integrate spi flashes
>>>>>> into mtd ... and mtd users can then read/write/erase
>>>>>> with the mtd_* functions ...
>>>>>>
>>>>>> Maybe someone has time to convert common/sf.c to use them?
>>>>>>
>>>>>> So, the final question is, can this patches go into mainline?
>>>>>
>>>>>
>>>>> The only point I'm concerned here is If I need to use sf with mtd support
>>>>> without using ubifs or any flash filesystem, the code in sf_mtd.c ops
>>>>> becomes
>>>>> unused.
>>>>
>>>>
>>>> Why you want to enable mtd support for sf, if you not use it?
>>>> do not define CONFIG_SPI_FLASH_MTD in this case?
>>>
>>> Suppose I need sf-mtd and enabled CONFIG_SPI_FLASH_MTD so
>>> when I did it from sf_cmd the code in sf-mtd.c is never been used for
>>> erase/read/write. Is that true right?
>>
>> you obviously do not understand the intention of sf_mtd. It is only an
>> adapter for translating mtd_read/mtd_write commands into
>> spi_flash_read/spi_flash_write commands. It is not intended to use it
>> within sf_cmd or the SPI flash subsystem. Such an adapter is needed
>> for subsystems like UBI which can only operate on top of the MTD
>> layer. Thus if you want to use UBI on SPI flash, you will need sf_mtd.
>> And using UBI on SPI flash is a legitimate use-case for many users.
>
> Say, for example I'm trying to do raw mtd partition without use of any flash
> filesystem say UBI. for this also I must t register the spi flash to mtd right?
>
> So I will enable CONFIG_SPI_FLASH_MTD and add the mtd partitions

If you want to use cmd_mtdparts with SPI flash, you have to enable
CONFIG_SPI_FLASH_MTD to have your SPI flash device registered with the
MTD layer.

>
> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
> mtdparts variable not set, see 'help mtdparts'
>
> and then I will able to do partition read/write using sf_cmd.

No. cmd_mtdparts operates on the MTD device list. That does not mean
that sf_cmd have to use mtd_read()/mtd_write() functions. Heiko only
added an option for convenience to use MTD partitions with sf_cmd. But
that option only derive offset and size from a MTD partition to
delegate it to spi_flash_read()/spi_flash_write(). That is a
difference to cmd_nand, where nand_read()/nand_write() always delegate
to mtd_read()/mtd_write().

>
> From your point above "It is not intended to use it within sf_cmd or
> the SPI flash subsystem."
>
> If I didn't enable or register with mtd, how come the device gets register
> with mtd and how could I add the partitions?
>
> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
> mtdparts variable not set, see 'help mtdparts'
> Device nor0 not found!
>
> So my point was if I do a raw mtd partition with spi flash I must use sf_mtd
> but there is no usage of sf_mtd_info ops code.
>
> I'm just concentrated on raw mtd usage on spi flash, think on that area and
> let me know your inputs.

yes for cmd_mtdparts you will need sf_mtd to have your SPI flash
device registered with the MTD layer because cmd_mtdparts operates on
the global MTD device list. The callbacks _read()/_write()/_erase() in
the mtd_info struct are unused then. Are you concerned about four
unused callback functions with minimal code footprint in your use
case? In that case we could add a CONFIG_SPI_FLASH_MTD_OPS option
which conditionally compile the callbacks.

Maybe it makes sense in the long term if all flash media is
consistently accessed through the MTD layer especially in conjunction
with driver model and device-tree. If we can replace cmd_nand,
cmd_flash, cmd_sf etc. with a single cmd_mtd respectively env_nand,
env_flash, env_sf with a single env_mtd we could likely reduce the
code footprint. But that discussion is independent from the sf_mtd
patch.

-- 
- Daniel

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

[Jagan Teki]

On 22 June 2015 at 17:23, Daniel Schwierzeck
<daniel.schwierzeck@gmail.com> wrote:
> 2015-06-22 8:43 GMT+02:00 Jagan Teki <jteki@openedev.com>:
>> On 16 June 2015 at 15:36, Daniel Schwierzeck
>> <daniel.schwierzeck@gmail.com> wrote:
>>> 2015-06-16 11:36 GMT+02:00 Jagan Teki <jteki@openedev.com>:
>>>> On 16 June 2015 at 14:48, Heiko Schocher denx <hs@denx.de> wrote:
>>>>> Hello Jagan,
>>>>>
>>>>>
>>>>> Am 16.06.2015 um 10:52 schrieb Jagan Teki:
>>>>>>
>>>>>> Hi Heiko,
>>>>>>
>>>>>> On 16 June 2015 at 14:13, Heiko Schocher denx <hs@denx.de> wrote:
>>>>>>>
>>>>>>> Hello Jagan,
>>>>>>>
>>>>>>>
>>>>>>> Am 16.06.2015 um 10:04 schrieb Jagan Teki:
>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Heiko,
>>>>>>>>
>>>>>>>> On 20 May 2015 at 12:16, Heiko Schocher <hs@denx.de> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hello Jagan,
>>>>>>>>>
>>>>>>>>> Am 19.05.2015 22:09, schrieb Jagan Teki:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Hi Heiko,
>>>>>>>>>>
>>>>>>>>>> I have tested this sf-mtd stuff, please see below and enabled prints
>>>>>>>>>> in all the func calls.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks for testing!
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>>>>>>>>>> mtdparts variable not set, see 'help mtdparts'
>>>>>>>>>> zynq-uboot> mtdparts
>>>>>>>>>>
>>>>>>>>>> device nor0 <zynq-sf.0>, # parts = 1
>>>>>>>>>>     #: name                size            offset          mask_flags
>>>>>>>>>>     0: env                 0x00010000      0x00000000      0
>>>>>>>>>>
>>>>>>>>>> active partition: nor0,0 - (env) 0x00010000 @ 0x00000000
>>>>>>>>>>
>>>>>>>>>> defaults:
>>>>>>>>>> mtdids  : nor0=zynq-sf.0
>>>>>>>>>> mtdparts: none
>>>>>>>>>> zynq-uboot> sf erase env 0x10000
>>>>>>>>>> spi_flash_erase
>>>>>>>>>> spi_flash_cmd_erase_ops
>>>>>>>>>> SF: erase d8  0  0  0 (0)
>>>>>>>>>> SF: 65536 bytes @ 0x0 Erased: OK
>>>>>>>>>> zynq-uboot> mw.b 0x100 0x44 0x10000
>>>>>>>>>> zynq-uboot> sf write 0x100 env
>>>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>>>> spi_flash_cmd_write_ops
>>>>>>>>>> SF: 65536 bytes @ 0x0 Written: OK
>>>>>>>>>> zynq-uboot> sf read 0x40000 env
>>>>>>>>>> device 0 offset 0x0, size 0x10000
>>>>>>>>>> spi_flash_cmd_read_ops
>>>>>>>>>> off = 0x10000, len = 0
>>>>>>>>>> SF: 65536 bytes @ 0x0 Read: OK
>>>>>>>>>> zynq-uboot> cmp.b 0x100 0x40000 0x10000
>>>>>>>>>> Total of 65536 byte(s) were the same
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Looks good ...
>>>>>>>>>
>>>>>>>>>> I wonder none of the sf_mtd_info._* getting called, why?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hmm.. good question .. you use the "sf ..." commands, they do not
>>>>>>>>> use the mtd interface, right?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> I'm fine with the testing, but mtd code in sf seems used only for in UBI
>>>>>>>> only.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> a fast "grep mtd_read" in the u-boot source shows, that yaffs2
>>>>>>> also uses mtd_read ... I did no yaffs2 test, but I think, yaffs2 can
>>>>>>> be used also with spi flashes now ... if this is wise, I don;t know ...
>>>>>>>
>>>>>>> I tested with UBI on a spi flash, and that works ... in the same
>>>>>>> fashion it currently does on nand for example ...
>>>>>>>
>>>>>>>> I wouldn't see this is a better approach where mtd code is considered as
>>>>>>>> to
>>>>>>>> be unknown thing for sf.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I do not understand you here complete ...
>>>>>>>
>>>>>>> drivers/mtd/spi/sf_mtd.c
>>>>>>>
>>>>>>> adds just spi flash specific functions to integrate spi flashes
>>>>>>> into mtd ... and mtd users can then read/write/erase
>>>>>>> with the mtd_* functions ...
>>>>>>>
>>>>>>> Maybe someone has time to convert common/sf.c to use them?
>>>>>>>
>>>>>>> So, the final question is, can this patches go into mainline?
>>>>>>
>>>>>>
>>>>>> The only point I'm concerned here is If I need to use sf with mtd support
>>>>>> without using ubifs or any flash filesystem, the code in sf_mtd.c ops
>>>>>> becomes
>>>>>> unused.
>>>>>
>>>>>
>>>>> Why you want to enable mtd support for sf, if you not use it?
>>>>> do not define CONFIG_SPI_FLASH_MTD in this case?
>>>>
>>>> Suppose I need sf-mtd and enabled CONFIG_SPI_FLASH_MTD so
>>>> when I did it from sf_cmd the code in sf-mtd.c is never been used for
>>>> erase/read/write. Is that true right?
>>>
>>> you obviously do not understand the intention of sf_mtd. It is only an
>>> adapter for translating mtd_read/mtd_write commands into
>>> spi_flash_read/spi_flash_write commands. It is not intended to use it
>>> within sf_cmd or the SPI flash subsystem. Such an adapter is needed
>>> for subsystems like UBI which can only operate on top of the MTD
>>> layer. Thus if you want to use UBI on SPI flash, you will need sf_mtd.
>>> And using UBI on SPI flash is a legitimate use-case for many users.
>>
>> Say, for example I'm trying to do raw mtd partition without use of any flash
>> filesystem say UBI. for this also I must t register the spi flash to mtd right?
>>
>> So I will enable CONFIG_SPI_FLASH_MTD and add the mtd partitions
>
> If you want to use cmd_mtdparts with SPI flash, you have to enable
> CONFIG_SPI_FLASH_MTD to have your SPI flash device registered with the
> MTD layer.
>
>>
>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>> mtdparts variable not set, see 'help mtdparts'
>>
>> and then I will able to do partition read/write using sf_cmd.
>
> No. cmd_mtdparts operates on the MTD device list. That does not mean
> that sf_cmd have to use mtd_read()/mtd_write() functions. Heiko only
> added an option for convenience to use MTD partitions with sf_cmd. But
> that option only derive offset and size from a MTD partition to
> delegate it to spi_flash_read()/spi_flash_write(). That is a
> difference to cmd_nand, where nand_read()/nand_write() always delegate
> to mtd_read()/mtd_write().
>
>>
>> From your point above "It is not intended to use it within sf_cmd or
>> the SPI flash subsystem."
>>
>> If I didn't enable or register with mtd, how come the device gets register
>> with mtd and how could I add the partitions?
>>
>> zynq-uboot> mtdparts add nor0 0x10000 at 0x0 env
>> mtdparts variable not set, see 'help mtdparts'
>> Device nor0 not found!
>>
>> So my point was if I do a raw mtd partition with spi flash I must use sf_mtd
>> but there is no usage of sf_mtd_info ops code.
>>
>> I'm just concentrated on raw mtd usage on spi flash, think on that area and
>> let me know your inputs.
>
> yes for cmd_mtdparts you will need sf_mtd to have your SPI flash
> device registered with the MTD layer because cmd_mtdparts operates on
> the global MTD device list. The callbacks _read()/_write()/_erase() in
> the mtd_info struct are unused then. Are you concerned about four
> unused callback functions with minimal code footprint in your use
> case? In that case we could add a CONFIG_SPI_FLASH_MTD_OPS option
> which conditionally compile the callbacks.

I just calculate the text size of elf it seems ~290 bytes got increased with
raw mtd partitioning use case, so I'm going without using any macro.

>
> Maybe it makes sense in the long term if all flash media is
> consistently accessed through the MTD layer especially in conjunction
> with driver model and device-tree. If we can replace cmd_nand,
> cmd_flash, cmd_sf etc. with a single cmd_mtd respectively env_nand,
> env_flash, env_sf with a single env_mtd we could likely reduce the
> code footprint. But that discussion is independent from the sf_mtd
> patch.

I will send the same series again with some useful comments and one typo
fix, please find that re-comment so-that I shall pull these stuff on my tree.

thanks!
-- 
Jagan | openedev.

[U-Boot] [PATCH v6 2/4] 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>
Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

---

Changes in v6: None
Changes in v2:
- none
Series-changes: 3
- add comments from scott wood:
  - align MTD_DEV_TYPE_NAND correct
  - remove unnecessary inline
  - rework "jffs2 header" problem later
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
Series-changes: 4
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Series-changes: 5
- add comment from Scott Wood:
  keep the continuation line aligned with the arguments
Series-changes: 6
- add Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
- fix Tom Rinis mail addr
- add comment from Scott Wood:
  - fix indentation level
  - add mtd_ prefix
  - move str2off and str2long into common place, as they are no
    mtd specific functions and change return value from int to bool

 common/cmd_nand.c       | 148 ++++++++++--------------------------------------
 common/cmd_onenand.c    |  19 ++-----
 common/cmd_test.c       |  12 +---
 drivers/mtd/Makefile    |   4 +-
 drivers/mtd/mtd_uboot.c |  99 ++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |   5 ++
 include/vsprintf.h      |   2 +
 lib/vsprintf.c          |  16 ++++++
 8 files changed, 164 insertions(+), 141 deletions(-)
 create mode 100644 drivers/mtd/mtd_uboot.c

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 9433c80..1482462 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 (mtd_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;
 		}
@@ -597,8 +493,12 @@ 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)
+		if (mtd_arg_off_size(argc - o, argv + o, &dev, &off, &size,
+				     &maxsize, MTD_DEV_TYPE_NAND,
+				     nand_info[dev].size) != 0)
+			return 1;
+
+		if (set_dev(dev))
 			return 1;
 
 		nand = &nand_info[dev];
@@ -658,7 +558,12 @@ 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 (mtd_arg_off(argv[3], &dev, &off, &size, &maxsize,
+					MTD_DEV_TYPE_NAND,
+					nand_info[dev].size))
+				return 1;
+
+			if (set_dev(dev))
 				return 1;
 
 			nand = &nand_info[dev];
@@ -675,8 +580,13 @@ 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 (mtd_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 */
@@ -814,8 +724,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		if (s && !strcmp(s, ".allexcept"))
 			allexcept = 1;
 
-		if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
-				 &maxsize) < 0)
+		if (mtd_arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
+				     &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/common/cmd_test.c b/common/cmd_test.c
index c93fe78..7285f75 100644
--- a/common/cmd_test.c
+++ b/common/cmd_test.c
@@ -5,15 +5,6 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
-/*
- * Define _STDBOOL_H here to avoid macro expansion of true and false.
- * If the future code requires macro true or false, remove this define
- * and undef true and false before U_BOOT_CMD. This define and comment
- * shall be removed if change to U_BOOT_CMD is made to take string
- * instead of stringifying it.
- */
-#define _STDBOOL_H
-
 #include <common.h>
 #include <command.h>
 #include <fs.h>
@@ -191,6 +182,9 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return expr;
 }
 
+#undef true
+#undef false
+
 U_BOOT_CMD(
 	test,	CONFIG_SYS_MAXARGS,	1,	do_test,
 	"minimal test like /bin/sh",
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..7197007
--- /dev/null
+++ b/drivers/mtd/mtd_uboot.c
@@ -0,0 +1,99 @@
+/*
+ * (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>
+
+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 mtd_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 mtd_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 = mtd_arg_off(argv[0], idx, off, size, maxsize, devtype,
+			  chipsize);
+	if (ret)
+		return ret;
+
+	if (argc == 1)
+		goto print;
+
+	if (!str2off(argv[1], size)) {
+		printf("'%s' is not a number\n", argv[1]);
+		return -1;
+	}
+
+	if (*size > *maxsize) {
+		puts("Size exceeds partition or device limit\n");
+		return -1;
+	}
+
+print:
+	printf("device %d ", *idx);
+	if (*size == chipsize)
+		puts("whole chip\n");
+	else
+		printf("offset 0x%llx, size 0x%llx\n",
+		       (unsigned long long)*off, (unsigned long long)*size);
+	return 0;
+}
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 8666413..33669da 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -482,5 +482,10 @@ 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 mtd_arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
+		loff_t *maxsize, int devtype, int chipsize);
+int mtd_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__ */
diff --git a/include/vsprintf.h b/include/vsprintf.h
index 09c8abd..d2fcca3 100644
--- a/include/vsprintf.h
+++ b/include/vsprintf.h
@@ -196,4 +196,6 @@ int vscnprintf(char *buf, size_t size, const char *fmt, va_list args);
  */
 void print_grouped_ull(unsigned long long int_val, int digits);
 
+bool str2off(const char *p, loff_t *num);
+bool str2long(const char *p, ulong *num);
 #endif
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index bedc865..a9b8a3a 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -909,3 +909,19 @@ void print_grouped_ull(unsigned long long int_val, int digits)
 		grab = 3;
 	}
 }
+
+bool str2off(const char *p, loff_t *num)
+{
+	char *endptr;
+
+	*num = simple_strtoull(p, &endptr, 16);
+	return *p != '\0' && *endptr == '\0';
+}
+
+bool str2long(const char *p, ulong *num)
+{
+	char *endptr;
+
+	*num = simple_strtoul(p, &endptr, 16);
+	return *p != '\0' && *endptr == '\0';
+}
-- 
2.1.0

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

[Scott Wood]

On Mon, 2015-04-27 at 07:42 +0200, Heiko Schocher wrote:
> move common functions from cmd_nand.c (for calculating offset
> and size from cmdline paramter) to common place, so they could
> used from other commands which use mtd partitions.
> 
> For onenand the arg_off_size() is left in common/cmd_onenand.c.
> It should use now the common arg_off() function, but as I could
> not test onenand I let it there ...
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> 
> ---
> 
> Changes in v6: None
> Changes in v2:
> - none
> Series-changes: 3
> - add comments from scott wood:
>   - align MTD_DEV_TYPE_NAND correct
>   - remove unnecessary inline
>   - rework "jffs2 header" problem later
> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> Series-changes: 4
> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
> Series-changes: 5
> - add comment from Scott Wood:
>   keep the continuation line aligned with the arguments
> Series-changes: 6
> - add Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> - fix Tom Rinis mail addr
> - add comment from Scott Wood:
>   - fix indentation level
>   - add mtd_ prefix
>   - move str2off and str2long into common place, as they are no
>     mtd specific functions and change return value from int to bool
> 
>  common/cmd_nand.c       | 148 ++++++++++--------------------------------------
>  common/cmd_onenand.c    |  19 ++-----
>  common/cmd_test.c       |  12 +---
>  drivers/mtd/Makefile    |   4 +-
>  drivers/mtd/mtd_uboot.c |  99 ++++++++++++++++++++++++++++++++
>  include/linux/mtd/mtd.h |   5 ++
>  include/vsprintf.h      |   2 +
>  lib/vsprintf.c          |  16 ++++++
>  8 files changed, 164 insertions(+), 141 deletions(-)
>  create mode 100644 drivers/mtd/mtd_uboot.c

Acked-by: Scott Wood <scottwood@freescale.com>

-Scott

[U-Boot] [PATCH v6 3/4] 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>
Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

---

Changes in v6: None
Changes in v2:
- none
Series-changes: 3
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
Series-changes: 4
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Series-changes: 5
- no changes
series-changes: 6
- add comment from Jagan Teki:
  - append help text
  - add Reviewed-by from Jagannadha Sutradharudu Teki

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

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 0250011..3bf9119 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -13,6 +13,8 @@
 #include <mapmem.h>
 #include <spi.h>
 #include <spi_flash.h>
+#include <jffs2/jffs2.h>
+#include <linux/mtd/mtd.h>
 
 #include <asm/io.h>
 #include <dm/device-internal.h>
@@ -254,23 +256,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 (mtd_arg_off_size(argc - 2, &argv[2], &dev, &offset, &len,
+			     &maxsize, MTD_DEV_TYPE_NOR, flash->size))
 		return -1;
 
 	/* Consistency checking */
@@ -309,31 +309,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 (mtd_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;
@@ -558,13 +558,17 @@ 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' or from start of mtd\n"
+	"					  `partition'to memory at `addr'\n"
+	"sf write addr offset|partition len	- write `len' bytes from memory\n"
+	"				          at `addr' to flash at `offset'\n"
+	"					  or to start of mtd `partition'\n"
+	"sf erase offset|partition [+]len	- erase `len' bytes from `offset'\n"
+	"	^				  or from start of mtd `partition'\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'\n"
+	"					  or to start of mtd `partition'\n"
 	SF_TEST_HELP
 );
-- 
2.1.0

[U-Boot] [PATCH v6 4/4] mtd, spi: check if flash pointer is used

[Heiko Schocher]

if flash pointer is used free it, before probing a new
flash and storing it in flash.

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

Changes in v6:
- add comments from Jagan Teki:
  new patch in this patchserie, extract this piece
  of code into a new patch.

Changes in v2: None

 common/cmd_sf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 3bf9119..e58058d 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -135,7 +135,12 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 
 	flash = dev_get_uclass_priv(new);
 #else
+	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;
-- 
2.1.0

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

[Heiko Schocher]

Hello Jagan,

Am 27.04.2015 07:42, 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
> +               if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
> +                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>
> CHECK: Alignment should match open parenthesis
> +                       if (arg_off(argv[3], &dev, &off, &size, &maxsize,
> +                           MTD_DEV_TYPE_NAND, nand_info[dev].size))
>
> CHECK: Alignment should match open parenthesis
> +                       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 ...
>
> resend rebased version of this series, as v3 is pending since
> September 2014...
>
> Changes in v6:
> - add comments from Jagan Teki:
>    new patch in this patchserie, extract this piece
>    of code into a new patch.
>
> Changes in v2:
> - add comment from Daniel Schwierzeck:
>    fix compile error from original patch with
>    "static inline" rather than "static __maybe_unused"
> Changes in v3:
> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> - add comments from scott wood:
>    - align MTD_DEV_TYPE_NAND correct
>    - remove unnecessary inline
>    - rework "jffs2 header" problem later
> Changes in v4:
> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
> Changes in v5:
> - add comment from Scott Wood:
>    keep the continuation line aligned with the arguments
>
> Changes in v6:
> - add comments from Jagan Teki:
>    move code, which checks if flash pointer is used
>    into a new patch.
>    - use #ifdef in Code
>    - call mtd register before the spi_release_bus
> - add Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
> - fix Tom Rinis mail addr
> - add comment from Scott Wood:
>    - fix indentation level
>    - add mtd_ prefix
>    - move str2off and str2long into common place, as they are no
>      mtd specific functions and change return value from int to bool
> - add comment from Jagan Teki:
>    - append help text
>    - add Reviewed-by from Jagannadha Sutradharudu Teki
>
> Daniel Schwierzeck (1):
>    mtd, spi: add MTD layer driver
>
> Heiko Schocher (3):
>    mtd, nand: move common functions from cmd_nand.c to common place
>    spi, sf: use offset and size in sf cmd from mtdpartition
>    mtd, spi: check if flash pointer is used
>
>   README                        |   3 +
>   common/cmd_nand.c             | 148 +++++++++---------------------------------
>   common/cmd_onenand.c          |  19 ++----
>   common/cmd_sf.c               |  61 +++++++++--------
>   common/cmd_test.c             |  12 +---
>   drivers/mtd/Makefile          |   4 +-
>   drivers/mtd/mtd_uboot.c       |  99 ++++++++++++++++++++++++++++
>   drivers/mtd/spi/Makefile      |   1 +
>   drivers/mtd/spi/sf_internal.h |   5 ++
>   drivers/mtd/spi/sf_mtd.c      | 104 +++++++++++++++++++++++++++++
>   drivers/mtd/spi/sf_probe.c    |  10 +--
>   include/linux/mtd/mtd.h       |   5 ++
>   include/vsprintf.h            |   2 +
>   lib/vsprintf.c                |  16 +++++
>   14 files changed, 317 insertions(+), 172 deletions(-)
>   create mode 100644 drivers/mtd/mtd_uboot.c
>   create mode 100644 drivers/mtd/spi/sf_mtd.c

I got no more comment for this v6. Are there any more issues?
If not I vote for apllying it soon, so we have some tests before
the next release... thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Jagan Teki]

On 11 May 2015 at 11:19, Heiko Schocher <hs@denx.de> wrote:
> Hello Jagan,
>
>
> Am 27.04.2015 07:42, 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
>> +               if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
>> +                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>>
>> CHECK: Alignment should match open parenthesis
>> +                       if (arg_off(argv[3], &dev, &off, &size, &maxsize,
>> +                           MTD_DEV_TYPE_NAND, nand_info[dev].size))
>>
>> CHECK: Alignment should match open parenthesis
>> +                       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 ...
>>
>> resend rebased version of this series, as v3 is pending since
>> September 2014...
>>
>> Changes in v6:
>> - add comments from Jagan Teki:
>>    new patch in this patchserie, extract this piece
>>    of code into a new patch.
>>
>> Changes in v2:
>> - add comment from Daniel Schwierzeck:
>>    fix compile error from original patch with
>>    "static inline" rather than "static __maybe_unused"
>> Changes in v3:
>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>> - add comments from scott wood:
>>    - align MTD_DEV_TYPE_NAND correct
>>    - remove unnecessary inline
>>    - rework "jffs2 header" problem later
>> Changes in v4:
>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>> Changes in v5:
>> - add comment from Scott Wood:
>>    keep the continuation line aligned with the arguments
>>
>> Changes in v6:
>> - add comments from Jagan Teki:
>>    move code, which checks if flash pointer is used
>>    into a new patch.
>>    - use #ifdef in Code
>>    - call mtd register before the spi_release_bus
>> - add Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>> - fix Tom Rinis mail addr
>> - add comment from Scott Wood:
>>    - fix indentation level
>>    - add mtd_ prefix
>>    - move str2off and str2long into common place, as they are no
>>      mtd specific functions and change return value from int to bool
>> - add comment from Jagan Teki:
>>    - append help text
>>    - add Reviewed-by from Jagannadha Sutradharudu Teki
>>
>> Daniel Schwierzeck (1):
>>    mtd, spi: add MTD layer driver
>>
>> Heiko Schocher (3):
>>    mtd, nand: move common functions from cmd_nand.c to common place
>>    spi, sf: use offset and size in sf cmd from mtdpartition
>>    mtd, spi: check if flash pointer is used
>>
>>   README                        |   3 +
>>   common/cmd_nand.c             | 148
>> +++++++++---------------------------------
>>   common/cmd_onenand.c          |  19 ++----
>>   common/cmd_sf.c               |  61 +++++++++--------
>>   common/cmd_test.c             |  12 +---
>>   drivers/mtd/Makefile          |   4 +-
>>   drivers/mtd/mtd_uboot.c       |  99 ++++++++++++++++++++++++++++
>>   drivers/mtd/spi/Makefile      |   1 +
>>   drivers/mtd/spi/sf_internal.h |   5 ++
>>   drivers/mtd/spi/sf_mtd.c      | 104 +++++++++++++++++++++++++++++
>>   drivers/mtd/spi/sf_probe.c    |  10 +--
>>   include/linux/mtd/mtd.h       |   5 ++
>>   include/vsprintf.h            |   2 +
>>   lib/vsprintf.c                |  16 +++++
>>   14 files changed, 317 insertions(+), 172 deletions(-)
>>   create mode 100644 drivers/mtd/mtd_uboot.c
>>   create mode 100644 drivers/mtd/spi/sf_mtd.c
>
>
> I got no more comment for this v6. Are there any more issues?
> If not I vote for apllying it soon, so we have some tests before
> the next release... thanks!

Let me give some more time I will test and see any final comments.

I will pick this series, once all fine.

thanks!
-- 
Jagan Teki,
Openedev.

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

[Heiko Schocher]

Hello Jagan,

Am 11.05.2015 08:01, schrieb Jagan Teki:
> On 11 May 2015 at 11:19, Heiko Schocher <hs@denx.de> wrote:
>> Hello Jagan,
>>
>>
>> Am 27.04.2015 07:42, 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
>>> +               if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
>>> +                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>>>
>>> CHECK: Alignment should match open parenthesis
>>> +                       if (arg_off(argv[3], &dev, &off, &size, &maxsize,
>>> +                           MTD_DEV_TYPE_NAND, nand_info[dev].size))
>>>
>>> CHECK: Alignment should match open parenthesis
>>> +                       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 ...
>>>
>>> resend rebased version of this series, as v3 is pending since
>>> September 2014...
>>>
>>> Changes in v6:
>>> - add comments from Jagan Teki:
>>>     new patch in this patchserie, extract this piece
>>>     of code into a new patch.
>>>
>>> Changes in v2:
>>> - add comment from Daniel Schwierzeck:
>>>     fix compile error from original patch with
>>>     "static inline" rather than "static __maybe_unused"
>>> Changes in v3:
>>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>> - add comments from scott wood:
>>>     - align MTD_DEV_TYPE_NAND correct
>>>     - remove unnecessary inline
>>>     - rework "jffs2 header" problem later
>>> Changes in v4:
>>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>>> Changes in v5:
>>> - add comment from Scott Wood:
>>>     keep the continuation line aligned with the arguments
>>>
>>> Changes in v6:
>>> - add comments from Jagan Teki:
>>>     move code, which checks if flash pointer is used
>>>     into a new patch.
>>>     - use #ifdef in Code
>>>     - call mtd register before the spi_release_bus
>>> - add Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>> - fix Tom Rinis mail addr
>>> - add comment from Scott Wood:
>>>     - fix indentation level
>>>     - add mtd_ prefix
>>>     - move str2off and str2long into common place, as they are no
>>>       mtd specific functions and change return value from int to bool
>>> - add comment from Jagan Teki:
>>>     - append help text
>>>     - add Reviewed-by from Jagannadha Sutradharudu Teki
>>>
>>> Daniel Schwierzeck (1):
>>>     mtd, spi: add MTD layer driver
>>>
>>> Heiko Schocher (3):
>>>     mtd, nand: move common functions from cmd_nand.c to common place
>>>     spi, sf: use offset and size in sf cmd from mtdpartition
>>>     mtd, spi: check if flash pointer is used
>>>
>>>    README                        |   3 +
>>>    common/cmd_nand.c             | 148
>>> +++++++++---------------------------------
>>>    common/cmd_onenand.c          |  19 ++----
>>>    common/cmd_sf.c               |  61 +++++++++--------
>>>    common/cmd_test.c             |  12 +---
>>>    drivers/mtd/Makefile          |   4 +-
>>>    drivers/mtd/mtd_uboot.c       |  99 ++++++++++++++++++++++++++++
>>>    drivers/mtd/spi/Makefile      |   1 +
>>>    drivers/mtd/spi/sf_internal.h |   5 ++
>>>    drivers/mtd/spi/sf_mtd.c      | 104 +++++++++++++++++++++++++++++
>>>    drivers/mtd/spi/sf_probe.c    |  10 +--
>>>    include/linux/mtd/mtd.h       |   5 ++
>>>    include/vsprintf.h            |   2 +
>>>    lib/vsprintf.c                |  16 +++++
>>>    14 files changed, 317 insertions(+), 172 deletions(-)
>>>    create mode 100644 drivers/mtd/mtd_uboot.c
>>>    create mode 100644 drivers/mtd/spi/sf_mtd.c
>>
>>
>> I got no more comment for this v6. Are there any more issues?
>> If not I vote for apllying it soon, so we have some tests before
>> the next release... thanks!
>
> Let me give some more time I will test and see any final comments.
>
> I will pick this series, once all fine.

Ok, thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

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

[Heiko Schocher]

Hello Jagan,

Am 11.05.2015 08:01, schrieb Jagan Teki:
> On 11 May 2015 at 11:19, Heiko Schocher <hs@denx.de> wrote:
>> Hello Jagan,
>>
>>
>> Am 27.04.2015 07:42, 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
>>> +               if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
>>> +                   MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>>>
>>> CHECK: Alignment should match open parenthesis
>>> +                       if (arg_off(argv[3], &dev, &off, &size, &maxsize,
>>> +                           MTD_DEV_TYPE_NAND, nand_info[dev].size))
>>>
>>> CHECK: Alignment should match open parenthesis
>>> +                       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 ...
>>>
>>> resend rebased version of this series, as v3 is pending since
>>> September 2014...
>>>
>>> Changes in v6:
>>> - add comments from Jagan Teki:
>>>     new patch in this patchserie, extract this piece
>>>     of code into a new patch.
>>>
>>> Changes in v2:
>>> - add comment from Daniel Schwierzeck:
>>>     fix compile error from original patch with
>>>     "static inline" rather than "static __maybe_unused"
>>> Changes in v3:
>>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>>> - add comments from scott wood:
>>>     - align MTD_DEV_TYPE_NAND correct
>>>     - remove unnecessary inline
>>>     - rework "jffs2 header" problem later
>>> Changes in v4:
>>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>>> Changes in v5:
>>> - add comment from Scott Wood:
>>>     keep the continuation line aligned with the arguments
>>>
>>> Changes in v6:
>>> - add comments from Jagan Teki:
>>>     move code, which checks if flash pointer is used
>>>     into a new patch.
>>>     - use #ifdef in Code
>>>     - call mtd register before the spi_release_bus
>>> - add Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>>> - fix Tom Rinis mail addr
>>> - add comment from Scott Wood:
>>>     - fix indentation level
>>>     - add mtd_ prefix
>>>     - move str2off and str2long into common place, as they are no
>>>       mtd specific functions and change return value from int to bool
>>> - add comment from Jagan Teki:
>>>     - append help text
>>>     - add Reviewed-by from Jagannadha Sutradharudu Teki
>>>
>>> Daniel Schwierzeck (1):
>>>     mtd, spi: add MTD layer driver
>>>
>>> Heiko Schocher (3):
>>>     mtd, nand: move common functions from cmd_nand.c to common place
>>>     spi, sf: use offset and size in sf cmd from mtdpartition
>>>     mtd, spi: check if flash pointer is used
>>>
>>>    README                        |   3 +
>>>    common/cmd_nand.c             | 148
>>> +++++++++---------------------------------
>>>    common/cmd_onenand.c          |  19 ++----
>>>    common/cmd_sf.c               |  61 +++++++++--------
>>>    common/cmd_test.c             |  12 +---
>>>    drivers/mtd/Makefile          |   4 +-
>>>    drivers/mtd/mtd_uboot.c       |  99 ++++++++++++++++++++++++++++
>>>    drivers/mtd/spi/Makefile      |   1 +
>>>    drivers/mtd/spi/sf_internal.h |   5 ++
>>>    drivers/mtd/spi/sf_mtd.c      | 104 +++++++++++++++++++++++++++++
>>>    drivers/mtd/spi/sf_probe.c    |  10 +--
>>>    include/linux/mtd/mtd.h       |   5 ++
>>>    include/vsprintf.h            |   2 +
>>>    lib/vsprintf.c                |  16 +++++
>>>    14 files changed, 317 insertions(+), 172 deletions(-)
>>>    create mode 100644 drivers/mtd/mtd_uboot.c
>>>    create mode 100644 drivers/mtd/spi/sf_mtd.c
>>
>>
>> I got no more comment for this v6. Are there any more issues?
>> If not I vote for apllying it soon, so we have some tests before
>> the next release... thanks!
>
> Let me give some more time I will test and see any final comments.
>
> I will pick this series, once all fine.

ping?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany