Re: [PATCH v4 5/5] mtd: spi-nor: micron-st: add comment for mt35xu02gcba

From: Pratyush Yadav <pratyush@kernel.org>
To: Haibo Chen <haibo.chen@nxp.com>
Cc: Tudor Ambarus <tudor.ambarus@linaro.org>,
	 Pratyush Yadav <pratyush@kernel.org>,
	 Michael Walle <mwalle@kernel.org>,
	 Miquel Raynal <miquel.raynal@bootlin.com>,
	 Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	 linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	 imx@lists.linux.dev
Subject: Re: [PATCH v4 5/5] mtd: spi-nor: micron-st: add comment for mt35xu02gcba
Date: Thu, 13 Nov 2025 15:47:09 +0100	[thread overview]
Message-ID: <mafs08qga9ehe.fsf@kernel.org> (raw)
In-Reply-To: <20251112-nor-v4-5-e4637be82a0a@nxp.com> (Haibo Chen's message of "Wed, 12 Nov 2025 19:05:13 +0800")

On Wed, Nov 12 2025, Haibo Chen wrote:

> The MT35XU02GCBA flash device does not support chip erase,
> according to its datasheet. It supports die erase, which
> means the current driver implementation will likely need
> to be converted to use die erase.
>
> Furthermore, similar to the MT35XU01GBBA, the
> SPI_NOR_IO_MODE_EN_VOLATILE flag probably needs to be enabled.
>
> Link: https://datasheet.octopart.com/MT35XU02GCBA1G12-0AAT-Micron-datasheet-138896808.pdf
> Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mtd/spi-nor/micron-st.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index c89c06b1fc61a581fea2e18732be2501a15715f9..f94e9d2d17bf4aa7c36ba3aa37d34f767a9f93ac 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -204,6 +204,16 @@ static const struct flash_info micron_nor_parts[] = {
>  		.fixup_flags = SPI_NOR_IO_MODE_EN_VOLATILE,
>  		.fixups = &mt35xu01gbba_fixups,
>  	}, {
> +		/*
> +		 * The MT35XU02GCBA flash device does not support
> +		 * chip erase, according to its datasheet.
> +		 * It supports die erase, which means the current
> +		 * driver implementation will likely need to be
> +		 * converted to use die erase.
> +		 * Furthermore, similar to the MT35XU01GBBA, the
> +		 * SPI_NOR_IO_MODE_EN_VOLATILE flag probably needs
> +		 * to be enabled.
> +		 */

Maybe I am missing some context from previous patches, but why are we
adding this comment here instead of fixing the support for this flash?
What does "the driver will likely need to be converted to use die erase"
mean? If it doesn't support chip erase, then it _does_ need to be fixed
by using die erase.

Also, why does the flash "probably" need SPI_NOR_IO_MODE_EN_VOLATILE?
Are you guessing based on datasheet and do not have the hardware at
hand?

The changelog should also explain _why_ this comment is added here, and
not just repeat the text.

>  		.id = SNOR_ID(0x2c, 0x5b, 0x1c),
>  		.name = "mt35xu02g",
>  		.sector_size = SZ_128K,

-- 
Regards,
Pratyush Yadav

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/