* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-09 14:55 ` [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check Miquel Raynal
@ 2026-03-10 9:24 ` Tudor Ambarus
2026-03-10 9:38 ` Tudor Ambarus
2026-03-17 9:20 ` Miquel Raynal
2026-03-11 7:02 ` Takahiro.Kuwano
2026-03-13 11:10 ` Pratyush Yadav
2 siblings, 2 replies; 16+ messages in thread
From: Tudor Ambarus @ 2026-03-10 9:24 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger
Cc: Vignesh Raghavendra, Pratyush Yadav, Michael Walle, linux-mtd,
Jakub Czapiga, Thomas Petazzoni, Takahiro Kuwano
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@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@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@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;
>> }
>> }
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-10 9:24 ` Tudor Ambarus
@ 2026-03-10 9:38 ` Tudor Ambarus
2026-03-17 9:21 ` Miquel Raynal
2026-03-17 9:20 ` Miquel Raynal
1 sibling, 1 reply; 16+ messages in thread
From: Tudor Ambarus @ 2026-03-10 9:38 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger
Cc: Vignesh Raghavendra, Pratyush Yadav, Michael Walle, linux-mtd,
Jakub Czapiga, Thomas Petazzoni, Takahiro Kuwano
On 3/10/26 11:24 AM, Tudor Ambarus wrote:
> for page reads and page programs (despite its generic name).
ah, and there's no such thing as page reads in SPI NORs, one can use
a single read instruction to read the entire flash memory. We all
understood what you meant, but please reword this so that's clear for
newcomers.
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-10 9:38 ` Tudor Ambarus
@ 2026-03-17 9:21 ` Miquel Raynal
0 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2026-03-17 9:21 UTC (permalink / raw)
To: Tudor Ambarus
Cc: Richard Weinberger, Vignesh Raghavendra, Pratyush Yadav,
Michael Walle, linux-mtd, Jakub Czapiga, Thomas Petazzoni,
Takahiro Kuwano
On 10/03/2026 at 11:38:47 +02, Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> On 3/10/26 11:24 AM, Tudor Ambarus wrote:
>> for page reads and page programs (despite its generic name).
> ah, and there's no such thing as page reads in SPI NORs, one can use
> a single read instruction to read the entire flash memory. We all
> understood what you meant, but please reword this so that's clear for
> newcomers.
True. I changed the commit log and mentioned "reads and writes" instead.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-10 9:24 ` Tudor Ambarus
2026-03-10 9:38 ` Tudor Ambarus
@ 2026-03-17 9:20 ` Miquel Raynal
2026-03-17 9:25 ` Tudor Ambarus
1 sibling, 1 reply; 16+ messages in thread
From: Miquel Raynal @ 2026-03-17 9:20 UTC (permalink / raw)
To: Tudor Ambarus
Cc: Richard Weinberger, Vignesh Raghavendra, Pratyush Yadav,
Michael Walle, linux-mtd, Jakub Czapiga, Thomas Petazzoni,
Takahiro Kuwano
Hello,
>>> 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.
> ```
I've added the precision about the invalid combination of non-zero
address length and zero bus width. I feel like it's a repetition of my
previous sentence, but I'm fine with it if it clarifies.
Thanks!
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-17 9:20 ` Miquel Raynal
@ 2026-03-17 9:25 ` Tudor Ambarus
0 siblings, 0 replies; 16+ messages in thread
From: Tudor Ambarus @ 2026-03-17 9:25 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Pratyush Yadav,
Michael Walle, linux-mtd, Jakub Czapiga, Thomas Petazzoni,
Takahiro Kuwano
On 3/17/26 11:20 AM, Miquel Raynal wrote:
> Hello,
>
>>>> 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.
>> ```
>
> I've added the precision about the invalid combination of non-zero
> address length and zero bus width. I feel like it's a repetition of my
oh, yes, I missed that.
> previous sentence, but I'm fine with it if it clarifies.
I'm fine either way. Thanks!
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-09 14:55 ` [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check Miquel Raynal
2026-03-10 9:24 ` Tudor Ambarus
@ 2026-03-11 7:02 ` Takahiro.Kuwano
2026-03-13 11:10 ` Pratyush Yadav
2 siblings, 0 replies; 16+ messages in thread
From: Takahiro.Kuwano @ 2026-03-11 7:02 UTC (permalink / raw)
To: miquel.raynal, richard
Cc: vigneshr, tudor.ambarus, pratyush, michael, linux-mtd, czapiga,
thomas.petazzoni
> Hello SPI NOR folks :-)
>
> + Takahiro
>
Hello!
> On 08/01/2026 at 13:14:29 +01, Miquel Raynal <miquel.raynal@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.
> >
> > 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@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;
> > }
> > }
Acked-by: Takahiro Kuwano <takahiro.kuwano@infineon.com>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-09 14:55 ` [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check Miquel Raynal
2026-03-10 9:24 ` Tudor Ambarus
2026-03-11 7:02 ` Takahiro.Kuwano
@ 2026-03-13 11:10 ` Pratyush Yadav
2026-03-13 14:34 ` Miquel Raynal
2 siblings, 1 reply; 16+ messages in thread
From: Pratyush Yadav @ 2026-03-13 11:10 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
Pratyush Yadav, Michael Walle, linux-mtd, Jakub Czapiga,
Thomas Petazzoni, Takahiro Kuwano
On Mon, Mar 09 2026, Miquel Raynal wrote:
> Hello SPI NOR folks :-)
>
> + Takahiro
>
> On 08/01/2026 at 13:14:29 +01, Miquel Raynal <miquel.raynal@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.
>>
>> 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@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.
With Tudor's suggestions:
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
I am going through the SPI NOR patch backlog. Since you plan to take
these directly via mtd/fixes, I'll not queue these.
[...]
--
Regards,
Pratyush Yadav
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-13 11:10 ` Pratyush Yadav
@ 2026-03-13 14:34 ` Miquel Raynal
2026-03-13 16:42 ` Pratyush Yadav
0 siblings, 1 reply; 16+ messages in thread
From: Miquel Raynal @ 2026-03-13 14:34 UTC (permalink / raw)
To: Pratyush Yadav
Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus,
Michael Walle, linux-mtd, Jakub Czapiga, Thomas Petazzoni,
Takahiro Kuwano
On 13/03/2026 at 11:10:50 GMT, Pratyush Yadav <pratyush@kernel.org> wrote:
> On Mon, Mar 09 2026, Miquel Raynal wrote:
>
>> Hello SPI NOR folks :-)
>>
>> + Takahiro
>>
>> On 08/01/2026 at 13:14:29 +01, Miquel Raynal <miquel.raynal@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.
>>>
>>> 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@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.
>
> With Tudor's suggestions:
>
> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
>
> I am going through the SPI NOR patch backlog. Since you plan to take
> these directly via mtd/fixes, I'll not queue these.
I was too busy this week to follow-up, but indeed I might just apply
with the suggestions from Tudor if I'm able to send a fixes PR by
Saturday, otherwise I'll go for the normal route and submit a v2. Patch
2 however is not eligible for -fixes, it clearly is an enhancement that
is not a bug fix or whatever, so you can take it when I send a v2
through SPI NOR next, as AFAIR it will not conflict with patch 1.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
2026-03-13 14:34 ` Miquel Raynal
@ 2026-03-13 16:42 ` Pratyush Yadav
0 siblings, 0 replies; 16+ messages in thread
From: Pratyush Yadav @ 2026-03-13 16:42 UTC (permalink / raw)
To: Miquel Raynal
Cc: Pratyush Yadav, Richard Weinberger, Vignesh Raghavendra,
Tudor Ambarus, Michael Walle, linux-mtd, Jakub Czapiga,
Thomas Petazzoni, Takahiro Kuwano
On Fri, Mar 13 2026, Miquel Raynal wrote:
> On 13/03/2026 at 11:10:50 GMT, Pratyush Yadav <pratyush@kernel.org> wrote:
>
>> On Mon, Mar 09 2026, Miquel Raynal wrote:
[...]
>>>
>>> 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.
>>
>> With Tudor's suggestions:
>>
>> Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
>>
>> I am going through the SPI NOR patch backlog. Since you plan to take
>> these directly via mtd/fixes, I'll not queue these.
>
> I was too busy this week to follow-up, but indeed I might just apply
> with the suggestions from Tudor if I'm able to send a fixes PR by
> Saturday, otherwise I'll go for the normal route and submit a v2. Patch
> 2 however is not eligible for -fixes, it clearly is an enhancement that
> is not a bug fix or whatever, so you can take it when I send a v2
> through SPI NOR next, as AFAIR it will not conflict with patch 1.
Sounds good.
--
Regards,
Pratyush Yadav
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread