Re: [PATCH v2 1/2] mtd: spi-nor: core: add flag for doing optional SFDP

From: "Michael Walle" <mwalle@kernel.org>
To: "Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"Esben Haabendal" <esben@geanix.com>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>
Cc: <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	"Rasmus Villemoes" <rasmus.villemoes@prevas.dk>
Subject: Re: [PATCH v2 1/2] mtd: spi-nor: core: add flag for doing optional SFDP
Date: Thu, 06 Jun 2024 15:59:01 +0200	[thread overview]
Message-ID: <D1SZKLZBDDBA.1Z7ZD4UEOX05F@kernel.org> (raw)
In-Reply-To: <a379a411-2c9e-4d9d-aa8f-4c4f3463cc27@linaro.org>

[-- Attachment #1.1: Type: text/plain, Size: 1969 bytes --]

On Thu Jun 6, 2024 at 3:31 PM CEST, Tudor Ambarus wrote:
> On 6/3/24 14:09, Esben Haabendal wrote:
> > A dedicated flag for triggering call to
> > spi_nor_sfdp_init_params_deprecated() allows enabling optional SFDP read
> > and parse, with fallback to legacy flash parameters, without having dual,
> > quad or octal parameters set in the legacy flash parameters.
> > 
> > With this, spi-nor flash parts without SFDP that is replaced with a
> > different flash NOR flash part that does have SFDP, but shares the same
> > manufacturer and device ID is easily handled.
> > 
> > Signed-off-by: Esben Haabendal <esben@geanix.com>
> > ---
> >  drivers/mtd/spi-nor/core.c | 3 ++-
> >  drivers/mtd/spi-nor/core.h | 1 +
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 3e1f1913536b..1c4d66fc993b 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2933,7 +2933,8 @@ static void spi_nor_init_params_deprecated(struct spi_nor *nor)
> >  
> >  	spi_nor_manufacturer_init_params(nor);
> >  
> > -	if (nor->info->no_sfdp_flags & (SPI_NOR_DUAL_READ |
> > +	if (nor->info->no_sfdp_flags & (SPI_NOR_TRY_SFDP |
>
> I don't like that we update deprecated methods. The solution though is
> elegant.

I actually had the same concern. But currently there is no
non-deprecated way to handle this case, right?

Right now we have the following cases:
 (1) pure SFDP parsing
 (2) non-SFDP flashes with static configuration only
 (3) legacy implementation, where the magic flags decide whether we
     use SFDP

Which case is eventually used depends on the ID of the flash -
assuming there will only be IDs which either fall into (1) *or* (2).
That assumption is clearly wrong :)

I'd propose a new case in spi_nor_init_params()
 (4) try SFDP with a fallback to the static flags from the
     flash_info db.

-michael

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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