[PATCH v4 4/6] mtd: spi-nor: macronix: Handle ID collision b/w MX25L12805D and MX25L12835F

[Tudor.Ambarus at microchip.com]

On 3/1/22 09:55, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,

Hi!

> 
> Am Mo., 28. Feb. 2022 um 14:45 Uhr schrieb Tudor Ambarus
> <tudor.ambarus at microchip.com>:
>>
>> Macronix has a bad habbit of reusing flash IDs. While MX25L12835F supports
>> RDSFDP opcode, MX25L12805D does not. Given that it is unlikely that RDSFDP
>> will cause any problems for the old MX25L12805D, differentiate between the
>> two flashes by parsing SFDP.
>>
>> cc: Heiko Thiery <heiko.thiery at gmail.com>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
>> Acked-by: Pratyush Yadav <p.yadav at ti.com>
>> ---
>> # cat /sys/devices/platform/soc at 0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0
>> .0/spi-nor/sfdp | xxd -p
>> 53464450000101ff00000109300000ffc2000104600000ffffffffffffff
>> ffffffffffffffffffffffffffffffffffffe520f1ffffffff0744eb086b
>> 083b04bbfeffffffffff00ffffff44eb0c200f5210d800ffffffffffffff
>> ffffffffffff003600279df9c06485cbffffffffffff
>>
>>  drivers/mtd/spi-nor/macronix.c | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
>> index 2754bbef3d2e..45c2f2c50e56 100644
>> --- a/drivers/mtd/spi-nor/macronix.c
>> +++ b/drivers/mtd/spi-nor/macronix.c
>> @@ -26,6 +26,24 @@ static const struct spi_nor_fixups mx25l3205d_fixups = {
>>         .post_bfpt = mx25l3205d_post_bfpt_fixups,
>>  };
>>
>> +static int mx25l12805d_post_bfpt_fixups(struct spi_nor *nor,
>> +                               const struct sfdp_parameter_header *bfpt_header,
>> +                               const struct sfdp_bfpt *bfpt)
>> +{
>> +       /*
>> +        * Macronix has a bad habit of reusing flash IDs: MX25L12835F collides
>> +        * with MX25L12805D. MX25L12835F defines SFDP tables, while the older
>> +        * variant does not.
>> +        */
>> +       nor->name = "mx25l12835f";
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct spi_nor_fixups mx25l12805d_fixups = {
>> +       .post_bfpt = mx25l12805d_post_bfpt_fixups,
>> +};
>> +
>>  static int
>>  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
>>                             const struct sfdp_parameter_header *bfpt_header,
>> @@ -82,8 +100,11 @@ static const struct flash_info macronix_nor_parts[] = {
>>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128)
>>                 NO_SFDP_FLAGS(SECT_4K) },
>>         { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256)
>> +               /* ID collision with mx25l12835f. */
>> +               PARSE_SFDP
>>                 FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP)
>> -               NO_SFDP_FLAGS(SECT_4K) },
>> +               NO_SFDP_FLAGS(SECT_4K)
>> +               .fixups = &mx25l12805d_fixups },
>>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256) },
>>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32)
>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
>> --
>> 2.25.1
>>
> 
> I tried this patch and saw that the flash is no longer detected. I did

Thanks! No longer detected? ReadID fails? What do you get on the console?

> some debugging and see now that the correct function to set the quad
> mode (spi_nor_sr1_bit6_quad_enable) is not called. Instead the
> spi_nor_sr2_bit1_quad_enable() is invoked. Further debbuging showed me
> that the macronix specific fixup is not called.
> 

ok, this bit is clear

> For the flash that does support SFDP parsing the
> spi_nor_manufacturer_init_params() is not called. Is that expected to
> be?

Yes, I would like to get rid of the default_init() hooks for new code.
I'm addressing it right now. Would be good if you can help testing it.
Will add you in to:

Cheers,
ta