[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e079751d-e775-4dec-9959-c9c3fdcc8ecf@linaro.org>
Date: Thu, 26 Sep 2024 11:08:25 +0100
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Esben Haabendal <esben@...nix.com>, Pratyush Yadav <pratyush@...nel.org>,
Michael Walle <mwalle@...nel.org>, Miquel Raynal
<miquel.raynal@...tlin.com>, Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
Rasmus Villemoes <rasmus.villemoes@...vas.dk>,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 00/15] mtd: spi-nor: macronix: workaround for device id
re-use
Hiya, Esben,
On 7/11/24 2:00 PM, Esben Haabendal wrote:
> Following up to various discussions, this series have now been
> modified so that it gets rid of the old deprecated approach
> for detecting when to do optional SFDP parsing.
>
> Before these changes, spi-nor flashes were handled in 4 different
> ways:
>
I'm adding a bit of extra context on each point you made. All your
points contain static init, you missed spi_nor_init_default_params()
There's a 0/ case where we have indeed just SFDP initialized flashes,
and that is the generic flash driver.
0/ SFDP only, generic flash driver
> (1) SFDP only [size==0]
1/ static + SFDP
spi_nor_init_default_params(nor);
spi_nor_parse_sfdp();
>
> (2a) static config only [size!=0 && no_sfdp_flags & SPI_NOR_SKIP_SFDP]
2a/ is
spi_nor_init_default_params(nor);
spi_nor_no_sfdp_init_params(nor);
>
> (2b) static config only
> [size!=0 &&
> !(no_sfdp_flags & (SPI_NOR_SKIP_SFDP |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ))]
2b/ is
spi_nor_init_default_params(nor);
spi_nor_init_params_deprecated(nor); //where parse SFDP is not called
spi_nor_no_sfdp_init_params(nor);
spi_nor_manufacturer_init_params(nor);>
> (3) SFDP with fallback to static config
> [size!=0 &&
> !(no_sfdp_flags & SPI_NOR_SKIP_SFDP) &&
> (no_sfdp_flags & SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ))]
>
3/ is 2b/ with parse SFDP called and rollback mechanism
spi_nor_init_default_params(nor);
spi_nor_init_params_deprecated(nor);
spi_nor_no_sfdp_init_params(nor);
spi_nor_manufacturer_init_params(nor);
spi_nor_sfdp_init_params_deprecated(nor);
All cases from above are followed by a call to spi_nor_late_init_params().
> Cases (2a) and (2b) have been handled slightly different, with
> manufacturer and flash_info ->default_init() hooks being called in
> case (2b) but not in case (2a).
default_init() was a mistake and we shall remove it and replace it with
late_init(). The challenge is to do that without affecting backwards
compatibility. But let's move this aside for the moment
>
> With this series, that is changed to this simpler approach instead:
>
> (1) SFDP only [size==0]
>
> (2) static config only
> [size!=0 && !(no_sfdp_flags & SPI_NOR_TRY_SFDP)]
>
> (3) SFDP with fallback to static config
> [size!=0 && (no_sfdp_flags & SPI_NOR_TRY_SFDP)]
>
> Existing struct flash_info entries are modified, so that all those
> that was case (2a) or (2b) are now case (2), and those that were (1)
> and (3) are still (1) and (3).
We indeed want 2a/ and 2b/ to be squashed, ideally by removing the
default_init() hook.
And if we really want SFDP-only init, we shall not call
spi_nor_init_default_params() in this case.
Your patches do not apply on latest SPI NOR, I tried locally. Can you
point me to a branch of yours where I can see them? No need to resend.
Thanks,
ta
Powered by blists - more mailing lists