public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/3] spi, sf: add mtdparts feature to spi and sf commands
@ 2014-07-14  7:39 Heiko Schocher
  2014-07-14  7:39 ` [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver Heiko Schocher
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Heiko Schocher @ 2014-07-14  7:39 UTC (permalink / raw)
  To: u-boot

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

steps:

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

  and addapted it to current mainline.

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

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

With this patchset, the sf command looks now:

=> sf
sf - SPI flash sub-system

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

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

There are the followings checkpatch warnings:

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

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

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

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

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

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

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

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

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

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

 README                        |   3 +
 common/cmd_nand.c             | 140 ++++++++----------------------------------
 common/cmd_onenand.c          |  19 ++----
 common/cmd_sf.c               |  59 +++++++++---------
 drivers/mtd/Makefile          |   4 +-
 drivers/mtd/mtd_uboot.c       | 114 ++++++++++++++++++++++++++++++++++
 drivers/mtd/spi/Makefile      |   1 +
 drivers/mtd/spi/sf_internal.h |  13 ++++
 drivers/mtd/spi/sf_mtd.c      | 104 +++++++++++++++++++++++++++++++
 drivers/mtd/spi/sf_probe.c    |   5 ++
 include/linux/mtd/mtd.h       |   7 +++
 11 files changed, 310 insertions(+), 159 deletions(-)
 create mode 100644 drivers/mtd/mtd_uboot.c
 create mode 100644 drivers/mtd/spi/sf_mtd.c

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver
  2014-07-14  7:39 [U-Boot] [PATCH 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
@ 2014-07-14  7:39 ` Heiko Schocher
  2014-07-16 18:42   ` Daniel Schwierzeck
  2014-07-14  7:39 ` [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
  2014-07-14  7:39 ` [U-Boot] [PATCH 3/3] spi, sf: use offset and size in sf cmd from mtdpartition Heiko Schocher
  2 siblings, 1 reply; 9+ messages in thread
From: Heiko Schocher @ 2014-07-14  7:39 UTC (permalink / raw)
  To: u-boot

From: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

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

changes from Heiko Schocher against this patch:
- remove compile error if not defining CONFIG_SPI_FLASH_MTD:

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

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

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>

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

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

diff --git a/README b/README
index d34692f..1eecba6 100644
--- a/README
+++ b/README
@@ -2930,6 +2930,9 @@ CBFS (Coreboot Filesystem) support
 		memories can be connected with a given cs line.
 		currently Xilinx Zynq qspi support these type of connections.
 
+		CONFIG_SPI_FLASH_MTD
+		add  MTD translation layer driver.
+
 - SystemACE Support:
 		CONFIG_SYSTEMACE
 
diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index b4ceb71..7653d7e 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -121,16 +121,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
 			return -1;
 	}
 
+	if (flash)
+		spi_flash_free(flash);
+
 	new = spi_flash_probe(bus, cs, speed, mode);
+	flash = new;
+
 	if (!new) {
 		printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
 		return 1;
 	}
 
-	if (flash)
-		spi_flash_free(flash);
-	flash = new;
-
 	return 0;
 }
 
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
index 9e18fb4..b15d273 100644
--- a/drivers/mtd/spi/Makefile
+++ b/drivers/mtd/spi/Makefile
@@ -12,6 +12,7 @@ endif
 
 obj-$(CONFIG_CMD_SF) += sf.o
 obj-$(CONFIG_SPI_FLASH) += sf_params.o sf_probe.o sf_ops.o
+obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
 obj-$(CONFIG_SPI_FRAM_RAMTRON) += ramtron.o
 obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
 obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 6bcd522..c883d1d 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -156,4 +156,17 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
 int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		size_t len, void *data);
 
+#ifdef CONFIG_SPI_FLASH_MTD
+int spi_flash_mtd_register(struct spi_flash *flash);
+void spi_flash_mtd_unregister(void);
+#else
+static inline int spi_flash_mtd_register(struct spi_flash *flash)
+{
+	return 0;
+}
+static __maybe_unused 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 36ae5e0..f6d793b 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -355,6 +355,10 @@ static struct spi_flash *spi_flash_probe_slave(struct spi_slave *spi)
 	/* Release spi bus */
 	spi_release_bus(spi);
 
+	ret = spi_flash_mtd_register(flash);
+	if (ret)
+		goto err_claim_bus;
+
 	return flash;
 
 err_read_id:
@@ -386,6 +390,7 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,
 
 void spi_flash_free(struct spi_flash *flash)
 {
+	spi_flash_mtd_unregister();
 	spi_free_slave(flash->spi);
 	free(flash);
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place
  2014-07-14  7:39 [U-Boot] [PATCH 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
  2014-07-14  7:39 ` [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver Heiko Schocher
@ 2014-07-14  7:39 ` Heiko Schocher
  2014-07-30  0:29   ` Scott Wood
  2014-07-14  7:39 ` [U-Boot] [PATCH 3/3] spi, sf: use offset and size in sf cmd from mtdpartition Heiko Schocher
  2 siblings, 1 reply; 9+ messages in thread
From: Heiko Schocher @ 2014-07-14  7:39 UTC (permalink / raw)
  To: u-boot

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

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

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Scott Wood <scottwood@freescale.com>
Cc: Tom Rini <trini@ti.com>
---
 common/cmd_nand.c       | 140 +++++++++---------------------------------------
 common/cmd_onenand.c    |  19 +++----
 drivers/mtd/Makefile    |   4 +-
 drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |   7 +++
 5 files changed, 154 insertions(+), 130 deletions(-)
 create mode 100644 drivers/mtd/mtd_uboot.c

diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index f9ced9d..d6232e3 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -133,115 +133,6 @@ static int set_dev(int dev)
 	return 0;
 }
 
-static inline int str2off(const char *p, loff_t *num)
-{
-	char *endptr;
-
-	*num = simple_strtoull(p, &endptr, 16);
-	return *p != '\0' && *endptr == '\0';
-}
-
-static inline int str2long(const char *p, ulong *num)
-{
-	char *endptr;
-
-	*num = simple_strtoul(p, &endptr, 16);
-	return *p != '\0' && *endptr == '\0';
-}
-
-static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
-		loff_t *maxsize)
-{
-#ifdef CONFIG_CMD_MTDPARTS
-	struct mtd_device *dev;
-	struct part_info *part;
-	u8 pnum;
-	int ret;
-
-	ret = mtdparts_init();
-	if (ret)
-		return ret;
-
-	ret = find_dev_and_part(partname, &dev, &pnum, &part);
-	if (ret)
-		return ret;
-
-	if (dev->id->type != MTD_DEV_TYPE_NAND) {
-		puts("not a NAND device\n");
-		return -1;
-	}
-
-	*off = part->offset;
-	*size = part->size;
-	*maxsize = part->size;
-	*idx = dev->id->num;
-
-	ret = set_dev(*idx);
-	if (ret)
-		return ret;
-
-	return 0;
-#else
-	puts("offset is not a number\n");
-	return -1;
-#endif
-}
-
-static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
-		loff_t *maxsize)
-{
-	if (!str2off(arg, off))
-		return get_part(arg, idx, off, size, maxsize);
-
-	if (*off >= nand_info[*idx].size) {
-		puts("Offset exceeds device limit\n");
-		return -1;
-	}
-
-	*maxsize = nand_info[*idx].size - *off;
-	*size = *maxsize;
-	return 0;
-}
-
-static int arg_off_size(int argc, char *const argv[], int *idx,
-			loff_t *off, loff_t *size, loff_t *maxsize)
-{
-	int ret;
-
-	if (argc == 0) {
-		*off = 0;
-		*size = nand_info[*idx].size;
-		*maxsize = *size;
-		goto print;
-	}
-
-	ret = arg_off(argv[0], idx, off, size, maxsize);
-	if (ret)
-		return ret;
-
-	if (argc == 1)
-		goto print;
-
-	if (!str2off(argv[1], size)) {
-		printf("'%s' is not a number\n", argv[1]);
-		return -1;
-	}
-
-	if (*size > *maxsize) {
-		puts("Size exceeds partition or device limit\n");
-		return -1;
-	}
-
-print:
-	printf("device %d ", *idx);
-	if (*size == nand_info[*idx].size)
-		puts("whole chip\n");
-	else
-		printf("offset 0x%llx, size 0x%llx\n",
-		       (unsigned long long)*off, (unsigned long long)*size);
-	return 0;
-}
-
 #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK
 static void print_status(ulong start, ulong end, ulong erasesize, int status)
 {
@@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
 			goto usage;
 
 		/* We don't care about size, or maxsize. */
-		if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
+		if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
+		    MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
+			puts("Offset or partition name expected\n");
+			return 1;
+		}
+		if (set_dev(idx)) {
 			puts("Offset or partition name expected\n");
 			return 1;
 		}
@@ -592,7 +488,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		printf("\nNAND %s: ", cmd);
 		/* skip first two or three arguments, look for offset and size */
 		if (arg_off_size(argc - o, argv + o, &dev, &off, &size,
-				 &maxsize) != 0)
+		    &maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size) != 0)
+			return 1;
+
+		if (set_dev(dev))
 			return 1;
 
 		nand = &nand_info[dev];
@@ -654,7 +553,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		if (s && !strcmp(s, ".raw")) {
 			raw = 1;
 
-			if (arg_off(argv[3], &dev, &off, &size, &maxsize))
+			if (arg_off(argv[3], &dev, &off, &size, &maxsize,
+			    MTD_DEV_TYPE_NAND, nand_info[dev].size))
+				return 1;
+
+			if (set_dev(dev))
 				return 1;
 
 			if (argc > 4 && !str2long(argv[4], &pagecount)) {
@@ -669,8 +572,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 			rwsize = pagecount * (nand->writesize + nand->oobsize);
 		} else {
-			if (arg_off_size(argc - 3, argv + 3, &dev,
-						&off, &size, &maxsize) != 0)
+			if (arg_off_size(argc - 3, argv + 3, &dev, &off, &size,
+			    &maxsize, MTD_DEV_TYPE_NAND,
+			    nand_info[dev].size) != 0)
+				return 1;
+
+			if (set_dev(dev))
 				return 1;
 
 			/* size is unspecified */
@@ -816,7 +723,10 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			allexcept = 1;
 
 		if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
-				 &maxsize) < 0)
+		    &maxsize, MTD_DEV_TYPE_NAND, nand_info[dev].size) < 0)
+			return 1;
+
+		if (set_dev(dev))
 			return 1;
 
 		if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c
index 06cc140..feab01a 100644
--- a/common/cmd_onenand.c
+++ b/common/cmd_onenand.c
@@ -24,15 +24,8 @@ static struct mtd_info *mtd;
 static loff_t next_ofs;
 static loff_t skip_ofs;
 
-static inline int str2long(char *p, ulong *num)
-{
-	char *endptr;
-
-	*num = simple_strtoul(p, &endptr, 16);
-	return (*p != '\0' && *endptr == '\0') ? 1 : 0;
-}
-
-static int arg_off_size(int argc, char * const argv[], ulong *off, size_t *size)
+static int arg_off_size_onenand(int argc, char * const argv[], ulong *off,
+				size_t *size)
 {
 	if (argc >= 1) {
 		if (!(str2long(argv[0], off))) {
@@ -399,7 +392,7 @@ static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char * const a
 	addr = (ulong)simple_strtoul(argv[1], NULL, 16);
 
 	printf("\nOneNAND read: ");
-	if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
+	if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
 		return 1;
 
 	ret = onenand_block_read(ofs, len, &retlen, (u8 *)addr, oob);
@@ -425,7 +418,7 @@ static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char * const
 	addr = (ulong)simple_strtoul(argv[1], NULL, 16);
 
 	printf("\nOneNAND write: ");
-	if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
+	if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
 		return 1;
 
 	ret = onenand_block_write(ofs, len, &retlen, (u8 *)addr, withoob);
@@ -461,7 +454,7 @@ static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char * const
 	printf("\nOneNAND erase: ");
 
 	/* skip first two or three arguments, look for offset and size */
-	if (arg_off_size(argc, argv, &ofs, &len) != 0)
+	if (arg_off_size_onenand(argc, argv, &ofs, &len) != 0)
 		return 1;
 
 	ret = onenand_block_erase(ofs, len, force);
@@ -486,7 +479,7 @@ static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char * const a
 	printf("\nOneNAND test: ");
 
 	/* skip first two or three arguments, look for offset and size */
-	if (arg_off_size(argc - 1, argv + 1, &ofs, &len) != 0)
+	if (arg_off_size_onenand(argc - 1, argv + 1, &ofs, &len) != 0)
 		return 1;
 
 	ret = onenand_block_test(ofs, len);
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 5467a951..a623f4c 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -5,8 +5,8 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
-ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)))
-obj-y += mtdcore.o
+ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)))
+obj-y += mtdcore.o mtd_uboot.o
 endif
 obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o
 obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
new file mode 100644
index 0000000..66e49c3
--- /dev/null
+++ b/drivers/mtd/mtd_uboot.c
@@ -0,0 +1,114 @@
+/*
+ * (C) Copyright 2014
+ * Heiko Schocher, DENX Software Engineering, hs at denx.de.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#include <common.h>
+#include <linux/mtd/mtd.h>
+#include <jffs2/jffs2.h>
+
+inline int str2off(const char *p, loff_t *num)
+{
+	char *endptr;
+
+	*num = simple_strtoull(p, &endptr, 16);
+	return *p != '\0' && *endptr == '\0';
+}
+
+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, int devtype)
+{
+#ifdef CONFIG_CMD_MTDPARTS
+	struct mtd_device *dev;
+	struct part_info *part;
+	u8 pnum;
+	int ret;
+
+	ret = mtdparts_init();
+	if (ret)
+		return ret;
+
+	ret = find_dev_and_part(partname, &dev, &pnum, &part);
+	if (ret)
+		return ret;
+
+	if (dev->id->type != devtype) {
+		printf("not same typ %d != %d\n", dev->id->type, devtype);
+		return -1;
+	}
+
+	*off = part->offset;
+	*size = part->size;
+	*maxsize = part->size;
+	*idx = dev->id->num;
+
+	return 0;
+#else
+	puts("offset is not a number\n");
+	return -1;
+#endif
+}
+
+int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
+		loff_t *maxsize, int devtype, int chipsize)
+{
+	if (!str2off(arg, off))
+		return get_part(arg, idx, off, size, maxsize, devtype);
+
+	if (*off >= chipsize) {
+		puts("Offset exceeds device limit\n");
+		return -1;
+	}
+
+	*maxsize = chipsize - *off;
+	*size = *maxsize;
+	return 0;
+}
+
+int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
+		 loff_t *size, loff_t *maxsize, int devtype, int chipsize)
+{
+	int ret;
+
+	if (argc == 0) {
+		*off = 0;
+		*size = chipsize;
+		*maxsize = *size;
+		goto print;
+	}
+
+	ret = arg_off(argv[0], idx, off, size, maxsize, devtype, chipsize);
+	if (ret)
+		return ret;
+
+	if (argc == 1)
+		goto print;
+
+	if (!str2off(argv[1], size)) {
+		printf("'%s' is not a number\n", argv[1]);
+		return -1;
+	}
+
+	if (*size > *maxsize) {
+		puts("Size exceeds partition or device limit\n");
+		return -1;
+	}
+
+print:
+	printf("device %d ", *idx);
+	if (*size == chipsize)
+		puts("whole chip\n");
+	else
+		printf("offset 0x%llx, size 0x%llx\n",
+		       (unsigned long long)*off, (unsigned long long)*size);
+	return 0;
+}
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index e6cfd55..8561b78 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -483,5 +483,12 @@ int add_mtd_device(struct mtd_info *mtd);
 int del_mtd_device(struct mtd_info *mtd);
 int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
 int del_mtd_partitions(struct mtd_info *);
+
+int str2off(const char *p, loff_t *num);
+int str2long(const char *p, ulong *num);
+int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
+		loff_t *maxsize, int devtype, int chipsize);
+int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
+		 loff_t *size, loff_t *maxsize, int devtype, int chipsize);
 #endif
 #endif /* __MTD_MTD_H__ */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/3] spi, sf: use offset and size in sf cmd from mtdpartition
  2014-07-14  7:39 [U-Boot] [PATCH 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
  2014-07-14  7:39 ` [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver Heiko Schocher
  2014-07-14  7:39 ` [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
@ 2014-07-14  7:39 ` Heiko Schocher
  2 siblings, 0 replies; 9+ messages in thread
From: Heiko Schocher @ 2014-07-14  7:39 UTC (permalink / raw)
  To: u-boot

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

=> sf
sf - SPI flash sub-system

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

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

Signed-off-by: Heiko Schocher <hs@denx.de>
Cc: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
---
 common/cmd_sf.c | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 7653d7e..fc306ee 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -10,6 +10,8 @@
 #include <div64.h>
 #include <malloc.h>
 #include <spi_flash.h>
+#include <jffs2/jffs2.h>
+#include <linux/mtd/mtd.h>
 
 #include <asm/io.h>
 
@@ -244,23 +246,21 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset,
 static int do_spi_flash_read_write(int argc, char * const argv[])
 {
 	unsigned long addr;
-	unsigned long offset;
-	unsigned long len;
 	void *buf;
 	char *endp;
 	int ret = 1;
+	int dev = 0;
+	loff_t offset, len, maxsize;
 
-	if (argc < 4)
+	if (argc < 3)
 		return -1;
 
 	addr = simple_strtoul(argv[1], &endp, 16);
 	if (*argv[1] == 0 || *endp != 0)
 		return -1;
-	offset = simple_strtoul(argv[2], &endp, 16);
-	if (*argv[2] == 0 || *endp != 0)
-		return -1;
-	len = simple_strtoul(argv[3], &endp, 16);
-	if (*argv[3] == 0 || *endp != 0)
+
+	if (arg_off_size(argc - 2, &argv[2], &dev, &offset, &len, &maxsize,
+	    MTD_DEV_TYPE_NOR, flash->size))
 		return -1;
 
 	/* Consistency checking */
@@ -299,31 +299,31 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 
 static int do_spi_flash_erase(int argc, char * const argv[])
 {
-	unsigned long offset;
-	unsigned long len;
-	char *endp;
 	int ret;
+	int dev = 0;
+	loff_t offset, len, maxsize;
+	ulong size;
 
 	if (argc < 3)
 		return -1;
 
-	offset = simple_strtoul(argv[1], &endp, 16);
-	if (*argv[1] == 0 || *endp != 0)
+	if (arg_off(argv[1], &dev, &offset, &len, &maxsize,
+	    MTD_DEV_TYPE_NOR, flash->size))
 		return -1;
 
-	ret = sf_parse_len_arg(argv[2], &len);
+	ret = sf_parse_len_arg(argv[2], &size);
 	if (ret != 1)
 		return -1;
 
 	/* Consistency checking */
-	if (offset + len > flash->size) {
+	if (offset + size > flash->size) {
 		printf("ERROR: attempting %s past flash size (%#x)\n",
 		       argv[0], flash->size);
 		return 1;
 	}
 
-	ret = spi_flash_erase(flash, offset, len);
-	printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)len, (u32)offset,
+	ret = spi_flash_erase(flash, offset, size);
+	printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)size, (u32)offset,
 	       ret ? "ERROR" : "OK");
 
 	return ret == 0 ? 0 : 1;
@@ -545,13 +545,13 @@ U_BOOT_CMD(
 	"SPI flash sub-system",
 	"probe [[bus:]cs] [hz] [mode]	- init flash device on given SPI bus\n"
 	"				  and chip select\n"
-	"sf read addr offset len	- read `len' bytes starting at\n"
-	"				  `offset' to memory at `addr'\n"
-	"sf write addr offset len	- write `len' bytes from memory\n"
-	"				  at `addr' to flash at `offset'\n"
-	"sf erase offset [+]len		- erase `len' bytes from `offset'\n"
-	"				  `+len' round up `len' to block size\n"
-	"sf update addr offset len	- erase and write `len' bytes from memory\n"
-	"				  at `addr' to flash at `offset'"
+	"sf read addr offset|partition len	- read `len' bytes starting at\n"
+	"				          `offset' to memory at `addr'\n"
+	"sf write addr offset|partition len	- write `len' bytes from memory\n"
+	"				          at `addr' to flash at `offset'\n"
+	"sf erase offset|partition [+]len	- erase `len' bytes from `offset'\n"
+	"			            	  `+len' round up `len' to block size\n"
+	"sf update addr offset|partition len	- erase and write `len' bytes from memory\n"
+	"				          at `addr' to flash at `offset'"
 	SF_TEST_HELP
 );
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver
  2014-07-14  7:39 ` [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver Heiko Schocher
@ 2014-07-16 18:42   ` Daniel Schwierzeck
  2014-07-17  4:27     ` Heiko Schocher
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Schwierzeck @ 2014-07-16 18:42 UTC (permalink / raw)
  To: u-boot

Dear Heiko,

2014-07-14 9:39 GMT+02:00 Heiko Schocher <hs@denx.de>:
> 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:

thanks for picking up this patch

>
>   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
...
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 6bcd522..c883d1d 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -156,4 +156,17 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>  int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>                 size_t len, void *data);
>
> +#ifdef CONFIG_SPI_FLASH_MTD
> +int spi_flash_mtd_register(struct spi_flash *flash);
> +void spi_flash_mtd_unregister(void);
> +#else
> +static inline int spi_flash_mtd_register(struct spi_flash *flash)
> +{
> +       return 0;
> +}
> +static __maybe_unused void spi_flash_mtd_unregister(void)
> +{
> +}
> +#endif
> +
>  #endif /* _SF_INTERNAL_H_ */

there is a copy&paste error in the original patch. But the fix should
be "static inline" rather than "static __maybe_unused"

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver
  2014-07-16 18:42   ` Daniel Schwierzeck
@ 2014-07-17  4:27     ` Heiko Schocher
  0 siblings, 0 replies; 9+ messages in thread
From: Heiko Schocher @ 2014-07-17  4:27 UTC (permalink / raw)
  To: u-boot

Hello Daniel,

Am 16.07.2014 20:42, schrieb Daniel Schwierzeck:
> Dear Heiko,
>
> 2014-07-14 9:39 GMT+02:00 Heiko Schocher<hs@denx.de>:
>> 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:
>
> thanks for picking up this patch

thanks for your work!

>>    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
> ...
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index 6bcd522..c883d1d 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -156,4 +156,17 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>>   int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>>                  size_t len, void *data);
>>
>> +#ifdef CONFIG_SPI_FLASH_MTD
>> +int spi_flash_mtd_register(struct spi_flash *flash);
>> +void spi_flash_mtd_unregister(void);
>> +#else
>> +static inline int spi_flash_mtd_register(struct spi_flash *flash)
>> +{
>> +       return 0;
>> +}
>> +static __maybe_unused void spi_flash_mtd_unregister(void)
>> +{
>> +}
>> +#endif
>> +
>>   #endif /* _SF_INTERNAL_H_ */
>
> there is a copy&paste error in the original patch. But the fix should
> be "static inline" rather than "static __maybe_unused"

Ah, thanks! Fixed this, MAKEALL started ...

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place
  2014-07-14  7:39 ` [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
@ 2014-07-30  0:29   ` Scott Wood
  2014-08-11  8:40     ` Heiko Schocher
  0 siblings, 1 reply; 9+ messages in thread
From: Scott Wood @ 2014-07-30  0:29 UTC (permalink / raw)
  To: u-boot

On Mon, 2014-07-14 at 09:39 +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>
> Cc: Scott Wood <scottwood@freescale.com>
> Cc: Tom Rini <trini@ti.com>
> ---
>  common/cmd_nand.c       | 140 +++++++++---------------------------------------
>  common/cmd_onenand.c    |  19 +++----
>  drivers/mtd/Makefile    |   4 +-
>  drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++
>  include/linux/mtd/mtd.h |   7 +++
>  5 files changed, 154 insertions(+), 130 deletions(-)
>  create mode 100644 drivers/mtd/mtd_uboot.c

ACK the concept, some nits below.

Have you given any more thought to formally taking over the
custodianship?  If not, Tom, are you expecting another pull request from
me or were you going to apply Heiko's patches directly?

> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
>  			goto usage;
>  
>  		/* We don't care about size, or maxsize. */
> -		if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
> +		if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
> +		    MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
> +			puts("Offset or partition name expected\n");
> +			return 1;
> +		}

MTD_DEV_TYPE_NAND should be aligned with argv[2] (if you're going to do
alignment rather than just a couple tabs), not arg_off.  Likewise
elsewhere.

> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> new file mode 100644
> index 0000000..66e49c3
> --- /dev/null
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -0,0 +1,114 @@
> +/*
> + * (C) Copyright 2014
> + * Heiko Schocher, DENX Software Engineering, hs at denx.de.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <linux/mtd/mtd.h>
> +#include <jffs2/jffs2.h>

What do you use from the jffs2 header?

> +inline int str2off(const char *p, loff_t *num)
> +{
> +	char *endptr;
> +
> +	*num = simple_strtoull(p, &endptr, 16);
> +	return *p != '\0' && *endptr == '\0';
> +}
> +
> +inline int str2long(const char *p, ulong *num)
> +{
> +	char *endptr;
> +
> +	*num = simple_strtoul(p, &endptr, 16);
> +	return *p != '\0' && *endptr == '\0';
> +}

Should drop the inline -- it didn't make much sense in the old location
as it wasn't in a header file (GCC can auto-inline where appropriate),
and it makes even less sense if it's not static.

-Scott

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place
  2014-07-30  0:29   ` Scott Wood
@ 2014-08-11  8:40     ` Heiko Schocher
  2014-08-15  0:10       ` Scott Wood
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Schocher @ 2014-08-11  8:40 UTC (permalink / raw)
  To: u-boot

Hello Scott,

Am 30.07.2014 02:29, schrieb Scott Wood:
> On Mon, 2014-07-14 at 09:39 +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>
>> Cc: Scott Wood<scottwood@freescale.com>
>> Cc: Tom Rini<trini@ti.com>
>> ---
>>   common/cmd_nand.c       | 140 +++++++++---------------------------------------
>>   common/cmd_onenand.c    |  19 +++----
>>   drivers/mtd/Makefile    |   4 +-
>>   drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++
>>   include/linux/mtd/mtd.h |   7 +++
>>   5 files changed, 154 insertions(+), 130 deletions(-)
>>   create mode 100644 drivers/mtd/mtd_uboot.c
>
> ACK the concept, some nits below.
>
> Have you given any more thought to formally taking over the
> custodianship?  If not, Tom, are you expecting another pull request from
> me or were you going to apply Heiko's patches directly?

I thinking about it, yes, but I am really busy ... but it seems
nobody else want to volunteer ... I discussed with Wolfgang, if
it would make sense to create a mtd branch ... so, if you could not
longer maintain the nand branch, we can set it to orphaned (in the
hope, someone will volunteer ...) and I can also handle the nand
patches ... but Wolfgang is currently on vacation, so this step
maybe take a while ...

>> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
>>   			goto usage;
>>
>>   		/* We don't care about size, or maxsize. */
>> -		if (arg_off(argv[2],&idx,&addr,&maxsize,&maxsize)) {
>> +		if (arg_off(argv[2],&idx,&addr,&maxsize,&maxsize,
>> +		    MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>> +			puts("Offset or partition name expected\n");
>> +			return 1;
>> +		}
>
> MTD_DEV_TYPE_NAND should be aligned with argv[2] (if you're going to do
> alignment rather than just a couple tabs), not arg_off.  Likewise
> elsewhere.

fixed.

>> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
>> new file mode 100644
>> index 0000000..66e49c3
>> --- /dev/null
>> +++ b/drivers/mtd/mtd_uboot.c
>> @@ -0,0 +1,114 @@
>> +/*
>> + * (C) Copyright 2014
>> + * Heiko Schocher, DENX Software Engineering, hs at denx.de.
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +#include<common.h>
>> +#include<linux/mtd/mtd.h>
>> +#include<jffs2/jffs2.h>
>
> What do you use from the jffs2 header?

Yes, good question ... because missing common mtd defines, like:

struct mtd_device {

found in include/jffs2/load_kernel.h
used in common/cmd_nand.c for example ... its really time to
cleanup the mtd subsystem!

Where to move this common defines? include/linux/mtd/mtd.h ?

But before the mtd/ubi/ubifs sync patches are not in mainline, I do
not want change this ... maybe it is okay to do this in a second step?

>> +inline int str2off(const char *p, loff_t *num)
>> +{
>> +	char *endptr;
>> +
>> +	*num = simple_strtoull(p,&endptr, 16);
>> +	return *p != '\0'&&  *endptr == '\0';
>> +}
>> +
>> +inline int str2long(const char *p, ulong *num)
>> +{
>> +	char *endptr;
>> +
>> +	*num = simple_strtoul(p,&endptr, 16);
>> +	return *p != '\0'&&  *endptr == '\0';
>> +}
>
> Should drop the inline -- it didn't make much sense in the old location
> as it wasn't in a header file (GCC can auto-inline where appropriate),
> and it makes even less sense if it's not static.

removed, thanks for the review!

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place
  2014-08-11  8:40     ` Heiko Schocher
@ 2014-08-15  0:10       ` Scott Wood
  0 siblings, 0 replies; 9+ messages in thread
From: Scott Wood @ 2014-08-15  0:10 UTC (permalink / raw)
  To: u-boot

On Mon, 2014-08-11 at 10:40 +0200, Heiko Schocher wrote:
> Hello Scott,
> 
> Am 30.07.2014 02:29, schrieb Scott Wood:
> > On Mon, 2014-07-14 at 09:39 +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>
> >> Cc: Scott Wood<scottwood@freescale.com>
> >> Cc: Tom Rini<trini@ti.com>
> >> ---
> >>   common/cmd_nand.c       | 140 +++++++++---------------------------------------
> >>   common/cmd_onenand.c    |  19 +++----
> >>   drivers/mtd/Makefile    |   4 +-
> >>   drivers/mtd/mtd_uboot.c | 114 +++++++++++++++++++++++++++++++++++++++
> >>   include/linux/mtd/mtd.h |   7 +++
> >>   5 files changed, 154 insertions(+), 130 deletions(-)
> >>   create mode 100644 drivers/mtd/mtd_uboot.c
> >
> > ACK the concept, some nits below.
> >
> > Have you given any more thought to formally taking over the
> > custodianship?  If not, Tom, are you expecting another pull request from
> > me or were you going to apply Heiko's patches directly?
> 
> I thinking about it, yes, but I am really busy ... but it seems
> nobody else want to volunteer ... I discussed with Wolfgang, if
> it would make sense to create a mtd branch ... so, if you could not
> longer maintain the nand branch, we can set it to orphaned (in the
> hope, someone will volunteer ...) and I can also handle the nand
> patches ... but Wolfgang is currently on vacation, so this step
> maybe take a while ...

OK.  I can still review things now and then, but it's getting hard to be
responsive to the extent expected of a custodian.

> >> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> >> new file mode 100644
> >> index 0000000..66e49c3
> >> --- /dev/null
> >> +++ b/drivers/mtd/mtd_uboot.c
> >> @@ -0,0 +1,114 @@
> >> +/*
> >> + * (C) Copyright 2014
> >> + * Heiko Schocher, DENX Software Engineering, hs at denx.de.
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +#include<common.h>
> >> +#include<linux/mtd/mtd.h>
> >> +#include<jffs2/jffs2.h>
> >
> > What do you use from the jffs2 header?
> 
> Yes, good question ... because missing common mtd defines, like:
> 
> struct mtd_device {
> 
> found in include/jffs2/load_kernel.h
> used in common/cmd_nand.c for example ... its really time to
> cleanup the mtd subsystem!
> 
> Where to move this common defines? include/linux/mtd/mtd.h ?
> 
> But before the mtd/ubi/ubifs sync patches are not in mainline, I do
> not want change this ... maybe it is okay to do this in a second step?

Sure.

-Scott

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-08-15  0:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-14  7:39 [U-Boot] [PATCH 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
2014-07-14  7:39 ` [U-Boot] [PATCH 1/3] mtd, spi: add MTD layer driver Heiko Schocher
2014-07-16 18:42   ` Daniel Schwierzeck
2014-07-17  4:27     ` Heiko Schocher
2014-07-14  7:39 ` [U-Boot] [PATCH 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
2014-07-30  0:29   ` Scott Wood
2014-08-11  8:40     ` Heiko Schocher
2014-08-15  0:10       ` Scott Wood
2014-07-14  7:39 ` [U-Boot] [PATCH 3/3] spi, sf: use offset and size in sf cmd from mtdpartition Heiko Schocher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox