2018-10-03 16:57:38

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH 0/3] spi-nor: Add Octal SPI support

This series adds support for octal mode of mt35x flash. Also, adds
support for OSPI version of Cadence QSPI controller.

Based on top of patches adding basic support for mt35xu512aba here:
https://patchwork.ozlabs.org/cover/971437/

Vignesh R (3):
mtd: spi-nor: Add Octal mode support for mt35xu512aba
dt-bindings: cadence-quadspi: Add new compatible for AM654 SoC
mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller

.../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
include/linux/mtd/spi-nor.h | 2 ++
4 files changed, 22 insertions(+), 1 deletion(-)

--
2.19.0



2018-10-03 16:58:12

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH 3/3] mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller

Cadence OSPI controller IP supports Octal IO (x8 IO lines),
It also has an integrated PHY. IP register layout is very
similar to existing QSPI IP except for additional bits to support Octal
and Octal DDR mode. Therefore, extend current driver to support Octal
mode.

Signed-off-by: Vignesh R <[email protected]>
---
drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index e24db817154e..48b00e75a879 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -101,6 +101,7 @@ struct cqspi_st {
#define CQSPI_INST_TYPE_SINGLE 0
#define CQSPI_INST_TYPE_DUAL 1
#define CQSPI_INST_TYPE_QUAD 2
+#define CQSPI_INST_TYPE_OCTAL 3

#define CQSPI_DUMMY_CLKS_PER_BYTE 8
#define CQSPI_DUMMY_BYTES_MAX 4
@@ -898,6 +899,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
case SNOR_PROTO_1_1_4:
f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
break;
+ case SNOR_PROTO_1_1_8:
+ f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
+ break;
default:
return -EINVAL;
}
@@ -1205,6 +1209,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
SNOR_HWCAPS_READ_FAST |
SNOR_HWCAPS_READ_1_1_2 |
SNOR_HWCAPS_READ_1_1_4 |
+ SNOR_HWCAPS_READ_1_1_8 |
SNOR_HWCAPS_PP,
};
struct platform_device *pdev = cqspi->pdev;
@@ -1456,6 +1461,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
.compatible = "ti,k2g-qspi",
.data = (void *)CQSPI_NEEDS_WR_DELAY,
},
+ {
+ .compatible = "ti,am654-ospi",
+ .data = (void *)CQSPI_NEEDS_WR_DELAY,
+ },
{ /* end of table */ }
};

--
2.19.0


2018-10-03 17:00:29

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH 2/3] dt-bindings: cadence-quadspi: Add new compatible for AM654 SoC

AM654 SoC has Cadence Octal SPI controller, which is similar to Cadence
QSPI controller but supports Octal IO(x8 data lines) and Double Data
Rate(DDR) mode. Add new compatible to support OSPI controller on TI's
AM654 SoCs.

Signed-off-by: Vignesh R <[email protected]>
---
Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index bb2075df9b38..4345c3a6f530 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -4,6 +4,7 @@ Required properties:
- compatible : should be one of the following:
Generic default - "cdns,qspi-nor".
For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
+ For TI AM654 SoC - "ti,am654-ospi", "cdns,qspi-nor".
- reg : Contains two entries, each of which is a tuple consisting of a
physical address and length. The first entry is the address and
length of the controller register set. The second entry is the
--
2.19.0


2018-10-03 17:01:11

by Vignesh Raghavendra

[permalink] [raw]
Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
supports read/write over 8 IO lines simulatenously. Add support for
Octal read mode for Micron mt35xu512aba.
Unfortunately, this flash is only complaint to SFDP JESD216B and does not
seem to support newer JESD216C standard that provides auto detection of
Octal mode capabilities and opcodes. Therefore, this capability is
manually added using new SPI_NOR_OCTAL_READ flag.

Signed-off-by: Vignesh R <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
include/linux/mtd/spi-nor.h | 2 ++
2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index aff5e6ff0b2c..4926e805a8cb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -90,6 +90,7 @@ struct flash_info {
#define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */
#define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
#define USE_CLSR BIT(14) /* use CLSR command */
+#define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */

int (*quad_enable)(struct spi_nor *nor);
};
@@ -209,6 +210,7 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
{ SPINOR_OP_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B },
{ SPINOR_OP_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B },
{ SPINOR_OP_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B },
+ { SPINOR_OP_READ_1_1_8, SPINOR_OP_READ_1_1_8_4B },

{ SPINOR_OP_READ_1_1_1_DTR, SPINOR_OP_READ_1_1_1_DTR_4B },
{ SPINOR_OP_READ_1_2_2_DTR, SPINOR_OP_READ_1_2_2_DTR_4B },
@@ -1406,7 +1408,7 @@ static const struct flash_info spi_nor_ids[] = {
{ "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },

/* Micron */
- { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K | USE_FSR | SPI_NOR_4B_OPCODES) },
+ { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },

/* PMC */
{ "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) },
@@ -3199,6 +3201,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
SNOR_PROTO_1_1_4);
}

+ if (info->flags & SPI_NOR_OCTAL_READ) {
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
+ spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
+ 0, 8, SPINOR_OP_READ_1_1_8,
+ SNOR_PROTO_1_1_8);
+ }
+
/* Page Program settings. */
params->hwcaps.mask |= SNOR_HWCAPS_PP;
spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 8b1acf68b7ac..ae9861ed7e0f 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -50,6 +50,7 @@
#define SPINOR_OP_READ_1_2_2 0xbb /* Read data bytes (Dual I/O SPI) */
#define SPINOR_OP_READ_1_1_4 0x6b /* Read data bytes (Quad Output SPI) */
#define SPINOR_OP_READ_1_4_4 0xeb /* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8 0x8b /* Read data bytes (Octal Output SPI) */
#define SPINOR_OP_PP 0x02 /* Page program (up to 256 bytes) */
#define SPINOR_OP_PP_1_1_4 0x32 /* Quad page program */
#define SPINOR_OP_PP_1_4_4 0x38 /* Quad page program */
@@ -73,6 +74,7 @@
#define SPINOR_OP_READ_1_2_2_4B 0xbc /* Read data bytes (Dual I/O SPI) */
#define SPINOR_OP_READ_1_1_4_4B 0x6c /* Read data bytes (Quad Output SPI) */
#define SPINOR_OP_READ_1_4_4_4B 0xec /* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8_4B 0x7c /* Read data bytes (Octal Output SPI) */
#define SPINOR_OP_PP_4B 0x12 /* Page program (up to 256 bytes) */
#define SPINOR_OP_PP_1_1_4_4B 0x34 /* Quad page program */
#define SPINOR_OP_PP_1_4_4_4B 0x3e /* Quad page program */
--
2.19.0


2018-10-03 19:20:42

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 0/3] spi-nor: Add Octal SPI support

On Wed, 3 Oct 2018 22:26:00 +0530
Vignesh R <[email protected]> wrote:

> This series adds support for octal mode of mt35x flash. Also, adds
> support for OSPI version of Cadence QSPI controller.
>
> Based on top of patches adding basic support for mt35xu512aba here:
> https://patchwork.ozlabs.org/cover/971437/
>
> Vignesh R (3):
> mtd: spi-nor: Add Octal mode support for mt35xu512aba
> dt-bindings: cadence-quadspi: Add new compatible for AM654 SoC
> mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller

The patchset looks good to me, I'll just wait the next release cycle
before applying it, since I'd prefer to have it stay a bit longer in
-next. This should leave enough time to let other people review this
stuff.

>
> .../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++

On a slightly different topic, do you plan to convert the Cadence
driver to spi-mem? And if you don't, is it because you don't have time
or because some features are missing in spi-mem (I remember you
mentioned a few things back when you were reviewing the spi-mem series)?

> drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
> include/linux/mtd/spi-nor.h | 2 ++
> 4 files changed, 22 insertions(+), 1 deletion(-)
>


2018-10-04 06:52:27

by Yogesh Narayan Gaur

[permalink] [raw]
Subject: RE: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

Hi Vignesh,

> -----Original Message-----
> From: Vignesh R [mailto:[email protected]]
> Sent: Wednesday, October 3, 2018 10:26 PM
> To: Boris Brezillon <[email protected]>; Marek Vasut
> <[email protected]>; Rob Herring <[email protected]>
> Cc: Brian Norris <[email protected]>; Yogesh Narayan Gaur
> <[email protected]>; Linux ARM Mailing List <linux-arm-
> [email protected]>; [email protected];
> [email protected]; [email protected]; Vignesh R
> <[email protected]>
> Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba
>
> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It supports
> read/write over 8 IO lines simulatenously. Add support for Octal read mode for
> Micron mt35xu512aba.
> Unfortunately, this flash is only complaint to SFDP JESD216B and does not seem
> to support newer JESD216C standard that provides auto detection of Octal
> mode capabilities and opcodes. Therefore, this capability is manually added
> using new SPI_NOR_OCTAL_READ flag.
>

Thanks for sending the patch-set of adding octal support.
If possible, can you share the MT35x datasheet?

I also have the patch ready in which I have added support for Read (1-1-8 and 1-8-8) protocol and Write (1-1-8 and 1-8-8).
Also have added support of Octal in driver/spi/spi.c framework.

IMO, we would collaborate our patches.
--
Regards
Yogesh Gaur

> Signed-off-by: Vignesh R <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
> include/linux/mtd/spi-nor.h | 2 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index
> aff5e6ff0b2c..4926e805a8cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -90,6 +90,7 @@ struct flash_info {
> #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip
> erase */
> #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
> #define USE_CLSR BIT(14) /* use CLSR command */
> +#define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */
>
> int (*quad_enable)(struct spi_nor *nor);
> };
> @@ -209,6 +210,7 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
> { SPINOR_OP_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B },
> { SPINOR_OP_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B },
> { SPINOR_OP_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B },
> + { SPINOR_OP_READ_1_1_8, SPINOR_OP_READ_1_1_8_4B },
>
> { SPINOR_OP_READ_1_1_1_DTR,
> SPINOR_OP_READ_1_1_1_DTR_4B },
> { SPINOR_OP_READ_1_2_2_DTR,
> SPINOR_OP_READ_1_2_2_DTR_4B },
> @@ -1406,7 +1408,7 @@ static const struct flash_info spi_nor_ids[] = {
> { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096, SECT_4K |
> USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
>
> /* Micron */
> - { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
> USE_FSR | SPI_NOR_4B_OPCODES) },
> + { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
> USE_FSR
> +| SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
>
> /* PMC */
> { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) },
> @@ -3199,6 +3201,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
> SNOR_PROTO_1_1_4);
> }
>
> + if (info->flags & SPI_NOR_OCTAL_READ) {
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> + spi_nor_set_read_settings(&params-
> >reads[SNOR_CMD_READ_1_1_8],
> + 0, 8, SPINOR_OP_READ_1_1_8,
> + SNOR_PROTO_1_1_8);
> + }
> +
> /* Page Program settings. */
> params->hwcaps.mask |= SNOR_HWCAPS_PP;
> spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index
> 8b1acf68b7ac..ae9861ed7e0f 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -50,6 +50,7 @@
> #define SPINOR_OP_READ_1_2_2 0xbb /* Read data bytes (Dual I/O
> SPI) */
> #define SPINOR_OP_READ_1_1_4 0x6b /* Read data bytes (Quad
> Output SPI) */
> #define SPINOR_OP_READ_1_4_4 0xeb /* Read data bytes (Quad I/O
> SPI) */
> +#define SPINOR_OP_READ_1_1_8 0x8b /* Read data bytes (Octal
> Output SPI) */
> #define SPINOR_OP_PP 0x02 /* Page program (up to 256 bytes) */
> #define SPINOR_OP_PP_1_1_4 0x32 /* Quad page program */
> #define SPINOR_OP_PP_1_4_4 0x38 /* Quad page program */
> @@ -73,6 +74,7 @@
> #define SPINOR_OP_READ_1_2_2_4B 0xbc /* Read data bytes (Dual I/O
> SPI) */
> #define SPINOR_OP_READ_1_1_4_4B 0x6c /* Read data bytes (Quad
> Output SPI) */
> #define SPINOR_OP_READ_1_4_4_4B 0xec /* Read data bytes (Quad I/O
> SPI) */
> +#define SPINOR_OP_READ_1_1_8_4B 0x7c /* Read data bytes (Octal
> Output SPI) */
> #define SPINOR_OP_PP_4B 0x12 /* Page program (up to 256
> bytes) */
> #define SPINOR_OP_PP_1_1_4_4B 0x34 /* Quad page program */
> #define SPINOR_OP_PP_1_4_4_4B 0x3e /* Quad page program */
> --
> 2.19.0


2018-10-04 07:39:56

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

+Julien, Zhengxunli and Mason from Macronix

Hi Yogesh,

On Thu, 4 Oct 2018 06:51:41 +0000
Yogesh Narayan Gaur <[email protected]> wrote:

> Hi Vignesh,
>
> > -----Original Message-----
> > From: Vignesh R [mailto:[email protected]]
> > Sent: Wednesday, October 3, 2018 10:26 PM
> > To: Boris Brezillon <[email protected]>; Marek Vasut
> > <[email protected]>; Rob Herring <[email protected]>
> > Cc: Brian Norris <[email protected]>; Yogesh Narayan Gaur
> > <[email protected]>; Linux ARM Mailing List <linux-arm-
> > [email protected]>; [email protected];
> > [email protected]; [email protected]; Vignesh R
> > <[email protected]>
> > Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba
> >
> > Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It supports
> > read/write over 8 IO lines simulatenously. Add support for Octal read mode for
> > Micron mt35xu512aba.
> > Unfortunately, this flash is only complaint to SFDP JESD216B and does not seem
> > to support newer JESD216C standard that provides auto detection of Octal
> > mode capabilities and opcodes. Therefore, this capability is manually added
> > using new SPI_NOR_OCTAL_READ flag.
> >
>
> Thanks for sending the patch-set of adding octal support.
> If possible, can you share the MT35x datasheet?
>
> I also have the patch ready in which I have added support for Read (1-1-8 and 1-8-8) protocol and Write (1-1-8 and 1-8-8).
> Also have added support of Octal in driver/spi/spi.c framework.
>
> IMO, we would collaborate our patches.

Looks like we are of stepping on each others toes here (see this branch
[1]). I guess it's not a problem if we agree on who is working on what.

Yogesh, you already sent "spi: add flags for octal I/O data
transfer" [3] which is only adding the new OCTAL flags but is not
patching spi.c and spi-mem.c to take those new flags into account. Here
is my version of this patch [2] (it's still missing an update of
SPI_MEM_MAX_BUSWIDTH). Let me know what you want to do (rework your
version to address the problem or take mine).

Regarding other patches in [2], they're mainly here to add support for
X-X-X and DTR modes and get the m25p80 logic integrated in spi-nor.c so
that we can really check which NOR operations are supported by the SPI
controller.

Regards,

Boris

[1]https://github.com/bbrezillon/linux/commits/spi-nor/octo
[2]https://github.com/bbrezillon/linux/commit/9854a8fdd23f64e79859fd07a71d4a1c57b812f2
[3]https://patchwork.ozlabs.org/patch/894916/

2018-10-04 08:49:25

by Yogesh Narayan Gaur

[permalink] [raw]
Subject: RE: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

Hi Boris,

> -----Original Message-----
> From: Boris Brezillon [mailto:[email protected]]
> Sent: Thursday, October 4, 2018 1:09 PM
> To: Yogesh Narayan Gaur <[email protected]>
> Cc: Vignesh R <[email protected]>; Marek Vasut <[email protected]>; Rob
> Herring <[email protected]>; Brian Norris <[email protected]>;
> Linux ARM Mailing List <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for
> mt35xu512aba
>
> +Julien, Zhengxunli and Mason from Macronix
>
> Hi Yogesh,
>
> On Thu, 4 Oct 2018 06:51:41 +0000
> Yogesh Narayan Gaur <[email protected]> wrote:
>
> > Hi Vignesh,
> >
> > > -----Original Message-----
> > > From: Vignesh R [mailto:[email protected]]
> > > Sent: Wednesday, October 3, 2018 10:26 PM
> > > To: Boris Brezillon <[email protected]>; Marek Vasut
> > > <[email protected]>; Rob Herring <[email protected]>
> > > Cc: Brian Norris <[email protected]>; Yogesh Narayan Gaur
> > > <[email protected]>; Linux ARM Mailing List <linux-arm-
> > > [email protected]>; [email protected];
> > > [email protected]; [email protected]; Vignesh R
> > > <[email protected]>
> > > Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for
> > > mt35xu512aba
> > >
> > > Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines.
> > > It supports read/write over 8 IO lines simulatenously. Add support
> > > for Octal read mode for Micron mt35xu512aba.
> > > Unfortunately, this flash is only complaint to SFDP JESD216B and
> > > does not seem to support newer JESD216C standard that provides auto
> > > detection of Octal mode capabilities and opcodes. Therefore, this
> > > capability is manually added using new SPI_NOR_OCTAL_READ flag.
> > >
> >
> > Thanks for sending the patch-set of adding octal support.
> > If possible, can you share the MT35x datasheet?
> >
> > I also have the patch ready in which I have added support for Read (1-1-8 and
> 1-8-8) protocol and Write (1-1-8 and 1-8-8).
> > Also have added support of Octal in driver/spi/spi.c framework.
> >
> > IMO, we would collaborate our patches.
>
> Looks like we are of stepping on each others toes here (see this branch [1]). I
> guess it's not a problem if we agree on who is working on what.
>
> Yogesh, you already sent "spi: add flags for octal I/O data transfer" [3] which is
> only adding the new OCTAL flags but is not patching spi.c and spi-mem.c to take
> those new flags into account. Here is my version of this patch [2] (it's still
> missing an update of SPI_MEM_MAX_BUSWIDTH). Let me know what you want
> to do (rework your version to address the problem or take mine).
>
I haven't said that I have sent the patches. My patches are ready and needed to be sent for review.
In these patches, I am adding support for X-X-X protocol for octal support and integrating them with spi and flash m25p80 interface.

Would send the patches by today evening for review.

Regards
Yogesh Gaur

> Regarding other patches in [2], they're mainly here to add support for X-X-X and
> DTR modes and get the m25p80 logic integrated in spi-nor.c so that we can
> really check which NOR operations are supported by the SPI controller.
>
> Regards,
>
> Boris
>
> [1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fbbrezillon%2Flinux%2Fcommits%2Fspi-
> nor%2Focto&amp;data=02%7C01%7Cyogeshnarayan.gaur%40nxp.com%7C3ad
> 4b6f2a4c9470e98f308d629cc8362%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636742355682081158&amp;sdata=vXg7nwZ6dLrUxoNw9t41GYMp
> oPdNWUWhLM6wZPmrIec%3D&amp;reserved=0
> [2]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Fbbrezillon%2Flinux%2Fcommit%2F9854a8fdd23f64e79859fd07a71d4
> a1c57b812f2&amp;data=02%7C01%7Cyogeshnarayan.gaur%40nxp.com%7C3ad
> 4b6f2a4c9470e98f308d629cc8362%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636742355682081158&amp;sdata=yEZqGYprHS8q2wuisFLpHg8bMj
> uuWDrl3ggWlA8LRoc%3D&amp;reserved=0
> [3]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> chwork.ozlabs.org%2Fpatch%2F894916%2F&amp;data=02%7C01%7Cyogeshnar
> ayan.gaur%40nxp.com%7C3ad4b6f2a4c9470e98f308d629cc8362%7C686ea1d3
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636742355682081158&amp;sdata=t
> 5DFvXgDOlyLn7pMieue9BB5q24DNiJMywGvNrWAYOo%3D&amp;reserved=0

2018-10-04 09:11:17

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

On Thu, 4 Oct 2018 08:47:33 +0000
Yogesh Narayan Gaur <[email protected]> wrote:

> >
> > Yogesh, you already sent "spi: add flags for octal I/O data
> > transfer" [3] which is only adding the new OCTAL flags but is not
> > patching spi.c and spi-mem.c to take those new flags into account.
> > Here is my version of this patch [2] (it's still missing an update
> > of SPI_MEM_MAX_BUSWIDTH). Let me know what you want to do (rework
> > your version to address the problem or take mine).
> I haven't said that I have sent the patches.

Well, you at least sent one of them, I know it because I was in
CC :P.

> My patches are ready and needed to be sent for review.

I initially told you to drop those patches because we didn't have a
user yet, that's about to change, so I guess we're all good.

> In these patches, I am adding support for X-X-X protocol for octal
> support and integrating them with spi and flash m25p80 interface.

From what I've seen you're not adding support of 8-8-8 but for 1-8-8 an
1-1-8, which is a lot easier to deal with (it's completely stateless,
which is what the spi-nor expects). Unless you're talking about other
patches you haven't sent yet...


2018-10-04 09:46:42

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

On Wed, 3 Oct 2018 22:26:01 +0530
Vignesh R <[email protected]> wrote:

> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
> supports read/write over 8 IO lines simulatenously. Add support for
> Octal read mode for Micron mt35xu512aba.
> Unfortunately, this flash is only complaint to SFDP JESD216B and does not
> seem to support newer JESD216C standard that provides auto detection of
> Octal mode capabilities and opcodes. Therefore, this capability is
> manually added using new SPI_NOR_OCTAL_READ flag.
>
> Signed-off-by: Vignesh R <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
> include/linux/mtd/spi-nor.h | 2 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index aff5e6ff0b2c..4926e805a8cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -90,6 +90,7 @@ struct flash_info {
> #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */
> #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
> #define USE_CLSR BIT(14) /* use CLSR command */
> +#define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */

Hm, we'll need to clarify what OCTAL means. I see at least 3 different
modes using 8 IO lines (1-1-8, 1-8-8 and 8-8-8) and all of them could
be qualified as "octal" modes.
So how about renaming this macro SPI_NOR_1_1_8_READ.

Also, I fear we'll soon run out of bits in ->flags if we keep adding
one flag per mode which is why I proposed a solution to let flash
chips tweak the flash parameters as they wish [1][2]. I'm not saying we
should do it now, but we should definitely plan for something like that.

[1]https://github.com/bbrezillon/linux/commit/9c672e4c85a91f1b0803c9c6e4b8f3aae5d79ffb
[2]https://github.com/bbrezillon/linux/commit/3a5515c8821314c06a3d84f9861aefe476bb711e

2018-10-04 10:35:56

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH 0/3] spi-nor: Add Octal SPI support

On Thursday 04 October 2018 12:50 AM, Boris Brezillon wrote:
> On Wed, 3 Oct 2018 22:26:00 +0530
> Vignesh R <[email protected]> wrote:
>
>> This series adds support for octal mode of mt35x flash. Also, adds
>> support for OSPI version of Cadence QSPI controller.
>>
>> Based on top of patches adding basic support for mt35xu512aba here:
>> https://patchwork.ozlabs.org/cover/971437/
>>
>> Vignesh R (3):
>> mtd: spi-nor: Add Octal mode support for mt35xu512aba
>> dt-bindings: cadence-quadspi: Add new compatible for AM654 SoC
>> mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller
>
> The patchset looks good to me, I'll just wait the next release cycle
> before applying it, since I'd prefer to have it stay a bit longer in
> -next. This should leave enough time to let other people review this
> stuff.
>

Ok, thats fine...

>>
>> .../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
>> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
>
> On a slightly different topic, do you plan to convert the Cadence
> driver to spi-mem? And if you don't, is it because you don't have time
> or because some features are missing in spi-mem (I remember you
> mentioned a few things back when you were reviewing the spi-mem series)?
>

I do not have plans to convert cadence QSPI driver to spi-mem yet,
mainly due to lack of time. Also, not sure if original author Marek and
other altera people are okay with that.

I see couple of issues in the way of conversion:
1. I would wait to know what direction would direct mapping APIs[1] go
before starting spi-mem conversion for Cadence QSPI driver. Else, we
have may to re write again if direct mapping APIs are merged.
2. New Cadence OSPI IP has an integrated PHY to support high throughput
OSPI flashes operating up 200MHz in Octal DDR mode. In order to work
with such flashes, PHY DLLs need to be calibrated. Highly simplified
calibration sequence is as below(See [2] for actual sequence):
-Read flash ID at low speed and store it.
-Enable PHY and set DLLs to a defined initial value
-Increment RX DLL value
-Read flash ID and check for correctness of data read
-repeat above two steps until a band of passing values is obtained for
RX DLL where flash ID is correctly read.
-DLL needs to set to middle of the passing band.

I am trying to figure out how to fit this into the spi-mem framework as
controller would to need to store READ ID opcode and expected JEDEC ID
before starting calibration sequence.


[1]https://patchwork.ozlabs.org/cover/924041/
[2]http://www.ti.com/lit/ug/spruid7a/spruid7a.pdf(12.3.2.4.14 PHY Module
page 9770-9772)

--
Regards
Vignesh

2018-10-04 10:38:37

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

Hi,

On Thursday 04 October 2018 12:21 PM, Yogesh Narayan Gaur wrote:
> Hi Vignesh,
>
>> -----Original Message-----
>> From: Vignesh R [mailto:[email protected]]
>> Sent: Wednesday, October 3, 2018 10:26 PM
>> To: Boris Brezillon <[email protected]>; Marek Vasut
>> <[email protected]>; Rob Herring <[email protected]>
>> Cc: Brian Norris <[email protected]>; Yogesh Narayan Gaur
>> <[email protected]>; Linux ARM Mailing List <linux-arm-
>> [email protected]>; [email protected];
>> [email protected]; [email protected]; Vignesh R
>> <[email protected]>
>> Subject: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba
>>
>> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It supports
>> read/write over 8 IO lines simulatenously. Add support for Octal read mode for
>> Micron mt35xu512aba.
>> Unfortunately, this flash is only complaint to SFDP JESD216B and does not seem
>> to support newer JESD216C standard that provides auto detection of Octal
>> mode capabilities and opcodes. Therefore, this capability is manually added
>> using new SPI_NOR_OCTAL_READ flag.
>>
>
> Thanks for sending the patch-set of adding octal support.
> If possible, can you share the MT35x datasheet?
>

I dont have datasheet, this patch is based on prior patches from here:
http://patchwork.ozlabs.org/patch/740942/

> I also have the patch ready in which I have added support for Read (1-1-8 and 1-8-8) protocol and Write (1-1-8 and 1-8-8).
> Also have added support of Octal in driver/spi/spi.c framework.
>
> IMO, we would collaborate our patches.

That would be great, thanks!

Regards
Vignesh

> --
> Regards
> Yogesh Gaur
>
>> Signed-off-by: Vignesh R <[email protected]>
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
>> include/linux/mtd/spi-nor.h | 2 ++
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index
>> aff5e6ff0b2c..4926e805a8cb 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -90,6 +90,7 @@ struct flash_info {
>> #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip
>> erase */
>> #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
>> #define USE_CLSR BIT(14) /* use CLSR command */
>> +#define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */
>>
>> int (*quad_enable)(struct spi_nor *nor);
>> };
>> @@ -209,6 +210,7 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
>> { SPINOR_OP_READ_1_2_2, SPINOR_OP_READ_1_2_2_4B },
>> { SPINOR_OP_READ_1_1_4, SPINOR_OP_READ_1_1_4_4B },
>> { SPINOR_OP_READ_1_4_4, SPINOR_OP_READ_1_4_4_4B },
>> + { SPINOR_OP_READ_1_1_8, SPINOR_OP_READ_1_1_8_4B },
>>
>> { SPINOR_OP_READ_1_1_1_DTR,
>> SPINOR_OP_READ_1_1_1_DTR_4B },
>> { SPINOR_OP_READ_1_2_2_DTR,
>> SPINOR_OP_READ_1_2_2_DTR_4B },
>> @@ -1406,7 +1408,7 @@ static const struct flash_info spi_nor_ids[] = {
>> { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096, SECT_4K |
>> USE_FSR | SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
>>
>> /* Micron */
>> - { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
>> USE_FSR | SPI_NOR_4B_OPCODES) },
>> + { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512, SECT_4K |
>> USE_FSR
>> +| SPI_NOR_OCTAL_READ | SPI_NOR_4B_OPCODES) },
>>
>> /* PMC */
>> { "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) },
>> @@ -3199,6 +3201,13 @@ static int spi_nor_init_params(struct spi_nor *nor,
>> SNOR_PROTO_1_1_4);
>> }
>>
>> + if (info->flags & SPI_NOR_OCTAL_READ) {
>> + params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
>> + spi_nor_set_read_settings(&params-
>>> reads[SNOR_CMD_READ_1_1_8],
>> + 0, 8, SPINOR_OP_READ_1_1_8,
>> + SNOR_PROTO_1_1_8);
>> + }
>> +
>> /* Page Program settings. */
>> params->hwcaps.mask |= SNOR_HWCAPS_PP;
>> spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index
>> 8b1acf68b7ac..ae9861ed7e0f 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -50,6 +50,7 @@
>> #define SPINOR_OP_READ_1_2_2 0xbb /* Read data bytes (Dual I/O
>> SPI) */
>> #define SPINOR_OP_READ_1_1_4 0x6b /* Read data bytes (Quad
>> Output SPI) */
>> #define SPINOR_OP_READ_1_4_4 0xeb /* Read data bytes (Quad I/O
>> SPI) */
>> +#define SPINOR_OP_READ_1_1_8 0x8b /* Read data bytes (Octal
>> Output SPI) */
>> #define SPINOR_OP_PP 0x02 /* Page program (up to 256 bytes) */
>> #define SPINOR_OP_PP_1_1_4 0x32 /* Quad page program */
>> #define SPINOR_OP_PP_1_4_4 0x38 /* Quad page program */
>> @@ -73,6 +74,7 @@
>> #define SPINOR_OP_READ_1_2_2_4B 0xbc /* Read data bytes (Dual I/O
>> SPI) */
>> #define SPINOR_OP_READ_1_1_4_4B 0x6c /* Read data bytes (Quad
>> Output SPI) */
>> #define SPINOR_OP_READ_1_4_4_4B 0xec /* Read data bytes (Quad I/O
>> SPI) */
>> +#define SPINOR_OP_READ_1_1_8_4B 0x7c /* Read data bytes (Octal
>> Output SPI) */
>> #define SPINOR_OP_PP_4B 0x12 /* Page program (up to 256
>> bytes) */
>> #define SPINOR_OP_PP_1_1_4_4B 0x34 /* Quad page program */
>> #define SPINOR_OP_PP_1_4_4_4B 0x3e /* Quad page program */
>> --
>> 2.19.0
>

--
Regards
Vignesh

2018-10-04 11:12:23

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba



On Thursday 04 October 2018 03:15 PM, Boris Brezillon wrote:
> On Wed, 3 Oct 2018 22:26:01 +0530
> Vignesh R <[email protected]> wrote:
>
>> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
>> supports read/write over 8 IO lines simulatenously. Add support for
>> Octal read mode for Micron mt35xu512aba.
>> Unfortunately, this flash is only complaint to SFDP JESD216B and does not
>> seem to support newer JESD216C standard that provides auto detection of
>> Octal mode capabilities and opcodes. Therefore, this capability is
>> manually added using new SPI_NOR_OCTAL_READ flag.
>>
>> Signed-off-by: Vignesh R <[email protected]>
>> ---
>> drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
>> include/linux/mtd/spi-nor.h | 2 ++
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index aff5e6ff0b2c..4926e805a8cb 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -90,6 +90,7 @@ struct flash_info {
>> #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */
>> #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
>> #define USE_CLSR BIT(14) /* use CLSR command */
>> +#define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */
>
> Hm, we'll need to clarify what OCTAL means. I see at least 3 different
> modes using 8 IO lines (1-1-8, 1-8-8 and 8-8-8) and all of them could
> be qualified as "octal" modes.
> So how about renaming this macro SPI_NOR_1_1_8_READ.
>

My understanding is that, if a flash is Octal IO capable, then it
supports all mode of Octal IO (1-1-8, 1-8-8 and 8-8-8). At least in the
current code, I see SPI_NOR_QUAD_READ seems to imply 1-1-4, 1-4-4 and
4-4-4 modes.
Moreover, these capabilities should be auto discoverable once flash
start populating SFDP JESD216C tables (IIRC, JESD216B already takes care
of QUAD IO modes). So, I don't think we need different flags for 1-1-8,
1-8-8 and 8-8-8. Please let me if that's not the case.

For this patch, I wanted to start small and support only 1-1-8 mode
which is the easiest.


> Also, I fear we'll soon run out of bits in ->flags if we keep adding
> one flag per mode which is why I proposed a solution to let flash
> chips tweak the flash parameters as they wish [1][2]. I'm not saying we
> should do it now, but we should definitely plan for something like that.
>
> [1]https://github.com/bbrezillon/linux/commit/9c672e4c85a91f1b0803c9c6e4b8f3aae5d79ffb
> [2]https://github.com/bbrezillon/linux/commit/3a5515c8821314c06a3d84f9861aefe476bb711e
>

Hmm, it almost seems like Macronix flash warrants its own driver now.

--
Regards
Vignesh

2018-10-04 11:18:09

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 0/3] spi-nor: Add Octal SPI support

On Thu, 4 Oct 2018 16:05:36 +0530
Vignesh R <[email protected]> wrote:

> >>
> >> .../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
> >> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
> >
> > On a slightly different topic, do you plan to convert the Cadence
> > driver to spi-mem? And if you don't, is it because you don't have time
> > or because some features are missing in spi-mem (I remember you
> > mentioned a few things back when you were reviewing the spi-mem series)?
> >
>
> I do not have plans to convert cadence QSPI driver to spi-mem yet,
> mainly due to lack of time. Also, not sure if original author Marek and
> other altera people are okay with that.
>
> I see couple of issues in the way of conversion:
> 1. I would wait to know what direction would direct mapping APIs[1] go
> before starting spi-mem conversion for Cadence QSPI driver. Else, we
> have may to re write again if direct mapping APIs are merged.

I'd suggest reviewing the proposal I posted so that you can influence
the design of this new API ;-).

> 2. New Cadence OSPI IP has an integrated PHY to support high throughput
> OSPI flashes operating up 200MHz in Octal DDR mode. In order to work
> with such flashes, PHY DLLs need to be calibrated. Highly simplified
> calibration sequence is as below(See [2] for actual sequence):
> -Read flash ID at low speed and store it.
> -Enable PHY and set DLLs to a defined initial value
> -Increment RX DLL value
> -Read flash ID and check for correctness of data read
> -repeat above two steps until a band of passing values is obtained for
> RX DLL where flash ID is correctly read.
> -DLL needs to set to middle of the passing band.

Is the Read ID operation hardcoded or do you just use it as a way to
trigger predictable transfers on the IO bus?

>
> I am trying to figure out how to fit this into the spi-mem framework as
> controller would to need to store READ ID opcode and expected JEDEC ID
> before starting calibration sequence.

I think this should be split in 2:

- the SPI NOR framework passing the operation to use to do the
calibration (here a READ ID)
- the SPI controller framework replaying the same operation with
different DLL configs until it finds the best match

So, it would basically be added as a new hook:

int (*calibrate)(struct spi_mem *mem,
const struct spi_mem_op *tmpl);

and a new function provided by the spi-mem API

int spi_mem_calibrate(struct spi_mem *mem,
const struct spi_mem_op *tmpl);

and calibration outcome would be somehow attached to the spi_mem
object.

This way we stay memory agnostic but still provide the necessary blocks
at the spi-mem level to do such callibrations.

Would that work?

2018-10-04 11:28:44

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 1/3] mtd: spi-nor: Add Octal mode support for mt35xu512aba

On Thu, 4 Oct 2018 16:42:22 +0530
Vignesh R <[email protected]> wrote:

> On Thursday 04 October 2018 03:15 PM, Boris Brezillon wrote:
> > On Wed, 3 Oct 2018 22:26:01 +0530
> > Vignesh R <[email protected]> wrote:
> >
> >> Micron's mt35xu512aba flash is an Octal flash that has x8 IO lines. It
> >> supports read/write over 8 IO lines simulatenously. Add support for
> >> Octal read mode for Micron mt35xu512aba.
> >> Unfortunately, this flash is only complaint to SFDP JESD216B and does not
> >> seem to support newer JESD216C standard that provides auto detection of
> >> Octal mode capabilities and opcodes. Therefore, this capability is
> >> manually added using new SPI_NOR_OCTAL_READ flag.
> >>
> >> Signed-off-by: Vignesh R <[email protected]>
> >> ---
> >> drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
> >> include/linux/mtd/spi-nor.h | 2 ++
> >> 2 files changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> >> index aff5e6ff0b2c..4926e805a8cb 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -90,6 +90,7 @@ struct flash_info {
> >> #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */
> >> #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */
> >> #define USE_CLSR BIT(14) /* use CLSR command */
> >> +#define SPI_NOR_OCTAL_READ BIT(15) /* Flash supports Octal Read */
> >
> > Hm, we'll need to clarify what OCTAL means. I see at least 3 different
> > modes using 8 IO lines (1-1-8, 1-8-8 and 8-8-8) and all of them could
> > be qualified as "octal" modes.
> > So how about renaming this macro SPI_NOR_1_1_8_READ.
> >
>
> My understanding is that, if a flash is Octal IO capable, then it
> supports all mode of Octal IO (1-1-8, 1-8-8 and 8-8-8). At least in the
> current code, I see SPI_NOR_QUAD_READ seems to imply 1-1-4, 1-4-4 and
> 4-4-4 modes.

I already have one NOR which is not following this rule (mx25uw51245g)
and is only supporting 1-1-1 and 8-8-8, nothing in between.

> Moreover, these capabilities should be auto discoverable once flash
> start populating SFDP JESD216C tables (IIRC, JESD216B already takes care
> of QUAD IO modes). So, I don't think we need different flags for 1-1-8,
> 1-8-8 and 8-8-8. Please let me if that's not the case.

Except we already have NORs out there that do not have a valid SFDP
table but still support a subset of the octal modes (mx25uw51245g is in
this case, but I'm pretty sure there are others).

>
> For this patch, I wanted to start small and support only 1-1-8 mode
> which is the easiest.

Which is fine, just wanted to know what SPI_NOR_OCTAL_READ meant.

>
>
> > Also, I fear we'll soon run out of bits in ->flags if we keep adding
> > one flag per mode which is why I proposed a solution to let flash
> > chips tweak the flash parameters as they wish [1][2]. I'm not saying we
> > should do it now, but we should definitely plan for something like that.
> >
> > [1]https://github.com/bbrezillon/linux/commit/9c672e4c85a91f1b0803c9c6e4b8f3aae5d79ffb
> > [2]https://github.com/bbrezillon/linux/commit/3a5515c8821314c06a3d84f9861aefe476bb711e
> >
>
> Hmm, it almost seems like Macronix flash warrants its own driver now.
>

Actually, we already have quite a bit of code that is vendor (and even
chip) specific in spi-nor.c, and I indeed think it would be beneficial
to have one driver per manufacturer so that the core is not polluted by
those chip/manufacturer specific hacks. The thing is, the spi_nor
interface (and the way hwcaps selection works) is not ready for that.

2018-10-08 15:37:29

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH 0/3] spi-nor: Add Octal SPI support

Hi Boris,

Sorry I missed this mail.

On Thursday 04 October 2018 04:47 PM, Boris Brezillon wrote:
> On Thu, 4 Oct 2018 16:05:36 +0530
> Vignesh R <[email protected]> wrote:
>
>>>>
>>>> .../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
>>>> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
>>>
>>> On a slightly different topic, do you plan to convert the Cadence
>>> driver to spi-mem? And if you don't, is it because you don't have time
>>> or because some features are missing in spi-mem (I remember you
>>> mentioned a few things back when you were reviewing the spi-mem series)?
>>>
>>
>> I do not have plans to convert cadence QSPI driver to spi-mem yet,
>> mainly due to lack of time. Also, not sure if original author Marek and
>> other altera people are okay with that.
>>
>> I see couple of issues in the way of conversion:
>> 1. I would wait to know what direction would direct mapping APIs[1] go
>> before starting spi-mem conversion for Cadence QSPI driver. Else, we
>> have may to re write again if direct mapping APIs are merged.
>
> I'd suggest reviewing the proposal I posted so that you can influence
> the design of this new API ;-).
>

I did take a look and proposal seems fine. Will try to prototype and
test cadence QSPI driver with these. Thanks for the patches!


>> 2. New Cadence OSPI IP has an integrated PHY to support high throughput
>> OSPI flashes operating up 200MHz in Octal DDR mode. In order to work
>> with such flashes, PHY DLLs need to be calibrated. Highly simplified
>> calibration sequence is as below(See [2] for actual sequence):
>> -Read flash ID at low speed and store it.
>> -Enable PHY and set DLLs to a defined initial value
>> -Increment RX DLL value
>> -Read flash ID and check for correctness of data read
>> -repeat above two steps until a band of passing values is obtained for
>> RX DLL where flash ID is correctly read.
>> -DLL needs to set to middle of the passing band.
>
> Is the Read ID operation hardcoded or do you just use it as a way to
> trigger predictable transfers on the IO bus?
>

Just a way to trigger predictable data reads.

>>
>> I am trying to figure out how to fit this into the spi-mem framework as
>> controller would to need to store READ ID opcode and expected JEDEC ID
>> before starting calibration sequence.
>
> I think this should be split in 2:
>
> - the SPI NOR framework passing the operation to use to do the
> calibration (here a READ ID)
> - the SPI controller framework replaying the same operation with
> different DLL configs until it finds the best match
>
> So, it would basically be added as a new hook:
>
> int (*calibrate)(struct spi_mem *mem,
> const struct spi_mem_op *tmpl);
>
> and a new function provided by the spi-mem API
>
> int spi_mem_calibrate(struct spi_mem *mem,
> const struct spi_mem_op *tmpl);
>
> and calibration outcome would be somehow attached to the spi_mem
> object.
>
> This way we stay memory agnostic but still provide the necessary blocks
> at the spi-mem level to do such callibrations.
>
> Would that work?
>

That would work and hopefully is not intrusive to spi-mem framework.

--
Regards
Vignesh

2018-10-12 08:53:30

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 0/3] spi-nor: Add Octal SPI support

Hi Vignesh,

On Mon, 8 Oct 2018 21:06:02 +0530
Vignesh R <[email protected]> wrote:

> Hi Boris,
>
> Sorry I missed this mail.
>
> On Thursday 04 October 2018 04:47 PM, Boris Brezillon wrote:
> > On Thu, 4 Oct 2018 16:05:36 +0530
> > Vignesh R <[email protected]> wrote:
> >
> >>>>
> >>>> .../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
> >>>> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
> >>>
> >>> On a slightly different topic, do you plan to convert the Cadence
> >>> driver to spi-mem? And if you don't, is it because you don't have time
> >>> or because some features are missing in spi-mem (I remember you
> >>> mentioned a few things back when you were reviewing the spi-mem series)?
> >>>
> >>
> >> I do not have plans to convert cadence QSPI driver to spi-mem yet,
> >> mainly due to lack of time. Also, not sure if original author Marek and
> >> other altera people are okay with that.
> >>
> >> I see couple of issues in the way of conversion:
> >> 1. I would wait to know what direction would direct mapping APIs[1] go
> >> before starting spi-mem conversion for Cadence QSPI driver. Else, we
> >> have may to re write again if direct mapping APIs are merged.
> >
> > I'd suggest reviewing the proposal I posted so that you can influence
> > the design of this new API ;-).
> >
>
> I did take a look and proposal seems fine. Will try to prototype and
> test cadence QSPI driver with these. Thanks for the patches!

That's great news! Let me know how it goes, and don't hesitate to ask
if you have any questions.

>
>
> >> 2. New Cadence OSPI IP has an integrated PHY to support high throughput
> >> OSPI flashes operating up 200MHz in Octal DDR mode. In order to work
> >> with such flashes, PHY DLLs need to be calibrated. Highly simplified
> >> calibration sequence is as below(See [2] for actual sequence):
> >> -Read flash ID at low speed and store it.
> >> -Enable PHY and set DLLs to a defined initial value
> >> -Increment RX DLL value
> >> -Read flash ID and check for correctness of data read
> >> -repeat above two steps until a band of passing values is obtained for
> >> RX DLL where flash ID is correctly read.
> >> -DLL needs to set to middle of the passing band.
> >
> > Is the Read ID operation hardcoded or do you just use it as a way to
> > trigger predictable transfers on the IO bus?
> >
>
> Just a way to trigger predictable data reads.

Good.


>
> >>
> >> I am trying to figure out how to fit this into the spi-mem framework as
> >> controller would to need to store READ ID opcode and expected JEDEC ID
> >> before starting calibration sequence.
> >
> > I think this should be split in 2:
> >
> > - the SPI NOR framework passing the operation to use to do the
> > calibration (here a READ ID)
> > - the SPI controller framework replaying the same operation with
> > different DLL configs until it finds the best match
> >
> > So, it would basically be added as a new hook:
> >
> > int (*calibrate)(struct spi_mem *mem,
> > const struct spi_mem_op *tmpl);
> >
> > and a new function provided by the spi-mem API
> >
> > int spi_mem_calibrate(struct spi_mem *mem,
> > const struct spi_mem_op *tmpl);
> >
> > and calibration outcome would be somehow attached to the spi_mem
> > object.
> >
> > This way we stay memory agnostic but still provide the necessary blocks
> > at the spi-mem level to do such callibrations.
> >
> > Would that work?
> >
>
> That would work and hopefully is not intrusive to spi-mem framework.
>

Okay. Don't hesitate to post a proposal along those lines and I'll try
to review it.

Thanks,

Boris

2018-10-15 19:12:00

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/3] dt-bindings: cadence-quadspi: Add new compatible for AM654 SoC

On Wed, 3 Oct 2018 22:26:02 +0530, Vignesh R wrote:
> AM654 SoC has Cadence Octal SPI controller, which is similar to Cadence
> QSPI controller but supports Octal IO(x8 data lines) and Double Data
> Rate(DDR) mode. Add new compatible to support OSPI controller on TI's
> AM654 SoCs.
>
> Signed-off-by: Vignesh R <[email protected]>
> ---
> Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
> 1 file changed, 1 insertion(+)
>

Reviewed-by: Rob Herring <[email protected]>

2018-12-09 08:47:37

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH 0/3] spi-nor: Add Octal SPI support

Hi Boris,

On 03/10/18 10:26 PM, Vignesh R wrote:
> This series adds support for octal mode of mt35x flash. Also, adds
> support for OSPI version of Cadence QSPI controller.
>
> Based on top of patches adding basic support for mt35xu512aba here:
> https://patchwork.ozlabs.org/cover/971437/
>
> Vignesh R (3):
> mtd: spi-nor: Add Octal mode support for mt35xu512aba


> dt-bindings: cadence-quadspi: Add new compatible for AM654 SoC
> mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller

Could you consider above two patches for v4.21 once [1][2] are merged?

[1] https://patchwork.ozlabs.org/patch/1006717/
[2] https://patchwork.ozlabs.org/patch/1006715/

Let me know if this needs to be re-posted.

>
> .../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
> drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
> include/linux/mtd/spi-nor.h | 2 ++
> 4 files changed, 22 insertions(+), 1 deletion(-)
>

--
Regards
Vignesh

2018-12-10 09:18:08

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 3/3] mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller

On Wed, 3 Oct 2018 22:26:03 +0530
Vignesh R <[email protected]> wrote:

> Cadence OSPI controller IP supports Octal IO (x8 IO lines),
> It also has an integrated PHY. IP register layout is very
> similar to existing QSPI IP except for additional bits to support Octal
> and Octal DDR mode. Therefore, extend current driver to support Octal
> mode.
>
> Signed-off-by: Vignesh R <[email protected]>
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index e24db817154e..48b00e75a879 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -101,6 +101,7 @@ struct cqspi_st {
> #define CQSPI_INST_TYPE_SINGLE 0
> #define CQSPI_INST_TYPE_DUAL 1
> #define CQSPI_INST_TYPE_QUAD 2
> +#define CQSPI_INST_TYPE_OCTAL 3
>
> #define CQSPI_DUMMY_CLKS_PER_BYTE 8
> #define CQSPI_DUMMY_BYTES_MAX 4
> @@ -898,6 +899,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
> case SNOR_PROTO_1_1_4:
> f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
> break;
> + case SNOR_PROTO_1_1_8:
> + f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
> + break;
> default:
> return -EINVAL;
> }
> @@ -1205,6 +1209,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> SNOR_HWCAPS_READ_FAST |
> SNOR_HWCAPS_READ_1_1_2 |
> SNOR_HWCAPS_READ_1_1_4 |
> + SNOR_HWCAPS_READ_1_1_8 |

Is this really supported on qspi versions of this IP? I guess not given
the description in the commit message and the name of the new
compatible (ospi instead of qspi).

> SNOR_HWCAPS_PP,
> };
> struct platform_device *pdev = cqspi->pdev;
> @@ -1456,6 +1461,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
> .compatible = "ti,k2g-qspi",
> .data = (void *)CQSPI_NEEDS_WR_DELAY,
> },
> + {
> + .compatible = "ti,am654-ospi",
> + .data = (void *)CQSPI_NEEDS_WR_DELAY,
> + },
> { /* end of table */ }
> };
>


2018-12-10 09:20:05

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 0/3] spi-nor: Add Octal SPI support

On Sun, 9 Dec 2018 14:17:18 +0530
Vignesh R <[email protected]> wrote:

> Hi Boris,
>
> On 03/10/18 10:26 PM, Vignesh R wrote:
> > This series adds support for octal mode of mt35x flash. Also, adds
> > support for OSPI version of Cadence QSPI controller.
> >
> > Based on top of patches adding basic support for mt35xu512aba here:
> > https://patchwork.ozlabs.org/cover/971437/
> >
> > Vignesh R (3):
> > mtd: spi-nor: Add Octal mode support for mt35xu512aba
>
>
> > dt-bindings: cadence-quadspi: Add new compatible for AM654 SoC
> > mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller
>
> Could you consider above two patches for v4.21 once [1][2] are merged?
>
> [1] https://patchwork.ozlabs.org/patch/1006717/
> [2] https://patchwork.ozlabs.org/patch/1006715/

I asked Tudor to have a look at those patches. I'll apply them once I
have his R-b.

>
> Let me know if this needs to be re-posted.
>
> >
> > .../devicetree/bindings/mtd/cadence-quadspi.txt | 1 +
> > drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
> > drivers/mtd/spi-nor/spi-nor.c | 11 ++++++++++-
> > include/linux/mtd/spi-nor.h | 2 ++
> > 4 files changed, 22 insertions(+), 1 deletion(-)
> >
>


2018-12-10 12:43:24

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 3/3] mtd: spi-nor: cadence-quadspi: Add support for Octal SPI controller

On Mon, 10 Dec 2018 16:49:29 +0530
Vignesh R <[email protected]> wrote:

> On 10/12/18 2:15 PM, Boris Brezillon wrote:
> > On Wed, 3 Oct 2018 22:26:03 +0530
> > Vignesh R <[email protected]> wrote:
> >
> >> Cadence OSPI controller IP supports Octal IO (x8 IO lines),
> >> It also has an integrated PHY. IP register layout is very
> >> similar to existing QSPI IP except for additional bits to support Octal
> >> and Octal DDR mode. Therefore, extend current driver to support Octal
> >> mode.
> >>
> >> Signed-off-by: Vignesh R <[email protected]>
> >> ---
> >> drivers/mtd/spi-nor/cadence-quadspi.c | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> >> index e24db817154e..48b00e75a879 100644
> >> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> >> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> >> @@ -101,6 +101,7 @@ struct cqspi_st {
> >> #define CQSPI_INST_TYPE_SINGLE 0
> >> #define CQSPI_INST_TYPE_DUAL 1
> >> #define CQSPI_INST_TYPE_QUAD 2
> >> +#define CQSPI_INST_TYPE_OCTAL 3
> >>
> >> #define CQSPI_DUMMY_CLKS_PER_BYTE 8
> >> #define CQSPI_DUMMY_BYTES_MAX 4
> >> @@ -898,6 +899,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
> >> case SNOR_PROTO_1_1_4:
> >> f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
> >> break;
> >> + case SNOR_PROTO_1_1_8:
> >> + f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
> >> + break;
> >> default:
> >> return -EINVAL;
> >> }
> >> @@ -1205,6 +1209,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> >> SNOR_HWCAPS_READ_FAST |
> >> SNOR_HWCAPS_READ_1_1_2 |
> >> SNOR_HWCAPS_READ_1_1_4 |
> >> + SNOR_HWCAPS_READ_1_1_8 |
> >
> > Is this really supported on qspi versions of this IP? I guess not given
> > the description in the commit message and the name of the new
> > compatible (ospi instead of qspi).
>
> No, qspi version does not support Octal mode. I guess you are pointing
> out its logically wrong for driver with "*-qspi" compatible to declare
> SNOR_HWCAPS_READ_1_1_8 capability.

Exactly.

> Will update patch to declare SNOR_HWCAPS_READ_1_1_8 based on compatible.

Thanks.