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

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Pratyush Yadav <pratyush@kernel.org>
Cc: Richard Weinberger <richard@nod.at>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	 Tudor Ambarus <tudor.ambarus@linaro.org>,
	 Michael Walle <michael@walle.cc>,
	 <linux-mtd@lists.infradead.org>,
	 Jakub Czapiga <czapiga@google.com>,
	 Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Takahiro Kuwano <takahiro.kuwano@infineon.com>
Subject: Re: [PATCH 1/2] mtd: spi-nor: Fix RDCR controller capability core check
Date: Fri, 13 Mar 2026 15:34:41 +0100	[thread overview]
Message-ID: <87ms0bkdby.fsf@bootlin.com> (raw)
In-Reply-To: <2vxz4imkf0hx.fsf@kernel.org> (Pratyush Yadav's message of "Fri, 13 Mar 2026 11:10:50 +0000")

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/