Re: spi-nor: maxronix MX25L12835F support

Heiko Thiery
2021-02-18T10:16:32Z

Hi Tudor,

[...]

> >> Thinking loud, now we do a static initialization of flash params, that
> >> can be overwritten dynamically by SFDP. How about doing the params init
> >> the other way around. Try first to dynamically discover the params via
> >> SFDP, and if SFDP fails or if it is not defined, do the static init via
> >> flags. That would spare some code. And new flash IDs will have less flags
> >> declared, and we'll better track faulty SFDP flashes.
> >
> > I am a newbie but it sounds reasonable. I made a first attempt and
>
> Let's first see if all parties find the idea good (I'll have to double check
> it myself). Vignesh and others might help.
>
> Until then can you try the patch form below and see if you can do the
> reads in quad mode?
>
> Cheers,
> ta
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 0522304f52fa..718d0b75df91 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
> spi_nor_manufacturer_init_params(nor);
>
> if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> - SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
> + SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
> + SPI_NOR_AIM_SFDP)) &&
> !(nor->info->flags & SPI_NOR_SKIP_SFDP))
> spi_nor_sfdp_init_params(nor);
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 4a3f7f150b5d..3495549815e6 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -338,6 +338,11 @@ struct flash_info {
> * protection bits. Usually these will
> * power-up in a write-protected state.
> */
> +#define SPI_NOR_AIM_SFDP BIT(23) /* Try to parse SFDP. Used by flashes
> + * that share the same JEDEC-ID, but
> + * where a flash defines the SFDP tables
> + * and the other doesn't.
> + */
>
> /* Part specific fixup hooks. */
> const struct spi_nor_fixups *fixups;
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 9203abaac229..1ebce775eae4 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
> { "mx25u4035", INFO(0xc22533, 0, 64 * 1024, 8, SECT_4K) },
> { "mx25u8035", INFO(0xc22534, 0, 64 * 1024, 16, SECT_4K) },
> { "mx25u6435f", INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
> - { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
> + { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
> + SECT_4K | SPI_NOR_AIM_SFDP) },
> { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
> { "mx25r1635f", INFO(0xc22815, 0, 64 * 1024, 32,
> SECT_4K | SPI_NOR_DUAL_READ |

I tried your patch and it works like expected. I can now read the
whole flash in ~2sec while without that it was ~6sec.

# time dd if=/dev/mtd0 of=dump.bin
32768+0 records in
32768+0 records out
real 0m 2.08s
user 0m 0.01s
sys 0m 2.06s

vs.

# time dd if=/dev/mtd0 of=dump.bin
32768+0 records in
32768+0 records out
real 0m 6.16s
user 0m 0.05s
sys 0m 6.09s

Should I prepare a patch with that change or will you do?

Thank you

--
Heiko