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

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	 Tudor Ambarus <tudor.ambarus@linaro.org>,
	 Pratyush Yadav <pratyush@kernel.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: Mon, 09 Mar 2026 15:55:28 +0100	[thread overview]
Message-ID: <873429oxwf.fsf@bootlin.com> (raw)
In-Reply-To: <20260108121430.1096844-1-miquel.raynal@bootlin.com> (Miquel Raynal's message of "Thu, 8 Jan 2026 13:14:29 +0100")

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.

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/