* [U-Boot] [PATCH v5 0/3] spi, sf: add mtdparts feature to spi and sf commands
@ 2015-04-20 5:47 Heiko Schocher
2015-04-20 5:47 ` [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver Heiko Schocher
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Heiko Schocher @ 2015-04-20 5:47 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
+ 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 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
- 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
- none
Series-changes: 3
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
Series-changes: 4
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Series-changes: 5
- no changes
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 | 142 ++++++++----------------------------------
common/cmd_onenand.c | 19 ++----
common/cmd_sf.c | 57 +++++++++--------
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, 312 insertions(+), 157 deletions(-)
create mode 100644 drivers/mtd/mtd_uboot.c
create mode 100644 drivers/mtd/spi/sf_mtd.c
--
2.1.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver
2015-04-20 5:47 [U-Boot] [PATCH v5 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
@ 2015-04-20 5:47 ` Heiko Schocher
2015-04-22 10:52 ` Jagan Teki
2015-04-20 5:47 ` [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
2015-04-20 5:47 ` [U-Boot] [PATCH v5 3/3] spi, sf: use offset and size in sf cmd from mtdpartition Heiko Schocher
2 siblings, 1 reply; 16+ messages in thread
From: Heiko Schocher @ 2015-04-20 5:47 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>
---
Changes in v2:
- add comment from Daniel Schwierzeck:
fix compile error from original patch with
"static inline" rather than "static __maybe_unused"
Series-changes: 3
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
Series-changes: 4
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Series-changes: 5
- no changes
README | 3 ++
common/cmd_sf.c | 7 ++-
drivers/mtd/spi/Makefile | 1 +
drivers/mtd/spi/sf_internal.h | 13 ++++++
drivers/mtd/spi/sf_mtd.c | 104 ++++++++++++++++++++++++++++++++++++++++++
drivers/mtd/spi/sf_probe.c | 5 ++
6 files changed, 131 insertions(+), 2 deletions(-)
create mode 100644 drivers/mtd/spi/sf_mtd.c
diff --git a/README b/README
index 9b748cc..56dcfd2 100644
--- a/README
+++ b/README
@@ -3077,6 +3077,9 @@ CBFS (Coreboot Filesystem) support
operation will not execute. The only way to exit this
hardware-protected mode is to drive W#/VPP HIGH.
+ CONFIG_SPI_FLASH_MTD
+ add MTD translation layer driver.
+
- SystemACE Support:
CONFIG_SYSTEMACE
diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 5c788e9..25a59e5 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -132,14 +132,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
flash = new->uclass_priv;
#else
+ if (flash)
+ spi_flash_free(flash);
+
new = spi_flash_probe(bus, cs, speed, mode);
+ flash = new;
+
if (!new) {
printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
return 1;
}
- if (flash)
- spi_flash_free(flash);
flash = new;
#endif
diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
index c61b784e..f8580cd 100644
--- a/drivers/mtd/spi/Makefile
+++ b/drivers/mtd/spi/Makefile
@@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
#endif
obj-$(CONFIG_CMD_SF) += sf.o
obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
+obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
index 785f7a9..dc75f43 100644
--- a/drivers/mtd/spi/sf_internal.h
+++ b/drivers/mtd/spi/sf_internal.h
@@ -221,4 +221,17 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
size_t len, void *data);
+#ifdef CONFIG_SPI_FLASH_MTD
+int spi_flash_mtd_register(struct spi_flash *flash);
+void spi_flash_mtd_unregister(void);
+#else
+static inline int spi_flash_mtd_register(struct spi_flash *flash)
+{
+ return 0;
+}
+static inline void spi_flash_mtd_unregister(void)
+{
+}
+#endif
+
#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 4103723..93209fe 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -399,6 +399,10 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
/* Release spi bus */
spi_release_bus(spi);
+ ret = spi_flash_mtd_register(flash);
+ if (ret)
+ return ret;
+
return 0;
err_read_id:
@@ -449,6 +453,7 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,
void spi_flash_free(struct spi_flash *flash)
{
+ spi_flash_mtd_unregister();
spi_free_slave(flash->spi);
free(flash);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-20 5:47 [U-Boot] [PATCH v5 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
2015-04-20 5:47 ` [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver Heiko Schocher
@ 2015-04-20 5:47 ` Heiko Schocher
2015-04-22 10:53 ` Jagan Teki
2015-04-22 22:47 ` Scott Wood
2015-04-20 5:47 ` [U-Boot] [PATCH v5 3/3] spi, sf: use offset and size in sf cmd from mtdpartition Heiko Schocher
2 siblings, 2 replies; 16+ messages in thread
From: Heiko Schocher @ 2015-04-20 5:47 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>
---
Changes in v2:
- none
Series-changes: 3
- add comments from scott wood:
- align MTD_DEV_TYPE_NAND correct
- remove unnecessary inline
- rework "jffs2 header" problem later
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
Series-changes: 4
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Series-changes: 5
- add comment from Scott Wood:
keep the continuation line aligned with the arguments
common/cmd_nand.c | 142 +++++++++---------------------------------------
common/cmd_onenand.c | 19 ++-----
drivers/mtd/Makefile | 4 +-
drivers/mtd/mtd_uboot.c | 114 ++++++++++++++++++++++++++++++++++++++
include/linux/mtd/mtd.h | 7 +++
5 files changed, 156 insertions(+), 130 deletions(-)
create mode 100644 drivers/mtd/mtd_uboot.c
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 17fa7ea..184335d 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -133,115 +133,6 @@ static int set_dev(int dev)
return 0;
}
-static inline int str2off(const char *p, loff_t *num)
-{
- char *endptr;
-
- *num = simple_strtoull(p, &endptr, 16);
- return *p != '\0' && *endptr == '\0';
-}
-
-static inline int str2long(const char *p, ulong *num)
-{
- char *endptr;
-
- *num = simple_strtoul(p, &endptr, 16);
- return *p != '\0' && *endptr == '\0';
-}
-
-static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
- loff_t *maxsize)
-{
-#ifdef CONFIG_CMD_MTDPARTS
- struct mtd_device *dev;
- struct part_info *part;
- u8 pnum;
- int ret;
-
- ret = mtdparts_init();
- if (ret)
- return ret;
-
- ret = find_dev_and_part(partname, &dev, &pnum, &part);
- if (ret)
- return ret;
-
- if (dev->id->type != MTD_DEV_TYPE_NAND) {
- puts("not a NAND device\n");
- return -1;
- }
-
- *off = part->offset;
- *size = part->size;
- *maxsize = part->size;
- *idx = dev->id->num;
-
- ret = set_dev(*idx);
- if (ret)
- return ret;
-
- return 0;
-#else
- puts("offset is not a number\n");
- return -1;
-#endif
-}
-
-static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
- loff_t *maxsize)
-{
- if (!str2off(arg, off))
- return get_part(arg, idx, off, size, maxsize);
-
- if (*off >= nand_info[*idx].size) {
- puts("Offset exceeds device limit\n");
- return -1;
- }
-
- *maxsize = nand_info[*idx].size - *off;
- *size = *maxsize;
- return 0;
-}
-
-static int arg_off_size(int argc, char *const argv[], int *idx,
- loff_t *off, loff_t *size, loff_t *maxsize)
-{
- int ret;
-
- if (argc == 0) {
- *off = 0;
- *size = nand_info[*idx].size;
- *maxsize = *size;
- goto print;
- }
-
- ret = arg_off(argv[0], idx, off, size, maxsize);
- if (ret)
- return ret;
-
- if (argc == 1)
- goto print;
-
- if (!str2off(argv[1], size)) {
- printf("'%s' is not a number\n", argv[1]);
- return -1;
- }
-
- if (*size > *maxsize) {
- puts("Size exceeds partition or device limit\n");
- return -1;
- }
-
-print:
- printf("device %d ", *idx);
- if (*size == nand_info[*idx].size)
- puts("whole chip\n");
- else
- printf("offset 0x%llx, size 0x%llx\n",
- (unsigned long long)*off, (unsigned long long)*size);
- return 0;
-}
-
#ifdef CONFIG_CMD_NAND_LOCK_UNLOCK
static void print_status(ulong start, ulong end, ulong erasesize, int status)
{
@@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
goto usage;
/* We don't care about size, or maxsize. */
- if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
+ if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
+ MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
+ puts("Offset or partition name expected\n");
+ return 1;
+ }
+ if (set_dev(idx)) {
puts("Offset or partition name expected\n");
return 1;
}
@@ -595,7 +491,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
printf("\nNAND %s: ", cmd);
/* skip first two or three arguments, look for offset and size */
if (arg_off_size(argc - o, argv + o, &dev, &off, &size,
- &maxsize) != 0)
+ &maxsize, MTD_DEV_TYPE_NAND,
+ nand_info[dev].size) != 0)
+ return 1;
+
+ if (set_dev(dev))
return 1;
nand = &nand_info[dev];
@@ -655,7 +555,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (s && !strcmp(s, ".raw")) {
raw = 1;
- if (arg_off(argv[3], &dev, &off, &size, &maxsize))
+ if (arg_off(argv[3], &dev, &off, &size, &maxsize,
+ MTD_DEV_TYPE_NAND, nand_info[dev].size))
+ return 1;
+
+ if (set_dev(dev))
return 1;
nand = &nand_info[dev];
@@ -672,8 +576,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
rwsize = pagecount * (nand->writesize + nand->oobsize);
} else {
- if (arg_off_size(argc - 3, argv + 3, &dev,
- &off, &size, &maxsize) != 0)
+ if (arg_off_size(argc - 3, argv + 3, &dev, &off, &size,
+ &maxsize, MTD_DEV_TYPE_NAND,
+ nand_info[dev].size) != 0)
+ return 1;
+
+ if (set_dev(dev))
return 1;
/* size is unspecified */
@@ -812,7 +720,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
allexcept = 1;
if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
- &maxsize) < 0)
+ &maxsize, MTD_DEV_TYPE_NAND,
+ nand_info[dev].size) < 0)
+ return 1;
+
+ if (set_dev(dev))
return 1;
if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c
index 06cc140..feab01a 100644
--- a/common/cmd_onenand.c
+++ b/common/cmd_onenand.c
@@ -24,15 +24,8 @@ static struct mtd_info *mtd;
static loff_t next_ofs;
static loff_t skip_ofs;
-static inline int str2long(char *p, ulong *num)
-{
- char *endptr;
-
- *num = simple_strtoul(p, &endptr, 16);
- return (*p != '\0' && *endptr == '\0') ? 1 : 0;
-}
-
-static int arg_off_size(int argc, char * const argv[], ulong *off, size_t *size)
+static int arg_off_size_onenand(int argc, char * const argv[], ulong *off,
+ size_t *size)
{
if (argc >= 1) {
if (!(str2long(argv[0], off))) {
@@ -399,7 +392,7 @@ static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char * const a
addr = (ulong)simple_strtoul(argv[1], NULL, 16);
printf("\nOneNAND read: ");
- if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
+ if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
return 1;
ret = onenand_block_read(ofs, len, &retlen, (u8 *)addr, oob);
@@ -425,7 +418,7 @@ static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char * const
addr = (ulong)simple_strtoul(argv[1], NULL, 16);
printf("\nOneNAND write: ");
- if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
+ if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
return 1;
ret = onenand_block_write(ofs, len, &retlen, (u8 *)addr, withoob);
@@ -461,7 +454,7 @@ static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char * const
printf("\nOneNAND erase: ");
/* skip first two or three arguments, look for offset and size */
- if (arg_off_size(argc, argv, &ofs, &len) != 0)
+ if (arg_off_size_onenand(argc, argv, &ofs, &len) != 0)
return 1;
ret = onenand_block_erase(ofs, len, force);
@@ -486,7 +479,7 @@ static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char * const a
printf("\nOneNAND test: ");
/* skip first two or three arguments, look for offset and size */
- if (arg_off_size(argc - 1, argv + 1, &ofs, &len) != 0)
+ if (arg_off_size_onenand(argc - 1, argv + 1, &ofs, &len) != 0)
return 1;
ret = onenand_block_test(ofs, len);
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 5467a951..a623f4c 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -5,8 +5,8 @@
# SPDX-License-Identifier: GPL-2.0+
#
-ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)))
-obj-y += mtdcore.o
+ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)))
+obj-y += mtdcore.o mtd_uboot.o
endif
obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o
obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
new file mode 100644
index 0000000..a70d40a
--- /dev/null
+++ b/drivers/mtd/mtd_uboot.c
@@ -0,0 +1,114 @@
+/*
+ * (C) Copyright 2014
+ * Heiko Schocher, DENX Software Engineering, hs at denx.de.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+#include <common.h>
+#include <linux/mtd/mtd.h>
+#include <jffs2/jffs2.h>
+
+int str2off(const char *p, loff_t *num)
+{
+ char *endptr;
+
+ *num = simple_strtoull(p, &endptr, 16);
+ return *p != '\0' && *endptr == '\0';
+}
+
+int str2long(const char *p, ulong *num)
+{
+ char *endptr;
+
+ *num = simple_strtoul(p, &endptr, 16);
+ return *p != '\0' && *endptr == '\0';
+}
+
+static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
+ loff_t *maxsize, int devtype)
+{
+#ifdef CONFIG_CMD_MTDPARTS
+ struct mtd_device *dev;
+ struct part_info *part;
+ u8 pnum;
+ int ret;
+
+ ret = mtdparts_init();
+ if (ret)
+ return ret;
+
+ ret = find_dev_and_part(partname, &dev, &pnum, &part);
+ if (ret)
+ return ret;
+
+ if (dev->id->type != devtype) {
+ printf("not same typ %d != %d\n", dev->id->type, devtype);
+ return -1;
+ }
+
+ *off = part->offset;
+ *size = part->size;
+ *maxsize = part->size;
+ *idx = dev->id->num;
+
+ return 0;
+#else
+ puts("offset is not a number\n");
+ return -1;
+#endif
+}
+
+int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
+ loff_t *maxsize, int devtype, int chipsize)
+{
+ if (!str2off(arg, off))
+ return get_part(arg, idx, off, size, maxsize, devtype);
+
+ if (*off >= chipsize) {
+ puts("Offset exceeds device limit\n");
+ return -1;
+ }
+
+ *maxsize = chipsize - *off;
+ *size = *maxsize;
+ return 0;
+}
+
+int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
+ loff_t *size, loff_t *maxsize, int devtype, int chipsize)
+{
+ int ret;
+
+ if (argc == 0) {
+ *off = 0;
+ *size = chipsize;
+ *maxsize = *size;
+ goto print;
+ }
+
+ ret = arg_off(argv[0], idx, off, size, maxsize, devtype, chipsize);
+ if (ret)
+ return ret;
+
+ if (argc == 1)
+ goto print;
+
+ if (!str2off(argv[1], size)) {
+ printf("'%s' is not a number\n", argv[1]);
+ return -1;
+ }
+
+ if (*size > *maxsize) {
+ puts("Size exceeds partition or device limit\n");
+ return -1;
+ }
+
+print:
+ printf("device %d ", *idx);
+ if (*size == chipsize)
+ puts("whole chip\n");
+ else
+ printf("offset 0x%llx, size 0x%llx\n",
+ (unsigned long long)*off, (unsigned long long)*size);
+ return 0;
+}
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 8666413..2861af5 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd);
int del_mtd_device(struct mtd_info *mtd);
int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
int del_mtd_partitions(struct mtd_info *);
+
+int str2off(const char *p, loff_t *num);
+int str2long(const char *p, ulong *num);
+int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
+ loff_t *maxsize, int devtype, int chipsize);
+int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
+ loff_t *size, loff_t *maxsize, int devtype, int chipsize);
#endif
#endif /* __MTD_MTD_H__ */
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 3/3] spi, sf: use offset and size in sf cmd from mtdpartition
2015-04-20 5:47 [U-Boot] [PATCH v5 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
2015-04-20 5:47 ` [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver Heiko Schocher
2015-04-20 5:47 ` [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
@ 2015-04-20 5:47 ` Heiko Schocher
2015-04-22 10:58 ` Jagan Teki
2 siblings, 1 reply; 16+ messages in thread
From: Heiko Schocher @ 2015-04-20 5:47 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>
---
Changes in v2:
- none
Series-changes: 3
- rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
Series-changes: 4
- rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
Series-changes: 5
- no changes
common/cmd_sf.c | 50 +++++++++++++++++++++++++-------------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/common/cmd_sf.c b/common/cmd_sf.c
index 25a59e5..1932ac9 100644
--- a/common/cmd_sf.c
+++ b/common/cmd_sf.c
@@ -12,6 +12,8 @@
#include <malloc.h>
#include <spi.h>
#include <spi_flash.h>
+#include <jffs2/jffs2.h>
+#include <linux/mtd/mtd.h>
#include <asm/io.h>
#include <dm/device-internal.h>
@@ -258,23 +260,21 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset,
static int do_spi_flash_read_write(int argc, char * const argv[])
{
unsigned long addr;
- unsigned long offset;
- unsigned long len;
void *buf;
char *endp;
int ret = 1;
+ int dev = 0;
+ loff_t offset, len, maxsize;
- if (argc < 4)
+ if (argc < 3)
return -1;
addr = simple_strtoul(argv[1], &endp, 16);
if (*argv[1] == 0 || *endp != 0)
return -1;
- offset = simple_strtoul(argv[2], &endp, 16);
- if (*argv[2] == 0 || *endp != 0)
- return -1;
- len = simple_strtoul(argv[3], &endp, 16);
- if (*argv[3] == 0 || *endp != 0)
+
+ if (arg_off_size(argc - 2, &argv[2], &dev, &offset, &len, &maxsize,
+ MTD_DEV_TYPE_NOR, flash->size))
return -1;
/* Consistency checking */
@@ -313,31 +313,31 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
static int do_spi_flash_erase(int argc, char * const argv[])
{
- unsigned long offset;
- unsigned long len;
- char *endp;
int ret;
+ int dev = 0;
+ loff_t offset, len, maxsize;
+ ulong size;
if (argc < 3)
return -1;
- offset = simple_strtoul(argv[1], &endp, 16);
- if (*argv[1] == 0 || *endp != 0)
+ if (arg_off(argv[1], &dev, &offset, &len, &maxsize,
+ MTD_DEV_TYPE_NOR, flash->size))
return -1;
- ret = sf_parse_len_arg(argv[2], &len);
+ ret = sf_parse_len_arg(argv[2], &size);
if (ret != 1)
return -1;
/* Consistency checking */
- if (offset + len > flash->size) {
+ if (offset + size > flash->size) {
printf("ERROR: attempting %s past flash size (%#x)\n",
argv[0], flash->size);
return 1;
}
- ret = spi_flash_erase(flash, offset, len);
- printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)len, (u32)offset,
+ ret = spi_flash_erase(flash, offset, size);
+ printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)size, (u32)offset,
ret ? "ERROR" : "OK");
return ret == 0 ? 0 : 1;
@@ -562,13 +562,13 @@ U_BOOT_CMD(
"SPI flash sub-system",
"probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n"
" and chip select\n"
- "sf read addr offset len - read `len' bytes starting at\n"
- " `offset' to memory at `addr'\n"
- "sf write addr offset len - write `len' bytes from memory\n"
- " at `addr' to flash at `offset'\n"
- "sf erase offset [+]len - erase `len' bytes from `offset'\n"
- " `+len' round up `len' to block size\n"
- "sf update addr offset len - erase and write `len' bytes from memory\n"
- " at `addr' to flash at `offset'"
+ "sf read addr offset|partition len - read `len' bytes starting at\n"
+ " `offset' to memory at `addr'\n"
+ "sf write addr offset|partition len - write `len' bytes from memory\n"
+ " at `addr' to flash at `offset'\n"
+ "sf erase offset|partition [+]len - erase `len' bytes from `offset'\n"
+ " `+len' round up `len' to block size\n"
+ "sf update addr offset|partition len - erase and write `len' bytes from memory\n"
+ " at `addr' to flash at `offset'"
SF_TEST_HELP
);
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver
2015-04-20 5:47 ` [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver Heiko Schocher
@ 2015-04-22 10:52 ` Jagan Teki
2015-04-23 5:27 ` Heiko Schocher
0 siblings, 1 reply; 16+ messages in thread
From: Jagan Teki @ 2015-04-22 10:52 UTC (permalink / raw)
To: u-boot
On 20 April 2015 at 11:17, Heiko Schocher <hs@denx.de> wrote:
> From: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>
> add MTD layer driver for spi, original patch from:
> http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=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 v2:
> - add comment from Daniel Schwierzeck:
> fix compile error from original patch with
> "static inline" rather than "static __maybe_unused"
> Series-changes: 3
> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> Series-changes: 4
> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
> Series-changes: 5
> - no changes
>
> README | 3 ++
> common/cmd_sf.c | 7 ++-
> drivers/mtd/spi/Makefile | 1 +
> drivers/mtd/spi/sf_internal.h | 13 ++++++
> drivers/mtd/spi/sf_mtd.c | 104 ++++++++++++++++++++++++++++++++++++++++++
> drivers/mtd/spi/sf_probe.c | 5 ++
> 6 files changed, 131 insertions(+), 2 deletions(-)
> create mode 100644 drivers/mtd/spi/sf_mtd.c
>
> diff --git a/README b/README
> index 9b748cc..56dcfd2 100644
> --- a/README
> +++ b/README
> @@ -3077,6 +3077,9 @@ CBFS (Coreboot Filesystem) support
> operation will not execute. The only way to exit this
> hardware-protected mode is to drive W#/VPP HIGH.
>
> + CONFIG_SPI_FLASH_MTD
> + add MTD translation layer driver.
> +
> - SystemACE Support:
> CONFIG_SYSTEMACE
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index 5c788e9..25a59e5 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -132,14 +132,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>
> flash = new->uclass_priv;
> #else
> + if (flash)
> + spi_flash_free(flash);
> +
> new = spi_flash_probe(bus, cs, speed, mode);
> + flash = new;
> +
This could be really a another patch - as it's not related to mtd.
> if (!new) {
> printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
> return 1;
> }
>
> - if (flash)
> - spi_flash_free(flash);
> flash = new;
> #endif
>
> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
> index c61b784e..f8580cd 100644
> --- a/drivers/mtd/spi/Makefile
> +++ b/drivers/mtd/spi/Makefile
> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
> #endif
> obj-$(CONFIG_CMD_SF) += sf.o
> obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
> obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
> obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> index 785f7a9..dc75f43 100644
> --- a/drivers/mtd/spi/sf_internal.h
> +++ b/drivers/mtd/spi/sf_internal.h
> @@ -221,4 +221,17 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
> int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
> size_t len, void *data);
>
> +#ifdef CONFIG_SPI_FLASH_MTD
> +int spi_flash_mtd_register(struct spi_flash *flash);
> +void spi_flash_mtd_unregister(void);
> +#else
> +static inline int spi_flash_mtd_register(struct spi_flash *flash)
> +{
> + return 0;
> +}
> +static inline void spi_flash_mtd_unregister(void)
> +{
> +}
> +#endif
From the previous comments to reply - https://patchwork.ozlabs.org/patch/386102/
Adding #def isn't a polluting thing in code size contains software's like u-boot
and did the same for many of my code.
So please do as below
#ifdef CONFIG_SPI_FLASH_MTD
int spi_flash_mtd_register(struct spi_flash *flash);
void spi_flash_mtd_unregister(void);
#endif
void spi_flash_free(struct spi_flash *flash)
{
#ifdef CONFIG_SPI_FLASH_MTD
spi_flash_mtd_unregister();
#endif
}
#ifdef CONFIG_SPI_FLASH_MTD
ret = spi_flash_mtd_register(flash);
if (ret)
goto err_claim_bus;
#endif
> +
> #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 4103723..93209fe 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -399,6 +399,10 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
> /* Release spi bus */
> spi_release_bus(spi);
>
> + ret = spi_flash_mtd_register(flash);
> + if (ret)
> + return ret;
mtd register call is before the spi_release_bus isn't it?
> +
> return 0;
>
> err_read_id:
> @@ -449,6 +453,7 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,
>
> void spi_flash_free(struct spi_flash *flash)
> {
> + spi_flash_mtd_unregister();
> spi_free_slave(flash->spi);
> free(flash);
> }
> --
> 2.1.0
>
thanks!
--
Jagan.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-20 5:47 ` [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
@ 2015-04-22 10:53 ` Jagan Teki
2015-04-22 22:47 ` Scott Wood
1 sibling, 0 replies; 16+ messages in thread
From: Jagan Teki @ 2015-04-22 10:53 UTC (permalink / raw)
To: u-boot
On 20 April 2015 at 11:17, Heiko Schocher <hs@denx.de> wrote:
> move common functions from cmd_nand.c (for calculating offset
> and size from cmdline paramter) to common place, so they could
> used from other commands which use mtd partitions.
>
> For onenand the arg_off_size() is left in common/cmd_onenand.c.
> It should use now the common arg_off() function, but as I could
> not test onenand I let it there ...
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>
> ---
>
> Changes in v2:
> - none
> Series-changes: 3
> - add comments from scott wood:
> - align MTD_DEV_TYPE_NAND correct
> - remove unnecessary inline
> - rework "jffs2 header" problem later
> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> Series-changes: 4
> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
> Series-changes: 5
> - add comment from Scott Wood:
> keep the continuation line aligned with the arguments
>
> common/cmd_nand.c | 142 +++++++++---------------------------------------
> common/cmd_onenand.c | 19 ++-----
> drivers/mtd/Makefile | 4 +-
> drivers/mtd/mtd_uboot.c | 114 ++++++++++++++++++++++++++++++++++++++
> include/linux/mtd/mtd.h | 7 +++
> 5 files changed, 156 insertions(+), 130 deletions(-)
> create mode 100644 drivers/mtd/mtd_uboot.c
>
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index 17fa7ea..184335d 100644
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -133,115 +133,6 @@ static int set_dev(int dev)
> return 0;
> }
>
> -static inline int str2off(const char *p, loff_t *num)
> -{
> - char *endptr;
> -
> - *num = simple_strtoull(p, &endptr, 16);
> - return *p != '\0' && *endptr == '\0';
> -}
> -
> -static inline int str2long(const char *p, ulong *num)
> -{
> - char *endptr;
> -
> - *num = simple_strtoul(p, &endptr, 16);
> - return *p != '\0' && *endptr == '\0';
> -}
> -
> -static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
> - loff_t *maxsize)
> -{
> -#ifdef CONFIG_CMD_MTDPARTS
> - struct mtd_device *dev;
> - struct part_info *part;
> - u8 pnum;
> - int ret;
> -
> - ret = mtdparts_init();
> - if (ret)
> - return ret;
> -
> - ret = find_dev_and_part(partname, &dev, &pnum, &part);
> - if (ret)
> - return ret;
> -
> - if (dev->id->type != MTD_DEV_TYPE_NAND) {
> - puts("not a NAND device\n");
> - return -1;
> - }
> -
> - *off = part->offset;
> - *size = part->size;
> - *maxsize = part->size;
> - *idx = dev->id->num;
> -
> - ret = set_dev(*idx);
> - if (ret)
> - return ret;
> -
> - return 0;
> -#else
> - puts("offset is not a number\n");
> - return -1;
> -#endif
> -}
> -
> -static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
> - loff_t *maxsize)
> -{
> - if (!str2off(arg, off))
> - return get_part(arg, idx, off, size, maxsize);
> -
> - if (*off >= nand_info[*idx].size) {
> - puts("Offset exceeds device limit\n");
> - return -1;
> - }
> -
> - *maxsize = nand_info[*idx].size - *off;
> - *size = *maxsize;
> - return 0;
> -}
> -
> -static int arg_off_size(int argc, char *const argv[], int *idx,
> - loff_t *off, loff_t *size, loff_t *maxsize)
> -{
> - int ret;
> -
> - if (argc == 0) {
> - *off = 0;
> - *size = nand_info[*idx].size;
> - *maxsize = *size;
> - goto print;
> - }
> -
> - ret = arg_off(argv[0], idx, off, size, maxsize);
> - if (ret)
> - return ret;
> -
> - if (argc == 1)
> - goto print;
> -
> - if (!str2off(argv[1], size)) {
> - printf("'%s' is not a number\n", argv[1]);
> - return -1;
> - }
> -
> - if (*size > *maxsize) {
> - puts("Size exceeds partition or device limit\n");
> - return -1;
> - }
> -
> -print:
> - printf("device %d ", *idx);
> - if (*size == nand_info[*idx].size)
> - puts("whole chip\n");
> - else
> - printf("offset 0x%llx, size 0x%llx\n",
> - (unsigned long long)*off, (unsigned long long)*size);
> - return 0;
> -}
> -
> #ifdef CONFIG_CMD_NAND_LOCK_UNLOCK
> static void print_status(ulong start, ulong end, ulong erasesize, int status)
> {
> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
> goto usage;
>
> /* We don't care about size, or maxsize. */
> - if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
> + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
> + MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
> + puts("Offset or partition name expected\n");
> + return 1;
> + }
> + if (set_dev(idx)) {
> puts("Offset or partition name expected\n");
> return 1;
> }
> @@ -595,7 +491,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> printf("\nNAND %s: ", cmd);
> /* skip first two or three arguments, look for offset and size */
> if (arg_off_size(argc - o, argv + o, &dev, &off, &size,
> - &maxsize) != 0)
> + &maxsize, MTD_DEV_TYPE_NAND,
> + nand_info[dev].size) != 0)
> + return 1;
> +
> + if (set_dev(dev))
> return 1;
>
> nand = &nand_info[dev];
> @@ -655,7 +555,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> if (s && !strcmp(s, ".raw")) {
> raw = 1;
>
> - if (arg_off(argv[3], &dev, &off, &size, &maxsize))
> + if (arg_off(argv[3], &dev, &off, &size, &maxsize,
> + MTD_DEV_TYPE_NAND, nand_info[dev].size))
> + return 1;
> +
> + if (set_dev(dev))
> return 1;
>
> nand = &nand_info[dev];
> @@ -672,8 +576,12 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
> rwsize = pagecount * (nand->writesize + nand->oobsize);
> } else {
> - if (arg_off_size(argc - 3, argv + 3, &dev,
> - &off, &size, &maxsize) != 0)
> + if (arg_off_size(argc - 3, argv + 3, &dev, &off, &size,
> + &maxsize, MTD_DEV_TYPE_NAND,
> + nand_info[dev].size) != 0)
> + return 1;
> +
> + if (set_dev(dev))
> return 1;
>
> /* size is unspecified */
> @@ -812,7 +720,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> allexcept = 1;
>
> if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
> - &maxsize) < 0)
> + &maxsize, MTD_DEV_TYPE_NAND,
> + nand_info[dev].size) < 0)
> + return 1;
> +
> + if (set_dev(dev))
> return 1;
>
> if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
> diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c
> index 06cc140..feab01a 100644
> --- a/common/cmd_onenand.c
> +++ b/common/cmd_onenand.c
> @@ -24,15 +24,8 @@ static struct mtd_info *mtd;
> static loff_t next_ofs;
> static loff_t skip_ofs;
>
> -static inline int str2long(char *p, ulong *num)
> -{
> - char *endptr;
> -
> - *num = simple_strtoul(p, &endptr, 16);
> - return (*p != '\0' && *endptr == '\0') ? 1 : 0;
> -}
> -
> -static int arg_off_size(int argc, char * const argv[], ulong *off, size_t *size)
> +static int arg_off_size_onenand(int argc, char * const argv[], ulong *off,
> + size_t *size)
> {
> if (argc >= 1) {
> if (!(str2long(argv[0], off))) {
> @@ -399,7 +392,7 @@ static int do_onenand_read(cmd_tbl_t * cmdtp, int flag, int argc, char * const a
> addr = (ulong)simple_strtoul(argv[1], NULL, 16);
>
> printf("\nOneNAND read: ");
> - if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
> + if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
> return 1;
>
> ret = onenand_block_read(ofs, len, &retlen, (u8 *)addr, oob);
> @@ -425,7 +418,7 @@ static int do_onenand_write(cmd_tbl_t * cmdtp, int flag, int argc, char * const
> addr = (ulong)simple_strtoul(argv[1], NULL, 16);
>
> printf("\nOneNAND write: ");
> - if (arg_off_size(argc - 2, argv + 2, &ofs, &len) != 0)
> + if (arg_off_size_onenand(argc - 2, argv + 2, &ofs, &len) != 0)
> return 1;
>
> ret = onenand_block_write(ofs, len, &retlen, (u8 *)addr, withoob);
> @@ -461,7 +454,7 @@ static int do_onenand_erase(cmd_tbl_t * cmdtp, int flag, int argc, char * const
> printf("\nOneNAND erase: ");
>
> /* skip first two or three arguments, look for offset and size */
> - if (arg_off_size(argc, argv, &ofs, &len) != 0)
> + if (arg_off_size_onenand(argc, argv, &ofs, &len) != 0)
> return 1;
>
> ret = onenand_block_erase(ofs, len, force);
> @@ -486,7 +479,7 @@ static int do_onenand_test(cmd_tbl_t * cmdtp, int flag, int argc, char * const a
> printf("\nOneNAND test: ");
>
> /* skip first two or three arguments, look for offset and size */
> - if (arg_off_size(argc - 1, argv + 1, &ofs, &len) != 0)
> + if (arg_off_size_onenand(argc - 1, argv + 1, &ofs, &len) != 0)
> return 1;
>
> ret = onenand_block_test(ofs, len);
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 5467a951..a623f4c 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -5,8 +5,8 @@
> # SPDX-License-Identifier: GPL-2.0+
> #
>
> -ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)))
> -obj-y += mtdcore.o
> +ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)))
> +obj-y += mtdcore.o mtd_uboot.o
> endif
> obj-$(CONFIG_MTD_PARTITIONS) += mtdpart.o
> obj-$(CONFIG_MTD_CONCAT) += mtdconcat.o
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> new file mode 100644
> index 0000000..a70d40a
> --- /dev/null
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -0,0 +1,114 @@
> +/*
> + * (C) Copyright 2014
> + * Heiko Schocher, DENX Software Engineering, hs at denx.de.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#include <common.h>
> +#include <linux/mtd/mtd.h>
> +#include <jffs2/jffs2.h>
> +
> +int str2off(const char *p, loff_t *num)
> +{
> + char *endptr;
> +
> + *num = simple_strtoull(p, &endptr, 16);
> + return *p != '\0' && *endptr == '\0';
> +}
> +
> +int str2long(const char *p, ulong *num)
> +{
> + char *endptr;
> +
> + *num = simple_strtoul(p, &endptr, 16);
> + return *p != '\0' && *endptr == '\0';
> +}
> +
> +static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
> + loff_t *maxsize, int devtype)
> +{
> +#ifdef CONFIG_CMD_MTDPARTS
> + struct mtd_device *dev;
> + struct part_info *part;
> + u8 pnum;
> + int ret;
> +
> + ret = mtdparts_init();
> + if (ret)
> + return ret;
> +
> + ret = find_dev_and_part(partname, &dev, &pnum, &part);
> + if (ret)
> + return ret;
> +
> + if (dev->id->type != devtype) {
> + printf("not same typ %d != %d\n", dev->id->type, devtype);
> + return -1;
> + }
> +
> + *off = part->offset;
> + *size = part->size;
> + *maxsize = part->size;
> + *idx = dev->id->num;
> +
> + return 0;
> +#else
> + puts("offset is not a number\n");
> + return -1;
> +#endif
> +}
> +
> +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
> + loff_t *maxsize, int devtype, int chipsize)
> +{
> + if (!str2off(arg, off))
> + return get_part(arg, idx, off, size, maxsize, devtype);
> +
> + if (*off >= chipsize) {
> + puts("Offset exceeds device limit\n");
> + return -1;
> + }
> +
> + *maxsize = chipsize - *off;
> + *size = *maxsize;
> + return 0;
> +}
> +
> +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
> + loff_t *size, loff_t *maxsize, int devtype, int chipsize)
> +{
> + int ret;
> +
> + if (argc == 0) {
> + *off = 0;
> + *size = chipsize;
> + *maxsize = *size;
> + goto print;
> + }
> +
> + ret = arg_off(argv[0], idx, off, size, maxsize, devtype, chipsize);
> + if (ret)
> + return ret;
> +
> + if (argc == 1)
> + goto print;
> +
> + if (!str2off(argv[1], size)) {
> + printf("'%s' is not a number\n", argv[1]);
> + return -1;
> + }
> +
> + if (*size > *maxsize) {
> + puts("Size exceeds partition or device limit\n");
> + return -1;
> + }
> +
> +print:
> + printf("device %d ", *idx);
> + if (*size == chipsize)
> + puts("whole chip\n");
> + else
> + printf("offset 0x%llx, size 0x%llx\n",
> + (unsigned long long)*off, (unsigned long long)*size);
> + return 0;
> +}
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 8666413..2861af5 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd);
> int del_mtd_device(struct mtd_info *mtd);
> int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
> int del_mtd_partitions(struct mtd_info *);
> +
> +int str2off(const char *p, loff_t *num);
> +int str2long(const char *p, ulong *num);
> +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
> + loff_t *maxsize, int devtype, int chipsize);
> +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
> + loff_t *size, loff_t *maxsize, int devtype, int chipsize);
> #endif
> #endif /* __MTD_MTD_H__ */
> --
> 2.1.0
>
thanks!
--
Jagan.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 3/3] spi, sf: use offset and size in sf cmd from mtdpartition
2015-04-20 5:47 ` [U-Boot] [PATCH v5 3/3] spi, sf: use offset and size in sf cmd from mtdpartition Heiko Schocher
@ 2015-04-22 10:58 ` Jagan Teki
2015-04-23 5:38 ` Heiko Schocher
0 siblings, 1 reply; 16+ messages in thread
From: Jagan Teki @ 2015-04-22 10:58 UTC (permalink / raw)
To: u-boot
On 20 April 2015 at 11:17, Heiko Schocher <hs@denx.de> wrote:
> with this patch, it is possible to get the offset and size information
> from the mtdpartiton setting in "mtdparts", similiar to the
> "nand" commandos.
>
> => sf
> sf - SPI flash sub-system
>
> Usage:
> sf probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus
> and chip select
> sf read addr offset|partition len - read `len' bytes starting at
> `offset' to memory at `addr'
> sf write addr offset|partition len - write `len' bytes from memory
> at `addr' to flash at `offset'
> sf erase offset|partition [+]len - erase `len' bytes from `offset'
> `+len' round up `len' to block size
> sf update addr offset|partition len - erase and write `len' bytes from memory
> at `addr' to flash at `offset'
> =>
> for example "env" is defined in mtdparts:
>
> => sf read 13000000 env
> device 0 offset 0xd0000, size 0x10000
> SF: 65536 bytes @ 0xd0000 Read: OK
> =>
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
>
> ---
>
> Changes in v2:
> - none
> Series-changes: 3
> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
> Series-changes: 4
> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
> Series-changes: 5
> - no changes
>
> common/cmd_sf.c | 50 +++++++++++++++++++++++++-------------------------
> 1 file changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
> index 25a59e5..1932ac9 100644
> --- a/common/cmd_sf.c
> +++ b/common/cmd_sf.c
> @@ -12,6 +12,8 @@
> #include <malloc.h>
> #include <spi.h>
> #include <spi_flash.h>
> +#include <jffs2/jffs2.h>
> +#include <linux/mtd/mtd.h>
>
> #include <asm/io.h>
> #include <dm/device-internal.h>
> @@ -258,23 +260,21 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset,
> static int do_spi_flash_read_write(int argc, char * const argv[])
> {
> unsigned long addr;
> - unsigned long offset;
> - unsigned long len;
> void *buf;
> char *endp;
> int ret = 1;
> + int dev = 0;
> + loff_t offset, len, maxsize;
>
> - if (argc < 4)
> + if (argc < 3)
> return -1;
>
> addr = simple_strtoul(argv[1], &endp, 16);
> if (*argv[1] == 0 || *endp != 0)
> return -1;
> - offset = simple_strtoul(argv[2], &endp, 16);
> - if (*argv[2] == 0 || *endp != 0)
> - return -1;
> - len = simple_strtoul(argv[3], &endp, 16);
> - if (*argv[3] == 0 || *endp != 0)
> +
> + if (arg_off_size(argc - 2, &argv[2], &dev, &offset, &len, &maxsize,
> + MTD_DEV_TYPE_NOR, flash->size))
> return -1;
>
> /* Consistency checking */
> @@ -313,31 +313,31 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
>
> static int do_spi_flash_erase(int argc, char * const argv[])
> {
> - unsigned long offset;
> - unsigned long len;
> - char *endp;
> int ret;
> + int dev = 0;
> + loff_t offset, len, maxsize;
> + ulong size;
>
> if (argc < 3)
> return -1;
>
> - offset = simple_strtoul(argv[1], &endp, 16);
> - if (*argv[1] == 0 || *endp != 0)
> + if (arg_off(argv[1], &dev, &offset, &len, &maxsize,
> + MTD_DEV_TYPE_NOR, flash->size))
> return -1;
>
> - ret = sf_parse_len_arg(argv[2], &len);
> + ret = sf_parse_len_arg(argv[2], &size);
> if (ret != 1)
> return -1;
>
> /* Consistency checking */
> - if (offset + len > flash->size) {
> + if (offset + size > flash->size) {
> printf("ERROR: attempting %s past flash size (%#x)\n",
> argv[0], flash->size);
> return 1;
> }
>
> - ret = spi_flash_erase(flash, offset, len);
> - printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)len, (u32)offset,
> + ret = spi_flash_erase(flash, offset, size);
> + printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)size, (u32)offset,
> ret ? "ERROR" : "OK");
>
> return ret == 0 ? 0 : 1;
> @@ -562,13 +562,13 @@ U_BOOT_CMD(
> "SPI flash sub-system",
> "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n"
> " and chip select\n"
> - "sf read addr offset len - read `len' bytes starting at\n"
> - " `offset' to memory at `addr'\n"
> - "sf write addr offset len - write `len' bytes from memory\n"
> - " at `addr' to flash at `offset'\n"
> - "sf erase offset [+]len - erase `len' bytes from `offset'\n"
> - " `+len' round up `len' to block size\n"
> - "sf update addr offset len - erase and write `len' bytes from memory\n"
> - " at `addr' to flash at `offset'"
> + "sf read addr offset|partition len - read `len' bytes starting at\n"
> + " `offset' to memory at `addr'\n"
Append partition on help text as well more readable.
> + "sf write addr offset|partition len - write `len' bytes from memory\n"
> + " at `addr' to flash at `offset'\n"
> + "sf erase offset|partition [+]len - erase `len' bytes from `offset'\n"
> + " `+len' round up `len' to block size\n"
> + "sf update addr offset|partition len - erase and write `len' bytes from memory\n"
> + " at `addr' to flash at `offset'"
> SF_TEST_HELP
> );
> --
> 2.1.0
>
Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
thanks!
--
Jagan.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-20 5:47 ` [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
2015-04-22 10:53 ` Jagan Teki
@ 2015-04-22 22:47 ` Scott Wood
2015-04-23 5:57 ` Heiko Schocher
1 sibling, 1 reply; 16+ messages in thread
From: Scott Wood @ 2015-04-22 22:47 UTC (permalink / raw)
To: u-boot
On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:
> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
> goto usage;
>
> /* We don't care about size, or maxsize. */
> - if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
> + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
> + MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
> + puts("Offset or partition name expected\n");
> + return 1;
> + }
Use only one tab per indentation level.
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 8666413..2861af5 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd);
> int del_mtd_device(struct mtd_info *mtd);
> int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
> int del_mtd_partitions(struct mtd_info *);
> +
> +int str2off(const char *p, loff_t *num);
> +int str2long(const char *p, ulong *num);
These should be moved somewhere more generic, especially if they're no
longer file-local.
> +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
> + loff_t *maxsize, int devtype, int chipsize);
> +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
> + loff_t *size, loff_t *maxsize, int devtype, int chipsize);
Add an mtd prefix.
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver
2015-04-22 10:52 ` Jagan Teki
@ 2015-04-23 5:27 ` Heiko Schocher
0 siblings, 0 replies; 16+ messages in thread
From: Heiko Schocher @ 2015-04-23 5:27 UTC (permalink / raw)
To: u-boot
Hello Jagan,
Am 22.04.2015 12:52, schrieb Jagan Teki:
> On 20 April 2015 at 11:17, Heiko Schocher <hs@denx.de> wrote:
>> From: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>>
>> add MTD layer driver for spi, original patch from:
>> http://git.denx.de/?p=u-boot/u-boot-mips.git;a=commitdiff;h=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 v2:
>> - add comment from Daniel Schwierzeck:
>> fix compile error from original patch with
>> "static inline" rather than "static __maybe_unused"
>> Series-changes: 3
>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>> Series-changes: 4
>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>> Series-changes: 5
>> - no changes
>>
>> README | 3 ++
>> common/cmd_sf.c | 7 ++-
>> drivers/mtd/spi/Makefile | 1 +
>> drivers/mtd/spi/sf_internal.h | 13 ++++++
>> drivers/mtd/spi/sf_mtd.c | 104 ++++++++++++++++++++++++++++++++++++++++++
>> drivers/mtd/spi/sf_probe.c | 5 ++
>> 6 files changed, 131 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/mtd/spi/sf_mtd.c
>>
>> diff --git a/README b/README
>> index 9b748cc..56dcfd2 100644
>> --- a/README
>> +++ b/README
>> @@ -3077,6 +3077,9 @@ CBFS (Coreboot Filesystem) support
>> operation will not execute. The only way to exit this
>> hardware-protected mode is to drive W#/VPP HIGH.
>>
>> + CONFIG_SPI_FLASH_MTD
>> + add MTD translation layer driver.
>> +
>> - SystemACE Support:
>> CONFIG_SYSTEMACE
>>
>> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
>> index 5c788e9..25a59e5 100644
>> --- a/common/cmd_sf.c
>> +++ b/common/cmd_sf.c
>> @@ -132,14 +132,17 @@ static int do_spi_flash_probe(int argc, char * const argv[])
>>
>> flash = new->uclass_priv;
>> #else
>> + if (flash)
>> + spi_flash_free(flash);
>> +
>> new = spi_flash_probe(bus, cs, speed, mode);
>> + flash = new;
>> +
>
> This could be really a another patch - as it's not related to mtd.
Yes, done.
>> if (!new) {
>> printf("Failed to initialize SPI flash at %u:%u\n", bus, cs);
>> return 1;
>> }
>>
>> - if (flash)
>> - spi_flash_free(flash);
>> flash = new;
>> #endif
>>
>> diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
>> index c61b784e..f8580cd 100644
>> --- a/drivers/mtd/spi/Makefile
>> +++ b/drivers/mtd/spi/Makefile
>> @@ -17,5 +17,6 @@ obj-$(CONFIG_SPI_FLASH) += sf_probe.o
>> #endif
>> obj-$(CONFIG_CMD_SF) += sf.o
>> obj-$(CONFIG_SPI_FLASH) += sf_ops.o sf_params.o
>> +obj-$(CONFIG_SPI_FLASH_MTD) += sf_mtd.o
>> obj-$(CONFIG_SPI_FLASH_SANDBOX) += sandbox.o
>> obj-$(CONFIG_SPI_M95XXX) += eeprom_m95xxx.o
>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index 785f7a9..dc75f43 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -221,4 +221,17 @@ int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
>> int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>> size_t len, void *data);
>>
>> +#ifdef CONFIG_SPI_FLASH_MTD
>> +int spi_flash_mtd_register(struct spi_flash *flash);
>> +void spi_flash_mtd_unregister(void);
>> +#else
>> +static inline int spi_flash_mtd_register(struct spi_flash *flash)
>> +{
>> + return 0;
>> +}
>> +static inline void spi_flash_mtd_unregister(void)
>> +{
>> +}
>> +#endif
>
>>From the previous comments to reply - https://patchwork.ozlabs.org/patch/386102/
>
> Adding #def isn't a polluting thing in code size contains software's like u-boot
> and did the same for many of my code.
>
> So please do as below
>
> #ifdef CONFIG_SPI_FLASH_MTD
> int spi_flash_mtd_register(struct spi_flash *flash);
> void spi_flash_mtd_unregister(void);
> #endif
>
> void spi_flash_free(struct spi_flash *flash)
> {
> #ifdef CONFIG_SPI_FLASH_MTD
> spi_flash_mtd_unregister();
> #endif
> }
>
> #ifdef CONFIG_SPI_FLASH_MTD
> ret = spi_flash_mtd_register(flash);
> if (ret)
> goto err_claim_bus;
> #endif
really? ... I thought we want to prevent #ifdefs in Code ...
Changed...
>> +
>> #endif /* _SF_INTERNAL_H_ */
>> diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
>> new file mode 100644
>> index 0000000..0b9cb62
>> --- /dev/null
>> +++ b/drivers/mtd/spi/sf_mtd.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Copyright (C) 2012-2014 Daniel Schwierzeck, daniel.schwierzeck 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 4103723..93209fe 100644
>> --- a/drivers/mtd/spi/sf_probe.c
>> +++ b/drivers/mtd/spi/sf_probe.c
>> @@ -399,6 +399,10 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct spi_flash *flash)
>> /* Release spi bus */
>> spi_release_bus(spi);
>>
>> + ret = spi_flash_mtd_register(flash);
>> + if (ret)
>> + return ret;
>
> mtd register call is before the spi_release_bus isn't it?
Yep, fixed.
>> +
>> return 0;
>>
>> err_read_id:
>> @@ -449,6 +453,7 @@ struct spi_flash *spi_flash_probe_fdt(const void *blob, int slave_node,
>>
>> void spi_flash_free(struct spi_flash *flash)
>> {
>> + spi_flash_mtd_unregister();
>> spi_free_slave(flash->spi);
>> free(flash);
>> }
>> --
>> 2.1.0
>>
>
> thanks!
Thanks for the review
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 3/3] spi, sf: use offset and size in sf cmd from mtdpartition
2015-04-22 10:58 ` Jagan Teki
@ 2015-04-23 5:38 ` Heiko Schocher
0 siblings, 0 replies; 16+ messages in thread
From: Heiko Schocher @ 2015-04-23 5:38 UTC (permalink / raw)
To: u-boot
Hello Jagan,
Am 22.04.2015 12:58, schrieb Jagan Teki:
> On 20 April 2015 at 11:17, Heiko Schocher <hs@denx.de> wrote:
>> with this patch, it is possible to get the offset and size information
>> from the mtdpartiton setting in "mtdparts", similiar to the
>> "nand" commandos.
>>
>> => sf
>> sf - SPI flash sub-system
>>
>> Usage:
>> sf probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus
>> and chip select
>> sf read addr offset|partition len - read `len' bytes starting at
>> `offset' to memory at `addr'
>> sf write addr offset|partition len - write `len' bytes from memory
>> at `addr' to flash at `offset'
>> sf erase offset|partition [+]len - erase `len' bytes from `offset'
>> `+len' round up `len' to block size
>> sf update addr offset|partition len - erase and write `len' bytes from memory
>> at `addr' to flash at `offset'
>> =>
>> for example "env" is defined in mtdparts:
>>
>> => sf read 13000000 env
>> device 0 offset 0xd0000, size 0x10000
>> SF: 65536 bytes @ 0xd0000 Read: OK
>> =>
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>>
>> ---
>>
>> Changes in v2:
>> - none
>> Series-changes: 3
>> - rebase with d6c1ffc7d23f4fe4ae8c91101861055b8e1501b6
>> Series-changes: 4
>> - rebased against 385a08a60f042061b004642d6b9bb6cfb794ad5a
>> Series-changes: 5
>> - no changes
>>
>> common/cmd_sf.c | 50 +++++++++++++++++++++++++-------------------------
>> 1 file changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/common/cmd_sf.c b/common/cmd_sf.c
>> index 25a59e5..1932ac9 100644
>> --- a/common/cmd_sf.c
>> +++ b/common/cmd_sf.c
>> @@ -12,6 +12,8 @@
>> #include <malloc.h>
>> #include <spi.h>
>> #include <spi_flash.h>
>> +#include <jffs2/jffs2.h>
>> +#include <linux/mtd/mtd.h>
>>
>> #include <asm/io.h>
>> #include <dm/device-internal.h>
>> @@ -258,23 +260,21 @@ static int spi_flash_update(struct spi_flash *flash, u32 offset,
>> static int do_spi_flash_read_write(int argc, char * const argv[])
>> {
>> unsigned long addr;
>> - unsigned long offset;
>> - unsigned long len;
>> void *buf;
>> char *endp;
>> int ret = 1;
>> + int dev = 0;
>> + loff_t offset, len, maxsize;
>>
>> - if (argc < 4)
>> + if (argc < 3)
>> return -1;
>>
>> addr = simple_strtoul(argv[1], &endp, 16);
>> if (*argv[1] == 0 || *endp != 0)
>> return -1;
>> - offset = simple_strtoul(argv[2], &endp, 16);
>> - if (*argv[2] == 0 || *endp != 0)
>> - return -1;
>> - len = simple_strtoul(argv[3], &endp, 16);
>> - if (*argv[3] == 0 || *endp != 0)
>> +
>> + if (arg_off_size(argc - 2, &argv[2], &dev, &offset, &len, &maxsize,
>> + MTD_DEV_TYPE_NOR, flash->size))
>> return -1;
>>
>> /* Consistency checking */
>> @@ -313,31 +313,31 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
>>
>> static int do_spi_flash_erase(int argc, char * const argv[])
>> {
>> - unsigned long offset;
>> - unsigned long len;
>> - char *endp;
>> int ret;
>> + int dev = 0;
>> + loff_t offset, len, maxsize;
>> + ulong size;
>>
>> if (argc < 3)
>> return -1;
>>
>> - offset = simple_strtoul(argv[1], &endp, 16);
>> - if (*argv[1] == 0 || *endp != 0)
>> + if (arg_off(argv[1], &dev, &offset, &len, &maxsize,
>> + MTD_DEV_TYPE_NOR, flash->size))
>> return -1;
>>
>> - ret = sf_parse_len_arg(argv[2], &len);
>> + ret = sf_parse_len_arg(argv[2], &size);
>> if (ret != 1)
>> return -1;
>>
>> /* Consistency checking */
>> - if (offset + len > flash->size) {
>> + if (offset + size > flash->size) {
>> printf("ERROR: attempting %s past flash size (%#x)\n",
>> argv[0], flash->size);
>> return 1;
>> }
>>
>> - ret = spi_flash_erase(flash, offset, len);
>> - printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)len, (u32)offset,
>> + ret = spi_flash_erase(flash, offset, size);
>> + printf("SF: %zu bytes @ %#x Erased: %s\n", (size_t)size, (u32)offset,
>> ret ? "ERROR" : "OK");
>>
>> return ret == 0 ? 0 : 1;
>> @@ -562,13 +562,13 @@ U_BOOT_CMD(
>> "SPI flash sub-system",
>> "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n"
>> " and chip select\n"
>> - "sf read addr offset len - read `len' bytes starting at\n"
>> - " `offset' to memory at `addr'\n"
>> - "sf write addr offset len - write `len' bytes from memory\n"
>> - " at `addr' to flash at `offset'\n"
>> - "sf erase offset [+]len - erase `len' bytes from `offset'\n"
>> - " `+len' round up `len' to block size\n"
>> - "sf update addr offset len - erase and write `len' bytes from memory\n"
>> - " at `addr' to flash at `offset'"
>> + "sf read addr offset|partition len - read `len' bytes starting at\n"
>> + " `offset' to memory at `addr'\n"
>
> Append partition on help text as well more readable.
Added.
>> + "sf write addr offset|partition len - write `len' bytes from memory\n"
>> + " at `addr' to flash at `offset'\n"
>> + "sf erase offset|partition [+]len - erase `len' bytes from `offset'\n"
>> + " `+len' round up `len' to block size\n"
>> + "sf update addr offset|partition len - erase and write `len' bytes from memory\n"
>> + " at `addr' to flash at `offset'"
>> SF_TEST_HELP
>> );
>> --
>> 2.1.0
>>
>
> Reviewed-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com>
>
> thanks!
Thanks for your time!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-22 22:47 ` Scott Wood
@ 2015-04-23 5:57 ` Heiko Schocher
2015-04-23 6:55 ` Scott Wood
0 siblings, 1 reply; 16+ messages in thread
From: Heiko Schocher @ 2015-04-23 5:57 UTC (permalink / raw)
To: u-boot
Hello Scott,
Am 23.04.2015 00:47, schrieb Scott Wood:
> On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:
>> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
>> goto usage;
>>
>> /* We don't care about size, or maxsize. */
>> - if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
>> + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
>> + MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>> + puts("Offset or partition name expected\n");
>> + return 1;
>> + }
>
> Use only one tab per indentation level.
You mean the line "MTD_DEV_TYPE_NAND, nand_info[idx].size)) {" ?
I have to move MTD_xx to the opening bracket to avoid checkpatch.pl
errors ...
If you mean the "puts ..." line ... fixed.
>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
>> index 8666413..2861af5 100644
>> --- a/include/linux/mtd/mtd.h
>> +++ b/include/linux/mtd/mtd.h
>> @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd);
>> int del_mtd_device(struct mtd_info *mtd);
>> int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
>> int del_mtd_partitions(struct mtd_info *);
>> +
>> +int str2off(const char *p, loff_t *num);
>> +int str2long(const char *p, ulong *num);
>
> These should be moved somewhere more generic, especially if they're no
> longer file-local.
Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe
we add a "mtd_" prefix to them? I think these functions are mtd specific ...
>> +int arg_off(const char *arg, int *idx, loff_t *off, loff_t *size,
>> + loff_t *maxsize, int devtype, int chipsize);
>> +int arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
>> + loff_t *size, loff_t *maxsize, int devtype, int chipsize);
>
> Add an mtd prefix.
Done.
Thanks for your time!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-23 5:57 ` Heiko Schocher
@ 2015-04-23 6:55 ` Scott Wood
2015-04-23 11:12 ` Heiko Schocher
0 siblings, 1 reply; 16+ messages in thread
From: Scott Wood @ 2015-04-23 6:55 UTC (permalink / raw)
To: u-boot
On Thu, 2015-04-23 at 07:57 +0200, Heiko Schocher wrote:
> Hello Scott,
>
> Am 23.04.2015 00:47, schrieb Scott Wood:
> > On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:
> >> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
> >> goto usage;
> >>
> >> /* We don't care about size, or maxsize. */
> >> - if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
> >> + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
> >> + MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
> >> + puts("Offset or partition name expected\n");
> >> + return 1;
> >> + }
> >
> > Use only one tab per indentation level.
>
> You mean the line "MTD_DEV_TYPE_NAND, nand_info[idx].size)) {" ?
>
> I have to move MTD_xx to the opening bracket to avoid checkpatch.pl
> errors ...
>
> If you mean the "puts ..." line ... fixed.
I meant the puts and return lines.
> >> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> >> index 8666413..2861af5 100644
> >> --- a/include/linux/mtd/mtd.h
> >> +++ b/include/linux/mtd/mtd.h
> >> @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd);
> >> int del_mtd_device(struct mtd_info *mtd);
> >> int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
> >> int del_mtd_partitions(struct mtd_info *);
> >> +
> >> +int str2off(const char *p, loff_t *num);
> >> +int str2long(const char *p, ulong *num);
> >
> > These should be moved somewhere more generic, especially if they're no
> > longer file-local.
>
> Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe
> we add a "mtd_" prefix to them? I think these functions are mtd specific ...
What is mtd-specific about them?
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-23 6:55 ` Scott Wood
@ 2015-04-23 11:12 ` Heiko Schocher
2015-04-23 17:48 ` Scott Wood
0 siblings, 1 reply; 16+ messages in thread
From: Heiko Schocher @ 2015-04-23 11:12 UTC (permalink / raw)
To: u-boot
Hello Scott,
Am 23.04.2015 08:55, schrieb Scott Wood:
> On Thu, 2015-04-23 at 07:57 +0200, Heiko Schocher wrote:
>> Hello Scott,
>>
>> Am 23.04.2015 00:47, schrieb Scott Wood:
>>> On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:
>>>> @@ -322,7 +213,12 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
>>>> goto usage;
>>>>
>>>> /* We don't care about size, or maxsize. */
>>>> - if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
>>>> + if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize,
>>>> + MTD_DEV_TYPE_NAND, nand_info[idx].size)) {
>>>> + puts("Offset or partition name expected\n");
>>>> + return 1;
>>>> + }
>>>
>>> Use only one tab per indentation level.
>>
>> You mean the line "MTD_DEV_TYPE_NAND, nand_info[idx].size)) {" ?
>>
>> I have to move MTD_xx to the opening bracket to avoid checkpatch.pl
>> errors ...
>>
>> If you mean the "puts ..." line ... fixed.
>
> I meant the puts and return lines.
>
>>>> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
>>>> index 8666413..2861af5 100644
>>>> --- a/include/linux/mtd/mtd.h
>>>> +++ b/include/linux/mtd/mtd.h
>>>> @@ -482,5 +482,12 @@ int add_mtd_device(struct mtd_info *mtd);
>>>> int del_mtd_device(struct mtd_info *mtd);
>>>> int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
>>>> int del_mtd_partitions(struct mtd_info *);
>>>> +
>>>> +int str2off(const char *p, loff_t *num);
>>>> +int str2long(const char *p, ulong *num);
>>>
>>> These should be moved somewhere more generic, especially if they're no
>>> longer file-local.
>>
>> Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe
>> we add a "mtd_" prefix to them? I think these functions are mtd specific ...
>
> What is mtd-specific about them?
Hmm... I thought:
return *p != '\0' && *endptr == '\0';
is more or less mtd specific ... but you are right, it is not really
mtd specific ... so I move them to "./lib/vsprintf.c" ... Ok?
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-23 11:12 ` Heiko Schocher
@ 2015-04-23 17:48 ` Scott Wood
2015-04-24 4:59 ` Heiko Schocher
0 siblings, 1 reply; 16+ messages in thread
From: Scott Wood @ 2015-04-23 17:48 UTC (permalink / raw)
To: u-boot
On Thu, 2015-04-23 at 13:12 +0200, Heiko Schocher wrote:
> Hello Scott,
>
> Am 23.04.2015 08:55, schrieb Scott Wood:
> > On Thu, 2015-04-23 at 07:57 +0200, Heiko Schocher wrote:
> >> Hello Scott,
> >>
> >> Am 23.04.2015 00:47, schrieb Scott Wood:
> >>> On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:
> >>>> +int str2off(const char *p, loff_t *num);
> >>>> +int str2long(const char *p, ulong *num);
> >>>
> >>> These should be moved somewhere more generic, especially if they're no
> >>> longer file-local.
> >>
> >> Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe
> >> we add a "mtd_" prefix to them? I think these functions are mtd specific ...
> >
> > What is mtd-specific about them?
>
> Hmm... I thought:
>
> return *p != '\0' && *endptr == '\0';
>
> is more or less mtd specific ... but you are right, it is not really
> mtd specific ... so I move them to "./lib/vsprintf.c" ... Ok?
OK. Maybe change the return to bool while you're at it, to make it
clear that it isn't return-zero-on-success.
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-23 17:48 ` Scott Wood
@ 2015-04-24 4:59 ` Heiko Schocher
2015-04-24 5:25 ` Scott Wood
0 siblings, 1 reply; 16+ messages in thread
From: Heiko Schocher @ 2015-04-24 4:59 UTC (permalink / raw)
To: u-boot
Hello Scott,
Am 23.04.2015 19:48, schrieb Scott Wood:
> On Thu, 2015-04-23 at 13:12 +0200, Heiko Schocher wrote:
>> Hello Scott,
>>
>> Am 23.04.2015 08:55, schrieb Scott Wood:
>>> On Thu, 2015-04-23 at 07:57 +0200, Heiko Schocher wrote:
>>>> Hello Scott,
>>>>
>>>> Am 23.04.2015 00:47, schrieb Scott Wood:
>>>>> On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:
>>>>>> +int str2off(const char *p, loff_t *num);
>>>>>> +int str2long(const char *p, ulong *num);
>>>>>
>>>>> These should be moved somewhere more generic, especially if they're no
>>>>> longer file-local.
>>>>
>>>> Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe
>>>> we add a "mtd_" prefix to them? I think these functions are mtd specific ...
>>>
>>> What is mtd-specific about them?
>>
>> Hmm... I thought:
>>
>> return *p != '\0' && *endptr == '\0';
>>
>> is more or less mtd specific ... but you are right, it is not really
>> mtd specific ... so I move them to "./lib/vsprintf.c" ... Ok?
>
> OK. Maybe change the return to bool while you're at it, to make it
> clear that it isn't return-zero-on-success.
Hmm.. tried this, but I get:
CC common/cmd_test.o
In file included from /home/hs/abb/imx6/u-boot/include/common.h:760:0,
from /home/hs/abb/imx6/u-boot/common/cmd_test.c:17:
/home/hs/abb/imx6/u-boot/include/vsprintf.h:176:1: error: unknown type name 'bool'
/home/hs/abb/imx6/u-boot/include/vsprintf.h:177:1: error: unknown type name 'bool'
/home/hs/abb/imx6/u-boot/scripts/Makefile.build:276: recipe for target 'common/cmd_test.o' failed
make[2]: *** [common/cmd_test.o] Error 1
/home/hs/abb/imx6/u-boot/Makefile:1156: recipe for target 'common' failed
make[1]: *** [common] Error 2
reason is in common/cmd_test.c:
/*
* 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>
Hmm... I tend to say, this is another patch changing the returntype
from int to bool ...
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place
2015-04-24 4:59 ` Heiko Schocher
@ 2015-04-24 5:25 ` Scott Wood
0 siblings, 0 replies; 16+ messages in thread
From: Scott Wood @ 2015-04-24 5:25 UTC (permalink / raw)
To: u-boot
On Fri, 2015-04-24 at 06:59 +0200, Heiko Schocher wrote:
> Hello Scott,
>
> Am 23.04.2015 19:48, schrieb Scott Wood:
> > On Thu, 2015-04-23 at 13:12 +0200, Heiko Schocher wrote:
> >> Hello Scott,
> >>
> >> Am 23.04.2015 08:55, schrieb Scott Wood:
> >>> On Thu, 2015-04-23 at 07:57 +0200, Heiko Schocher wrote:
> >>>> Hello Scott,
> >>>>
> >>>> Am 23.04.2015 00:47, schrieb Scott Wood:
> >>>>> On Mon, 2015-04-20 at 07:47 +0200, Heiko Schocher wrote:
> >>>>>> +int str2off(const char *p, loff_t *num);
> >>>>>> +int str2long(const char *p, ulong *num);
> >>>>>
> >>>>> These should be moved somewhere more generic, especially if they're no
> >>>>> longer file-local.
> >>>>
> >>>> Hmm... the code is currently in "drivers/mtd/mtd_uboot.c" ... maybe
> >>>> we add a "mtd_" prefix to them? I think these functions are mtd specific ...
> >>>
> >>> What is mtd-specific about them?
> >>
> >> Hmm... I thought:
> >>
> >> return *p != '\0' && *endptr == '\0';
> >>
> >> is more or less mtd specific ... but you are right, it is not really
> >> mtd specific ... so I move them to "./lib/vsprintf.c" ... Ok?
> >
> > OK. Maybe change the return to bool while you're at it, to make it
> > clear that it isn't return-zero-on-success.
>
> Hmm.. tried this, but I get:
>
> CC common/cmd_test.o
> In file included from /home/hs/abb/imx6/u-boot/include/common.h:760:0,
> from /home/hs/abb/imx6/u-boot/common/cmd_test.c:17:
> /home/hs/abb/imx6/u-boot/include/vsprintf.h:176:1: error: unknown type name 'bool'
> /home/hs/abb/imx6/u-boot/include/vsprintf.h:177:1: error: unknown type name 'bool'
> /home/hs/abb/imx6/u-boot/scripts/Makefile.build:276: recipe for target 'common/cmd_test.o' failed
> make[2]: *** [common/cmd_test.o] Error 1
> /home/hs/abb/imx6/u-boot/Makefile:1156: recipe for target 'common' failed
> make[1]: *** [common] Error 2
>
> reason is in common/cmd_test.c:
>
> /*
> * 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
Ugh. Maybe add a variant of U_BOOT_CMD_COMPLETE that takes a string for
the user-visible name that is separate from the C-visible symbol used
for the ll entry.
Or you could either define bool manually, or do what the comment says
and undef true/false.
> #include <common.h>
>
> Hmm... I tend to say, this is another patch changing the returntype
> from int to bool ...
It's related because you're moving it from being a local static function
to being an API exposed treewide, so higher standards apply.
Another option would be to convert it to returning zero on success and a
negative error code on error.
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-04-24 5:25 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-20 5:47 [U-Boot] [PATCH v5 0/3] spi, sf: add mtdparts feature to spi and sf commands Heiko Schocher
2015-04-20 5:47 ` [U-Boot] [PATCH v5 1/3] mtd, spi: add MTD layer driver Heiko Schocher
2015-04-22 10:52 ` Jagan Teki
2015-04-23 5:27 ` Heiko Schocher
2015-04-20 5:47 ` [U-Boot] [PATCH v5 2/3] mtd, nand: move common functions from cmd_nand.c to common place Heiko Schocher
2015-04-22 10:53 ` Jagan Teki
2015-04-22 22:47 ` Scott Wood
2015-04-23 5:57 ` Heiko Schocher
2015-04-23 6:55 ` Scott Wood
2015-04-23 11:12 ` Heiko Schocher
2015-04-23 17:48 ` Scott Wood
2015-04-24 4:59 ` Heiko Schocher
2015-04-24 5:25 ` Scott Wood
2015-04-20 5:47 ` [U-Boot] [PATCH v5 3/3] spi, sf: use offset and size in sf cmd from mtdpartition Heiko Schocher
2015-04-22 10:58 ` Jagan Teki
2015-04-23 5:38 ` Heiko Schocher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.