[RFC PATCH 3/3] mtd: spi-nor: rework flash parameter initialization
Tudor Ambarus
tudor.ambarus at linaro.org
Fri Jun 5 06:17:02 PDT 2026
On 6/1/26 3:52 PM, Michael Walle wrote:
> Rework how the flash parameters are initialized. It used to make a
> difference whether a flash is in the in-kernel database, whether it
> supports multi I/O and so on. Recently, that makes more and more
> problems because flash IDs are reused among legacy flashes (w/o SFDP
> support) and newer flashes [1,2,3,4].
>
> Simplify the whole parameter initialization, by starting with the
> parameters we have in the in-kernel database and the try to parse SFDP
> (and let it override any settings).
You need to specify here as well about the potential problem of issuing
an unsupported command (rdsfdp), although practice showed us it's okay.
I can't remember who raised first the potential of issuing unsupported
commands. Was it macronix, sst?
>
> [1] https://lore.kernel.org/linux-mtd/CAAyq3SYX9UPwhC_Ume_S2yxhQwimRvB=Y6O_+FFqokhNmw7jQg@mail.gmail.com/
> [2] https://lore.kernel.org/linux-mtd/DGB4745HRCFI.1DRYTHXURWZJO@kernel.org/
> [3] https://lore.kernel.org/linux-mtd/DD10GE4EOCD7.CPTN7198QFUV@kernel.org/
> [4] https://lore.kernel.org/linux-mtd/DD6SI06QNEE4.2YCRTWJHEAAQM@kernel.org/
>
> Signed-off-by: Michael Walle <mwalle at kernel.org>
> ---
> drivers/mtd/spi-nor/core.c | 65 +++++++++++++-------------------------
> drivers/mtd/spi-nor/core.h | 1 -
> 2 files changed, 22 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index fbf8c2d9c6b5..67e0377b606f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2868,11 +2868,10 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
>
> /**
> * spi_nor_no_sfdp_init_params() - Initialize the flash's parameters and
> - * settings based on nor->info->sfdp_flags. This method should be called only by
> - * flashes that do not define SFDP tables. If the flash supports SFDP but the
> - * information is wrong and the settings from this function can not be retrieved
> - * by parsing SFDP, one should instead use the fixup hooks and update the wrong
> - * bits.
> + * settings based on nor->info->sfdp_flags.
> + * If the flash supports SFDP but the information is wrong and the settings from
> + * this function can not be retrieved by parsing SFDP, one should instead use
> + * the fixup hooks and update the wrong bits.
> * @nor: pointer to a 'struct spi_nor'.
> */
> static void spi_nor_no_sfdp_init_params(struct spi_nor *nor)
> @@ -3053,43 +3052,25 @@ static int spi_nor_late_init_params(struct spi_nor *nor)
> }
>
> /**
> - * spi_nor_sfdp_init_params_deprecated() - Deprecated way of initializing flash
> - * parameters and settings based on JESD216 SFDP standard.
> + * spi_nor_try_parse_sfdp() - Tries to parse flash parameters based
> + * on JESD216 SFDP standard.
> * @nor: pointer to a 'struct spi_nor'.
> *
> * The method has a roll-back mechanism: in case the SFDP parsing fails, the
> - * legacy flash parameters and settings will be restored.
> + * flash parameters and settings will be restored.
> */
> -static void spi_nor_sfdp_init_params_deprecated(struct spi_nor *nor)
> +static int spi_nor_try_parse_sfdp(struct spi_nor *nor)
> {
> struct spi_nor_flash_parameter sfdp_params;
> + int ret;
>
> memcpy(&sfdp_params, nor->params, sizeof(sfdp_params));
>
> - if (spi_nor_parse_sfdp(nor))
> + ret = spi_nor_parse_sfdp(nor);
> + if (ret)
> memcpy(nor->params, &sfdp_params, sizeof(*nor->params));
> -}
> -
> -/**
> - * spi_nor_init_params_deprecated() - Deprecated way of initializing flash
> - * parameters and settings.
> - * @nor: pointer to a 'struct spi_nor'.
> - *
> - * The method assumes that flash doesn't support SFDP so it initializes flash
> - * parameters in spi_nor_no_sfdp_init_params() which later on can be overwritten
> - * when parsing SFDP, if supported.
> - */
> -static void spi_nor_init_params_deprecated(struct spi_nor *nor)
> -{
> - spi_nor_no_sfdp_init_params(nor);
>
> - spi_nor_manufacturer_init_params(nor);
> -
> - if (nor->info->no_sfdp_flags & (SPI_NOR_DUAL_READ |
> - SPI_NOR_QUAD_READ |
> - SPI_NOR_OCTAL_READ |
> - SPI_NOR_OCTAL_DTR_READ))
> - spi_nor_sfdp_init_params_deprecated(nor);
> + return ret;
> }
>
> /**
> @@ -3152,7 +3133,8 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
> *
> * 1/ Default flash parameters initialization. The initializations are done
> * based on nor->info data:
> - * spi_nor_info_init_params()
> + * spi_nor_init_default_params()
> + * spi_nor_no_sfdp_init_params()
> *
> * which can be overwritten by:
> * 2/ Manufacturer flash parameters initialization. The initializations are
> @@ -3163,7 +3145,7 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
> * which can be overwritten by:
> * 3/ SFDP flash parameters initialization. JESD216 SFDP is a standard and
> * should be more accurate that the above.
> - * spi_nor_parse_sfdp() or spi_nor_no_sfdp_init_params()
> + * spi_nor_try_parse_sfdp()
> *
> * Please note that there is a ->post_bfpt() fixup hook that can overwrite
> * the flash parameters and settings immediately after parsing the Basic
> @@ -3189,17 +3171,14 @@ static int spi_nor_init_params(struct spi_nor *nor)
> return -ENOMEM;
>
> spi_nor_init_default_params(nor);
> + spi_nor_no_sfdp_init_params(nor);
> + spi_nor_manufacturer_init_params(nor);
>
> - if (spi_nor_needs_sfdp(nor)) {
> - ret = spi_nor_parse_sfdp(nor);
> - if (ret) {
> - dev_err(nor->dev, "BFPT parsing failed. Please consider using SPI_NOR_SKIP_SFDP when declaring the flash\n");
> - return ret;
> - }
> - } else if (nor->info->no_sfdp_flags & SPI_NOR_SKIP_SFDP) {
> - spi_nor_no_sfdp_init_params(nor);
> - } else {
> - spi_nor_init_params_deprecated(nor);
> + ret = spi_nor_try_parse_sfdp(nor);
I find it fine. I'm concerned about the inconsistent rollback
mechanism, but this is a preexisting problem.
Reviewed-by: Tudor Ambarus <tudor.ambarus at linaro.org>
> + if (ret && spi_nor_needs_sfdp(nor)) {
> + dev_err(nor->dev,
> + "SFDP parsing failed. You need to manually declare the flash parameters.\n");
> + return ret;
> }
>
> ret = spi_nor_late_init_params(nor);
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 2ebc5c3caca1..3d7c31bff5cb 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -548,7 +548,6 @@ struct flash_info {
> #define SPI_NOR_HAS_CMP BIT(10)
>
> u8 no_sfdp_flags;
> -#define SPI_NOR_SKIP_SFDP BIT(0)
sashiko says you need to grep this further and remove all instances.
> #define SECT_4K BIT(1)
> #define SPI_NOR_DUAL_READ BIT(3)
> #define SPI_NOR_QUAD_READ BIT(4)
More information about the linux-mtd
mailing list