Re: spi-nor: maxronix MX25L12835F support

[<Tudor.Ambarus@microchip.com>]

On 3/1/21 3:36 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 01/03/21 11:11AM, Tudor.Ambarus@microchip.com wrote:
>> 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?
> 
> I think printing the correct flash name is somewhat important. Other
> than the handful of people who are reading this thread, few would know
> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
> cause a lot of confusion among people trying to debug any issues.
> 
> So my vote goes for having separate entries for both the flashes and
> then adding a fixup hook to select the correct one by checking if SFDP
> read works.
> 

You can't have separate flash entries with the same JEDEC ID, because
you will always hit the first one that it is defined in the array.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/