* [PATCH v2 1/3] cmd: mtd: add logs when bad blocks are encountered
2026-01-12 9:09 ` [PATCH v2 " Peter Suti
@ 2026-01-12 9:09 ` Peter Suti
2026-01-12 13:35 ` Quentin Schulz
2026-01-12 9:09 ` [PATCH v2 2/3] cmd: mtd: fix read failure on default full partition read with bad blocks Peter Suti
2026-01-12 9:09 ` [PATCH v2 3/3] cmd: mtd: fix default length calculation when offset is specified Peter Suti
2 siblings, 1 reply; 6+ messages in thread
From: Peter Suti @ 2026-01-12 9:09 UTC (permalink / raw)
To: u-boot
Cc: Peter Suti, Tom Rini, Michael Trimarchi, Mikhail Kshevetskiy,
Miquel Raynal, Quentin Schulz, Andrew Goodbody
Signed-off-by: Peter Suti <peter.suti@streamunlimited.com>
---
cmd/mtd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c
index 7f25144098b..32733e8f11f 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -559,8 +559,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
/* Search for the first good block after the given offset */
off = start_off;
- while (mtd_block_isbad(mtd, off))
+ while (mtd_block_isbad(mtd, off)) {
+ printf("Bad block: failed to read at offset 0x%llx, skipping.\n", off);
off += mtd->erasesize;
+ }
led_activity_blink();
@@ -572,6 +574,7 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
/* Skip the block if it is bad */
if (mtd_is_aligned_with_block_size(mtd, off) &&
mtd_block_isbad(mtd, off)) {
+ printf("Bad block: failed to read at offset 0x%llx, skipping.\n", off);
off += mtd->erasesize;
continue;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/3] cmd: mtd: add logs when bad blocks are encountered
2026-01-12 9:09 ` [PATCH v2 1/3] cmd: mtd: add logs when bad blocks are encountered Peter Suti
@ 2026-01-12 13:35 ` Quentin Schulz
0 siblings, 0 replies; 6+ messages in thread
From: Quentin Schulz @ 2026-01-12 13:35 UTC (permalink / raw)
To: Peter Suti, u-boot
Cc: Tom Rini, Michael Trimarchi, Mikhail Kshevetskiy, Miquel Raynal,
Andrew Goodbody
Hi Peter,
On 1/12/26 10:09 AM, Peter Suti wrote:
> [You don't often get email from peter.suti@streamunlimited.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
We generally do not like having commits with empty commit logs. What
made you want to write this patch? Why should we take this in?
> Signed-off-by: Peter Suti <peter.suti@streamunlimited.com>
> ---
> cmd/mtd.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index 7f25144098b..32733e8f11f 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -559,8 +559,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
>
> /* Search for the first good block after the given offset */
> off = start_off;
> - while (mtd_block_isbad(mtd, off))
> + while (mtd_block_isbad(mtd, off)) {
> + printf("Bad block: failed to read at offset 0x%llx, skipping.\n", off);
I'm not verse in flashes... do we know if a block is a bad block by
reading it?
Because the code you're looking at can be reached by mtd read, mtd write
or mtd dump, so maybe we shouldn't say "read" in the error message?
Finally, is this something we should be printing in the command? What
about the other users (env/mtd.c, dfu, the onenand command). Should we
rather print (using the logger even maybe?) in the MTD core? Also,
should we print even if this is not critical? I assume skipping bad
blocks isn't always an issue?
Cheers,
Quentin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] cmd: mtd: fix read failure on default full partition read with bad blocks
2026-01-12 9:09 ` [PATCH v2 " Peter Suti
2026-01-12 9:09 ` [PATCH v2 1/3] cmd: mtd: add logs when bad blocks are encountered Peter Suti
@ 2026-01-12 9:09 ` Peter Suti
2026-01-12 9:09 ` [PATCH v2 3/3] cmd: mtd: fix default length calculation when offset is specified Peter Suti
2 siblings, 0 replies; 6+ messages in thread
From: Peter Suti @ 2026-01-12 9:09 UTC (permalink / raw)
To: u-boot
Cc: Peter Suti, Radek Dostál, Tom Rini, Michael Trimarchi,
Mikhail Kshevetskiy, Miquel Raynal, Andrew Goodbody
When performing an 'mtd read' on a NAND partition, the presence of bad
blocks can cause the command to fail with error -22 (EINVAL) if the
requested size is equal to (or close to) the partition size.
When size is not provided, the whole partition is read by default.
This issue occurs because the bad block skipping logic increments the
physical read offset ('off') without decrementing the 'remaining' byte
count. If enough bad blocks are skipped, the 'off' pointer eventually
slides past the end of the partition boundary ('mtd->size') while
'remaining' is still non-zero. The subsequent call to 'mtd_read_oob'
then fails its internal boundary checks.
The Scenario:
Partition Size: 0x1200000
Bad Blocks: 1
Command: mtd read swufit ${loadaddr}
(Defaults to reading full 0x1200000 bytes)
1. U-Boot attempts to read 0x1200000 valid bytes.
2. It encounters the bad block and skips it (off += mtd->erasesize).
3. To satisfy the full request, it must read
(0x1200000 + mtd->erasesize) physical bytes.
4. The read pointer 'off' exceeds 'mtd->size'.
5. Command aborts with: "Read on swufit failed with error -22".
Add a boundary check at the start of the read loop. If the read pointer
'off' reaches the end of the physical device, check if the user explicitly
requested a size (argc >= 2).
* If no size was specified (default read): Stop the read gracefully,
print a notice, and return success. We assume the user wants "all
available data" up to the physical limit.
* If a size was explicitly specified: Continue standard execution. This
will result in an error (-22) as expected, since the specific requirement
cannot be met.
It is physically impossible to read the full logical size of a partition
if it contains bad blocks (as the bad blocks consume physical space
required for the data). In this case, truncating the read is reasonable.
For common use cases like loading FIT images (which often
contain padding at the end), missing the trailing 0xFF data is acceptable,
whereas failing the entire load operation is not.
NOTE: This changes the behavior of 'mtd read' when no size is provided.
Previously, it failed if the full read was impossible. Now, it
succeeds with a truncated buffer. The caller is responsible for
verifying data integrity (e.g., via fitImage hash verification).
Signed-off-by: Peter Suti <peter.suti@streamunlimited.com>
[polished the comment and commit message]
Signed-off-by: Radek Dostál <radek.dostal@streamunlimited.com>
---
cmd/mtd.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/cmd/mtd.c b/cmd/mtd.c
index 32733e8f11f..1d1845bce44 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -571,6 +571,28 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
/* Loop over the pages to do the actual read/write */
while (remaining) {
+ /*
+ * Boundary Check: Bad block skipping may push the physical read offset
+ * past the partition end (mtd->size).
+ * If the read pointer 'off' has reached the end of the physical flash,
+ * we physically cannot read more.
+ *
+ * Check 'argc' to see if the user explicitly requested a specific size.
+ * * If argc < 2, the user did NOT specify a size (defaulted to partition size).
+ * In this case, we treat "end of flash" as a natural stopping point
+ * and truncate the read gracefully.
+ * * If argc >= 2, the user explicitly asked for N bytes.
+ * In this case, we do NOT truncate. We let the loop continue, which
+ * will trigger the standard error (-22) because the request cannot be met.
+ */
+ if (argc < 2 && off >= mtd->size) {
+ printf("Notice: Reached end of partition with %lld bytes remaining. "
+ "Truncating read.\n", remaining);
+ remaining = 0;
+ ret = CMD_RET_SUCCESS;
+ break;
+ }
+
/* Skip the block if it is bad */
if (mtd_is_aligned_with_block_size(mtd, off) &&
mtd_block_isbad(mtd, off)) {
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] cmd: mtd: fix default length calculation when offset is specified
2026-01-12 9:09 ` [PATCH v2 " Peter Suti
2026-01-12 9:09 ` [PATCH v2 1/3] cmd: mtd: add logs when bad blocks are encountered Peter Suti
2026-01-12 9:09 ` [PATCH v2 2/3] cmd: mtd: fix read failure on default full partition read with bad blocks Peter Suti
@ 2026-01-12 9:09 ` Peter Suti
2 siblings, 0 replies; 6+ messages in thread
From: Peter Suti @ 2026-01-12 9:09 UTC (permalink / raw)
To: u-boot
Cc: Peter Suti, Tom Rini, Mikhail Kshevetskiy, Michael Trimarchi,
Miquel Raynal, Andrew Goodbody
When the 'mtd read' command is issued with an offset but no size (argc=1),
the length defaults to the full partition size ('mtd->size').
This calculation is incorrect because reading 'mtd->size' bytes starting
from a non-zero offset results in a read request that extends past the
end of the partition. This causes the MTD layer to return error -22
(EINVAL).
Fix the default length calculation to be 'mtd->size - offset' so that
reads starting from an offset effectively read "to the end of the
partition" rather than attempting to read out of bounds.
Signed-off-by: Peter Suti <peter.suti@streamunlimited.com>
---
cmd/mtd.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c
index 1d1845bce44..29189925b24 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -519,7 +519,17 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
goto out_put_mtd;
}
- default_len = dump ? mtd->writesize : mtd->size;
+ if (dump) {
+ default_len = mtd->writesize;
+ } else {
+ if (start_off >= mtd->size) {
+ printf("Start offset 0x%llx is greater or equal to mtd size 0x%llx\n", start_off, mtd->size);
+ ret = CMD_RET_FAILURE;
+ goto out_put_mtd;
+ }
+ default_len = mtd->size - start_off;
+ }
+
len = argc > 1 ? hextoul(argv[1], NULL) : default_len;
if (!mtd_is_aligned_with_min_io_size(mtd, len)) {
len = round_up(len, mtd->writesize);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread