Re: spi-nor: maxronix MX25L12835F support

[<Tudor.Ambarus@microchip.com>]

On 2/18/21 12:15 PM, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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
> 
> 

Great, thanks!

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

Let's wait for a few days, so others can intervene. I'd like to
clarify what's happening on mx66l51235l too.

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