* [PATCH RFC] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it
@ 2022-03-01 12:49 ` Michael Walle
0 siblings, 0 replies; 6+ messages in thread
From: Michael Walle @ 2022-03-01 12:49 UTC (permalink / raw)
To: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra
Cc: linux-mtd, linux-kernel, heiko.thiery, Michael Walle
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(-)
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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH RFC] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it
2022-03-01 12:49 ` Michael Walle
@ 2022-03-01 15:25 ` Heiko Thiery
-1 siblings, 0 replies; 6+ messages in thread
From: Heiko Thiery @ 2022-03-01 15:25 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFC] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it
@ 2022-03-01 15:25 ` Heiko Thiery
0 siblings, 0 replies; 6+ messages in thread
From: Heiko Thiery @ 2022-03-01 15:25 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
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
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it
2022-03-01 12:49 ` Michael Walle
@ 2022-03-04 7:14 ` Tudor.Ambarus
-1 siblings, 0 replies; 6+ messages in thread
From: Tudor.Ambarus @ 2022-03-04 7:14 UTC (permalink / raw)
To: michael, p.yadav, miquel.raynal, richard, vigneshr
Cc: linux-mtd, linux-kernel, heiko.thiery
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH RFC] mtd: spi-nor: unset quad_enable if SFDP doesn't specify it
@ 2022-03-04 7:14 ` Tudor.Ambarus
0 siblings, 0 replies; 6+ messages in thread
From: Tudor.Ambarus @ 2022-03-04 7:14 UTC (permalink / raw)
To: michael, p.yadav, miquel.raynal, richard, vigneshr
Cc: linux-mtd, linux-kernel, heiko.thiery
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
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 6+ messages in thread