spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus at microchip.com]

Hi,

+ Vignesh

On 2/18/21 9:43 AM, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor and all,
> 
> Am Di., 16. Feb. 2021 um 12:15 Uhr schrieb <Tudor.Ambarus at microchip.com>:
>>
>> 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.
> 
> I am a newbie but it sounds reasonable. I made a first attempt and

Let's first see if all parties find the idea good (I'll have to double check
it myself). Vignesh and others might help.

Until then can you try the patch form below and see if you can do the
reads in quad mode?

Cheers,
ta

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 0522304f52fa..718d0b75df91 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
        spi_nor_manufacturer_init_params(nor);
 
        if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
+                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
+                                SPI_NOR_AIM_SFDP)) &&
            !(nor->info->flags & SPI_NOR_SKIP_SFDP))
                spi_nor_sfdp_init_params(nor);
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..3495549815e6 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -338,6 +338,11 @@ struct flash_info {
                                         * protection bits. Usually these will
                                         * power-up in a write-protected state.
                                         */
+#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 |