Re: spi-nor: maxronix MX25L12835F support

[<Tudor.Ambarus@microchip.com>]

On 3/1/21 12:52 PM, Vignesh Raghavendra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi,
> 
> On 2/28/21 3:22 AM, Heiko Thiery wrote:
> 
> [...]
>>>>> +#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.
>>
>> Since a few days have passed and no one has commented, I would like to
>> bring up the subject again.
>>
>> I can send a patch for the changes you suggested. What do you think?
>>
> 
> Why not have a single entry for mx66l51235l/mx25l12805d with superset
> capabilities declared. And then use info->fixups->post_sfdp() to fixup
> capabilities for mx66l51235l based on absence of SFDP tables?

do you mean to add SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ flags in order to
trigger the SFDP parsing and then to undo the read init that is done in
spi_nor_info_init_params()? Too much hustle.

> 
> SPI_NOR_AIM_SFDP seems redundant to me. SPI NOR Framework should anyway
> be using SFDP for detecting flash capabilities and away from flash_info
> based static data.

Yeah, that's what I suggested a bit earlier in the thread. To first try to
detect the caps by parsing SFDP and then if SFDP not supported then to do
the static init via the flash flags. I'll have to check the implications,
it will impact every flash.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/