Re: spi-nor: maxronix MX25L12835F support

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>
To: Heiko Thiery <heiko.thiery@gmail.com>,
	Vignesh Raghavendra <vigneshr@ti.com>
Cc: Tudor.Ambarus@microchip.com, Michael Walle <michael@walle.cc>,
	p.yadav@ti.com, ycllin@mxic.com.tw, zhengxunli@mxic.com.tw,
	juliensu@mxic.com.tw, linux-mtd@lists.infradead.org
Subject: Re: spi-nor: maxronix MX25L12835F support
Date: Fri, 19 Mar 2021 15:33:41 +0100	[thread overview]
Message-ID: <ee7bf6f7-dccf-2ff2-bb70-2b819997002b@weidmueller.com> (raw)
In-Reply-To: <CAEyMn7b+vHNiSRJTP2Qs3doK2n3xgCQm9YFBCYEn09GWLehQ0Q@mail.gmail.com>

Hi,

Am 04.03.2021 um 08:10 schrieb Heiko Thiery:
> Hi Vignesh,
> 
> Am Do., 4. März 2021 um 08:02 Uhr schrieb Vignesh Raghavendra <vigneshr@ti.com>:
>>
>>
>>
>> On 3/3/21 7:14 PM, Heiko Thiery wrote:
>>> Hi Vignesh,
>>>
>> [...]
>>>>>>>
>>>>>>> How about naming them something like "updated-flash-name ||
>>>>>>> first-name".
>>>>>>> Anyway, these are just workarounds. Manufacturers shouldn't use the
>>>>>>> same
>>>>>>> JEDEC ID for new flashes. They should at least add an extended ID.
>>>>>>
>>>>>> Mh, what about a list of names? I mean yes it is a workaround, but
>>>>>> there is actual hardware doing this, so IMHO linux has to deal with
>>>>>> it in some way. OTOH that list might be long and doesn't look good
>>>>>> in dmesg (or wherever that string might be used).
>>>>>>
>>>>>> It might come in handy to have a mechanism in place if someone
>>>>>> really cares about it though.
>>>>>>
>>>>>
>>>>> A list of names with differentiation at run-time, where possible,
>>>>> sounds good. Otherwise we'll stick to a default name, whatever that
>>>>> will be. Do you care to scratch a patch for the list of names idea?
>>>>>
>>>>> We'll still have a single flash entry, with a list of names, and we
>>>>> still need to either do the SFDP detection first, or to trigger the
>>>>> SFDP detection with an explicit flash info flag. I'll follow Pratyush's
>>>>> steps and evaluate the "detect SFDP first" idea.
>>>>>
>>>>
>>>> If we do go down the road of "detect SFDP first", we should add
>>>> SPI_NOR_SKIP_SFDP to flashes that currently don't claim DUAL/QUAD/OCTAL
>>>> capability currently in order to avoid any surprises due to wrong values
>>>> in the table.
>>>
>>> Does this mean that all entries that have DUAL/QUAD/OCTAL defined can
>>> have them removed And the correct values will be detected/set by SFDP?
>>>
>>
>> No, not all Dual/Quad/Octal SPI flashes have SFDP tables populated.
>> Removing DUAL/QUAD/OCTAL capabilities for a flash that does not have
>> SFDP tables populated (or has wrong values) will cause regression as
>> code may fallback to legacy SPI mode.
> 
> So in that case removing the flags can only be done on chips/flashes
> that are checked and reviewed for a valid functional SFDP detection.
> 

Isn't the assumption wrong that legacy flashes doesn't support SFDP? At 
the moment the driver enables SFDP only if the flash supports 
DUAL/QUAD/OCTAL mode. We should enable it for all flashes or allow the 
legacy flashes to explicit enable it.

What is the meaning of SPI_NOR_SKIP_SFDP? Is this a leftover and should 
be replaced by post_sfdp fixup?

What is the advantage of SFDP first? Because of backward compatibility 
you have to check the flash info data and you need a flag for compatible 
or incompatible flashes.

Independently of the SFDP read position we could add the 
SPI_NOR_SKIP_SFDP flag to all legacy flashes and enable SFDP for all 
flashes. Or we could remove the SPI_NOR_SKIP_SFDP flag and add an 
SPI_NOR_SFDP flag to explicitly enable SFDP.

Kind regards
   Stefan

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