linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Support flash devices used on Pensando boards
@ 2020-07-16 21:16 David Clear
  2020-07-16 21:16 ` [PATCH 1/2] mtd: spi-nor: Add support for Macronix mx66u2g45g David Clear
  2020-07-16 21:16 ` [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g David Clear
  0 siblings, 2 replies; 10+ messages in thread
From: David Clear @ 2020-07-16 21:16 UTC (permalink / raw)
  To: linux-mtd, tudor.ambarus; +Cc: snelson, David Clear

Pensando boards use Macronix and Micron flash devices in x2 mode.  This
patchset adds support for the previously unlisted Macronix mx66u2g45g, and
it enables x2 mode on the Micron mt25qu02g.

This patchset applies to the spi-nor/next tree.

David Clear (2):
  mtd: spi-nor: Add support for Macronix mx66u2g45g.
  mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g.

 drivers/mtd/spi-nor/macronix.c  | 3 +++
 drivers/mtd/spi-nor/micron-st.c | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)


base-commit: fb249e1007e0270e305ea674012dcc4f95f1304e
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] mtd: spi-nor: Add support for Macronix mx66u2g45g.
  2020-07-16 21:16 [PATCH 0/2] Support flash devices used on Pensando boards David Clear
@ 2020-07-16 21:16 ` David Clear
  2020-07-17 10:38   ` Tudor.Ambarus
  2020-07-16 21:16 ` [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g David Clear
  1 sibling, 1 reply; 10+ messages in thread
From: David Clear @ 2020-07-16 21:16 UTC (permalink / raw)
  To: linux-mtd, tudor.ambarus; +Cc: snelson, David Clear

The Macronix mx66u2g45g is a 1.8V, 2Gbit (256MB) device which
supports x1, x2, or x4 operation.

Signed-off-by: David Clear <dac2@pensando.io>
Acked-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/mtd/spi-nor/macronix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 0ae0815a3633..f97f3d127575 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -87,6 +87,9 @@ static const struct flash_info macronix_parts[] = {
 			      SPI_NOR_QUAD_READ) },
 	{ "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048,
 			      SPI_NOR_QUAD_READ) },
+	{ "mx66u2g45g",	 INFO(0xc2253c, 0, 64 * 1024, 4096,
+			      SECT_4K | SPI_NOR_DUAL_READ |
+			      SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 };
 
 static void macronix_default_init(struct spi_nor *nor)
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g.
  2020-07-16 21:16 [PATCH 0/2] Support flash devices used on Pensando boards David Clear
  2020-07-16 21:16 ` [PATCH 1/2] mtd: spi-nor: Add support for Macronix mx66u2g45g David Clear
@ 2020-07-16 21:16 ` David Clear
  2020-07-17 10:33   ` Tudor.Ambarus
  1 sibling, 1 reply; 10+ messages in thread
From: David Clear @ 2020-07-16 21:16 UTC (permalink / raw)
  To: linux-mtd, tudor.ambarus; +Cc: snelson, David Clear

The Micron mt25qu02g supports both x2 and x4 transactions.  Add the
SPI_NOR_DUAL_READ to its spi_nor_ids[] table entry.

Signed-off-by: David Clear <dac2@pensando.io>
Acked-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/mtd/spi-nor/micron-st.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 3dca5b9af3b6..ef3695080710 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -71,8 +71,8 @@ static const struct flash_info st_parts[] = {
 			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
 			      NO_CHIP_ERASE) },
 	{ "mt25qu02g",   INFO(0x20bb22, 0, 64 * 1024, 4096,
-			      SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
-			      NO_CHIP_ERASE) },
+			      SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
+			      SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
 
 	{ "m25p05",  INFO(0x202010,  0,  32 * 1024,   2, 0) },
 	{ "m25p10",  INFO(0x202011,  0,  32 * 1024,   4, 0) },
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g.
  2020-07-16 21:16 ` [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g David Clear
@ 2020-07-17 10:33   ` Tudor.Ambarus
       [not found]     ` <CA+nMikFDWgLMAhb2rUZnAaLUJdimKZZxqKsQACnXrmCMb_CLMw@mail.gmail.com>
  2020-07-17 18:00     ` David Clear
  0 siblings, 2 replies; 10+ messages in thread
From: Tudor.Ambarus @ 2020-07-17 10:33 UTC (permalink / raw)
  To: dac2, linux-mtd, vigneshr; +Cc: snelson

On 7/17/20 12:16 AM, David Clear wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The Micron mt25qu02g supports both x2 and x4 transactions.  Add the
> SPI_NOR_DUAL_READ to its spi_nor_ids[] table entry.

In spi_nor_select_read() we select the fastest read. Since this flash
supports Quad Read, the Dual Read will never get selected. As of now,
there is no benefit in adding SPI_NOR_DUAL_READ when SPI_NOR_QUAD_READ
is specified.

Both SPI_NOR_DUAL_READ and SPI_NOR_QUAD_READ trigger the parsing of
SFDP, maybe we should choose to add just the fastest SPI_NOR_*_READ
supported by the flash when declaring one. We would have to update
all the flash entries. Vignesh?

> 
> Signed-off-by: David Clear <dac2@pensando.io>
> Acked-by: Shannon Nelson <snelson@pensando.io>
> ---
>  drivers/mtd/spi-nor/micron-st.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index 3dca5b9af3b6..ef3695080710 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -71,8 +71,8 @@ static const struct flash_info st_parts[] = {
>                               SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>                               NO_CHIP_ERASE) },
>         { "mt25qu02g",   INFO(0x20bb22, 0, 64 * 1024, 4096,
> -                             SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> -                             NO_CHIP_ERASE) },
> +                             SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> +                             SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
> 
>         { "m25p05",  INFO(0x202010,  0,  32 * 1024,   2, 0) },
>         { "m25p10",  INFO(0x202011,  0,  32 * 1024,   4, 0) },
> --
> 2.17.1
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] mtd: spi-nor: Add support for Macronix mx66u2g45g.
  2020-07-16 21:16 ` [PATCH 1/2] mtd: spi-nor: Add support for Macronix mx66u2g45g David Clear
@ 2020-07-17 10:38   ` Tudor.Ambarus
       [not found]     ` <CA+nMikGEEOoKri5z2T3Ms-3cEi+G014ha+rwrwZWeRn=DxfU9w@mail.gmail.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Tudor.Ambarus @ 2020-07-17 10:38 UTC (permalink / raw)
  To: dac2, linux-mtd, vigneshr; +Cc: snelson

On 7/17/20 12:16 AM, David Clear wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The Macronix mx66u2g45g is a 1.8V, 2Gbit (256MB) device which
> supports x1, x2, or x4 operation.

Please specify in the commit message if you tested this flash
and with which controller.
One should do the following tests:
- read, erase, read back, hexdump -> to verify the erase op
- generate a random file, write, read back, compare
(e.g. sha1sum) -> to verify the write/read

For locking operations one should do some locking tests.

Vignesh, maybe we'll have to update the documentation to request
these kind of tests when one adds a flash entry.

Cheers,
ta

> 
> Signed-off-by: David Clear <dac2@pensando.io>
> Acked-by: Shannon Nelson <snelson@pensando.io>
> ---
>  drivers/mtd/spi-nor/macronix.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 0ae0815a3633..f97f3d127575 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -87,6 +87,9 @@ static const struct flash_info macronix_parts[] = {
>                               SPI_NOR_QUAD_READ) },
>         { "mx66l1g55g",  INFO(0xc2261b, 0, 64 * 1024, 2048,
>                               SPI_NOR_QUAD_READ) },
> +       { "mx66u2g45g",  INFO(0xc2253c, 0, 64 * 1024, 4096,
> +                             SECT_4K | SPI_NOR_DUAL_READ |
> +                             SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>  };
> 
>  static void macronix_default_init(struct spi_nor *nor)
> --
> 2.17.1
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Fwd: [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g.
       [not found]     ` <CA+nMikFDWgLMAhb2rUZnAaLUJdimKZZxqKsQACnXrmCMb_CLMw@mail.gmail.com>
@ 2020-07-17 16:42       ` David Clear
  0 siblings, 0 replies; 10+ messages in thread
From: David Clear @ 2020-07-17 16:42 UTC (permalink / raw)
  To: Tudor.Ambarus; +Cc: Shannon Nelson, linux-mtd, vigneshr

[resend due to mailing list bounce]
On Fri, Jul 17, 2020 at 3:33 AM <Tudor.Ambarus@microchip.com> wrote:
>
> > The Micron mt25qu02g supports both x2 and x4 transactions.  Add the
> > SPI_NOR_DUAL_READ to its spi_nor_ids[] table entry.
>
> In spi_nor_select_read() we select the fastest read. Since this flash
> supports Quad Read, the Dual Read will never get selected. As of now,
> there is no benefit in adding SPI_NOR_DUAL_READ when SPI_NOR_QUAD_READ
> is specified.

We're using these devices with the drivers/spi/spi-cadence-quadspi.c
controller.  Now that the Cadence driver is in the spi subsystem, the
spi-rx-bus-width property in the device-tree is honored, and the fastest
available read is selected, taking the bus-width into account.

Only two data pins are hooked up on our board, so we need the x2
mode to work.

> &qspi {
>         status = "okay";
>         flash0: flash@0 {
>                 compatible = "jedec,spi-nor";
>                 reg = <0>;
>                 spi-max-frequency = <50000000>;
>                 spi-rx-bus-width = <2>;

Without this property the interface runs in x1 mode.  Both <2> and
<4> appropriately take effect on this controller (x4 verified under
simulation).

Regards,
David.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Fwd: [PATCH 1/2] mtd: spi-nor: Add support for Macronix mx66u2g45g.
       [not found]     ` <CA+nMikGEEOoKri5z2T3Ms-3cEi+G014ha+rwrwZWeRn=DxfU9w@mail.gmail.com>
@ 2020-07-17 16:44       ` David Clear
  0 siblings, 0 replies; 10+ messages in thread
From: David Clear @ 2020-07-17 16:44 UTC (permalink / raw)
  To: Tudor.Ambarus; +Cc: Shannon Nelson, linux-mtd, vigneshr

[resend due to mailing list bounce]
On Fri, Jul 17, 2020 at 3:38 AM <Tudor.Ambarus@microchip.com> wrote:
>
> On 7/17/20 12:16 AM, David Clear wrote:
> > The Macronix mx66u2g45g is a 1.8V, 2Gbit (256MB) device which
> > supports x1, x2, or x4 operation.
>
> Please specify in the commit message if you tested this flash
> and with which controller.
> One should do the following tests:
> - read, erase, read back, hexdump -> to verify the erase op
> - generate a random file, write, read back, compare
> (e.g. sha1sum) -> to verify the write/read
>
> For locking operations one should do some locking tests.

We've been using with this part hooked up to the cadence-quadspi
controller (albeit with a 4.14 kernel and modified driver to
support x2 mode) for a while now, but yes I'll re-issue the patch
with a statement on testing once we have closure on the DUAL_READ
discussion.

Thanks for the review.  I appreciate the quick turnaround.

Regards,
David.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g.
  2020-07-17 10:33   ` Tudor.Ambarus
       [not found]     ` <CA+nMikFDWgLMAhb2rUZnAaLUJdimKZZxqKsQACnXrmCMb_CLMw@mail.gmail.com>
@ 2020-07-17 18:00     ` David Clear
  2020-07-18  5:35       ` Tudor.Ambarus
  1 sibling, 1 reply; 10+ messages in thread
From: David Clear @ 2020-07-17 18:00 UTC (permalink / raw)
  To: Tudor.Ambarus; +Cc: Shannon Nelson, linux-mtd, vigneshr

On Fri, Jul 17, 2020 at 3:33 AM <Tudor.Ambarus@microchip.com> wrote:
> On 7/17/20 12:16 AM, David Clear wrote:
> > The Micron mt25qu02g supports both x2 and x4 transactions.  Add the
> > SPI_NOR_DUAL_READ to its spi_nor_ids[] table entry.
>
> In spi_nor_select_read() we select the fastest read. Since this flash
> supports Quad Read, the Dual Read will never get selected. As of now,
> there is no benefit in adding SPI_NOR_DUAL_READ when SPI_NOR_QUAD_READ
> is specified.

Ok, I've got this now:  I went back and did some more testing with different
spi-rx-bus-width properties and I see it's redundant to specify both DUAL and
QUAD in the flash description.  Thanks for pointing out my error. I learned
something new.

I withdraw this patch and will work on getting the Macronix patch cleaned up and
tests documented.

Regards,
David.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g.
  2020-07-17 18:00     ` David Clear
@ 2020-07-18  5:35       ` Tudor.Ambarus
  2020-07-18 17:23         ` David Clear
  0 siblings, 1 reply; 10+ messages in thread
From: Tudor.Ambarus @ 2020-07-18  5:35 UTC (permalink / raw)
  To: dac2; +Cc: snelson, linux-mtd, vigneshr

Hi, David,

On 7/17/20 9:00 PM, David Clear wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Jul 17, 2020 at 3:33 AM <Tudor.Ambarus@microchip.com> wrote:
>> On 7/17/20 12:16 AM, David Clear wrote:
>>> The Micron mt25qu02g supports both x2 and x4 transactions.  Add the
>>> SPI_NOR_DUAL_READ to its spi_nor_ids[] table entry.
>>
>> In spi_nor_select_read() we select the fastest read. Since this flash
>> supports Quad Read, the Dual Read will never get selected. As of now,
>> there is no benefit in adding SPI_NOR_DUAL_READ when SPI_NOR_QUAD_READ
>> is specified.
> 
> Ok, I've got this now:  I went back and did some more testing with different
> spi-rx-bus-width properties and I see it's redundant to specify both DUAL and
> QUAD in the flash description.  Thanks for pointing out my error. I learned
> something new.
> 
> I withdraw this patch and will work on getting the Macronix patch cleaned up and
> tests documented.

No, please keep this patch as well. You were right. If we remove the
SPI_NOR_DUAL_READ flag, the dual read will work only because the SPI_NOR_QUAD_READ
flag triggers the parsing of SFDP, which will fill the dual read caps. It is
better to statically fill the 1-1-2 read, so that we can still use the dual read
if SFDP parsing fails and will not fill the dual read caps. Let's keep it.

Thanks, David.
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g.
  2020-07-18  5:35       ` Tudor.Ambarus
@ 2020-07-18 17:23         ` David Clear
  0 siblings, 0 replies; 10+ messages in thread
From: David Clear @ 2020-07-18 17:23 UTC (permalink / raw)
  To: Tudor.Ambarus; +Cc: Shannon Nelson, linux-mtd, vigneshr

On Fri, Jul 17, 2020 at 10:35 PM <Tudor.Ambarus@microchip.com> wrote:
> On 7/17/20 9:00 PM, David Clear wrote:
> > I withdraw this patch and will work on getting the Macronix patch cleaned up and
> > tests documented.
>
> No, please keep this patch as well. [...]

Ok, will do.  I'll re-submit the original patch with additional notes
on Macronix testing.

Regards,
David.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-18 17:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 21:16 [PATCH 0/2] Support flash devices used on Pensando boards David Clear
2020-07-16 21:16 ` [PATCH 1/2] mtd: spi-nor: Add support for Macronix mx66u2g45g David Clear
2020-07-17 10:38   ` Tudor.Ambarus
     [not found]     ` <CA+nMikGEEOoKri5z2T3Ms-3cEi+G014ha+rwrwZWeRn=DxfU9w@mail.gmail.com>
2020-07-17 16:44       ` Fwd: " David Clear
2020-07-16 21:16 ` [PATCH 2/2] mtd: spi-nor: Support SPI_NOR_DUAL_READ on Micron mt25qu02g David Clear
2020-07-17 10:33   ` Tudor.Ambarus
     [not found]     ` <CA+nMikFDWgLMAhb2rUZnAaLUJdimKZZxqKsQACnXrmCMb_CLMw@mail.gmail.com>
2020-07-17 16:42       ` Fwd: " David Clear
2020-07-17 18:00     ` David Clear
2020-07-18  5:35       ` Tudor.Ambarus
2020-07-18 17:23         ` David Clear

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).