Re: spi-nor: maxronix MX25L12835F support

From: Vignesh Raghavendra <vigneshr@ti.com>
To: Heiko Thiery <heiko.thiery@gmail.com>, <Tudor.Ambarus@microchip.com>
Cc: juliensu@mxic.com.tw, ycllin@mxic.com.tw,
	Michael Walle <michael@walle.cc>,
	linux-mtd@lists.infradead.org, p.yadav@ti.com,
	zhengxunli@mxic.com.tw
Subject: Re: spi-nor: maxronix MX25L12835F support
Date: Mon, 1 Mar 2021 16:22:52 +0530	[thread overview]
Message-ID: <9263ef4d-7944-00bb-8e2f-b7d33e5aab87@ti.com> (raw)
In-Reply-To: <CAEyMn7bY9CisxoWcpF=zR+DYJpW+=EeWLGE_gAB3WK9hvkcuJQ@mail.gmail.com>

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?

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.

Regards
Vignesh

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