| Message ID | 20210727045222.905056-4-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: spi-nor: Handle ID collisions and clean params init | expand |
Am 2021-07-27 06:51, 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@microchip.com> > Acked-by: Pratyush Yadav <p.yadav@ti.com> > --- > root@sama5d2-xplained:~# find / -iname spi-nor > /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor > /sys/devices/platform/ahb/ahb:apb/f8000000.spi/spi_master/spi0/spi0.0/spi-nor > /sys/bus/spi/drivers/spi-nor > root@sama5d2-xplained:~# ls -al > /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor > total 0 > drwxr-xr-x 2 root root 0 Mar 9 14:51 . > drwxr-xr-x 6 root root 0 Mar 9 14:50 .. > -r--r--r-- 1 root root 4096 Mar 9 14:51 jedec_id > -r--r--r-- 1 root root 4096 Mar 9 14:51 manufacturer > -r--r--r-- 1 root root 4096 Mar 9 14:51 partname > -r--r--r-- 1 root root 0 Mar 9 14:51 sfdp > root@sama5d2-xplained:~# cat > /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id > c22016 > root@sama5d2-xplained:~# cat > /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer > macronix > root@sama5d2-xplained:~# cat > /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/partname > mx25l3233f > root@sama5d2-xplained:~# cat > /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp > > mx25l3233f-sfdp > root@sama5d2-xplained:~# hexdump mx25l3233f-sfdp use xxd if possible and the sha1sum/md5sum is missing. > 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 > 0000010 00c2 0401 0060 ff00 ffff ffff ffff ffff > 0000020 ffff ffff ffff ffff ffff ffff ffff ffff > 0000030 20e5 fff1 ffff 01ff eb44 6b08 3b08 bb04 > 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f > 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff > 0000060 3600 2650 f99c 6477 cffe ffff ffff ffff > 0000070 > > 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 27498ed0cc0d..68f6ac060bc6 100644 > --- a/drivers/mtd/spi-nor/macronix.c > +++ b/drivers/mtd/spi-nor/macronix.c > @@ -8,6 +8,24 @@ > > #include "core.h" > > +static int mx25l3233f_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 struct spi_nor_fixups mx25l3233f_fixups = { > + .post_bfpt = mx25l3233f_post_bfpt_fixups, > +}; > + > static int > mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > const struct sfdp_parameter_header *bfpt_header, > @@ -39,7 +57,10 @@ static const struct flash_info macronix_parts[] = { > { "mx25l4005a", INFO(0xc22013, 0, 64 * 1024, 8, SECT_4K) }, > { "mx25l8005", INFO(0xc22014, 0, 64 * 1024, 16, 0) }, > { "mx25l1606e", INFO(0xc22015, 0, 64 * 1024, 32, SECT_4K) }, > - { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, SECT_4K) }, > + { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, SPI_NOR_PARSE_SFDP > | > + SECT_4K) > + /* ID collision with mx25l3233f. */ > + .fixups = &mx25l3233f_fixups }, Shouldn't we use mx25l3205d_fixups as name here? What if there are more flashes with the same id. Using the name of the colliding flash here doesn't really scale. -michael > { "mx25l3255e", INFO(0xc29e16, 0, 64 * 1024, 64, SECT_4K) }, > { "mx25l6405d", INFO(0xc22017, 0, 64 * 1024, 128, SECT_4K) }, > { "mx25u2033e", INFO(0xc22532, 0, 64 * 1024, 4, SECT_4K) },
On 8/24/21 1:42 AM, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2021-07-27 06:51, 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@microchip.com> >> Acked-by: Pratyush Yadav <p.yadav@ti.com> >> --- >> root@sama5d2-xplained:~# find / -iname spi-nor >> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor >> /sys/devices/platform/ahb/ahb:apb/f8000000.spi/spi_master/spi0/spi0.0/spi-nor >> /sys/bus/spi/drivers/spi-nor >> root@sama5d2-xplained:~# ls -al >> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor >> total 0 >> drwxr-xr-x 2 root root 0 Mar 9 14:51 . >> drwxr-xr-x 6 root root 0 Mar 9 14:50 .. >> -r--r--r-- 1 root root 4096 Mar 9 14:51 jedec_id >> -r--r--r-- 1 root root 4096 Mar 9 14:51 manufacturer >> -r--r--r-- 1 root root 4096 Mar 9 14:51 partname >> -r--r--r-- 1 root root 0 Mar 9 14:51 sfdp >> root@sama5d2-xplained:~# cat >> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id >> c22016 >> root@sama5d2-xplained:~# cat >> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer >> macronix >> root@sama5d2-xplained:~# cat >> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/partname >> mx25l3233f >> root@sama5d2-xplained:~# cat >> /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp >> > mx25l3233f-sfdp >> root@sama5d2-xplained:~# hexdump mx25l3233f-sfdp > > use xxd if possible and the sha1sum/md5sum is missing. ok, will update. > >> 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 >> 0000010 00c2 0401 0060 ff00 ffff ffff ffff ffff >> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff >> 0000030 20e5 fff1 ffff 01ff eb44 6b08 3b08 bb04 >> 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f >> 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff >> 0000060 3600 2650 f99c 6477 cffe ffff ffff ffff >> 0000070 >> >> 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 27498ed0cc0d..68f6ac060bc6 100644 >> --- a/drivers/mtd/spi-nor/macronix.c >> +++ b/drivers/mtd/spi-nor/macronix.c >> @@ -8,6 +8,24 @@ >> >> #include "core.h" >> >> +static int mx25l3233f_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 struct spi_nor_fixups mx25l3233f_fixups = { >> + .post_bfpt = mx25l3233f_post_bfpt_fixups, >> +}; >> + >> static int >> mx25l25635_post_bfpt_fixups(struct spi_nor *nor, >> const struct sfdp_parameter_header *bfpt_header, >> @@ -39,7 +57,10 @@ static const struct flash_info macronix_parts[] = { >> { "mx25l4005a", INFO(0xc22013, 0, 64 * 1024, 8, SECT_4K) }, >> { "mx25l8005", INFO(0xc22014, 0, 64 * 1024, 16, 0) }, >> { "mx25l1606e", INFO(0xc22015, 0, 64 * 1024, 32, SECT_4K) }, >> - { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, SECT_4K) }, >> + { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, SPI_NOR_PARSE_SFDP >> | >> + SECT_4K) >> + /* ID collision with mx25l3233f. */ >> + .fixups = &mx25l3233f_fixups }, > > Shouldn't we use mx25l3205d_fixups as name here? What if there are more > flashes with the same id. Using the name of the colliding flash here > doesn't really scale. I agree, will change. Cheers, ta > > -michael > >> { "mx25l3255e", INFO(0xc29e16, 0, 64 * 1024, 64, SECT_4K) }, >> { "mx25l6405d", INFO(0xc22017, 0, 64 * 1024, 128, SECT_4K) }, >> { "mx25u2033e", INFO(0xc22532, 0, 64 * 1024, 4, SECT_4K) },
diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c index 27498ed0cc0d..68f6ac060bc6 100644 --- a/drivers/mtd/spi-nor/macronix.c +++ b/drivers/mtd/spi-nor/macronix.c @@ -8,6 +8,24 @@ #include "core.h" +static int mx25l3233f_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 struct spi_nor_fixups mx25l3233f_fixups = { + .post_bfpt = mx25l3233f_post_bfpt_fixups, +}; + static int mx25l25635_post_bfpt_fixups(struct spi_nor *nor, const struct sfdp_parameter_header *bfpt_header, @@ -39,7 +57,10 @@ static const struct flash_info macronix_parts[] = { { "mx25l4005a", INFO(0xc22013, 0, 64 * 1024, 8, SECT_4K) }, { "mx25l8005", INFO(0xc22014, 0, 64 * 1024, 16, 0) }, { "mx25l1606e", INFO(0xc22015, 0, 64 * 1024, 32, SECT_4K) }, - { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, SECT_4K) }, + { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, SPI_NOR_PARSE_SFDP | + SECT_4K) + /* ID collision with mx25l3233f. */ + .fixups = &mx25l3233f_fixups }, { "mx25l3255e", INFO(0xc29e16, 0, 64 * 1024, 64, SECT_4K) }, { "mx25l6405d", INFO(0xc22017, 0, 64 * 1024, 128, SECT_4K) }, { "mx25u2033e", INFO(0xc22532, 0, 64 * 1024, 4, SECT_4K) },
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@microchip.com> Acked-by: Pratyush Yadav <p.yadav@ti.com> --- root@sama5d2-xplained:~# find / -iname spi-nor /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor /sys/devices/platform/ahb/ahb:apb/f8000000.spi/spi_master/spi0/spi0.0/spi-nor /sys/bus/spi/drivers/spi-nor root@sama5d2-xplained:~# ls -al /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor total 0 drwxr-xr-x 2 root root 0 Mar 9 14:51 . drwxr-xr-x 6 root root 0 Mar 9 14:50 .. -r--r--r-- 1 root root 4096 Mar 9 14:51 jedec_id -r--r--r-- 1 root root 4096 Mar 9 14:51 manufacturer -r--r--r-- 1 root root 4096 Mar 9 14:51 partname -r--r--r-- 1 root root 0 Mar 9 14:51 sfdp root@sama5d2-xplained:~# cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/jedec_id c22016 root@sama5d2-xplained:~# cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/manufacturer macronix root@sama5d2-xplained:~# cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/partname mx25l3233f root@sama5d2-xplained:~# cat /sys/devices/platform/ahb/ahb:apb/f0020000.spi/spi_master/spi1/spi1.0/spi-nor/sfdp > mx25l3233f-sfdp root@sama5d2-xplained:~# hexdump mx25l3233f-sfdp 0000000 4653 5044 0100 ff01 0000 0901 0030 ff00 0000010 00c2 0401 0060 ff00 ffff ffff ffff ffff 0000020 ffff ffff ffff ffff ffff ffff ffff ffff 0000030 20e5 fff1 ffff 01ff eb44 6b08 3b08 bb04 0000040 ffee ffff ffff ff00 ffff ff00 200c 520f 0000050 d810 ff00 ffff ffff ffff ffff ffff ffff 0000060 3600 2650 f99c 6477 cffe ffff ffff ffff 0000070 drivers/mtd/spi-nor/macronix.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)