Re: [PATCH] mtd: cfi_cmdset_0002: cap the write-buffer chunk at 256 bytes on an 8-bit bus

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: incomplete write-buffer size clamp for interleaved 8-bit chips
--

commit efef65c6e8de323e4294fd157f7f865d12e35292
Author: Orgad Shaneh <orgads@gmail.com>

mtd: cfi_cmdset_0002: cap the write-buffer chunk at 256 bytes on an 8-bit bus

This commit limits the write buffer chunk size to 256 bytes when the bank
width is exactly one byte. It prevents program aborts on 8-bit buses where
the chip advertises a larger write buffer than the single bus word can hold.

> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 517db2f2707f4..64fde81191d15 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -283,6 +283,17 @@ static void fixup_use_write_buffers(struct mtd_info *mtd)
[ ... ]
> +	/*
> +	 * The word count of the Write to Buffer command is a single bus
> +	 * word, so on an 8-bit bus a chunk holds at most 256 bytes no
> +	 * matter how large a buffer the chip advertises.
> +	 */
> +	if (map_bankwidth_is_1(map) && cfi->cfiq->MaxBufWriteSize > 8) {
[Severity: High]
Does this condition properly handle interleaved configurations where multiple
8-bit chips are used on a wider bus?

The map_bankwidth_is_1() macro checks if the total aggregate system bus width
is 1 byte. If two 8-bit chips are interleaved on a 16-bit bus, the aggregate
bus width is 2 bytes, causing map_bankwidth_is_1() to evaluate to false and
bypass the clamp.

When writing a large buffer in an interleaved setup (e.g., MaxBufWriteSize = 9
and interleave = 2), cfi_amdstd_write_buffers() will pass up to 1024 bytes to
do_write_buffer(). The loop divides this by map_bankwidth to compute 512 words.

It writes a command of 511 (0x1FF) to the bus, which cfi_build_cmd()
duplicates to 0x1FFFF. This gets truncated to 0xFFFF on the 16-bit bus, so
each 8-bit chip receives 0xFF (255) as the word count. They will expect exactly
256 words (bytes).

However, the subsequent data loop in do_write_buffer() would write 512 words
to each chip, causing them to abort the program on the 257th byte.

Should this condition check the per-chip data width instead of the aggregate
system bus width? For example, checking if
map_bankwidth(map) / cfi_interleave(cfi) == 1?

> +		cfi->cfiq->MaxBufWriteSize = 8;
> +		mtd->writebufsize = cfi_interleave(cfi) <<
> +				    cfi->cfiq->MaxBufWriteSize;
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915071339.15172-1-orgads@gmail.com?part=1