| Message ID | 20220228134505.203270-4-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: spi-nor: Handle ID collisions | expand |
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@microchip.com> > Acked-by: Pratyush Yadav <p.yadav@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. > > # 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
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@microchip.com> >> Acked-by: Pratyush Yadav <p.yadav@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
Am 2022-03-03 16:28, schrieb Tudor.Ambarus@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@microchip.com> >>> Acked-by: Pratyush Yadav <p.yadav@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. I'm confused, why is Heiko reporting that the CR/SR writing isn't working because a wrong quad_enable method is chosen, but here it will work. What am I missing? -michael
Am 2022-03-03 17:31, schrieb Heiko Thiery: .. >>>>> # 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. >> >> I'm confused, why is Heiko reporting that the CR/SR writing isn't >> working because a wrong quad_enable method is chosen, but here it >> will work. What am I missing? > > I suppose that the flash that supports the RSSFDP is JEDES216B > compatible including DWORD[15]. The flash that I have is only JEDES216 > compatible and has not the DWORD[15] defined. That was why I wrote "Judging by the length of the SFDP". I've converted both the mx25l12835f and mx25l3233f to binary and both are 112 bytes long. Both seem to have the short BFPT table, ie. no DWORD(15). Both seem to have a second table at offset 60h. -michael
On 3/3/22 18:45, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2022-03-03 17:31, schrieb Heiko Thiery: > .. > >>>>>> # 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. >>> >>> I'm confused, why is Heiko reporting that the CR/SR writing isn't >>> working because a wrong quad_enable method is chosen, but here it >>> will work. What am I missing? >> >> I suppose that the flash that supports the RSSFDP is JEDES216B >> compatible including DWORD[15]. The flash that I have is only JEDES216 >> compatible and has not the DWORD[15] defined. > > That was why I wrote "Judging by the length of the SFDP". I've > converted both the mx25l12835f and mx25l3233f to binary and both > are 112 bytes long. Both seem to have the short BFPT table, ie. > no DWORD(15). Both seem to have a second table at offset 60h. > I've just redone the test, I see: root@sama5d2-xplained:~# mtd_debug read /dev/mtd1 0 65536 read atmel_qspi f0020000.spi: op->cmd.opcode = 00eb, so SPINOR_OP_READ_1_4_4 as I said. Michael, you have the eyes of an eagle, only the first 9 BFPT dwords are defined: spi-nor spi1.0: bfpt_header->length = 9 spi-nor spi1.0: BFPT[DWORD(1)] = fff120e5 spi-nor spi1.0: BFPT[DWORD(2)] = 01ffffff spi-nor spi1.0: BFPT[DWORD(3)] = 6b08eb44 spi-nor spi1.0: BFPT[DWORD(4)] = bb043b08 spi-nor spi1.0: BFPT[DWORD(5)] = ffffffee spi-nor spi1.0: BFPT[DWORD(6)] = ff00ffff spi-nor spi1.0: BFPT[DWORD(7)] = ff00ffff spi-nor spi1.0: BFPT[DWORD(8)] = 520f200c spi-nor spi1.0: BFPT[DWORD(9)] = ff00d810 What happens is that the QE bit is non volatile and it's already set. spi-nor spi1.0: spi_nor_quad_enable spi-nor spi1.0: spi_nor_sr2_bit1_quad_enable atmel_qspi f0020000.spi: op->cmd.opcode = 0035 spi-nor spi1.0: spi_nor_sr2_bit1_quad_enable cr = ff atmel_qspi f0020000.spi: op->cmd.opcode = 0005 spi-nor spi1.0: sr = 40 spi_nor_sr2_bit1_quad_enable is called, RDCR is ignored so 0xff, but I did a read of the SR and surprise, it's value is 0x40, so QE set. This is a new kind of bug :). So yes, this patch has the same problem as Heiko's, I will update it. Thanks for the heads up!
Am 2022-03-04 01:36, schrieb Tudor.Ambarus@microchip.com: > On 3/3/22 18:45, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2022-03-03 17:31, schrieb Heiko Thiery: >> .. >> >>>>>>> # 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. >>>> >>>> I'm confused, why is Heiko reporting that the CR/SR writing isn't >>>> working because a wrong quad_enable method is chosen, but here it >>>> will work. What am I missing? >>> >>> I suppose that the flash that supports the RSSFDP is JEDES216B >>> compatible including DWORD[15]. The flash that I have is only >>> JEDES216 >>> compatible and has not the DWORD[15] defined. >> >> That was why I wrote "Judging by the length of the SFDP". I've >> converted both the mx25l12835f and mx25l3233f to binary and both >> are 112 bytes long. Both seem to have the short BFPT table, ie. >> no DWORD(15). Both seem to have a second table at offset 60h. >> > > I've just redone the test, I see: > root@sama5d2-xplained:~# mtd_debug read /dev/mtd1 0 65536 read > atmel_qspi f0020000.spi: op->cmd.opcode = 00eb, so > SPINOR_OP_READ_1_4_4 as I said. > > Michael, you have the eyes of an eagle, only the first 9 BFPT dwords > are defined: > spi-nor spi1.0: bfpt_header->length = 9 > spi-nor spi1.0: BFPT[DWORD(1)] = fff120e5 > spi-nor spi1.0: BFPT[DWORD(2)] = 01ffffff > spi-nor spi1.0: BFPT[DWORD(3)] = 6b08eb44 > spi-nor spi1.0: BFPT[DWORD(4)] = bb043b08 > spi-nor spi1.0: BFPT[DWORD(5)] = ffffffee > spi-nor spi1.0: BFPT[DWORD(6)] = ff00ffff > spi-nor spi1.0: BFPT[DWORD(7)] = ff00ffff > spi-nor spi1.0: BFPT[DWORD(8)] = 520f200c > spi-nor spi1.0: BFPT[DWORD(9)] = ff00d810 > > What happens is that the QE bit is non volatile and it's already set. > > spi-nor spi1.0: spi_nor_quad_enable > spi-nor spi1.0: spi_nor_sr2_bit1_quad_enable > atmel_qspi f0020000.spi: op->cmd.opcode = 0035 > spi-nor spi1.0: spi_nor_sr2_bit1_quad_enable cr = ff > atmel_qspi f0020000.spi: op->cmd.opcode = 0005 > spi-nor spi1.0: sr = 40 > > spi_nor_sr2_bit1_quad_enable is called, RDCR is ignored so 0xff, > but I did a read of the SR and surprise, it's value is 0x40, so QE set. > This is a new kind of bug :). So yes, this patch has the same problem > as Heiko's, I will update it. Thanks for the heads up! I've given this a little bit more thought. I'm pretty sure the QE bit is non-volatile on most flashes which share IO2 IO3 with hold#/reset#/wp#. Why is the QE bit needed in the first place? Because it will determine the function of the pins. For example, all our products will have WP# pin enabled and pulled low to make sure (parts of) the flash is write-protected. Needless to say, we cannot use quad mode. There might be edge cases where we accidentally disable the write protection pin. I'm not saying it is likely, though. E.g. on our boards we have a jumper to disable the write protection, now if linux decides for whatever reason to fiddle with the QE bit and set it to 1 that jumper might very well be useless after you booted linux once. Now that does ring a bell with "linux will just disable the write protection for no good reason on any flash", if you still remember ;) Anyways, just to let you know. I don't have any better idea. -michael
On 04/03/22 03:36PM, Michael Walle wrote: > Am 2022-03-04 01:36, schrieb Tudor.Ambarus@microchip.com: > > On 3/3/22 18:45, Michael Walle wrote: > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > know the content is safe > > > > > > Am 2022-03-03 17:31, schrieb Heiko Thiery: > > > .. > > > > > > > > > > > # 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. > > > > > > > > > > I'm confused, why is Heiko reporting that the CR/SR writing isn't > > > > > working because a wrong quad_enable method is chosen, but here it > > > > > will work. What am I missing? > > > > > > > > I suppose that the flash that supports the RSSFDP is JEDES216B > > > > compatible including DWORD[15]. The flash that I have is only > > > > JEDES216 > > > > compatible and has not the DWORD[15] defined. > > > > > > That was why I wrote "Judging by the length of the SFDP". I've > > > converted both the mx25l12835f and mx25l3233f to binary and both > > > are 112 bytes long. Both seem to have the short BFPT table, ie. > > > no DWORD(15). Both seem to have a second table at offset 60h. > > > > > > > I've just redone the test, I see: > > root@sama5d2-xplained:~# mtd_debug read /dev/mtd1 0 65536 read > > atmel_qspi f0020000.spi: op->cmd.opcode = 00eb, so > > SPINOR_OP_READ_1_4_4 as I said. > > > > Michael, you have the eyes of an eagle, only the first 9 BFPT dwords > > are defined: > > spi-nor spi1.0: bfpt_header->length = 9 > > spi-nor spi1.0: BFPT[DWORD(1)] = fff120e5 > > spi-nor spi1.0: BFPT[DWORD(2)] = 01ffffff > > spi-nor spi1.0: BFPT[DWORD(3)] = 6b08eb44 > > spi-nor spi1.0: BFPT[DWORD(4)] = bb043b08 > > spi-nor spi1.0: BFPT[DWORD(5)] = ffffffee > > spi-nor spi1.0: BFPT[DWORD(6)] = ff00ffff > > spi-nor spi1.0: BFPT[DWORD(7)] = ff00ffff > > spi-nor spi1.0: BFPT[DWORD(8)] = 520f200c > > spi-nor spi1.0: BFPT[DWORD(9)] = ff00d810 > > > > What happens is that the QE bit is non volatile and it's already set. > > > > spi-nor spi1.0: spi_nor_quad_enable > > spi-nor spi1.0: spi_nor_sr2_bit1_quad_enable > > atmel_qspi f0020000.spi: op->cmd.opcode = 0035 > > spi-nor spi1.0: spi_nor_sr2_bit1_quad_enable cr = ff > > atmel_qspi f0020000.spi: op->cmd.opcode = 0005 > > spi-nor spi1.0: sr = 40 > > > > spi_nor_sr2_bit1_quad_enable is called, RDCR is ignored so 0xff, > > but I did a read of the SR and surprise, it's value is 0x40, so QE set. > > This is a new kind of bug :). So yes, this patch has the same problem > > as Heiko's, I will update it. Thanks for the heads up! > > I've given this a little bit more thought. I'm pretty sure > the QE bit is non-volatile on most flashes which share IO2 > IO3 with hold#/reset#/wp#. Why is the QE bit needed in the > first place? Because it will determine the function of the > pins. For example, all our products will have WP# pin enabled > and pulled low to make sure (parts of) the flash is > write-protected. Needless to say, we cannot use quad mode. For those cases you should set spi-{rx,tx}-bus-width to 1 or 2 in device tree so SPI NOR does not attempt quad mode. If it does despite that, this is a bug which we should fix. > > There might be edge cases where we accidentally disable the > write protection pin. I'm not saying it is likely, though. > E.g. on our boards we have a jumper to disable the write > protection, now if linux decides for whatever reason to fiddle > with the QE bit and set it to 1 that jumper might very well > be useless after you booted linux once. Now that does ring a > bell with "linux will just disable the write protection for > no good reason on any flash", if you still remember ;) > > Anyways, just to let you know. I don't have any better > idea. > > -michael
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)
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> --- # 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 # sha1sum mx25l3233f-sfdp 1b6e0f75b4a6d08d570082992455affa72b2dc81 mx25l3233f-sfdp drivers/mtd/spi-nor/macronix.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)