* [U-Boot] [PATCH v3 0/4] Add NAND support to DFU, enable for am335x_evm
@ 2013-02-28 19:09 Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters Tom Rini
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Tom Rini @ 2013-02-28 19:09 UTC (permalink / raw)
To: u-boot
This series adds DFU support to NAND and was started by Pantelis. The
NAND changes have been compile-tested on all ARM and PowerPC targets and
run-time tested on ARM. DFU itself has been tested, for NAND, on
am335x_evm.
For practical reasons, this series depends on Pantelis' previous series
of generic DFU changes. Lukasz and I are discussing how to handle a few
changes there since one of them breaks file writing.
--
Tom
Changes in v3:
- Reworked skip_check_len changes to just add accounting for *used to
the logic.
- Allow for actual to be NULL in nand_(read|write)_skip_bad, only DFU
calls this with a non-NULL parameter. Make sure the comments for both
functions explain the parameters and their behavior.
- Other style changes requested by Scott.
- As nand_(write|read)_skip_bad passes back just a used length now.
- Rework logic in nand_block_op for nand_(read|write)_skip_bad returning
just a size for actual used length.
- Remove unused externs from drivers/dfu/dfu_nand.c
- Fix checkpatch.pl warnings in include/configs/am335x_evm.h
Changes in v2:
- NAND skip_check_len changes reworked to allow
nand_(read|write)_skip_bad to return this information to the caller.
- nand_block_op calls nand_(read|write)_skip_bad directly.
- Bugfix in dfu_nand to make sure we set dfu->skip_bad to 0 on each
iteration.
- Add CONFIG_CMD_MTDPARTS and relevant information to am335x_evm
- Enable DFU for NAND and MMC, set dfu_alt_info_(nand|mmc) as examples
for both in am335x_evm.h
- Increase CONFIG_SYS_MAXARGS due to hush parsing bugs that would
otherwise prevent 'setenv dfu_alt_info ${dfu_alt_info_nand}' on
am335x_evm
Pantelis Antoniou (2):
dfu: NAND specific routines for DFU operation
am335x_evm: Enable DFU for NAND and MMC, provide example alt_info for
both
Tom Rini (2):
nand: Extend nand_(read|write)_skip_bad with *actual and limit
parameters
am335x_evm: Add CONFIG_CMD_MTDPARTS and relevant defaults
common/cmd_nand.c | 56 ++++++------
common/env_nand.c | 3 +-
drivers/dfu/Makefile | 1 +
drivers/dfu/dfu.c | 8 ++
drivers/dfu/dfu_nand.c | 195 ++++++++++++++++++++++++++++++++++++++++++
drivers/mtd/nand/nand_util.c | 67 +++++++++++++--
include/configs/am335x_evm.h | 47 +++++++++-
include/dfu.h | 23 +++++
include/nand.h | 4 +-
9 files changed, 365 insertions(+), 39 deletions(-)
create mode 100644 drivers/dfu/dfu_nand.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
2013-02-28 19:09 [U-Boot] [PATCH v3 0/4] Add NAND support to DFU, enable for am335x_evm Tom Rini
@ 2013-02-28 19:09 ` Tom Rini
2013-03-01 1:37 ` Scott Wood
2013-02-28 19:09 ` [U-Boot] [PATCH v3 2/4] dfu: NAND specific routines for DFU operation Tom Rini
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Tom Rini @ 2013-02-28 19:09 UTC (permalink / raw)
To: u-boot
We make these two functions take a size_t pointer to how much space
was used on NAND to read or write the buffer (when reads/writes happen)
so that bad blocks can be accounted for. We also make them take an
loff_t limit on how much data can be read or written. This means that
we can now catch the case of when writing to a partition would exceed
the partition size due to bad blocks. To do this we also need to make
check_skip_len count not just complete blocks used but partial ones as
well. All callers of nand_(read|write)_skip_bad are adjusted to call
these with the most sensible limits available.
The changes were started by Pantelis and finished by Tom.
Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Tom Rini <trini@ti.com>
---
Changes in v3:
- Reworked skip_check_len changes to just add accounting for *used to
the logic.
- Allow for actual to be NULL in nand_(read|write)_skip_bad, only DFU
calls this with a non-NULL parameter. Make sure the comments for both
functions explain the parameters and their behavior.
- Other style changes requested by Scott.
- As nand_(write|read)_skip_bad passes back just a used length now.
Changes in v2:
- NAND skip_check_len changes reworked to allow
nand_(read|write)_skip_bad to return this information to the caller.
common/cmd_nand.c | 56 +++++++++++++++++++----------------
common/env_nand.c | 3 +-
drivers/mtd/nand/nand_util.c | 67 +++++++++++++++++++++++++++++++++++++-----
include/nand.h | 4 +--
4 files changed, 93 insertions(+), 37 deletions(-)
diff --git a/common/cmd_nand.c b/common/cmd_nand.c
index 495610c..0b07b8e 100644
--- a/common/cmd_nand.c
+++ b/common/cmd_nand.c
@@ -137,7 +137,8 @@ static inline int str2long(const char *p, ulong *num)
return *p != '\0' && *endptr == '\0';
}
-static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size)
+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;
@@ -160,6 +161,7 @@ static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size)
*off = part->offset;
*size = part->size;
+ *maxsize = part->size;
*idx = dev->id->num;
ret = set_dev(*idx);
@@ -173,10 +175,11 @@ static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size)
#endif
}
-static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *maxsize)
+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, maxsize);
+ return get_part(arg, idx, off, size, maxsize);
if (*off >= nand_info[*idx].size) {
puts("Offset exceeds device limit\n");
@@ -184,40 +187,34 @@ static int arg_off(const char *arg, int *idx, loff_t *off, loff_t *maxsize)
}
*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 *off, loff_t *size, loff_t *maxsize)
{
int ret;
- loff_t maxsize = 0;
if (argc == 0) {
*off = 0;
*size = nand_info[*idx].size;
+ *maxsize = *size;
goto print;
}
- ret = arg_off(argv[0], idx, off, &maxsize);
+ ret = arg_off(argv[0], idx, off, size, maxsize);
if (ret)
return ret;
- if (argc == 1) {
- *size = maxsize;
+ 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)
@@ -307,7 +304,8 @@ int do_nand_env_oob(cmd_tbl_t *cmdtp, int argc, char *const argv[])
if (argc < 3)
goto usage;
- if (arg_off(argv[2], &idx, &addr, &maxsize)) {
+ /* We don't care about size, or maxsize. */
+ if (arg_off(argv[2], &idx, &addr, &maxsize, &maxsize)) {
puts("Offset or partition name expected\n");
return 1;
}
@@ -432,7 +430,7 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
{
int i, ret = 0;
ulong addr;
- loff_t off, size;
+ loff_t off, size, maxsize;
char *cmd, *s;
nand_info_t *nand;
#ifdef CONFIG_SYS_NAND_QUIET
@@ -557,7 +555,8 @@ 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) != 0)
+ if (arg_off_size(argc - o, argv + o, &dev, &off, &size,
+ &maxsize) != 0)
return 1;
nand = &nand_info[dev];
@@ -625,7 +624,7 @@ 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))
+ if (arg_off(argv[3], &dev, &off, &size, &maxsize))
return 1;
if (argc > 4 && !str2long(argv[4], &pagecount)) {
@@ -641,7 +640,7 @@ 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) != 0)
+ &off, &size, &maxsize) != 0)
return 1;
rwsize = size;
@@ -651,9 +650,11 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
!strcmp(s, ".e") || !strcmp(s, ".i")) {
if (read)
ret = nand_read_skip_bad(nand, off, &rwsize,
+ NULL, maxsize,
(u_char *)addr);
else
ret = nand_write_skip_bad(nand, off, &rwsize,
+ NULL, maxsize,
(u_char *)addr, 0);
#ifdef CONFIG_CMD_NAND_TRIMFFS
} else if (!strcmp(s, ".trimffs")) {
@@ -661,8 +662,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
printf("Unknown nand command suffix '%s'\n", s);
return 1;
}
- ret = nand_write_skip_bad(nand, off, &rwsize,
- (u_char *)addr,
+ ret = nand_write_skip_bad(nand, off, &rwsize, NULL,
+ maxsize, (u_char *)addr,
WITH_DROP_FFS);
#endif
#ifdef CONFIG_CMD_NAND_YAFFS
@@ -671,8 +672,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
printf("Unknown nand command suffix '%s'.\n", s);
return 1;
}
- ret = nand_write_skip_bad(nand, off, &rwsize,
- (u_char *)addr,
+ ret = nand_write_skip_bad(nand, off, &rwsize, NULL,
+ maxsize, (u_char *)addr,
WITH_INLINE_OOB);
#endif
} else if (!strcmp(s, ".oob")) {
@@ -781,7 +782,8 @@ static int do_nand(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
if (s && !strcmp(s, ".allexcept"))
allexcept = 1;
- if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size) < 0)
+ if (arg_off_size(argc - 2, argv + 2, &dev, &off, &size,
+ &maxsize) < 0)
return 1;
if (!nand_unlock(&nand_info[dev], off, size, allexcept)) {
@@ -879,7 +881,8 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
printf("\nLoading from %s, offset 0x%lx\n", nand->name, offset);
cnt = nand->writesize;
- r = nand_read_skip_bad(nand, offset, &cnt, (u_char *) addr);
+ r = nand_read_skip_bad(nand, offset, &cnt, NULL, nand->size,
+ (u_char *) addr);
if (r) {
puts("** Read error\n");
bootstage_error(BOOTSTAGE_ID_NAND_HDR_READ);
@@ -911,7 +914,8 @@ static int nand_load_image(cmd_tbl_t *cmdtp, nand_info_t *nand,
}
bootstage_mark(BOOTSTAGE_ID_NAND_TYPE);
- r = nand_read_skip_bad(nand, offset, &cnt, (u_char *) addr);
+ r = nand_read_skip_bad(nand, offset, &cnt, NULL, nand->size,
+ (u_char *) addr);
if (r) {
puts("** Read error\n");
bootstage_error(BOOTSTAGE_ID_NAND_READ);
diff --git a/common/env_nand.c b/common/env_nand.c
index 5b69889..b745822 100644
--- a/common/env_nand.c
+++ b/common/env_nand.c
@@ -281,7 +281,8 @@ int readenv(size_t offset, u_char *buf)
} else {
char_ptr = &buf[amount_loaded];
if (nand_read_skip_bad(&nand_info[0], offset,
- &len, char_ptr))
+ &len, NULL,
+ nand_info[0].size, char_ptr))
return 1;
offset += blocksize;
diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
index ff2d348..a8d8e19 100644
--- a/drivers/mtd/nand/nand_util.c
+++ b/drivers/mtd/nand/nand_util.c
@@ -416,11 +416,13 @@ int nand_unlock(struct mtd_info *mtd, loff_t start, size_t length,
* @param nand NAND device
* @param offset offset in flash
* @param length image length
+ * @param used length of flash needed for the requested length
* @return 0 if the image fits and there are no bad blocks
* 1 if the image fits, but there are bad blocks
* -1 if the image does not fit
*/
-static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
+static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length,
+ size_t *used)
{
size_t len_excl_bad = 0;
int ret = 0;
@@ -442,8 +444,13 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
ret = 1;
offset += block_len;
+ *used += block_len;
}
+ /* If the length is not a multiple of block_len, adjust. */
+ if (len_excl_bad > length)
+ *used -= (len_excl_bad - length);
+
return ret;
}
@@ -476,20 +483,30 @@ static size_t drop_ffs(const nand_info_t *nand, const u_char *buf,
* Write image to NAND flash.
* Blocks that are marked bad are skipped and the is written to the next
* block instead as long as the image is short enough to fit even after
- * skipping the bad blocks.
+ * skipping the bad blocks. Due to bad blocks we may not be able to
+ * perform the requested write. In the case where the write would
+ * extend beyond the end of the NAND device, both length and actual (if
+ * not NULL) are set to 0. In the case where the write would extend
+ * beyond the limit we are passed, length is set to 0 and actual is set
+ * to the required length.
*
* @param nand NAND device
* @param offset offset in flash
* @param length buffer length
+ * @param actual set to size required to write length worth of
+ * buffer or 0, if not NULL
+ * @param lim maximum size that length may be in order to not
+ * exceed the buffer
* @param buffer buffer to read from
* @param flags flags modifying the behaviour of the write to NAND
* @return 0 in case of success
*/
int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer, int flags)
+ size_t *actual, loff_t lim, u_char *buffer, int flags)
{
int rval = 0, blocksize;
size_t left_to_write = *length;
+ size_t used_for_write = 0;
u_char *p_buffer = buffer;
int need_skip;
@@ -526,16 +543,28 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
if ((offset & (nand->writesize - 1)) != 0) {
printf("Attempt to write non page-aligned data\n");
*length = 0;
+ if (actual)
+ *actual = 0;
return -EINVAL;
}
- need_skip = check_skip_len(nand, offset, *length);
+ need_skip = check_skip_len(nand, offset, *length, &used_for_write);
+
+ if (actual)
+ *actual = used_for_write;
+
if (need_skip < 0) {
printf("Attempt to write outside the flash area\n");
*length = 0;
return -EINVAL;
}
+ if (used_for_write > lim) {
+ puts("Size of write exceeds partition or device limit\n");
+ *length = 0;
+ return -EFBIG;
+ }
+
if (!need_skip && !(flags & WITH_DROP_FFS)) {
rval = nand_write(nand, offset, length, buffer);
if (rval == 0)
@@ -626,36 +655,58 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
*
* Read image from NAND flash.
* Blocks that are marked bad are skipped and the next block is read
- * instead as long as the image is short enough to fit even after skipping the
- * bad blocks.
+ * instead as long as the image is short enough to fit even after
+ * skipping the bad blocks. Due to bad blocks we may not be able to
+ * perform the requested read. In the case where the read would extend
+ * beyond the end of the NAND device, both length and actual (if not
+ * NULL) are set to 0. In the case where the read would extend beyond
+ * the limit we are passed, length is set to 0 and actual is set to the
+ * required length.
*
* @param nand NAND device
* @param offset offset in flash
* @param length buffer length, on return holds number of read bytes
+ * @param actual set to size required to read length worth of buffer if
+ * not NULL
+ * @param lim maximum size that length may be in order to not exceed the
+ * buffer
* @param buffer buffer to write to
* @return 0 in case of success
*/
int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer)
+ size_t *actual, loff_t lim, u_char *buffer)
{
int rval;
size_t left_to_read = *length;
+ size_t used_for_read = 0;
u_char *p_buffer = buffer;
int need_skip;
if ((offset & (nand->writesize - 1)) != 0) {
printf("Attempt to read non page-aligned data\n");
*length = 0;
+ if (actual)
+ *actual = 0;
return -EINVAL;
}
- need_skip = check_skip_len(nand, offset, *length);
+ need_skip = check_skip_len(nand, offset, *length, &used_for_read);
+
+ if (actual)
+ *actual = used_for_read;
+
if (need_skip < 0) {
printf("Attempt to read outside the flash area\n");
*length = 0;
return -EINVAL;
}
+ if (used_for_read > lim) {
+ puts("Size of read exceeds partition or device limit\n");
+ *length = 0;
+ return -EFBIG;
+ }
+
if (!need_skip) {
rval = nand_read(nand, offset, length, buffer);
if (!rval || rval == -EUCLEAN)
diff --git a/include/nand.h b/include/nand.h
index dded4e2..f0f3bf9 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -129,7 +129,7 @@ struct nand_erase_options {
typedef struct nand_erase_options nand_erase_options_t;
int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer);
+ size_t *actual, loff_t lim, u_char *buffer);
#define WITH_YAFFS_OOB (1 << 0) /* whether write with yaffs format. This flag
* is a 'mode' meaning it cannot be mixed with
@@ -137,7 +137,7 @@ int nand_read_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
#define WITH_DROP_FFS (1 << 1) /* drop trailing all-0xff pages */
int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
- u_char *buffer, int flags);
+ size_t *actual, loff_t lim, u_char *buffer, int flags);
int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts);
int nand_torture(nand_info_t *nand, loff_t offset);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 2/4] dfu: NAND specific routines for DFU operation
2013-02-28 19:09 [U-Boot] [PATCH v3 0/4] Add NAND support to DFU, enable for am335x_evm Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters Tom Rini
@ 2013-02-28 19:09 ` Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 3/4] am335x_evm: Add CONFIG_CMD_MTDPARTS and relevant defaults Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 4/4] am335x_evm: Enable DFU for NAND and MMC, provide example alt_info for both Tom Rini
3 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2013-02-28 19:09 UTC (permalink / raw)
To: u-boot
From: Pantelis Antoniou <panto@antoniou-consulting.com>
Support for NAND storage devices to work with the DFU framework.
Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Tom Rini <trini@ti.com>
---
Changes in v3:
- Rework logic in nand_block_op for nand_(read|write)_skip_bad returning
just a size for actual used length.
- Remove unused externs from drivers/dfu/dfu_nand.c
Changes in v2:
- nand_block_op calls nand_(read|write)_skip_bad directly.
- Bugfix in dfu_nand to make sure we set dfu->skip_bad to 0 on each
iteration.
drivers/dfu/Makefile | 1 +
drivers/dfu/dfu.c | 8 ++
drivers/dfu/dfu_nand.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++
include/dfu.h | 23 ++++++
4 files changed, 227 insertions(+)
create mode 100644 drivers/dfu/dfu_nand.c
diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
index 7b717bc..153095d 100644
--- a/drivers/dfu/Makefile
+++ b/drivers/dfu/Makefile
@@ -27,6 +27,7 @@ LIB = $(obj)libdfu.o
COBJS-$(CONFIG_DFU_FUNCTION) += dfu.o
COBJS-$(CONFIG_DFU_MMC) += dfu_mmc.o
+COBJS-$(CONFIG_DFU_NAND) += dfu_nand.o
SRCS := $(COBJS-y:.o=.c)
OBJS := $(addprefix $(obj),$(COBJS-y))
diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
index fb9b417..44d29de 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -86,6 +86,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
/* initial state */
dfu->crc = 0;
dfu->offset = 0;
+ dfu->bad_skip = 0;
dfu->i_blk_seq_num = 0;
dfu->i_buf_start = dfu_buf;
dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
@@ -234,6 +235,8 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
dfu->i_buf = dfu->i_buf_start;
dfu->b_left = 0;
+ dfu->bad_skip = 0;
+
dfu->inited = 1;
}
@@ -263,6 +266,8 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
dfu->i_buf = dfu->i_buf_start;
dfu->b_left = 0;
+ dfu->bad_skip = 0;
+
dfu->inited = 0;
}
@@ -285,6 +290,9 @@ static int dfu_fill_entity(struct dfu_entity *dfu, char *s, int alt,
if (strcmp(interface, "mmc") == 0) {
if (dfu_fill_entity_mmc(dfu, s))
return -1;
+ } else if (strcmp(interface, "nand") == 0) {
+ if (dfu_fill_entity_nand(dfu, s))
+ return -1;
} else {
printf("%s: Device %s not (yet) supported!\n",
__func__, interface);
diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
new file mode 100644
index 0000000..b7f60dd
--- /dev/null
+++ b/drivers/dfu/dfu_nand.c
@@ -0,0 +1,195 @@
+/*
+ * dfu_nand.c -- DFU for NAND routines.
+ *
+ * Copyright (C) 2012-2013 Texas Instruments, Inc.
+ *
+ * Based on dfu_mmc.c which is:
+ * Copyright (C) 2012 Samsung Electronics
+ * author: Lukasz Majewski <l.majewski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <errno.h>
+#include <div64.h>
+#include <dfu.h>
+#include <linux/mtd/mtd.h>
+#include <jffs2/load_kernel.h>
+#include <nand.h>
+
+enum dfu_nand_op {
+ DFU_OP_READ = 1,
+ DFU_OP_WRITE,
+};
+
+static int nand_block_op(enum dfu_nand_op op, struct dfu_entity *dfu,
+ u64 offset, void *buf, long *len)
+{
+ loff_t start;
+ size_t count, actual;
+ int ret;
+ int dev;
+ nand_info_t *nand;
+
+ /* if buf == NULL return total size of the area */
+ if (buf == NULL) {
+ *len = dfu->data.nand.size;
+ return 0;
+ }
+
+ start = dfu->data.nand.start + offset + dfu->bad_skip;
+ count = *len;
+ if (start + count >
+ dfu->data.nand.start + dfu->data.nand.size) {
+ printf("%s: block_op out of bounds\n", __func__);
+ return -1;
+ }
+
+ dev = nand_curr_device;
+ if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
+ !nand_info[dev].name) {
+ printf("%s: invalid nand device\n", __func__);
+ return -1;
+ }
+
+ nand = &nand_info[dev];
+
+ if (op == DFU_OP_READ)
+ ret = nand_read_skip_bad(nand, start, &count, &actual,
+ nand->size, buf);
+ else
+ ret = nand_write_skip_bad(nand, start, &count, &actual,
+ nand->size, buf, 0);
+
+ if (ret != 0) {
+ printf("%s: nand_%s_skip_bad call failed at %llx!\n",
+ __func__, op == DFU_OP_READ ? "read" : "write",
+ start);
+ return ret;
+ }
+
+ /*
+ * Find out where we stopped writing data. This can be deeper into
+ * the NAND than we expected due to having to skip bad blocks. So
+ * we must take this into account for the next write, if any.
+ */
+ if (actual > count) {
+ printf("%s: skipped 0x%x bad bytes at 0x%llx\n", __func__,
+ actual - count, start);
+ dfu->bad_skip += actual - count;
+ }
+
+ return ret;
+}
+
+static inline int nand_block_write(struct dfu_entity *dfu,
+ u64 offset, void *buf, long *len)
+{
+ return nand_block_op(DFU_OP_WRITE, dfu, offset, buf, len);
+}
+
+static inline int nand_block_read(struct dfu_entity *dfu,
+ u64 offset, void *buf, long *len)
+{
+ return nand_block_op(DFU_OP_READ, dfu, offset, buf, len);
+}
+
+static int dfu_write_medium_nand(struct dfu_entity *dfu,
+ u64 offset, void *buf, long *len)
+{
+ int ret = -1;
+
+ switch (dfu->layout) {
+ case DFU_RAW_ADDR:
+ ret = nand_block_write(dfu, offset, buf, len);
+ break;
+ default:
+ printf("%s: Layout (%s) not (yet) supported!\n", __func__,
+ dfu_get_layout(dfu->layout));
+ }
+
+ return ret;
+}
+
+static int dfu_read_medium_nand(struct dfu_entity *dfu, u64 offset, void *buf,
+ long *len)
+{
+ int ret = -1;
+
+ switch (dfu->layout) {
+ case DFU_RAW_ADDR:
+ ret = nand_block_read(dfu, offset, buf, len);
+ break;
+ default:
+ printf("%s: Layout (%s) not (yet) supported!\n", __func__,
+ dfu_get_layout(dfu->layout));
+ }
+
+ return ret;
+}
+
+int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
+{
+ char *st;
+ int ret, dev, part;
+
+ dfu->dev_type = DFU_DEV_NAND;
+ st = strsep(&s, " ");
+ if (!strcmp(st, "raw")) {
+ dfu->layout = DFU_RAW_ADDR;
+ dfu->data.nand.start = simple_strtoul(s, &s, 16);
+ s++;
+ dfu->data.nand.size = simple_strtoul(s, &s, 16);
+ } else if (!strcmp(st, "part")) {
+ char mtd_id[32];
+ struct mtd_device *mtd_dev;
+ u8 part_num;
+ struct part_info *pi;
+
+ dfu->layout = DFU_RAW_ADDR;
+
+ dev = simple_strtoul(s, &s, 10);
+ s++;
+ part = simple_strtoul(s, &s, 10);
+
+ sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
+ printf("using id '%s'\n", mtd_id);
+
+ mtdparts_init();
+
+ ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
+ if (ret != 0) {
+ printf("Could not locate '%s'\n", mtd_id);
+ return -1;
+ }
+
+ dfu->data.nand.start = pi->offset;
+ dfu->data.nand.size = pi->size;
+
+ } else {
+ printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+ return -1;
+ }
+
+ dfu->read_medium = dfu_read_medium_nand;
+ dfu->write_medium = dfu_write_medium_nand;
+
+ /* initial state */
+ dfu->inited = 0;
+
+ return 0;
+}
diff --git a/include/dfu.h b/include/dfu.h
index 9c6b364..86b7d66 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -56,6 +56,15 @@ struct mmc_internal_data {
unsigned int part;
};
+struct nand_internal_data {
+ /* RAW programming */
+ u64 start;
+ u64 size;
+
+ unsigned int dev;
+ unsigned int part;
+};
+
static inline unsigned int get_mmc_blk_size(int dev)
{
return find_mmc_device(dev)->read_bl_len;
@@ -75,6 +84,7 @@ struct dfu_entity {
union {
struct mmc_internal_data mmc;
+ struct nand_internal_data nand;
} data;
int (*read_medium)(struct dfu_entity *dfu,
@@ -95,6 +105,8 @@ struct dfu_entity {
long r_left;
long b_left;
+ u32 bad_skip; /* for nand use */
+
unsigned int inited : 1;
};
@@ -119,4 +131,15 @@ static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
return -1;
}
#endif
+
+#ifdef CONFIG_DFU_NAND
+extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s);
+#else
+static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
+{
+ puts("NAND support not available!\n");
+ return -1;
+}
+#endif
+
#endif /* __DFU_ENTITY_H_ */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 3/4] am335x_evm: Add CONFIG_CMD_MTDPARTS and relevant defaults
2013-02-28 19:09 [U-Boot] [PATCH v3 0/4] Add NAND support to DFU, enable for am335x_evm Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 2/4] dfu: NAND specific routines for DFU operation Tom Rini
@ 2013-02-28 19:09 ` Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 4/4] am335x_evm: Enable DFU for NAND and MMC, provide example alt_info for both Tom Rini
3 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2013-02-28 19:09 UTC (permalink / raw)
To: u-boot
Signed-off-by: Tom Rini <trini@ti.com>
---
Changes in v3: None
Changes in v2:
- Add CONFIG_CMD_MTDPARTS and relevant information to am335x_evm
include/configs/am335x_evm.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index 59647d1..61b861d 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -60,6 +60,8 @@
"fdtfile=\0" \
"console=ttyO0,115200n8\0" \
"optargs=\0" \
+ "mtdids=" MTDIDS_DEFAULT "\0" \
+ "mtdparts=" MTDPARTS_DEFAULT "\0" \
"mmcdev=0\0" \
"mmcroot=/dev/mmcblk0p2 ro\0" \
"mmcrootfstype=ext4 rootwait\0" \
@@ -341,6 +343,13 @@
/* NAND support */
#ifdef CONFIG_NAND
#define CONFIG_CMD_NAND
+#define CONFIG_CMD_MTDPARTS
+#define MTDIDS_DEFAULT "nand0=omap2-nand.0"
+#define MTDPARTS_DEFAULT "mtdparts=omap2-nand.0:128k(SPL)," \
+ "128k(SPL.backup1)," \
+ "128k(SPL.backup2)," \
+ "128k(SPL.backup3),1920k(u-boot)," \
+ "128k(u-boot-env),5m(kernel),-(rootfs)"
#define CONFIG_NAND_OMAP_GPMC
#define GPMC_NAND_ECC_LP_x16_LAYOUT 1
#define CONFIG_SYS_NAND_BASE (0x08000000) /* physical address */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 4/4] am335x_evm: Enable DFU for NAND and MMC, provide example alt_info for both
2013-02-28 19:09 [U-Boot] [PATCH v3 0/4] Add NAND support to DFU, enable for am335x_evm Tom Rini
` (2 preceding siblings ...)
2013-02-28 19:09 ` [U-Boot] [PATCH v3 3/4] am335x_evm: Add CONFIG_CMD_MTDPARTS and relevant defaults Tom Rini
@ 2013-02-28 19:09 ` Tom Rini
3 siblings, 0 replies; 11+ messages in thread
From: Tom Rini @ 2013-02-28 19:09 UTC (permalink / raw)
To: u-boot
From: Pantelis Antoniou <panto@antoniou-consulting.com>
- Add CONFIG_DFU_NAND, CONFIG_DFU_MMC
- Set dfu_alt_info_nand and dfu_alt_info_mmc to show a working example
for both.
- Increase CONFIG_SYS_MAXARGS due to hush parsing bugs that would
otherwise disallow 'setenv dfu_alt_info ${dfu_alt_info_nand}'.
- Enable CONFIG_FAT_WRITE to allow updating on MMC
Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
Signed-off-by: Tom Rini <trini@ti.com>
---
Changes in v3:
- Fix checkpatch.pl warnings in include/configs/am335x_evm.h
Changes in v2:
- Enable DFU for NAND and MMC, set dfu_alt_info_(nand|mmc) as examples
for both in am335x_evm.h
- Increase CONFIG_SYS_MAXARGS due to hush parsing bugs that would
otherwise prevent 'setenv dfu_alt_info ${dfu_alt_info_nand}' on
am335x_evm
include/configs/am335x_evm.h | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index 61b861d..14ffda7 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -62,6 +62,8 @@
"optargs=\0" \
"mtdids=" MTDIDS_DEFAULT "\0" \
"mtdparts=" MTDPARTS_DEFAULT "\0" \
+ "dfu_alt_info_mmc=" DFU_ALT_INFO_MMC "\0" \
+ "dfu_alt_info_nand=" DFU_ALT_INFO_NAND "\0" \
"mmcdev=0\0" \
"mmcroot=/dev/mmcblk0p2 ro\0" \
"mmcrootfstype=ext4 rootwait\0" \
@@ -118,8 +120,8 @@
#define CONFIG_CMD_ECHO
-/* max number of command args */
-#define CONFIG_SYS_MAXARGS 16
+/* We set the max number of command args high to avoid HUSH bugs. */
+#define CONFIG_SYS_MAXARGS 64
/* Console I/O Buffer Size */
#define CONFIG_SYS_CBSIZE 512
@@ -148,6 +150,7 @@
#define CONFIG_CMD_MMC
#define CONFIG_DOS_PARTITION
#define CONFIG_CMD_FAT
+#define CONFIG_FAT_WRITE
#define CONFIG_CMD_EXT2
#define CONFIG_SPI
@@ -158,6 +161,36 @@
#define CONFIG_CMD_SF
#define CONFIG_SF_DEFAULT_SPEED (24000000)
+/* USB Composite download gadget - g_dnl */
+#define CONFIG_USB_GADGET
+#define CONFIG_USBDOWNLOAD_GADGET
+
+/* USB TI's IDs */
+#define CONFIG_USBD_HS
+#define CONFIG_G_DNL_VENDOR_NUM 0x0403
+#define CONFIG_G_DNL_PRODUCT_NUM 0xBD00
+#define CONFIG_G_DNL_MANUFACTURER "Texas Instruments"
+
+/* USB Device Firmware Update support */
+#define CONFIG_DFU_FUNCTION
+#define CONFIG_DFU_MMC
+#define CONFIG_DFU_NAND
+#define CONFIG_CMD_DFU
+#define DFU_ALT_INFO_MMC \
+ "boot part 0 1;" \
+ "rootfs part 0 2;" \
+ "MLO fat 0 1;" \
+ "u-boot.img fat 0 1;" \
+ "uEnv.txt fat 0 1"
+#define DFU_ALT_INFO_NAND \
+ "SPL part 0 1;" \
+ "SPL.backup1 part 0 2;" \
+ "SPL.backup2 part 0 3;" \
+ "SPL.backup3 part 0 4;" \
+ "u-boot part 0 5;" \
+ "kernel part 0 7;" \
+ "rootfs part 0 8"
+
/* Physical Memory Map */
#define CONFIG_NR_DRAM_BANKS 1 /* 1 bank of DRAM */
#define PHYS_DRAM_1 0x80000000 /* DRAM Bank #1 */
@@ -302,6 +335,7 @@
#define CONFIG_MUSB_GADGET
#define CONFIG_MUSB_PIO_ONLY
#define CONFIG_USB_GADGET_DUALSPEED
+#define CONFIG_USB_GADGET_VBUS_DRAW 2
#define CONFIG_MUSB_HOST
#define CONFIG_AM335X_USB0
#define CONFIG_AM335X_USB0_MODE MUSB_PERIPHERAL
--
1.7.9.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
2013-02-28 19:09 ` [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters Tom Rini
@ 2013-03-01 1:37 ` Scott Wood
2013-03-01 15:57 ` Tom Rini
0 siblings, 1 reply; 11+ messages in thread
From: Scott Wood @ 2013-03-01 1:37 UTC (permalink / raw)
To: u-boot
On 02/28/2013 01:09:05 PM, Tom Rini wrote:
> We make these two functions take a size_t pointer to how much space
> was used on NAND to read or write the buffer (when reads/writes
> happen)
> so that bad blocks can be accounted for. We also make them take an
> loff_t limit on how much data can be read or written. This means that
> we can now catch the case of when writing to a partition would exceed
> the partition size due to bad blocks. To do this we also need to make
> check_skip_len count not just complete blocks used but partial ones as
> well. All callers of nand_(read|write)_skip_bad are adjusted to call
> these with the most sensible limits available.
>
> The changes were started by Pantelis and finished by Tom.
>
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> Changes in v3:
> - Reworked skip_check_len changes to just add accounting for *used to
> the logic.
> - Allow for actual to be NULL in nand_(read|write)_skip_bad, only DFU
> calls this with a non-NULL parameter. Make sure the comments for
> both
> functions explain the parameters and their behavior.
> - Other style changes requested by Scott.
> - As nand_(write|read)_skip_bad passes back just a used length now.
>
> Changes in v2:
> - NAND skip_check_len changes reworked to allow
> nand_(read|write)_skip_bad to return this information to the caller.
>
> common/cmd_nand.c | 56
> +++++++++++++++++++----------------
> common/env_nand.c | 3 +-
> drivers/mtd/nand/nand_util.c | 67
> +++++++++++++++++++++++++++++++++++++-----
> include/nand.h | 4 +--
> 4 files changed, 93 insertions(+), 37 deletions(-)
Looks mostly good, just some minor issues:
> - if (*size > maxsize) {
> - puts("Size exceeds partition or device limit\n");
> - return -1;
> - }
> -
I assume you're removing this because you rely on the read/write
functions printing the error... what about other users of this such as
erase, lock, etc?
> @@ -476,20 +483,30 @@ static size_t drop_ffs(const nand_info_t *nand,
> const u_char *buf,
> * Write image to NAND flash.
> * Blocks that are marked bad are skipped and the is written to the
> next
> * block instead as long as the image is short enough to fit even
> after
> - * skipping the bad blocks.
> + * skipping the bad blocks. Due to bad blocks we may not be able to
> + * perform the requested write. In the case where the write would
> + * extend beyond the end of the NAND device, both length and actual
> (if
> + * not NULL) are set to 0. In the case where the write would extend
> + * beyond the limit we are passed, length is set to 0 and actual is
> set
> + * to the required length.
> *
> * @param nand NAND device
> * @param offset offset in flash
> * @param length buffer length
> + * @param actual set to size required to write length worth of
> + * buffer or 0, if not NULL
s/or 0/or 0 on error/
or
s/or 0/in case of success/
The read function doesn't have the "or 0" comment...
> + * @param lim maximum size that length may be in
> order to not
> + * exceed the buffer
s/that length may be/that actual may be/
> * @param buffer buffer to read from
> * @param flags flags modifying the behaviour of the
> write to NAND
> * @return 0 in case of success
> */
> int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t
> *length,
> - u_char *buffer, int flags)
> + size_t *actual, loff_t lim, u_char *buffer, int flags)
> {
> int rval = 0, blocksize;
> size_t left_to_write = *length;
> + size_t used_for_write = 0;
> u_char *p_buffer = buffer;
> int need_skip;
>
> @@ -526,16 +543,28 @@ int nand_write_skip_bad(nand_info_t *nand,
> loff_t offset, size_t *length,
> if ((offset & (nand->writesize - 1)) != 0) {
> printf("Attempt to write non page-aligned data\n");
> *length = 0;
> + if (actual)
> + *actual = 0;
> return -EINVAL;
> }
Again, what about the returns in the WITH_YAFFS_OOB section? Or if we
document that "actual" is undefined for error returns we can not worry
about this.
-Scott
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
2013-03-01 1:37 ` Scott Wood
@ 2013-03-01 15:57 ` Tom Rini
2013-03-01 16:07 ` Tom Rini
2013-03-02 2:59 ` Scott Wood
0 siblings, 2 replies; 11+ messages in thread
From: Tom Rini @ 2013-03-01 15:57 UTC (permalink / raw)
To: u-boot
On Thu, Feb 28, 2013 at 07:37:51PM -0600, Scott Wood wrote:
> On 02/28/2013 01:09:05 PM, Tom Rini wrote:
> >We make these two functions take a size_t pointer to how much space
> >was used on NAND to read or write the buffer (when reads/writes
> >happen)
> >so that bad blocks can be accounted for. We also make them take an
> >loff_t limit on how much data can be read or written. This means that
> >we can now catch the case of when writing to a partition would exceed
> >the partition size due to bad blocks. To do this we also need to make
> >check_skip_len count not just complete blocks used but partial ones as
> >well. All callers of nand_(read|write)_skip_bad are adjusted to call
> >these with the most sensible limits available.
> >
> >The changes were started by Pantelis and finished by Tom.
> >
> >Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >Signed-off-by: Tom Rini <trini@ti.com>
> >---
> >Changes in v3:
> >- Reworked skip_check_len changes to just add accounting for *used to
> > the logic.
> >- Allow for actual to be NULL in nand_(read|write)_skip_bad, only DFU
> > calls this with a non-NULL parameter. Make sure the comments
> >for both
> > functions explain the parameters and their behavior.
> >- Other style changes requested by Scott.
> >- As nand_(write|read)_skip_bad passes back just a used length now.
> >
> >Changes in v2:
> >- NAND skip_check_len changes reworked to allow
> > nand_(read|write)_skip_bad to return this information to the caller.
> >
> > common/cmd_nand.c | 56
> >+++++++++++++++++++----------------
> > common/env_nand.c | 3 +-
> > drivers/mtd/nand/nand_util.c | 67
> >+++++++++++++++++++++++++++++++++++++-----
> > include/nand.h | 4 +--
> > 4 files changed, 93 insertions(+), 37 deletions(-)
>
> Looks mostly good, just some minor issues:
>
> >- if (*size > maxsize) {
> >- puts("Size exceeds partition or device limit\n");
> >- return -1;
> >- }
> >-
>
> I assume you're removing this because you rely on the read/write
> functions printing the error... what about other users of this such
> as erase, lock, etc?
Ah true. And I don't see an easy way to make Harvey's patch cover limit
exceeds request, so we'll need to keep the limit stuff, I'll go add this
check back.
> >@@ -476,20 +483,30 @@ static size_t drop_ffs(const nand_info_t
> >*nand, const u_char *buf,
> > * Write image to NAND flash.
> > * Blocks that are marked bad are skipped and the is written to
> >the next
> > * block instead as long as the image is short enough to fit even
> >after
> >- * skipping the bad blocks.
> >+ * skipping the bad blocks. Due to bad blocks we may not be able to
> >+ * perform the requested write. In the case where the write would
> >+ * extend beyond the end of the NAND device, both length and
> >actual (if
> >+ * not NULL) are set to 0. In the case where the write would extend
> >+ * beyond the limit we are passed, length is set to 0 and actual
> >is set
> >+ * to the required length.
> > *
> > * @param nand NAND device
> > * @param offset offset in flash
> > * @param length buffer length
> >+ * @param actual set to size required to write length worth of
> >+ * buffer or 0, if not NULL
>
> s/or 0/or 0 on error/
> or
> s/or 0/in case of success/
>
> The read function doesn't have the "or 0" comment...
I'll go reword.
> >+ * @param lim maximum size that length may be in order to not
> >+ * exceed the buffer
>
> s/that length may be/that actual may be/
No? We check that the requested length to something will fit even if the
caller doesn't care to know what actual is.
> > * @param buffer buffer to read from
> > * @param flags flags modifying the behaviour of the write to
> >NAND
> > * @return 0 in case of success
> > */
> > int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t
> >*length,
> >- u_char *buffer, int flags)
> >+ size_t *actual, loff_t lim, u_char *buffer, int flags)
> > {
> > int rval = 0, blocksize;
> > size_t left_to_write = *length;
> >+ size_t used_for_write = 0;
> > u_char *p_buffer = buffer;
> > int need_skip;
> >
> >@@ -526,16 +543,28 @@ int nand_write_skip_bad(nand_info_t *nand,
> >loff_t offset, size_t *length,
> > if ((offset & (nand->writesize - 1)) != 0) {
> > printf("Attempt to write non page-aligned data\n");
> > *length = 0;
> >+ if (actual)
> >+ *actual = 0;
> > return -EINVAL;
> > }
>
> Again, what about the returns in the WITH_YAFFS_OOB section? Or if
> we document that "actual" is undefined for error returns we can not
> worry about this.
OK. Currently we don't set length to 0 on WITH_YAFFS_OOB errors, but we
ought to. And we can deal with actual the same way.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130301/706cb408/attachment.pgp>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
2013-03-01 15:57 ` Tom Rini
@ 2013-03-01 16:07 ` Tom Rini
2013-03-02 2:59 ` Scott Wood
1 sibling, 0 replies; 11+ messages in thread
From: Tom Rini @ 2013-03-01 16:07 UTC (permalink / raw)
To: u-boot
On Fri, Mar 01, 2013 at 10:57:40AM -0500, Tom Rini wrote:
> On Thu, Feb 28, 2013 at 07:37:51PM -0600, Scott Wood wrote:
> > On 02/28/2013 01:09:05 PM, Tom Rini wrote:
[snip]
> > >@@ -526,16 +543,28 @@ int nand_write_skip_bad(nand_info_t *nand,
> > >loff_t offset, size_t *length,
> > > if ((offset & (nand->writesize - 1)) != 0) {
> > > printf("Attempt to write non page-aligned data\n");
> > > *length = 0;
> > >+ if (actual)
> > >+ *actual = 0;
> > > return -EINVAL;
> > > }
> >
> > Again, what about the returns in the WITH_YAFFS_OOB section? Or if
> > we document that "actual" is undefined for error returns we can not
> > worry about this.
>
> OK. Currently we don't set length to 0 on WITH_YAFFS_OOB errors, but we
> ought to. And we can deal with actual the same way.
OK, I'm going to do a follow-up patch to deal with length, and that
CONFIG_CMD_NAND_YAFFS is broken as well.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130301/f3615cfb/attachment.pgp>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
2013-03-01 15:57 ` Tom Rini
2013-03-01 16:07 ` Tom Rini
@ 2013-03-02 2:59 ` Scott Wood
2013-03-03 14:04 ` Tom Rini
1 sibling, 1 reply; 11+ messages in thread
From: Scott Wood @ 2013-03-02 2:59 UTC (permalink / raw)
To: u-boot
On 03/01/2013 09:57:40 AM, Tom Rini wrote:
> On Thu, Feb 28, 2013 at 07:37:51PM -0600, Scott Wood wrote:
> > >+ * @param lim maximum size that length may be in
> order to not
> > >+ * exceed the buffer
> >
> > s/that length may be/that actual may be/
>
> No? We check that the requested length to something will fit even if
> the
> caller doesn't care to know what actual is.
What I mean is that we're not saying "if (length > lim) error". We
compute the actual, and if the actual exceeds "lim", then there's an
error.
-Scott
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
2013-03-02 2:59 ` Scott Wood
@ 2013-03-03 14:04 ` Tom Rini
2013-03-04 19:15 ` Scott Wood
0 siblings, 1 reply; 11+ messages in thread
From: Tom Rini @ 2013-03-03 14:04 UTC (permalink / raw)
To: u-boot
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 03/01/2013 09:59 PM, Scott Wood wrote:
> On 03/01/2013 09:57:40 AM, Tom Rini wrote:
>> On Thu, Feb 28, 2013 at 07:37:51PM -0600, Scott Wood wrote:
>>>> + * @param lim maximum size that length may be in
>>>> order to not + * exceed the buffer
>>>
>>> s/that length may be/that actual may be/
>>
>> No? We check that the requested length to something will fit
>> even if the caller doesn't care to know what actual is.
>
> What I mean is that we're not saying "if (length > lim) error".
> We compute the actual, and if the actual exceeds "lim", then
> there's an error.
OK, I see your point. My hang-up is that actual may be NULL. So,
"maximum size that length may be once bad blocks are accounted for in
order to not exceed the buffer" ?
- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQIcBAEBAgAGBQJRM1hRAAoJENk4IS6UOR1WJ0oQAJdWo6WlggN3bbZ71XWDMqoi
+9Tn1lNbCVc/S4ez1NJiLr18bTrjDtY7Gvq3I0hwKkee//QAG0JTcibf2SRXJ7Wo
3ShfTdEwBuzq8mvIahTvlaSaFUCfu3oSaYGlz6Kq+bfAjlhN2s3ogjCOlqUbBT/7
GwKuue4LM3jhF8YPlF5hgssz1HAQdtt8QTTODcq4WZRisVn82W8uk4d9xSUb5Wrw
WdmrmD4v4lvtvXzimEUXyovlwvVFAX148hh1LIHKAl4pFLHaEDQF7YqtTqVgOrBR
A8ZEPYPTG/+xVhEmzxQPJsJxDhy5Z6Ax/hDyS8XVpP1qtd4V51k0ZV1pPl2UBnCI
5kyjy3oNAfQFrcP0XSeEYZaprSuy/0mC3gLbDV8kcNRYirCet124HhZU7qo7XWgO
0e1GwIKpp0a+somLE1QViGgzhsOKWhj7LL24n9jhC/aowFW365g7FIjEM9RK3mlY
v7Z/YW/9BLfd+qsvQp5VH3i8yA9QEoTCB/0O8kiLrogXoiYF37EdCn4wQLaIjiJO
uxk5Y3m+HuEOF7AZHYjvajH/WrkGg13TE70/BbPZRHOBnw89mtIwE/Ox4wJeIJ5T
Y3yoUNj59iAWXk8tr5PIBgbRpijc8DcrfLHInlDaVVrq5IdWubbCeNf9KNB+atYc
wRpDSFXnPkzT5WSus8Hf
=VPjG
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 11+ messages in thread
* [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters
2013-03-03 14:04 ` Tom Rini
@ 2013-03-04 19:15 ` Scott Wood
0 siblings, 0 replies; 11+ messages in thread
From: Scott Wood @ 2013-03-04 19:15 UTC (permalink / raw)
To: u-boot
On 03/03/2013 08:04:01 AM, Tom Rini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 03/01/2013 09:59 PM, Scott Wood wrote:
> > On 03/01/2013 09:57:40 AM, Tom Rini wrote:
> >> On Thu, Feb 28, 2013 at 07:37:51PM -0600, Scott Wood wrote:
> >>>> + * @param lim maximum size that length may be in
> >>>> order to not + * exceed the buffer
> >>>
> >>> s/that length may be/that actual may be/
> >>
> >> No? We check that the requested length to something will fit
> >> even if the caller doesn't care to know what actual is.
> >
> > What I mean is that we're not saying "if (length > lim) error".
> > We compute the actual, and if the actual exceeds "lim", then
> > there's an error.
>
> OK, I see your point. My hang-up is that actual may be NULL.
That's just the pointer to store the actual length in, which is just an
implementation detail of how we return additional values. We still
compute the actual length. I'd hope it would be obvious that we're not
talking about the pointer here.
> So, "maximum size that length may be once bad blocks are accounted
> for in
> order to not exceed the buffer" ?
I think it would be better to not have to describe the concept twice.
-Scott
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-03-04 19:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-28 19:09 [U-Boot] [PATCH v3 0/4] Add NAND support to DFU, enable for am335x_evm Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 1/4] nand: Extend nand_(read|write)_skip_bad with *actual and limit parameters Tom Rini
2013-03-01 1:37 ` Scott Wood
2013-03-01 15:57 ` Tom Rini
2013-03-01 16:07 ` Tom Rini
2013-03-02 2:59 ` Scott Wood
2013-03-03 14:04 ` Tom Rini
2013-03-04 19:15 ` Scott Wood
2013-02-28 19:09 ` [U-Boot] [PATCH v3 2/4] dfu: NAND specific routines for DFU operation Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 3/4] am335x_evm: Add CONFIG_CMD_MTDPARTS and relevant defaults Tom Rini
2013-02-28 19:09 ` [U-Boot] [PATCH v3 4/4] am335x_evm: Enable DFU for NAND and MMC, provide example alt_info for both Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox