Re: spi-nor: maxronix MX25L12835F support

From: <Tudor.Ambarus@microchip.com>
To: <michael@walle.cc>, <p.yadav@ti.com>, <ycllin@mxic.com.tw>,
	<zhengxunli@mxic.com.tw>, <juliensu@mxic.com.tw>
Cc: heiko.thiery@gmail.com, linux-mtd@lists.infradead.org
Subject: Re: spi-nor: maxronix MX25L12835F support
Date: Tue, 16 Feb 2021 11:15:33 +0000	[thread overview]
Message-ID: <92b5b932-a672-9fb6-c604-5263a0668eb3@microchip.com> (raw)
In-Reply-To: <d7e81d38064ba59a24ec243cb84a1d97@walle.cc>

Hi, all,

+zhengxunli, juliensu & ycllin

On 2/16/21 11:48 AM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2021-02-16 10:27, schrieb Pratyush Yadav:
>> On 15/02/21 10:53PM, Heiko Thiery wrote:
>>> Hi all,
>>>
>>> I faced an issue with a SPI flash on our board. We use a macronix
>>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
>>> the MX25L12805D [2].
>>>
>>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
>>> while the older doesn't.
>>>
>>> I thought that I could do a fixup with a device specific
>>> post_bfpt_fixups() call but by now this seems not possible. The older
>>> MX25L12805D has no flags set that allows a call to
>>> spi_nor_sfdp_init_params() and implements the fixup.
>>>
>>> Has anyone an idea how to solve this?

Maybe macronix can help with some suggestions on how to differentiate
between flashes at runtime.

My first thought is to introduce a SPI_NOR_HAS_SFDP flag. For the flash
that doesn't support SFDP tables, there should be no functional change,
for the one that support SFDP it should fill the properties from the
SFDP tables.

>>
>> The post_sfdp fixup is always run regardless of whether the flash has
>> SFDP or not. You can try putting your flash-specific fixups there.
> 
> Well the problem here is, that the SFDP setup is skipped though the
> flash would support SFDP. If the jedec id wasn't already in the table,
> there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> parsed. But because there is already the legacy device (which likely
> doesn't support SFDP) it really doesn't fit.
> 
> Its unclear to me, why the SFDP is only parsed if one of the
> SPI_NOR_*_READ flags are set.

My guess is that a new SFDP flag was not necessary. SFDP defines multiple
tables, but there is just one that is mandatory, BFPT. BFPT defines DUAL
and QUAD parameters. From the spi-nor code, a BFPT without DUAL or QUAD
support doesn't make sense, even though DUAL or QUAD are not mandatory
in BFPT as I see in the standard. So probably it was just a way to avoid
adding a extra flag. We have to check the git history for a more accurate
description, this was just a guess.

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.

Cheers,
ta

> 
>>> [1]
>>> https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
>>> [2]
>>> https://www.mxic.com.tw/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf
>>
>> Both these links are broken.
> 
> mh, they work for me.
> 
> -michael
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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