[PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check

[Tudor Ambarus]

On 3/9/26 4:55 PM, Miquel Raynal wrote:
> Hello SPI NOR folks :-)
> 
> + Takahiro
> 
> On 08/01/2026 at 13:14:29 +01, Miquel Raynal <miquel.raynal at bootlin.com> wrote:
> 
>> Commit 5008c3ec3f89 ("mtd: spi-nor: core: Check read CR support") adds a
>> controller check to make sure the core will not use CR reads on
>> controllers not supporting them. The approach is valid but the fix is
>> incorrect. Unfortunately, the author could not catch it, because the
>> expected behavior was met. The patch indeed drops the RDCR capability,
>> but it does it for all controllers!
>>
>> The issue comes from the use of spi_nor_spimem_check_op() which is an
>> internal helper dedicated to check page operations, ie. it is only used
>> for page reads and page programs (despite its generic name).
>>
>> This helper looks for the biggest number of address bytes that can be
>> used for a page operation and tries 4 then 3. It then calls the usual
>> spi-mem helpers to do the checks. These will always fail because there
>> is now an inconsistency: the address cycles are forced to 4 (then 3)
>> bytes, but the bus width during the address cycles rightfully remains 0:
>> impossible, the operation is invalid.

I like Cheng's details on how it's failing. Would you please add what he
detailed there?
```
This modified operation is then rejected by spi_mem_check_op() in the
core spi-mem.c because it has a non-zero address length but a zero address
buswidth, which is an invalid combination.
```
Acked-by: Tudor Ambarus <tudor.ambarus at linaro.org>

>>
>> The correct check in this case is to directly call spi_mem_supports_op()
>> which doesn't messes up with the operation content.
>>
>> Fixes: 5008c3ec3f89 ("mtd: spi-nor: core: Check read CR support")
>> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> 
> These two patches are fixes which need to get in, I'd like to pick them
> for the next fixes MTD PR that I am preparing, but I was expecting some
> kind of acknowledgement on it.
> 
> Cheng Ming already faced the same issue on his side an identical patch
> already.
> 
> Thanks,
> Miquèl
> 
>> ---
>>  drivers/mtd/spi-nor/core.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index d3f8a78efd3b..1f2e312feec7 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -2466,7 +2466,7 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>>  
>>  		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>>  
>> -		if (spi_nor_spimem_check_op(nor, &op))
>> +		if (!spi_mem_supports_op(nor->spimem, &op))
>>  			nor->flags |= SNOR_F_NO_READ_CR;
>>  	}
>>  }