| Message ID | 20220301124935.2893622-1-michael@walle.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [RFC] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it | expand |
Hi Michael, Am Di., 1. März 2022 um 13:49 Uhr schrieb Michael Walle <michael@walle.cc>: > > For flashes which use the first JESD216 revision, we don't know > which enable method for quad mode we can use. The default one in > spi_nor_init_default_params() is wrong for at least the Macronix > MX25L12835F, thus clear the enable method. Flashes with such an > SFDP revision will have to use a flash (and SFDP revision) > specific fixup. > > Signed-off-by: Michael Walle <michael@walle.cc> Tested-by: Heiko Thiery <heiko.thiery@gmail.com> > --- > Please note, completely, untested. Heiko, could you test this without > having the second series from Tudor applied? Then you should have at > least a working flash without quad mode. I removed the second series and applied yours. With that it works. -- Heiko > > drivers/mtd/spi-nor/sfdp.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c > index a5211543d30d..c23e85274ff2 100644 > --- a/drivers/mtd/spi-nor/sfdp.c > +++ b/drivers/mtd/spi-nor/sfdp.c > @@ -549,6 +549,14 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > map->uniform_erase_type = map->uniform_region.offset & > SNOR_ERASE_TYPE_MASK; > > + /* > + * The first JESD216 revision doesn't specify a method to enable > + * quad mode. spi_nor_init_default_params() will set a legacy > + * default method to enable quad mode. We have to disable it > + * again. > + */ > + params->quad_enable = NULL; > + > /* Stop here if not JESD216 rev A or later. */ > if (bfpt_header->length == BFPT_DWORD_MAX_JESD216) > return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt); > @@ -562,7 +570,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > /* Quad Enable Requirements. */ > switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { > case BFPT_DWORD15_QER_NONE: > - params->quad_enable = NULL; > break; > > case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: > -- > 2.30.2 >
On 3/1/22 14:49, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > For flashes which use the first JESD216 revision, we don't know > which enable method for quad mode we can use. The default one in > spi_nor_init_default_params() is wrong for at least the Macronix > MX25L12835F, thus clear the enable method. Flashes with such an > SFDP revision will have to use a flash (and SFDP revision) > specific fixup. I expect this to break support for flashes that use the quad_enable method selected in spi_nor_init_default_params() and define the first JESD216 revision, but we should fix all sooner or later. I vote to queue this in. Maybe you can update the commit message and explain why would some flashes fail to enable quad mode, similar to what I did. Cheers, ta > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > Please note, completely, untested. Heiko, could you test this without > having the second series from Tudor applied? Then you should have at > least a working flash without quad mode. > > drivers/mtd/spi-nor/sfdp.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c > index a5211543d30d..c23e85274ff2 100644 > --- a/drivers/mtd/spi-nor/sfdp.c > +++ b/drivers/mtd/spi-nor/sfdp.c > @@ -549,6 +549,14 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > map->uniform_erase_type = map->uniform_region.offset & > SNOR_ERASE_TYPE_MASK; > > + /* > + * The first JESD216 revision doesn't specify a method to enable > + * quad mode. spi_nor_init_default_params() will set a legacy > + * default method to enable quad mode. We have to disable it > + * again. and here to update the comment and explain that you expect these flashes to set the quad enable method in a post_bfpt hook. > + */ > + params->quad_enable = NULL; > + > /* Stop here if not JESD216 rev A or later. */ > if (bfpt_header->length == BFPT_DWORD_MAX_JESD216) > return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt); > @@ -562,7 +570,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > /* Quad Enable Requirements. */ > switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { > case BFPT_DWORD15_QER_NONE: > - params->quad_enable = NULL; > break; > > case BFPT_DWORD15_QER_SR2_BIT1_BUGGY: > -- > 2.30.2 >
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c index a5211543d30d..c23e85274ff2 100644 --- a/drivers/mtd/spi-nor/sfdp.c +++ b/drivers/mtd/spi-nor/sfdp.c @@ -549,6 +549,14 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, map->uniform_erase_type = map->uniform_region.offset & SNOR_ERASE_TYPE_MASK; + /* + * The first JESD216 revision doesn't specify a method to enable + * quad mode. spi_nor_init_default_params() will set a legacy + * default method to enable quad mode. We have to disable it + * again. + */ + params->quad_enable = NULL; + /* Stop here if not JESD216 rev A or later. */ if (bfpt_header->length == BFPT_DWORD_MAX_JESD216) return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt); @@ -562,7 +570,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, /* Quad Enable Requirements. */ switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) { case BFPT_DWORD15_QER_NONE: - params->quad_enable = NULL; break; case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
For flashes which use the first JESD216 revision, we don't know which enable method for quad mode we can use. The default one in spi_nor_init_default_params() is wrong for at least the Macronix MX25L12835F, thus clear the enable method. Flashes with such an SFDP revision will have to use a flash (and SFDP revision) specific fixup. Signed-off-by: Michael Walle <michael@walle.cc> --- Please note, completely, untested. Heiko, could you test this without having the second series from Tudor applied? Then you should have at least a working flash without quad mode. drivers/mtd/spi-nor/sfdp.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)