Hi, On Fri May 24, 2024 at 12:48 PM CEST, Esben Haabendal wrote: > Macronix engineers apparantly do not understand the purpose of having > an ID actually identify the chip and its capabilities. Sigh. > > The original Macronix SPI NOR flash that identifies itself as 0xC22016 > with RDID was MX25L3205D. This chip does not support SFDP, but does > support the 2READ command (1-2-2). > > When Macronix announced EoL for MX25L3205D, the recommended > replacement part was MX25L3206E, which conveniently also identifies > itself as 0xC22016. It does not support 2READ, but supports DREAD > (1-1-2) instead, and supports SFDP for discovering this. > > When Macronix announced EoL for MX25L3206E, the recommended > replacement part was MX25L3233F, which also identifies itself as > 0xC22016. It supports DREAD, 2READ, and the quad modes QREAD (1-1-4) > and 4READ (1-4-4). This also support SFDP. Thanks for collecting all this info! > So far, all of these chips have been handled the same way by the Linux > driver. The SFDP information have not been read, and no dual and quad > read modes have been enabled. > > The trouble begins when we want to enable the faster read modes. The > RDID command only return the same 3 bytes for all 3 chips, so that > doesn't really help. > > But we can take advantage of the fact that only the old MX25L3205D > chip does not support SFDP, so by triggering the old initialization > mechanism where we try to read and parse SFDP, but has a fall-back > configuration in place, we can configure all 3 chips to their optimal > configurations. You are (mis)using the quad info bits to trigger an sfdp read, correct? In that case, I'd rather see a new flag in .no_sfdp_flags to explicitly trigger the SFDP read. Then your new flash would only need this flag and doesn't require the shenanigans with the fixup, right? > With this, MX25L3205D will get the faster 2READ command enabled, > speading up reads. This should be safe. > > MX25L3206E will get the faster DREAD command enabled. This should also > be safe. > > MX25L3233F will get all of DREAD, 2READ, QREAD and 4READ enabled. In > order for this to actually work, the WP#/SIO2 and HOLD#/SIO3 pins must > be correctly wired to the SPI controller. That should already be taken care of with the spi-{tx,rx}-bus-width. -michael > Signed-off-by: Esben Haabendal > --- > I only have access to boards with MX25L3233F flashes, so haven't been > able to test the backwards compatibility. If anybody has boards with > MX25L3205D and/or MX25L3206E, please help test this patch. Keep an eye > for read performance regression. > > It is worth nothing that both MX25L3205D and MX25L3206E are > end-of-life, and is unavailable from Macronix, so any new boards > featuring a Macronix flash with this ID will likely be using > MX25L3233F. > --- > drivers/mtd/spi-nor/macronix.c | 60 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 59 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c > index ea6be95e75a5..c1e64ee3baa3 100644 > --- a/drivers/mtd/spi-nor/macronix.c > +++ b/drivers/mtd/spi-nor/macronix.c > @@ -8,6 +8,63 @@ > > #include "core.h" > > +/* > + * There is a whole sequence of chips from Macronix that uses the same device > + * id. These are recommended as EoL replacement parts by Macronix, although they > + * are only partly software compatible. > + * > + * Recommended replacement for MX25L3205D was MX25L3206E. > + * Recommended replacement for MX25L3206E was MX25L3233F. > + * > + * MX25L3205D does not support RDSFDP. The other two does. > + * > + * MX25L3205D supports 1-2-2 (2READ) command. > + * MX25L3206E supports 1-1-2 (DREAD) command. > + * MX25L3233F supports 1-1-2 (DREAD), 1-2-2 (2READ), 1-1-4 (QREAD), and 1-4-4 > + * (4READ) commands. > + * > + * In order to trigger reading optional SFDP configuration, the > + * SPI_NOR_DUAL_READ|SPI_NOR_QUAD_READ flags are set, seemingly enabling 1-1-2 > + * and 1-1-4 for MX25L3205D. The other chips supporting RDSFDP will have the > + * correct read commands configured based on SFDP information. > + * > + * As none of the other will enable 1-1-4 and NOT 1-4-4, so we identify > + * MX25L3205D when we see that. > + */ > +static int > +mx25l3205d_late_init(struct spi_nor *nor) > +{ > + struct spi_nor_flash_parameter *params = nor->params; > + > + /* DREAD 2READ QREAD 4READ > + * 1-1-2 1-2-2 1-1-4 1-4-4 > + * Before SFDP parse 1 0 1 0 > + * 3206e after SFDP parse 1 0 0 0 > + * 3233f after SFDP parse 1 1 1 1 > + * 3205d after this func 0 1 0 0 > + */ > + if ((params->hwcaps.mask & SNOR_HWCAPS_READ_1_1_4) && > + !(params->hwcaps.mask & SNOR_HWCAPS_READ_1_4_4)) { > + /* Should be MX25L3205D */ > + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_2; > + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_2], > + 0, 0, 0, 0); > + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_1_1_4; > + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_1_4], > + 0, 0, 0, 0); > + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2; > + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_1_2_2], > + 0, 4, SPINOR_OP_READ_1_2_2, > + SNOR_PROTO_1_2_2); > + } > + > + return 0; > +} > + > +static const struct spi_nor_fixups mx25l3205d_fixups = { > + .late_init = mx25l3205d_late_init, > +}; > + > static int > mx25l25635_post_bfpt_fixups(struct spi_nor *nor, > const struct sfdp_parameter_header *bfpt_header, > @@ -61,7 +118,8 @@ static const struct flash_info macronix_nor_parts[] = { > .id = SNOR_ID(0xc2, 0x20, 0x16), > .name = "mx25l3205d", > .size = SZ_4M, > - .no_sfdp_flags = SECT_4K, > + .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ, > + .fixups = &mx25l3205d_fixups > }, { > .id = SNOR_ID(0xc2, 0x20, 0x17), > .name = "mx25l6405d", > > --- > base-commit: a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6 > change-id: 20240524-macronix-mx25l3205d-fixups-882e92eed7d7 > > Best regards,