[PATCH v4 3/6] mtd: spi-nor: macronix: Handle ID collision b/w MX25L3233F and MX25L3205D

[Tudor.Ambarus at microchip.com]

On 3/1/22 23:57, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2022-02-28 14:45, schrieb Tudor Ambarus:
>> Macronix has a bad habbit of reusing flash IDs. While MX25L3233F
>> supports
>> RDSFDP opcode, MX25L3205D does not support it and does not recommend
>> issuing opcodes that are not supported ("It is not recommended to adopt
>> any other code not in the command definition table, which will
>> potentially
>> enter the hidden mode.").
>>
>> We tested the RDSFDP on the MX25L3205D and the conclusion is that the
>> flash didn't reply anything. Given that it is unlikely that RDSFDP will
>> cause any problems for the old MX25L3205D, differentiate between the
>> two
>> flashes by parsing SFDP.
>>
>> Tested MX25L3233F. Generated a 256 Kbyte random data and did an erase,
>> write, read back and compare test. The flash uses for reads
>> SPINOR_OP_READ_1_4_4 0xeb, for erases SPINOR_OP_BE_4K 0x20, and for
>> writes
>> SPINOR_OP_PP 0x02.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
>> Acked-by: Pratyush Yadav <p.yadav at ti.com>
>> ---
>> #  cat
>> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id
>> c22016
>> # cat
>> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer
>> macronix
>> # cat
>> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/partname
>> mx25l3233f
>> # cat
>> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp
>> > mx25l3233f-sfdp
>>
>>
>> # xxd -p mx25l3233f-sfdp
>> 53464450000101ff00000109300000ffc2000104600000ffffffffffffff
>> ffffffffffffffffffffffffffffffffffffe520f1ffffffff0144eb086b
>> 083b04bbeeffffffffff00ffffff00ff0c200f5210d800ffffffffffffff
>> ffffffffffff003650269cf97764fecfffffffffffff
> 
> Is quad enable working or has this the same problem as
> the macronix flash in patch 4? Judging by the length of the SFDP
> this also lacks the required information to select an
> appropriate enable method. I haven't had closer look though.

it worked, yes. As I specified in the commit message, I tested it and it used
SPINOR_OP_READ_1_4_4 0xeb opcode for reads.

> 
>>
>> # sha1sum mx25l3233f-sfdp
>> 1b6e0f75b4a6d08d570082992455affa72b2dc81  mx25l3233f-sfdp
>>
>>  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 d81a4cb2812b..2754bbef3d2e 100644
>> --- a/drivers/mtd/spi-nor/macronix.c
>> +++ b/drivers/mtd/spi-nor/macronix.c
>> @@ -8,6 +8,24 @@
>>
>>  #include "core.h"
>>
>> +static int mx25l3205d_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: MX25L3233F collides
>> +      * with MX25L3205D. MX25L3233F defines SFDP tables, while the older
>> +      * variant does not.
>> +      */
>> +     nor->name = "mx25l3233f";
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct spi_nor_fixups mx25l3205d_fixups = {
>> +     .post_bfpt = mx25l3205d_post_bfpt_fixups,
>> +};
>> +
>>  static int
>>  mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
>>                           const struct sfdp_parameter_header *bfpt_header,
>> @@ -44,7 +62,10 @@ static const struct flash_info macronix_nor_parts[]
>> = {
>>       { "mx25l1606e",  INFO(0xc22015, 0, 64 * 1024,  32)
>>               NO_SFDP_FLAGS(SECT_4K) },
>>       { "mx25l3205d",  INFO(0xc22016, 0, 64 * 1024,  64)
>> -             NO_SFDP_FLAGS(SECT_4K) },
>> +             /* ID collision with mx25l3233f. */
>> +             PARSE_SFDP
>> +             NO_SFDP_FLAGS(SECT_4K)
>> +             .fixups = &mx25l3205d_fixups },
>>       { "mx25l3255e",  INFO(0xc29e16, 0, 64 * 1024,  64)
>>               NO_SFDP_FLAGS(SECT_4K) },
>>       { "mx25l6405d",  INFO(0xc22017, 0, 64 * 1024, 128)
> 
> -michael