* [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
@ 2018-11-29 18:15 Sverdlin, Alexander (Nokia - DE/Ulm)
2018-11-30 10:40 ` Boris Brezillon
0 siblings, 1 reply; 8+ messages in thread
From: Sverdlin, Alexander (Nokia - DE/Ulm) @ 2018-11-29 18:15 UTC (permalink / raw)
To: linux-mtd
Cc: Sverdlin, Alexander (Nokia - DE/Ulm),
Marek Vasut, David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Tudor Ambarus
JESD216 allows "variable address length" and "variable latency" in
Configuration Detection Command Descriptors, in other words "as-is".
And they are still unset during Sector Map Parameter Table parsing,
which led to "map_id" determined erroneously as 0 for, e.g. S25FS128S.
Fixes: b038e8e3b ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
---
drivers/mtd/spi-nor/spi-nor.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 828d03e..5557c89 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2893,6 +2893,16 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt)
nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
addr = smpt[i + 1];
+ /*
+ * JESD216 allows to omit particular address length or latency
+ * specification in the header and at this point they are still
+ * unset, so we need some heuristics. One example is S25FS128S.
+ */
+ if (!nor->addr_width)
+ nor->addr_width = 3;
+ if (!nor->read_dummy)
+ nor->read_dummy = 8;
+
err = spi_nor_read_raw(nor, addr, 1, &data_byte);
if (err)
goto out;
--
2.4.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
2018-11-29 18:15 [PATCH] mtd: spi-nor: Provide default address width and latency for map selection Sverdlin, Alexander (Nokia - DE/Ulm)
@ 2018-11-30 10:40 ` Boris Brezillon
2018-12-03 8:03 ` Sverdlin, Alexander (Nokia - DE/Ulm)
0 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2018-11-30 10:40 UTC (permalink / raw)
To: Sverdlin, Alexander (Nokia - DE/Ulm)
Cc: linux-mtd, Marek Vasut, David Woodhouse, Brian Norris,
Richard Weinberger, Tudor Ambarus
On Thu, 29 Nov 2018 18:15:34 +0000
"Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
wrote:
> JESD216 allows "variable address length" and "variable latency" in
> Configuration Detection Command Descriptors, in other words "as-is".
> And they are still unset during Sector Map Parameter Table parsing,
> which led to "map_id" determined erroneously as 0 for, e.g. S25FS128S.
>
> Fixes: b038e8e3b ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 828d03e..5557c89 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2893,6 +2893,16 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt)
> nor->read_opcode = SMPT_CMD_OPCODE(smpt[i]);
> addr = smpt[i + 1];
>
> + /*
> + * JESD216 allows to omit particular address length or latency
> + * specification in the header and at this point they are still
> + * unset, so we need some heuristics. One example is S25FS128S.
> + */
> + if (!nor->addr_width)
> + nor->addr_width = 3;
> + if (!nor->read_dummy)
> + nor->read_dummy = 8;
> +
Looks like the same problem was reported by Yogesh here [1]. One more
proof that parsing SFDP is not trivial. I mean, what's the point of
defining a generic tables to describe NOR capabilities if you then
depend on vendor/chip specific init to read those tables...
Anyway, I think this sort of initialization should be placed in a
pre_sfdp() fixup hook, as getting the right values likely requires
reading some volatile/non-volatile regs.
> err = spi_nor_read_raw(nor, addr, 1, &data_byte);
> if (err)
> goto out;
[1]https://lkml.org/lkml/2018/10/22/354
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
2018-11-30 10:40 ` Boris Brezillon
@ 2018-12-03 8:03 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-12-03 8:23 ` Boris Brezillon
0 siblings, 1 reply; 8+ messages in thread
From: Sverdlin, Alexander (Nokia - DE/Ulm) @ 2018-12-03 8:03 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd, Marek Vasut, David Woodhouse, Brian Norris,
Richard Weinberger, Tudor Ambarus
Hello Boris!
On 30/11/2018 11:40, Boris Brezillon wrote:
>> + /*
>> + * JESD216 allows to omit particular address length or latency
>> + * specification in the header and at this point they are still
>> + * unset, so we need some heuristics. One example is S25FS128S.
>> + */
>> + if (!nor->addr_width)
>> + nor->addr_width = 3;
>> + if (!nor->read_dummy)
>> + nor->read_dummy = 8;
>> +
> Looks like the same problem was reported by Yogesh here [1]. One more
This is indeed the same problem, although I was not aware of the [1] discussion.
> proof that parsing SFDP is not trivial. I mean, what's the point of
> defining a generic tables to describe NOR capabilities if you then
> depend on vendor/chip specific init to read those tables...
>
> Anyway, I think this sort of initialization should be placed in a
> pre_sfdp() fixup hook, as getting the right values likely requires
This is still on my TODO list, to learn about new hooks.
> reading some volatile/non-volatile regs.
This is the same instruction 65h which is used to read regs and which
appears in SMPT headers, it is a chicken-egg problem.
Therefore, I don't know if it's possible to provide smarter heuristics
here. But without it Spansion S25FS-S Family is now broken by SFDP.
>> err = spi_nor_read_raw(nor, addr, 1, &data_byte);
>> if (err)
>> goto out;
> [1]https://lkml.org/lkml/2018/10/22/354
>
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
2018-12-03 8:03 ` Sverdlin, Alexander (Nokia - DE/Ulm)
@ 2018-12-03 8:23 ` Boris Brezillon
2018-12-03 8:34 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-12-03 8:37 ` Sverdlin, Alexander (Nokia - DE/Ulm)
0 siblings, 2 replies; 8+ messages in thread
From: Boris Brezillon @ 2018-12-03 8:23 UTC (permalink / raw)
To: Sverdlin, Alexander (Nokia - DE/Ulm)
Cc: linux-mtd, Marek Vasut, David Woodhouse, Brian Norris,
Richard Weinberger, Tudor Ambarus
On Mon, 3 Dec 2018 08:03:18 +0000
"Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
wrote:
> Hello Boris!
>
> On 30/11/2018 11:40, Boris Brezillon wrote:
> >> + /*
> >> + * JESD216 allows to omit particular address length or latency
> >> + * specification in the header and at this point they are still
> >> + * unset, so we need some heuristics. One example is S25FS128S.
> >> + */
> >> + if (!nor->addr_width)
> >> + nor->addr_width = 3;
> >> + if (!nor->read_dummy)
> >> + nor->read_dummy = 8;
> >> +
> > Looks like the same problem was reported by Yogesh here [1]. One more
>
> This is indeed the same problem, although I was not aware of the [1] discussion.
>
> > proof that parsing SFDP is not trivial. I mean, what's the point of
> > defining a generic tables to describe NOR capabilities if you then
> > depend on vendor/chip specific init to read those tables...
> >
> > Anyway, I think this sort of initialization should be placed in a
> > pre_sfdp() fixup hook, as getting the right values likely requires
>
> This is still on my TODO list, to learn about new hooks.
->pre_sfdp() does not exist yet, but I'm about to add it for other
reasons, and it looks like a good place to put this sort of
initialization.
>
> > reading some volatile/non-volatile regs.
>
> This is the same instruction 65h which is used to read regs and which
> appears in SMPT headers, it is a chicken-egg problem.
Oh, right, I remember now. Not a smart decision from Spansion :-/.
>
> Therefore, I don't know if it's possible to provide smarter heuristics
> here.
Maybe:
ref_cr1 = read_CR1_using_RDCR()
for_each_possible_dummy_and_addr_width
cr1 = read_CR1_using_RDAR()
if (cr1 == ref_cr1)
break;
> But without it Spansion S25FS-S Family is now broken by SFDP.
Do you have [1] in your tree? IIRC, this fixed Yogesh issue.
[1]https://patchwork.ozlabs.org/patch/994765/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
2018-12-03 8:23 ` Boris Brezillon
@ 2018-12-03 8:34 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-12-03 8:37 ` Sverdlin, Alexander (Nokia - DE/Ulm)
1 sibling, 0 replies; 8+ messages in thread
From: Sverdlin, Alexander (Nokia - DE/Ulm) @ 2018-12-03 8:34 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd, Marek Vasut, David Woodhouse, Brian Norris,
Richard Weinberger, Tudor Ambarus
Hi!
On 03/12/2018 09:23, Boris Brezillon wrote:
>> This is the same instruction 65h which is used to read regs and which
>> appears in SMPT headers, it is a chicken-egg problem.
> Oh, right, I remember now. Not a smart decision from Spansion :-/.
>
>> Therefore, I don't know if it's possible to provide smarter heuristics
>> here.
> Maybe:
>
> ref_cr1 = read_CR1_using_RDCR()
>
> for_each_possible_dummy_and_addr_width
> cr1 = read_CR1_using_RDAR()
> if (cr1 == ref_cr1)
> break;
>
>
>> But without it Spansion S25FS-S Family is now broken by SFDP.
> Do you have [1] in your tree? IIRC, this fixed Yogesh issue.
>
> [1]https://patchwork.ozlabs.org/patch/994765/
This is definitely unrelated, in my case SMPT is happily processed leading
to completely wrong mapping because command 65h with addr_width == 0 always
returns 0.
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
2018-12-03 8:23 ` Boris Brezillon
2018-12-03 8:34 ` Sverdlin, Alexander (Nokia - DE/Ulm)
@ 2018-12-03 8:37 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-12-03 9:08 ` Boris Brezillon
1 sibling, 1 reply; 8+ messages in thread
From: Sverdlin, Alexander (Nokia - DE/Ulm) @ 2018-12-03 8:37 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd, Marek Vasut, David Woodhouse, Brian Norris,
Richard Weinberger, Tudor Ambarus
Hi!
On 03/12/2018 09:23, Boris Brezillon wrote:
>> This is the same instruction 65h which is used to read regs and which
>> appears in SMPT headers, it is a chicken-egg problem.
> Oh, right, I remember now. Not a smart decision from Spansion :-/.
>
>> Therefore, I don't know if it's possible to provide smarter heuristics
>> here.
> Maybe:
>
> ref_cr1 = read_CR1_using_RDCR()
>
> for_each_possible_dummy_and_addr_width
> cr1 = read_CR1_using_RDAR()
> if (cr1 == ref_cr1)
> break;
This will not work, as default value for CR1(N)V is 0 and that is the value
one gets with incorrect addr_width as well.
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
2018-12-03 8:37 ` Sverdlin, Alexander (Nokia - DE/Ulm)
@ 2018-12-03 9:08 ` Boris Brezillon
2018-12-03 9:34 ` Boris Brezillon
0 siblings, 1 reply; 8+ messages in thread
From: Boris Brezillon @ 2018-12-03 9:08 UTC (permalink / raw)
To: Sverdlin, Alexander (Nokia - DE/Ulm)
Cc: linux-mtd, Marek Vasut, David Woodhouse, Brian Norris,
Richard Weinberger, Tudor Ambarus
On Mon, 3 Dec 2018 08:37:17 +0000
"Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
wrote:
> Hi!
>
> On 03/12/2018 09:23, Boris Brezillon wrote:
> >> This is the same instruction 65h which is used to read regs and which
> >> appears in SMPT headers, it is a chicken-egg problem.
> > Oh, right, I remember now. Not a smart decision from Spansion :-/.
> >
> >> Therefore, I don't know if it's possible to provide smarter heuristics
> >> here.
> > Maybe:
> >
> > ref_cr1 = read_CR1_using_RDCR()
> >
> > for_each_possible_dummy_and_addr_width
> > cr1 = read_CR1_using_RDAR()
> > if (cr1 == ref_cr1)
> > break;
>
> This will not work, as default value for CR1(N)V is 0 and that is the value
> one gets with incorrect addr_width as well.
>
How about:
// set the WE bit in SR1 so that SR1 is not 0
write_enable()
ref_sr1 = read_SR1_using_RDSR1()
for_each_possible_dummy_and_addr_width
sr1 = read_SR1_using_RDAR()
if (sr1 == ref_sr1)
break;
write_disable()
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mtd: spi-nor: Provide default address width and latency for map selection
2018-12-03 9:08 ` Boris Brezillon
@ 2018-12-03 9:34 ` Boris Brezillon
0 siblings, 0 replies; 8+ messages in thread
From: Boris Brezillon @ 2018-12-03 9:34 UTC (permalink / raw)
To: Sverdlin, Alexander (Nokia - DE/Ulm)
Cc: linux-mtd, Marek Vasut, David Woodhouse, Brian Norris,
Richard Weinberger, Tudor Ambarus
On Mon, 3 Dec 2018 10:08:14 +0100
Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> On Mon, 3 Dec 2018 08:37:17 +0000
> "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
> wrote:
>
> > Hi!
> >
> > On 03/12/2018 09:23, Boris Brezillon wrote:
> > >> This is the same instruction 65h which is used to read regs and which
> > >> appears in SMPT headers, it is a chicken-egg problem.
> > > Oh, right, I remember now. Not a smart decision from Spansion :-/.
> > >
> > >> Therefore, I don't know if it's possible to provide smarter heuristics
> > >> here.
> > > Maybe:
> > >
> > > ref_cr1 = read_CR1_using_RDCR()
> > >
> > > for_each_possible_dummy_and_addr_width
> > > cr1 = read_CR1_using_RDAR()
> > > if (cr1 == ref_cr1)
> > > break;
> >
> > This will not work, as default value for CR1(N)V is 0 and that is the value
> > one gets with incorrect addr_width as well.
> >
>
> How about:
>
> // set the WE bit in SR1 so that SR1 is not 0
> write_enable()
>
> ref_sr1 = read_SR1_using_RDSR1()
> for_each_possible_dummy_and_addr_width
> sr1 = read_SR1_using_RDAR()
> if (sr1 == ref_sr1)
> break;
Oh, and you should read CR2 to make sure AL and RL fields match the
selected addr/dummy values.
>
> write_disable()
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-12-03 9:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-29 18:15 [PATCH] mtd: spi-nor: Provide default address width and latency for map selection Sverdlin, Alexander (Nokia - DE/Ulm)
2018-11-30 10:40 ` Boris Brezillon
2018-12-03 8:03 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-12-03 8:23 ` Boris Brezillon
2018-12-03 8:34 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-12-03 8:37 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-12-03 9:08 ` Boris Brezillon
2018-12-03 9:34 ` Boris Brezillon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.