RE: [PATCH v3 05/23] mtd: spi-nor: Create the concept of fixup table with match function

[<Takahiro.Kuwano@infineon.com>]

> Manufacturer ID tables increase and fixup() hooks proliferate. Having
> one possible structure per chip was fine until the fixups started being
> more and more common and needed, to some extend due to ID reuses. Mixing
> fixups and chips becomes hard and requires extra helpers to sort which
> ones are needed for a given chip, which every time this happens requires
> a lot of rework.
> 
> Replace the two-level fixup association (a manufacturer wide hook and a
> per flash_info hook) with a per-manufacturer list of fixups that can be
> looked up by flash ID and/or match function.
> 
> The match logic works as follows:
> - If there is an ID, it must match
> - If there is a match function, it must match (cumulative)
> - If there is no identifier (no ID nor any match function), it's a
>   catch-all entry typically used for flagging manufacturer fixups.
> 
> Suggested-by: Michael Walle <mwalle@kernel.org>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

[...]

> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index ca2703a5a338..f3862eb1a23a 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -905,7 +905,6 @@ static const struct flash_info spansion_nor_parts[] = {
>                 .sector_size = SZ_256K,
>                 .no_sfdp_flags = SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
>                 .mfr_flags = USE_CLSR,
> -               .fixups = &s25fs_s_nor_fixups,
>         }, {
>                 .id = SNOR_ID(0x01, 0x20, 0x18, 0x03, 0x00),
>                 .name = "s25sl12800",
> @@ -941,7 +940,6 @@ static const struct flash_info spansion_nor_parts[] = {
>                 .size = SZ_16M,
>                 .no_sfdp_flags = SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
>                 .mfr_flags = USE_CLSR,
> -               .fixups = &s25fs_s_nor_fixups,
>         }, {
>                 .id = SNOR_ID(0x01, 0x20, 0x18, 0x4d, 0x01),
>                 .name = "s25fl129p1",
> @@ -1001,76 +999,61 @@ static const struct flash_info spansion_nor_parts[] = {
>                 .id = SNOR_ID(0x34, 0x2a, 0x1a, 0x0f, 0x03, 0x90),
>                 .name = "s25hl512t",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s25hx_t_fixups
>         }, {
>                 .id = SNOR_ID(0x34, 0x2a, 0x1b, 0x0f, 0x03, 0x90),
>                 .name = "s25hl01gt",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s25hx_t_fixups
>         }, {
>                 .id = SNOR_ID(0x34, 0x2a, 0x1c, 0x0f, 0x00, 0x90),
>                 .name = "s25hl02gt",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s25hx_t_fixups
>         }, {
>                 .id = SNOR_ID(0x34, 0x2b, 0x19, 0x0f, 0x08, 0x90),
>                 .name = "s25fs256t",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s25fs256t_fixups
>         }, {
>                 .id = SNOR_ID(0x34, 0x2b, 0x1a, 0x0f, 0x03, 0x90),
>                 .name = "s25hs512t",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s25hx_t_fixups
>         }, {
>                 .id = SNOR_ID(0x34, 0x2b, 0x1b, 0x0f, 0x03, 0x90),
>                 .name = "s25hs01gt",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s25hx_t_fixups
>         }, {
>                 .id = SNOR_ID(0x34, 0x2b, 0x1c, 0x0f, 0x00, 0x90),
>                 .name = "s25hs02gt",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s25hx_t_fixups
>         }, {
>                 /* S28HL256T */
>                 .id = SNOR_ID(0x34, 0x5a, 0x19),
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 .id = SNOR_ID(0x34, 0x5a, 0x1a),
>                 .name = "s28hl512t",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 .id = SNOR_ID(0x34, 0x5a, 0x1b),
>                 .name = "s28hl01gt",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 /* S28HL02GT */
>                 .id = SNOR_ID(0x34, 0x5a, 0x1c),
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 .id = SNOR_ID(0x34, 0x5b, 0x19),
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 .id = SNOR_ID(0x34, 0x5b, 0x1a),
>                 .name = "s28hs512t",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 .id = SNOR_ID(0x34, 0x5b, 0x1b),
>                 .name = "s28hs01gt",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 .id = SNOR_ID(0x34, 0x5b, 0x1c),
>                 .name = "s28hs02gt",
>                 .mfr_flags = USE_CLPEF,
> -               .fixups = &s28hx_t_fixups,
>         }, {
>                 .id = SNOR_ID(0xef, 0x40, 0x13),
>                 .name = "s25fl004k",
> @@ -1170,9 +1153,31 @@ static const struct spi_nor_fixups spansion_nor_fixups = {
>         .late_init = spansion_nor_late_init,
>  };
> 
> +static const struct spi_nor_fixup spansion_fixups[] = {
> +       { .fixups = &spansion_nor_fixups },
> +       { .id = SNOR_ID(0x01, 0x02, 0x20, 0x4d, 0x00, 0x81), .fixups = &s25fs_s_nor_fixups },
> +       { .id = SNOR_ID(0x01, 0x20, 0x18, 0x4d, 0x01, 0x81), .fixups = &s25fs_s_nor_fixups },
> +       { .id = SNOR_ID(0x34, 0x2a, 0x1a, 0x0f, 0x03, 0x90), .fixups = &s25hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x2a, 0x1b, 0x0f, 0x03, 0x90), .fixups = &s25hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x2a, 0x1c, 0x0f, 0x00, 0x90), .fixups = &s25hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x2b, 0x19, 0x0f, 0x08, 0x90), .fixups = &s25fs256t_fixups },
> +       { .id = SNOR_ID(0x34, 0x2b, 0x1a, 0x0f, 0x03, 0x90), .fixups = &s25hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x2b, 0x1b, 0x0f, 0x03, 0x90), .fixups = &s25hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x2b, 0x1c, 0x0f, 0x00, 0x90), .fixups = &s25hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5a, 0x19), .fixups = &s28hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5a, 0x1a), .fixups = &s28hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5a, 0x1b), .fixups = &s28hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5a, 0x1c), .fixups = &s28hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5b, 0x19), .fixups = &s28hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5b, 0x1a), .fixups = &s28hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5b, 0x1b), .fixups = &s28hx_t_fixups },
> +       { .id = SNOR_ID(0x34, 0x5b, 0x1c), .fixups = &s28hx_t_fixups },
> +};
> +
>  const struct spi_nor_manufacturer spi_nor_spansion = {
>         .name = "spansion",
>         .parts = spansion_nor_parts,
>         .nparts = ARRAY_SIZE(spansion_nor_parts),
> -       .fixups = &spansion_nor_fixups,
> +       .fixups = spansion_fixups,
> +       .nfixups = ARRAY_SIZE(spansion_fixups),
>  };

With S25FS512S, S25HL512T, and S28HS512T,
Tested-by: Takahiro Kuwano <takahiro.kuwano@infineon.com>

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