[PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[Sudip Mukherjee]

Some Synopsys SSI controllers support enhanced SPI which includes
Dual mode, Quad mode and Octal mode. DWC_ssi includes clock stretching
feature in enhanced SPI modes which can be used to prevent FIFO underflow
and overflow conditions while transmitting or receiving the data respectively.
This is only tested on controller version 1.03a.

Ben Dooks (1):
  spi: dw-apb-ssi: add generic 1.03a version

Sudip Mukherjee (10):
  spi: dw: define capability for enhanced spi
  spi: dw: add check for support of dual/quad/octal
  spi: dw: define spi_frf for dual/quad/octal modes
  spi: dw: use TMOD_RO to read in enhanced spi modes
  spi: dw: define SPI_CTRLR0 register and its fields
  spi: dw: update SPI_CTRLR0 register
  spi: dw: update NDF while writing in enhanced spi mode
  spi: dw: update buffer for enhanced spi mode
  spi: dw: prepare the transfer routine for enhanced mode
  spi: dw: initialize dwc-ssi-1.03a controller

 .../bindings/spi/snps,dw-apb-ssi.yaml         |   1 +
 drivers/spi/spi-dw-core.c                     | 288 ++++++++++++++++--
 drivers/spi/spi-dw-mmio.c                     |  10 +
 drivers/spi/spi-dw.h                          |  19 ++
 4 files changed, 291 insertions(+), 27 deletions(-)

-- 
2.30.2

[PATCH 01/11] spi: dw: define capability for enhanced spi

[Sudip Mukherjee]

Some Synopsys SSI controllers support enhanced SPI which includes
Dual mode, Quad mode and Octal mode. Define the capability and mention
it in the controller supported modes.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 4 ++++
 drivers/spi/spi-dw.h      | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index f87d97ccd2d6..97e72da7c120 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -917,6 +917,10 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 
 	master->use_gpio_descriptors = true;
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
+	if (dws->caps & DW_SPI_CAP_EXT_SPI)
+		master->mode_bits |= SPI_TX_DUAL | SPI_RX_DUAL |
+				SPI_TX_QUAD | SPI_RX_QUAD |
+				SPI_TX_OCTAL | SPI_RX_OCTAL;
 	if (dws->caps & DW_SPI_CAP_DFS32)
 		master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
 	else
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 9e8eb2b52d5c..71d18e9291a3 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -32,6 +32,7 @@
 /* DW SPI controller capabilities */
 #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
 #define DW_SPI_CAP_DFS32		BIT(1)
+#define DW_SPI_CAP_EXT_SPI		BIT(2)
 
 /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
 #define DW_SPI_CTRLR0			0x00
-- 
2.30.2

Re: [PATCH 01/11] spi: dw: define capability for enhanced spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote:

> Some Synopsys SSI controllers support enhanced SPI which includes
> Dual mode, Quad mode and Octal mode. Define the capability and mention
> it in the controller supported modes.

> +#define DW_SPI_CAP_EXT_SPI		BIT(2)

This isn't at all descriptive, it'd be better to have a capability bit
for the various multi-line data modes (or possibly individual bits for
them, though board setup will stop us using things that aren't supported
in a given design anyway so it's a bit redundant) - that'd be a lot
clearer and avoid confusion further down the line when some other
feature gets added.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 01/11] spi: dw: define capability for enhanced spi

[Sudip Mukherjee]

On Tue, Aug 2, 2022 at 7:48 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote:
>
> > Some Synopsys SSI controllers support enhanced SPI which includes
> > Dual mode, Quad mode and Octal mode. Define the capability and mention
> > it in the controller supported modes.
>
> > +#define DW_SPI_CAP_EXT_SPI           BIT(2)
>
> This isn't at all descriptive, it'd be better to have a capability bit
> for the various multi-line data modes (or possibly individual bits for
> them, though board setup will stop us using things that aren't supported
> in a given design anyway so it's a bit redundant) - that'd be a lot
> clearer and avoid confusion further down the line when some other
> feature gets added.

Do you mean to add separate capability bit like:
DW_SPI_CAP_DUAL_SPI
DW_SPI_CAP_QUAD_SPI and
DW_SPI_CAP_OCTAL_SPI ?


--
Regards
Sudip

Re: [PATCH 01/11] spi: dw: define capability for enhanced spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On Wed, Aug 03, 2022 at 06:34:53PM +0100, Sudip Mukherjee wrote:
> On Tue, Aug 2, 2022 at 7:48 PM Mark Brown <broonie@kernel.org> wrote:
> > On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote:

> > > +#define DW_SPI_CAP_EXT_SPI           BIT(2)

> > This isn't at all descriptive, it'd be better to have a capability bit
> > for the various multi-line data modes (or possibly individual bits for
> > them, though board setup will stop us using things that aren't supported
> > in a given design anyway so it's a bit redundant) - that'd be a lot
> > clearer and avoid confusion further down the line when some other
> > feature gets added.

> Do you mean to add separate capability bit like:
> DW_SPI_CAP_DUAL_SPI
> DW_SPI_CAP_QUAD_SPI and
> DW_SPI_CAP_OCTAL_SPI ?

Either that or some rolled together capability with an at least somewhat
descriptive name.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 01/11] spi: dw: define capability for enhanced spi

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:45PM +0100, Sudip Mukherjee wrote:
> Some Synopsys SSI controllers support enhanced SPI which includes
> Dual mode, Quad mode and Octal mode. Define the capability and mention
> it in the controller supported modes.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 4 ++++
>  drivers/spi/spi-dw.h      | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index f87d97ccd2d6..97e72da7c120 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -917,6 +917,10 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>  
>  	master->use_gpio_descriptors = true;
>  	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;

> +	if (dws->caps & DW_SPI_CAP_EXT_SPI)
> +		master->mode_bits |= SPI_TX_DUAL | SPI_RX_DUAL |
> +				SPI_TX_QUAD | SPI_RX_QUAD |
> +				SPI_TX_OCTAL | SPI_RX_OCTAL;

Is it possible to auto-detect the highest supported mode by, for
instance, writing to the CTRL0.SPI_FRF field values up to the maximum
one? The DW SSI hardware manual says that the unsupported mode
combinations are reserved. Could the reserved modes still be written
to the SPI_FRF field? If not we could use it to set the
SPI_{TX,RX}_DUAL, SPI_{TX,RX}_QUAD, SPI_{TX,RX}_OCTAL in accordance
with the actual device capabilities rather than setting all of them.

>  	if (dws->caps & DW_SPI_CAP_DFS32)
>  		master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
>  	else
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 9e8eb2b52d5c..71d18e9291a3 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -32,6 +32,7 @@
>  /* DW SPI controller capabilities */
>  #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
>  #define DW_SPI_CAP_DFS32		BIT(1)

> +#define DW_SPI_CAP_EXT_SPI		BIT(2)

EXT-prefix is misleading. The feature is called "Enhanced SPI Modes",
not Extended SPI modes. Perhaps something like DW_SPI_CAP_EMODE ?

-Sergey

>  
>  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
>  #define DW_SPI_CTRLR0			0x00
> -- 
> 2.30.2
> 

[PATCH 02/11] spi: dw: add check for support of dual/quad/octal

[Sudip Mukherjee]

Before doing the mem op spi controller will be queried about the
buswidths it supports. Add the dual/quad/octal if the controller
has the DW_SPI_CAP_EXT_SPI capability.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 97e72da7c120..77529e359b6d 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -488,8 +488,23 @@ static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 static bool dw_spi_supports_mem_op(struct spi_mem *mem,
 				   const struct spi_mem_op *op)
 {
-	if (op->data.buswidth > 1 || op->addr.buswidth > 1 ||
-	    op->dummy.buswidth > 1 || op->cmd.buswidth > 1)
+	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);
+
+	/*
+	 * Only support TT0 mode in enhanced SPI for now.
+	 * TT0 = Instruction and Address will be sent in
+	 * Standard SPI Mode.
+	 */
+	if (op->addr.buswidth > 1 || op->dummy.buswidth > 1 ||
+	    op->cmd.buswidth > 1)
+		return false;
+
+	/* In enhanced SPI 1, 2, 4, 8 all are valid modes. */
+	if (op->data.buswidth > 1 && (!(dws->caps & DW_SPI_CAP_EXT_SPI)))
+		return false;
+
+	/* Only support upto 32 bit address in enhanced SPI for now. */
+	if (op->data.buswidth > 1 && op->addr.nbytes > 4)
 		return false;
 
 	return spi_mem_default_supports_op(mem, op);
-- 
2.30.2

Re: [PATCH 02/11] spi: dw: add check for support of dual/quad/octal

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:46PM +0100, Sudip Mukherjee wrote:
> Before doing the mem op spi controller will be queried about the
> buswidths it supports. Add the dual/quad/octal if the controller
> has the DW_SPI_CAP_EXT_SPI capability.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 97e72da7c120..77529e359b6d 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -488,8 +488,23 @@ static int dw_spi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>  static bool dw_spi_supports_mem_op(struct spi_mem *mem,
>  				   const struct spi_mem_op *op)
>  {
> -	if (op->data.buswidth > 1 || op->addr.buswidth > 1 ||
> -	    op->dummy.buswidth > 1 || op->cmd.buswidth > 1)

> +	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);
> +
> +	/*
> +	 * Only support TT0 mode in enhanced SPI for now.
> +	 * TT0 = Instruction and Address will be sent in
> +	 * Standard SPI Mode.
> +	 */
> +	if (op->addr.buswidth > 1 || op->dummy.buswidth > 1 ||
> +	    op->cmd.buswidth > 1)
> +		return false;
> +
> +	/* In enhanced SPI 1, 2, 4, 8 all are valid modes. */
> +	if (op->data.buswidth > 1 && (!(dws->caps & DW_SPI_CAP_EXT_SPI)))
> +		return false;
> +
> +	/* Only support upto 32 bit address in enhanced SPI for now. */
> +	if (op->data.buswidth > 1 && op->addr.nbytes > 4)

Sorry, it can't be done as you suggest. First you get to update the
supported mem-ops method with no adding actual ops support. Thus the
driver will permit submitting the enhanced SPI memory operations, but
won't handle them correctly. So the driver will be left broken after
this commit. Second the dummy bytes are supported for the Rx
operations only. Thirdly you don't check the length of the cmd field,
but you need to unless you add the 2-bytes long opcode support. The
conditional statement for the address data length is incorrect. Since
you get to fix the field length to 4 bytes the address will be always
sent as 4 bytes. Fifthly you haven't fixed the
dw_spi_adjust_mem_op_size() method, which permits SPI MEM Tx-transfers
of any size. It isn't true in your case. Finally you can easily add
the cmd and address sent via the same buswidth as specified for the
data by means of the SPI_CTRL0.TRANS_TYPE field settings, thus
supporting all the possible modes.

Instead I suggest for you to do the next things in this case:
1. Create separate dw_spi_supports_enh_mem_op() and
dw_spi_adjust_enh_mem_op_size() for the case of the Enhanced SPI
modes.
2. Return false if the dummy cycle is requested for the
Tx-operation.
3. Return false if:
   (op->addr.nbytes != 0 && op->addr.buswidth != 1 && op->addr.buswidth != op->data.buswidth) ||
   (op->cmd.buswidth != 1 && op->cmd.buswidth != op->addr.buswidth && op->cmd.buswidth != op->data.buswidth)
4. Return false if:
   (op->dummy.nbytes != 0 && op->dummy.nbytes / op->dummy.buswidth > 4)
5. Make sure the dw_spi_adjust_enh_mem_op_size() method fixes both Rx
and Tx transfer lengths.

All of the above needs to be added either in the framework of the
patch with the new SPI MEM ops functionality or in a ways so the
currently unsupported operations wouldn't be available for now.

-Sergey

>  		return false;
>  
>  	return spi_mem_default_supports_op(mem, op);
> -- 
> 2.30.2
> 

[PATCH 03/11] spi: dw: define spi_frf for dual/quad/octal modes

[Sudip Mukherjee]

The SPI mode needs to be mentioned in CTRLR0[23:22] register. Define a
configuration variable to keep the mode based on the buswidth, which will
then be used to update CR0. If the transfer is using dual/quad/octal
mode then mark enhanced_spi as true.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++
 drivers/spi/spi-dw.h      |  7 +++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 77529e359b6d..8c84a2e991b5 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -333,6 +333,14 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
 		/* CTRLR0[11:10] Transfer Mode */
 		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
 
+	if (dws->caps & DW_SPI_CAP_EXT_SPI) {
+		if (cfg->spi_frf)
+			cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK,
+				cfg->spi_frf);
+		else
+			cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK;
+	}
+
 	dw_writel(dws, DW_SPI_CTRLR0, cr0);
 
 	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
@@ -679,10 +687,31 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
 static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);
+	bool enhanced_spi = false;
 	struct dw_spi_cfg cfg;
 	unsigned long flags;
 	int ret;
 
+	if (dws->caps & DW_SPI_CAP_EXT_SPI) {
+		switch (op->data.buswidth) {
+		case 2:
+			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI;
+			enhanced_spi = true;
+			break;
+		case 4:
+			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI;
+			enhanced_spi = true;
+			break;
+		case 8:
+			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_OCT_SPI;
+			enhanced_spi = true;
+			break;
+		default:
+			cfg.spi_frf = 0;
+			break;
+		}
+	}
+
 	/*
 	 * Collect the outbound data into a single buffer to speed the
 	 * transmission up at least on the initial stage.
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 71d18e9291a3..b8cc20e0deaa 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -96,6 +96,12 @@
 #define DW_HSSI_CTRLR0_SRL			BIT(13)
 #define DW_HSSI_CTRLR0_MST			BIT(31)
 
+/* Bit fields in CTRLR0 for enhanced SPI */
+#define DW_HSSI_CTRLR0_SPI_FRF_MASK		GENMASK(23, 22)
+#define DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI		0x1
+#define DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI		0x2
+#define DW_SSI_CTRLR0_SPI_FRF_OCT_SPI		0x3
+
 /* Bit fields in CTRLR1 */
 #define DW_SPI_NDF_MASK				GENMASK(15, 0)
 
@@ -136,6 +142,7 @@ struct dw_spi_cfg {
 	u8 dfs;
 	u32 ndf;
 	u32 freq;
+	u8 spi_frf;
 };
 
 struct dw_spi;
-- 
2.30.2

Re: [PATCH 03/11] spi: dw: define spi_frf for dual/quad/octal modes

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:47PM +0100, Sudip Mukherjee wrote:
> The SPI mode needs to be mentioned in CTRLR0[23:22] register. Define a
> configuration variable to keep the mode based on the buswidth, which will
> then be used to update CR0. If the transfer is using dual/quad/octal
> mode then mark enhanced_spi as true.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++
>  drivers/spi/spi-dw.h      |  7 +++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 77529e359b6d..8c84a2e991b5 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -333,6 +333,14 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
>  		/* CTRLR0[11:10] Transfer Mode */
>  		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
>  

> +	if (dws->caps & DW_SPI_CAP_EXT_SPI) {

Drop this conditional statement. Just always set the spi_frf field.


> +		if (cfg->spi_frf)

Drop this conditional statement. Just always set the spi_frf. It shall
be zero if the Enhanced SPI modes are unsupported.

> +			cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK,

Since the SPI_FRF field has different offset in the DW APB and DW AHB
SSI controllers, you'll need to initialize the mode based on the
IP-core ID using the dw_spi_ip_is() macro (see the as it's done for
tmode).

> +				cfg->spi_frf);
> +		else

> +			cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK;

This is redundant. The initial value never has the SPI_FRF field set
(see the dw_spi_setup() and dw_spi_prepare_cr0() methods for details).

> +	}
> +
>  	dw_writel(dws, DW_SPI_CTRLR0, cr0);
>  
>  	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
> @@ -679,10 +687,31 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
>  static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  {
>  	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);
> +	bool enhanced_spi = false;
>  	struct dw_spi_cfg cfg;
>  	unsigned long flags;
>  	int ret;
>  

> +	if (dws->caps & DW_SPI_CAP_EXT_SPI) {
> +		switch (op->data.buswidth) {
> +		case 2:
> +			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI;
> +			enhanced_spi = true;
> +			break;
> +		case 4:
> +			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI;
> +			enhanced_spi = true;
> +			break;
> +		case 8:
> +			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_OCT_SPI;
> +			enhanced_spi = true;
> +			break;
> +		default:
> +			cfg.spi_frf = 0;
> +			break;
> +		}
> +	}
> +

Please create a separate dw_spi_exec_enh_mem_op() method for the
Enhanced SPI communications and parse the data.buswidth field there.
Also it would be better to define a new macro
DW_SSI_CTRLR0_SPI_FRF_STD_SPI and use it instead of the explicit zero
SPI_FRF value.

>  	/*
>  	 * Collect the outbound data into a single buffer to speed the
>  	 * transmission up at least on the initial stage.
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 71d18e9291a3..b8cc20e0deaa 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -96,6 +96,12 @@
>  #define DW_HSSI_CTRLR0_SRL			BIT(13)
>  #define DW_HSSI_CTRLR0_MST			BIT(31)
>  

> +/* Bit fields in CTRLR0 for enhanced SPI */

No need in the comment. The macros name is descriptive enough.

> +#define DW_HSSI_CTRLR0_SPI_FRF_MASK		GENMASK(23, 22)

Please also add the next new macros
#define DW_PSSI_CTRLR0_SPI_FRF_MASK		GENMASK(22, 21)
#define DW_SSI_CTRLR0_SPI_FRF_STD_SPI		0x0

and group them together with the rest of the APB SSI CTRL0 macros.

> +#define DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI		0x1
> +#define DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI		0x2
> +#define DW_SSI_CTRLR0_SPI_FRF_OCT_SPI		0x3

Please move these to be defined together with the APB SSI
CTRL0.SPI_FRF-related macros.

-Sergey

> +
>  /* Bit fields in CTRLR1 */
>  #define DW_SPI_NDF_MASK				GENMASK(15, 0)
>  
> @@ -136,6 +142,7 @@ struct dw_spi_cfg {
>  	u8 dfs;
>  	u32 ndf;
>  	u32 freq;
> +	u8 spi_frf;
>  };
>  
>  struct dw_spi;
> -- 
> 2.30.2
> 

Re: [PATCH 03/11] spi: dw: define spi_frf for dual/quad/octal modes

[Serge Semin]

On Sat, Aug 27, 2022 at 01:03:22AM +0300, Serge Semin wrote:
> On Tue, Aug 02, 2022 at 06:57:47PM +0100, Sudip Mukherjee wrote:
> > The SPI mode needs to be mentioned in CTRLR0[23:22] register. Define a
> > configuration variable to keep the mode based on the buswidth, which will
> > then be used to update CR0. If the transfer is using dual/quad/octal
> > mode then mark enhanced_spi as true.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> > ---
> >  drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++
> >  drivers/spi/spi-dw.h      |  7 +++++++
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index 77529e359b6d..8c84a2e991b5 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -333,6 +333,14 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
> >  		/* CTRLR0[11:10] Transfer Mode */
> >  		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
> >  
> 
> > +	if (dws->caps & DW_SPI_CAP_EXT_SPI) {
> 
> Drop this conditional statement. Just always set the spi_frf field.
> 
> 
> > +		if (cfg->spi_frf)
> 
> Drop this conditional statement. Just always set the spi_frf. It shall
> be zero if the Enhanced SPI modes are unsupported.
> 
> > +			cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK,
> 
> Since the SPI_FRF field has different offset in the DW APB and DW AHB
> SSI controllers, you'll need to initialize the mode based on the
> IP-core ID using the dw_spi_ip_is() macro (see the as it's done for
> tmode).
> 
> > +				cfg->spi_frf);
> > +		else
> 
> > +			cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK;
> 
> This is redundant. The initial value never has the SPI_FRF field set
> (see the dw_spi_setup() and dw_spi_prepare_cr0() methods for details).
> 
> > +	}
> > +
> >  	dw_writel(dws, DW_SPI_CTRLR0, cr0);
> >  
> >  	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
> > @@ -679,10 +687,31 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
> >  static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
> >  {
> >  	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);
> > +	bool enhanced_spi = false;
> >  	struct dw_spi_cfg cfg;
> >  	unsigned long flags;
> >  	int ret;
> >  
> 
> > +	if (dws->caps & DW_SPI_CAP_EXT_SPI) {
> > +		switch (op->data.buswidth) {
> > +		case 2:
> > +			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI;
> > +			enhanced_spi = true;
> > +			break;
> > +		case 4:
> > +			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI;
> > +			enhanced_spi = true;
> > +			break;
> > +		case 8:
> > +			cfg.spi_frf = DW_SSI_CTRLR0_SPI_FRF_OCT_SPI;
> > +			enhanced_spi = true;
> > +			break;
> > +		default:
> > +			cfg.spi_frf = 0;
> > +			break;
> > +		}
> > +	}
> > +
> 
> Please create a separate dw_spi_exec_enh_mem_op() method for the
> Enhanced SPI communications and parse the data.buswidth field there.
> Also it would be better to define a new macro
> DW_SSI_CTRLR0_SPI_FRF_STD_SPI and use it instead of the explicit zero
> SPI_FRF value.
> 
> >  	/*
> >  	 * Collect the outbound data into a single buffer to speed the
> >  	 * transmission up at least on the initial stage.
> > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> > index 71d18e9291a3..b8cc20e0deaa 100644
> > --- a/drivers/spi/spi-dw.h
> > +++ b/drivers/spi/spi-dw.h
> > @@ -96,6 +96,12 @@
> >  #define DW_HSSI_CTRLR0_SRL			BIT(13)
> >  #define DW_HSSI_CTRLR0_MST			BIT(31)
> >  
> 
> > +/* Bit fields in CTRLR0 for enhanced SPI */
> 
> No need in the comment. The macros name is descriptive enough.
> 
> > +#define DW_HSSI_CTRLR0_SPI_FRF_MASK		GENMASK(23, 22)
> 
> Please also add the next new macros
> #define DW_PSSI_CTRLR0_SPI_FRF_MASK		GENMASK(22, 21)

> #define DW_SSI_CTRLR0_SPI_FRF_STD_SPI		0x0
> 
> and group them together with the rest of the APB SSI CTRL0 macros.
> 
> > +#define DW_SSI_CTRLR0_SPI_FRF_DUAL_SPI		0x1
> > +#define DW_SSI_CTRLR0_SPI_FRF_QUAD_SPI		0x2
> > +#define DW_SSI_CTRLR0_SPI_FRF_OCT_SPI		0x3

One more thing s/DW_SSI/DW_SPI . All the common macros are defined
with the DW_SPI prefix.

-Sergey

> 
> Please move these to be defined together with the APB SSI
> CTRL0.SPI_FRF-related macros.
> 
> -Sergey
> 
> > +
> >  /* Bit fields in CTRLR1 */
> >  #define DW_SPI_NDF_MASK				GENMASK(15, 0)
> >  
> > @@ -136,6 +142,7 @@ struct dw_spi_cfg {
> >  	u8 dfs;
> >  	u32 ndf;
> >  	u32 freq;
> > +	u8 spi_frf;
> >  };
> >  
> >  struct dw_spi;
> > -- 
> > 2.30.2
> > 

[PATCH 04/11] spi: dw: use TMOD_RO to read in enhanced spi modes

[Sudip Mukherjee]

When we are using the enhanced spi modes we can not use EEPROM Read.
The Synopsys datasheet mentions EEPROM Read is not applicable in
enhanced SPI modes. We will need to use Receive only mode.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 8c84a2e991b5..8e624620864f 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -727,7 +727,10 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	cfg.dfs = 8;
 	cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
 	if (op->data.dir == SPI_MEM_DATA_IN) {
-		cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
+		if (enhanced_spi)
+			cfg.tmode = DW_SPI_CTRLR0_TMOD_RO;
+		else
+			cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
 		cfg.ndf = op->data.nbytes;
 	} else {
 		cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
-- 
2.30.2

Re: [PATCH 04/11] spi: dw: use TMOD_RO to read in enhanced spi modes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

On Tue, Aug 02, 2022 at 06:57:48PM +0100, Sudip Mukherjee wrote:
> When we are using the enhanced spi modes we can not use EEPROM Read.
> The Synopsys datasheet mentions EEPROM Read is not applicable in
> enhanced SPI modes. We will need to use Receive only mode.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 8c84a2e991b5..8e624620864f 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -727,7 +727,10 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  	cfg.dfs = 8;
>  	cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
>  	if (op->data.dir == SPI_MEM_DATA_IN) {
> -		cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
> +		if (enhanced_spi)
> +			cfg.tmode = DW_SPI_CTRLR0_TMOD_RO;
> +		else
> +			cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;

This is fixing the previous commit...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 04/11] spi: dw: use TMOD_RO to read in enhanced spi modes

[Sudip Mukherjee]

Hi Mark,

On Tue, Aug 2, 2022 at 8:13 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Aug 02, 2022 at 06:57:48PM +0100, Sudip Mukherjee wrote:
> > When we are using the enhanced spi modes we can not use EEPROM Read.
> > The Synopsys datasheet mentions EEPROM Read is not applicable in
> > enhanced SPI modes. We will need to use Receive only mode.
> >
> > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> > ---
> >  drivers/spi/spi-dw-core.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index 8c84a2e991b5..8e624620864f 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -727,7 +727,10 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
> >       cfg.dfs = 8;
> >       cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
> >       if (op->data.dir == SPI_MEM_DATA_IN) {
> > -             cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
> > +             if (enhanced_spi)
> > +                     cfg.tmode = DW_SPI_CTRLR0_TMOD_RO;
> > +             else
> > +                     cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;
>
> This is fixing the previous commit...

This was not actually meant to be a fixup patch. I intentionally made
it separate so that "enhanced_spi" is introduced in the previous
patch,
and then modified the tmode read protocol in this patch based on
enhanced_spi. But I can merge it with the previous patch like you have
suggested.

--
Regards
Sudip

Re: [PATCH 04/11] spi: dw: use TMOD_RO to read in enhanced spi modes

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:48PM +0100, Sudip Mukherjee wrote:
> When we are using the enhanced spi modes we can not use EEPROM Read.
> The Synopsys datasheet mentions EEPROM Read is not applicable in
> enhanced SPI modes. We will need to use Receive only mode.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 8c84a2e991b5..8e624620864f 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -727,7 +727,10 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  	cfg.dfs = 8;
>  	cfg.freq = clamp(mem->spi->max_speed_hz, 0U, dws->max_mem_freq);
>  	if (op->data.dir == SPI_MEM_DATA_IN) {
> -		cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;

> +		if (enhanced_spi)
> +			cfg.tmode = DW_SPI_CTRLR0_TMOD_RO;
> +		else
> +			cfg.tmode = DW_SPI_CTRLR0_TMOD_EPROMREAD;

Just drop this modification and initialize the dw_spi_cfg structure
from scratch in the Enhanced SPI-specific method:
dw_spi_exec_enh_mem_op().

-Sergey

>  		cfg.ndf = op->data.nbytes;
>  	} else {
>  		cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
> -- 
> 2.30.2
> 

[PATCH 05/11] spi: dw: define SPI_CTRLR0 register and its fields

[Sudip Mukherjee]

Synopsys SSI controllers supporting enhanced SPI mode of operation has
SPI Control Register at offset 0xf4 which controls the following:

CLK_STRETCH_EN: Enables clock stretching capability in SPI transfers.
In case of write, if the FIFO becomes empty DWC_ssi will stretch the
clock until FIFO has enough data to continue the transfer. In case of
read, if the receive FIFO becomes full DWC_ssi will stop the clock until
data has been read from the FIFO.

WAIT_CYCLES: Wait cycles in Dual/Quad/Octal mode between control frames
transmit and data reception.

INST_L: Dual/Quad/Octal mode instruction length in bits.

ADDR_L: defines Length of Address to be transmitted.

For now, we are only using 32bit Address length and 8 bit Instruction
length.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index b8cc20e0deaa..a7a4637d6d32 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -63,6 +63,17 @@
 #define DW_SPI_RX_SAMPLE_DLY		0xf0
 #define DW_SPI_CS_OVERRIDE		0xf4
 
+/* Register offsets (Defined in DWC SSI 1.03a) */
+#define DW_HSSI_SPI_CTRLR0		0xf4
+
+/* Bit fields in SPI_CTRLR0 (Defined in DWC SSI 1.03a) */
+#define DW_HSSI_SPI_CTRLR0_CLK_STRETCH_EN	BIT(30)
+#define DW_HSSI_SPI_CTRLR0_WAIT_CYCLE_MASK	GENMASK(15, 11)
+#define DW_HSSI_SPI_CTRLR0_INST_L_MASK		GENMASK(9, 8)
+#define DW_HSSI_SPI_CTRLR0_INST_L8		0x2
+#define DW_HSSI_SPI_CTRLR0_ADDR_L_MASK		GENMASK(5, 2)
+#define DW_HSSI_SPI_CTRLR0_ADDR_L32		0x8
+
 /* Bit fields in CTRLR0 (DWC APB SSI) */
 #define DW_PSSI_CTRLR0_DFS_MASK			GENMASK(3, 0)
 #define DW_PSSI_CTRLR0_DFS32_MASK		GENMASK(20, 16)
-- 
2.30.2

Re: [PATCH 05/11] spi: dw: define SPI_CTRLR0 register and its fields

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:49PM +0100, Sudip Mukherjee wrote:
> Synopsys SSI controllers supporting enhanced SPI mode of operation has
> SPI Control Register at offset 0xf4 which controls the following:
> 
> CLK_STRETCH_EN: Enables clock stretching capability in SPI transfers.
> In case of write, if the FIFO becomes empty DWC_ssi will stretch the
> clock until FIFO has enough data to continue the transfer. In case of
> read, if the receive FIFO becomes full DWC_ssi will stop the clock until
> data has been read from the FIFO.
> 
> WAIT_CYCLES: Wait cycles in Dual/Quad/Octal mode between control frames
> transmit and data reception.
> 
> INST_L: Dual/Quad/Octal mode instruction length in bits.
> 
> ADDR_L: defines Length of Address to be transmitted.
> 
> For now, we are only using 32bit Address length and 8 bit Instruction
> length.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index b8cc20e0deaa..a7a4637d6d32 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -63,6 +63,17 @@
>  #define DW_SPI_RX_SAMPLE_DLY		0xf0
>  #define DW_SPI_CS_OVERRIDE		0xf4
>  

> +/* Register offsets (Defined in DWC SSI 1.03a) */

Drop this comment. The CSR can exist in the DW APB SSI too

> +#define DW_HSSI_SPI_CTRLR0		0xf4

#define DW_SSI_SPI_CTRLR0		0xf4

> +
> +/* Bit fields in SPI_CTRLR0 (Defined in DWC SSI 1.03a) */

Drop the IP-core ID and version from the comment. As I said the eSPI
feature can be enabled for DW APB SSI too.

> +#define DW_HSSI_SPI_CTRLR0_CLK_STRETCH_EN	BIT(30)
> +#define DW_HSSI_SPI_CTRLR0_WAIT_CYCLE_MASK	GENMASK(15, 11)
> +#define DW_HSSI_SPI_CTRLR0_INST_L_MASK		GENMASK(9, 8)
> +#define DW_HSSI_SPI_CTRLR0_INST_L8		0x2
> +#define DW_HSSI_SPI_CTRLR0_ADDR_L_MASK		GENMASK(5, 2)
> +#define DW_HSSI_SPI_CTRLR0_ADDR_L32		0x8

Move this to the tail of the CSR fields macros definition (right after
the DW_SPI_DMACR_TDMAE macros) and s/HSSI/SPI .

-Sergey

> +
>  /* Bit fields in CTRLR0 (DWC APB SSI) */
>  #define DW_PSSI_CTRLR0_DFS_MASK			GENMASK(3, 0)
>  #define DW_PSSI_CTRLR0_DFS32_MASK		GENMASK(20, 16)
> -- 
> 2.30.2
> 

[PATCH 06/11] spi: dw: update SPI_CTRLR0 register

[Sudip Mukherjee]

If the controller supports enhanced SPI modes then update the register
or reset the register if the transfer is not using dual/quad/octal mode.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 8e624620864f..9d499bdf2ce6 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -676,6 +676,32 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
 	dw_spi_enable_chip(dws, 1);
 }
 
+static void update_spi_ctrl0(struct dw_spi *dws, const struct spi_mem_op *op, bool enable)
+{
+	u32 spi_ctrlr0;
+
+	spi_ctrlr0 = dw_readl(dws, DW_HSSI_SPI_CTRLR0);
+	if (enable) {
+		spi_ctrlr0 |= FIELD_PREP(DW_HSSI_SPI_CTRLR0_WAIT_CYCLE_MASK,
+					 op->dummy.nbytes * BITS_PER_BYTE);
+		/* 8 bit instruction length */
+		spi_ctrlr0 |= FIELD_PREP(DW_HSSI_SPI_CTRLR0_INST_L_MASK,
+					 DW_HSSI_SPI_CTRLR0_INST_L8);
+		/* 32 bit address length */
+		spi_ctrlr0 |= FIELD_PREP(DW_HSSI_SPI_CTRLR0_ADDR_L_MASK,
+					 DW_HSSI_SPI_CTRLR0_ADDR_L32);
+		/* Enable clock stretching */
+		spi_ctrlr0 |= DW_HSSI_SPI_CTRLR0_CLK_STRETCH_EN;
+	} else {
+		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_WAIT_CYCLE_MASK;
+		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_INST_L_MASK;
+		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_ADDR_L_MASK;
+		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_CLK_STRETCH_EN;
+	}
+
+	dw_writel(dws, DW_HSSI_SPI_CTRLR0, spi_ctrlr0);
+}
+
 /*
  * The SPI memory operation implementation below is the best choice for the
  * devices, which are selected by the native chip-select lane. It's
@@ -738,6 +764,9 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 
 	dw_spi_enable_chip(dws, 0);
 
+	if (dws->caps & DW_SPI_CAP_EXT_SPI)
+		update_spi_ctrl0(dws, op, enhanced_spi);
+
 	dw_spi_update_config(dws, mem->spi, &cfg);
 
 	dw_spi_mask_intr(dws, 0xff);
-- 
2.30.2

Re: [PATCH 06/11] spi: dw: update SPI_CTRLR0 register

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:50PM +0100, Sudip Mukherjee wrote:
> If the controller supports enhanced SPI modes then update the register
> or reset the register if the transfer is not using dual/quad/octal mode.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 8e624620864f..9d499bdf2ce6 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -676,6 +676,32 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
>  	dw_spi_enable_chip(dws, 1);
>  }
>  
> +static void update_spi_ctrl0(struct dw_spi *dws, const struct spi_mem_op *op, bool enable)
> +{
> +	u32 spi_ctrlr0;
> +
> +	spi_ctrlr0 = dw_readl(dws, DW_HSSI_SPI_CTRLR0);
> +	if (enable) {
> +		spi_ctrlr0 |= FIELD_PREP(DW_HSSI_SPI_CTRLR0_WAIT_CYCLE_MASK,

> +					 op->dummy.nbytes * BITS_PER_BYTE);
> +		/* 8 bit instruction length */
> +		spi_ctrlr0 |= FIELD_PREP(DW_HSSI_SPI_CTRLR0_INST_L_MASK,
> +					 DW_HSSI_SPI_CTRLR0_INST_L8);
> +		/* 32 bit address length */
> +		spi_ctrlr0 |= FIELD_PREP(DW_HSSI_SPI_CTRLR0_ADDR_L_MASK,
> +					 DW_HSSI_SPI_CTRLR0_ADDR_L32);

Just add new fields dw_spi_cfg.{trans_t,inst_l,addr_l,wait_c},
initialize them with the values taken from the spi_mem_op (trans_t -
based on the bus widths, inst_l - cmd.nbytes, addr_l - addr.nbytes,
wait_c - dummy.nbytes / dummy.buswidth) and use them to accordingly
update the SPI_CTRLR0 CSR in the dw_spi_update_config() method. Update
the CSR if spi_frf has value other than STD_SPI_FRF, otherwise
according to the HW manual the SPI_CTRLR0 register isn't relevant so
don't touch it.

> +		/* Enable clock stretching */
> +		spi_ctrlr0 |= DW_HSSI_SPI_CTRLR0_CLK_STRETCH_EN;
> +	} else {
> +		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_WAIT_CYCLE_MASK;
> +		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_INST_L_MASK;
> +		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_ADDR_L_MASK;
> +		spi_ctrlr0 &= ~DW_HSSI_SPI_CTRLR0_CLK_STRETCH_EN;
> +	}
> +
> +	dw_writel(dws, DW_HSSI_SPI_CTRLR0, spi_ctrlr0);
> +}
> +
>  /*
>   * The SPI memory operation implementation below is the best choice for the
>   * devices, which are selected by the native chip-select lane. It's
> @@ -738,6 +764,9 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  
>  	dw_spi_enable_chip(dws, 0);
>  

> +	if (dws->caps & DW_SPI_CAP_EXT_SPI)
> +		update_spi_ctrl0(dws, op, enhanced_spi);
> +

This won't be needed as long as you do as I suggested above.

-Sergey

>  	dw_spi_update_config(dws, mem->spi, &cfg);
>  
>  	dw_spi_mask_intr(dws, 0xff);
> -- 
> 2.30.2
> 

[PATCH 07/11] spi: dw: update NDF while writing in enhanced spi mode

[Sudip Mukherjee]

If the transfer of Transmit only mode is using dual/quad/octal SPI then
NDF needs to be updated with the number of data frames.
If the Transmit FIFO goes empty in-between, DWC_ssi masks the serial
clock and wait for rest of the data until the programmed amount of
frames are transferred successfully.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 9d499bdf2ce6..8cb30540ad5b 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -344,7 +344,9 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
 	dw_writel(dws, DW_SPI_CTRLR0, cr0);
 
 	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
-	    cfg->tmode == DW_SPI_CTRLR0_TMOD_RO)
+	    cfg->tmode == DW_SPI_CTRLR0_TMOD_RO ||
+	    (cfg->tmode == DW_SPI_CTRLR0_TMOD_TO &&
+	     (dws->caps & DW_SPI_CAP_EXT_SPI) && cfg->spi_frf))
 		dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf ? cfg->ndf - 1 : 0);
 
 	/* Note DW APB SSI clock divider doesn't support odd numbers */
@@ -760,6 +762,8 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 		cfg.ndf = op->data.nbytes;
 	} else {
 		cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;
+		if (enhanced_spi)
+			cfg.ndf = op->data.nbytes;
 	}
 
 	dw_spi_enable_chip(dws, 0);
-- 
2.30.2

Re: [PATCH 07/11] spi: dw: update NDF while writing in enhanced spi mode

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:51PM +0100, Sudip Mukherjee wrote:
> If the transfer of Transmit only mode is using dual/quad/octal SPI then
> NDF needs to be updated with the number of data frames.
> If the Transmit FIFO goes empty in-between, DWC_ssi masks the serial
> clock and wait for rest of the data until the programmed amount of
> frames are transferred successfully.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 9d499bdf2ce6..8cb30540ad5b 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -344,7 +344,9 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
>  	dw_writel(dws, DW_SPI_CTRLR0, cr0);
>  
>  	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
> -	    cfg->tmode == DW_SPI_CTRLR0_TMOD_RO)
> +	    cfg->tmode == DW_SPI_CTRLR0_TMOD_RO ||

> +	    (cfg->tmode == DW_SPI_CTRLR0_TMOD_TO &&
> +	     (dws->caps & DW_SPI_CAP_EXT_SPI) && cfg->spi_frf))

just (cfg->spi_frf != DW_SPI_CTRLR0_SPI_FRF_STD_SPI &&
      cfg->tmode == DW_SPI_CTRLR0_TMOD_TO)

>  		dw_writel(dws, DW_SPI_CTRLR1, cfg->ndf ? cfg->ndf - 1 : 0);
>  
>  	/* Note DW APB SSI clock divider doesn't support odd numbers */
> @@ -760,6 +762,8 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  		cfg.ndf = op->data.nbytes;
>  	} else {
>  		cfg.tmode = DW_SPI_CTRLR0_TMOD_TO;

> +		if (enhanced_spi)
> +			cfg.ndf = op->data.nbytes;

This shall be moved to the Enhanced SPI-specific mem-op execution method:
dw_spi_exec_enh_mem_op().

-Sergey

>  	}
>  
>  	dw_spi_enable_chip(dws, 0);
> -- 
> 2.30.2
> 

[PATCH 08/11] spi: dw: update buffer for enhanced spi mode

[Sudip Mukherjee]

In enhanced spi mode we will be writing the address to a single FIFO
location instead of writing to multiple FIFOs in the standard SPI mode.
Save the cmd and address bytes in the buffer accordingly.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 55 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 8cb30540ad5b..2564a2276572 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -520,7 +520,8 @@ static bool dw_spi_supports_mem_op(struct spi_mem *mem,
 	return spi_mem_default_supports_op(mem, op);
 }
 
-static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
+static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op,
+			       bool enhanced_spi)
 {
 	unsigned int i, j, len;
 	u8 *out;
@@ -548,17 +549,57 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
 	 */
 	for (i = 0; i < op->cmd.nbytes; ++i)
 		out[i] = DW_SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
-	for (j = 0; j < op->addr.nbytes; ++i, ++j)
-		out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
-	for (j = 0; j < op->dummy.nbytes; ++i, ++j)
-		out[i] = 0x0;
+
+	if (enhanced_spi) {
+		/*
+		 * Fill the remaining spaces of dws->reg_io_width bytes
+		 * size register with zero for cmd.
+		 */
+		for (; i < dws->reg_io_width; ++i)
+			out[i] = 0;
+		/*
+		 * Copy the address bytes in dws->reg_io_width bytes size
+		 * register and fill remaining spaces with zero.
+		 */
+		for (j = op->addr.nbytes; j > 0; ++i, --j)
+			out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j);
+		for (j = op->addr.nbytes; j < dws->reg_io_width; ++i, ++j)
+			out[i] = 0;
+	} else {
+		for (j = 0; j < op->addr.nbytes; ++i, ++j)
+			out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
+	}
+
+	if (!enhanced_spi) {
+		/*
+		 * dummy bytes are not needed in enhanced mode as
+		 * wait_cycles specified as number of SPI clock cycles
+		 * between control frames transmit and data reception
+		 * will be mentioned in enhanced spi mode.
+		 */
+		for (j = 0; j < op->dummy.nbytes; ++i, ++j)
+			out[i] = 0x0;
+	}
 
 	if (op->data.dir == SPI_MEM_DATA_OUT)
 		memcpy(&out[i], op->data.buf.out, op->data.nbytes);
 
 	dws->n_bytes = 1;
 	dws->tx = out;
-	dws->tx_len = len;
+
+	if (enhanced_spi) {
+		/*
+		 * In enhanced mode cmd will be one FIFO and address
+		 * will be one more FIFO.
+		 */
+		dws->tx_len = 1;
+		if (op->addr.nbytes)
+			dws->tx_len += 1;
+		if (op->data.dir == SPI_MEM_DATA_OUT)
+			dws->tx_len += op->data.nbytes;
+	} else {
+		dws->tx_len = len;
+	}
 	if (op->data.dir == SPI_MEM_DATA_IN) {
 		dws->rx = op->data.buf.in;
 		dws->rx_len = op->data.nbytes;
@@ -744,7 +785,7 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	 * Collect the outbound data into a single buffer to speed the
 	 * transmission up at least on the initial stage.
 	 */
-	ret = dw_spi_init_mem_buf(dws, op);
+	ret = dw_spi_init_mem_buf(dws, op, enhanced_spi);
 	if (ret)
 		return ret;
 
-- 
2.30.2

Re: [PATCH 08/11] spi: dw: update buffer for enhanced spi mode

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:52PM +0100, Sudip Mukherjee wrote:
> In enhanced spi mode we will be writing the address to a single FIFO
> location instead of writing to multiple FIFOs in the standard SPI mode.
> Save the cmd and address bytes in the buffer accordingly.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 55 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 8cb30540ad5b..2564a2276572 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -520,7 +520,8 @@ static bool dw_spi_supports_mem_op(struct spi_mem *mem,
>  	return spi_mem_default_supports_op(mem, op);
>  }
> 
 
> -static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
> +static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op,
> +			       bool enhanced_spi)

There is no point in modifying this method. Since clock stretching is
available you won't need to collect all the data in a single buffer.
So just create a new method dw_spi_init_enh_mem_buf() which would set
dws->tx/rx pointers and tx_len/rx_len fields with the
spi_mem_op.data.buf.{in,out} and the corresponding lengths. The command
and address data shall be written to the Tx FIFO to initiate the SPI MEM
transfers, since in accordance with the HW manual the SPI-bus transfers
won't start before it is done.

-Sergey

>  {
>  	unsigned int i, j, len;
>  	u8 *out;
> @@ -548,17 +549,57 @@ static int dw_spi_init_mem_buf(struct dw_spi *dws, const struct spi_mem_op *op)
>  	 */
>  	for (i = 0; i < op->cmd.nbytes; ++i)
>  		out[i] = DW_SPI_GET_BYTE(op->cmd.opcode, op->cmd.nbytes - i - 1);
> -	for (j = 0; j < op->addr.nbytes; ++i, ++j)
> -		out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
> -	for (j = 0; j < op->dummy.nbytes; ++i, ++j)
> -		out[i] = 0x0;
> +
> +	if (enhanced_spi) {
> +		/*
> +		 * Fill the remaining spaces of dws->reg_io_width bytes
> +		 * size register with zero for cmd.
> +		 */
> +		for (; i < dws->reg_io_width; ++i)
> +			out[i] = 0;
> +		/*
> +		 * Copy the address bytes in dws->reg_io_width bytes size
> +		 * register and fill remaining spaces with zero.
> +		 */
> +		for (j = op->addr.nbytes; j > 0; ++i, --j)
> +			out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j);
> +		for (j = op->addr.nbytes; j < dws->reg_io_width; ++i, ++j)
> +			out[i] = 0;
> +	} else {
> +		for (j = 0; j < op->addr.nbytes; ++i, ++j)
> +			out[i] = DW_SPI_GET_BYTE(op->addr.val, op->addr.nbytes - j - 1);
> +	}
> +
> +	if (!enhanced_spi) {
> +		/*
> +		 * dummy bytes are not needed in enhanced mode as
> +		 * wait_cycles specified as number of SPI clock cycles
> +		 * between control frames transmit and data reception
> +		 * will be mentioned in enhanced spi mode.
> +		 */
> +		for (j = 0; j < op->dummy.nbytes; ++i, ++j)
> +			out[i] = 0x0;
> +	}
>  
>  	if (op->data.dir == SPI_MEM_DATA_OUT)
>  		memcpy(&out[i], op->data.buf.out, op->data.nbytes);
>  
>  	dws->n_bytes = 1;
>  	dws->tx = out;
> -	dws->tx_len = len;
> +
> +	if (enhanced_spi) {
> +		/*
> +		 * In enhanced mode cmd will be one FIFO and address
> +		 * will be one more FIFO.
> +		 */
> +		dws->tx_len = 1;
> +		if (op->addr.nbytes)
> +			dws->tx_len += 1;
> +		if (op->data.dir == SPI_MEM_DATA_OUT)
> +			dws->tx_len += op->data.nbytes;
> +	} else {
> +		dws->tx_len = len;
> +	}
>  	if (op->data.dir == SPI_MEM_DATA_IN) {
>  		dws->rx = op->data.buf.in;
>  		dws->rx_len = op->data.nbytes;
> @@ -744,7 +785,7 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  	 * Collect the outbound data into a single buffer to speed the
>  	 * transmission up at least on the initial stage.
>  	 */
> -	ret = dw_spi_init_mem_buf(dws, op);
> +	ret = dw_spi_init_mem_buf(dws, op, enhanced_spi);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.30.2
> 

[PATCH 09/11] spi: dw: prepare the transfer routine for enhanced mode

[Sudip Mukherjee]

The transfer routine of dual/quad/octal is similar to standard SPI mode
except that we do not need to worry about CS being de-asserted and we
will be writing the address to a single FIFO location.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-core.c | 141 +++++++++++++++++++++++++++++++++-----
 1 file changed, 125 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 2564a2276572..d6afa75e7023 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -712,6 +712,28 @@ static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
 	return 0;
 }
 
+static void ext_transfer_delay(struct dw_spi *dws)
+{
+	struct spi_delay delay;
+	unsigned long ns, us;
+	u32 nents;
+
+	nents = dw_readl(dws, DW_SPI_TXFLR);
+	ns = NSEC_PER_SEC / dws->current_freq * nents;
+	ns *= dws->n_bytes * BITS_PER_BYTE;
+	if (ns <= NSEC_PER_USEC) {
+		delay.unit = SPI_DELAY_UNIT_NSECS;
+		delay.value = ns;
+	} else {
+		us = DIV_ROUND_UP(ns, NSEC_PER_USEC);
+		delay.unit = SPI_DELAY_UNIT_USECS;
+		delay.value = clamp_val(us, 0, USHRT_MAX);
+	}
+	/* wait until there is some space in TX FIFO */
+	while (!(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_TF_NOT_FULL))
+		spi_delay_exec(&delay, NULL);
+}
+
 static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
 {
 	dw_spi_enable_chip(dws, 0);
@@ -719,6 +741,82 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
 	dw_spi_enable_chip(dws, 1);
 }
 
+static int enhanced_transfer(struct dw_spi *dws, struct spi_device *spi,
+			     const struct spi_mem_op *op)
+{
+	u32 max, txw = 0, rxw;
+	bool cs_done = false;
+	void *buf = dws->tx;
+	int ret;
+
+	/* Send cmd as 32 bit value */
+	if (buf) {
+		txw = *(u32 *)(buf);
+		dw_write_io_reg(dws, DW_SPI_DR, txw);
+		buf += 4;
+		dws->tx_len--;
+		if (op->addr.nbytes) {
+			/*
+			 * Send address as 32 bit value if address
+			 * is present in the instruction.
+			 */
+			txw = *(u32 *)(buf);
+			dw_write_io_reg(dws, DW_SPI_DR, txw);
+			buf += 4;
+			dws->tx_len--;
+		}
+	}
+
+	do {
+		max = min_t(u32, dws->tx_len, dws->fifo_len -
+			    dw_readl(dws, DW_SPI_TXFLR));
+		while (max--) {
+			if (buf) {
+				txw = *(u8 *)(buf);
+				buf += dws->n_bytes;
+			}
+			dw_write_io_reg(dws, DW_SPI_DR, txw);
+			--dws->tx_len;
+		}
+		/* Enable CS after filling up FIFO */
+		if (!cs_done) {
+			dw_spi_set_cs(spi, false);
+			cs_done = true;
+		}
+		ext_transfer_delay(dws);
+		if (!dws->tx_len && !dws->rx_len) {
+			/*
+			 * We only need to wait for done if there is
+			 * nothing to receive and there is nothing more
+			 * to transmit. If we are receiving, then the
+			 * wait cycles will make sure we wait.
+			 */
+			ret = dw_spi_wait_mem_op_done(dws);
+			if (ret)
+				return ret;
+		}
+	} while (dws->tx_len);
+
+	buf = dws->rx;
+	while (dws->rx_len) {
+		max = dw_spi_rx_max(dws);
+
+		while (max--) {
+			rxw = dw_read_io_reg(dws, DW_SPI_DR);
+			if (buf) {
+				*(u8 *)(buf) = rxw;
+				buf += dws->n_bytes;
+			}
+			--dws->rx_len;
+		}
+
+		ret = dw_spi_check_status(dws, true);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 static void update_spi_ctrl0(struct dw_spi *dws, const struct spi_mem_op *op, bool enable)
 {
 	u32 spi_ctrlr0;
@@ -846,25 +944,36 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 	 * manually restricting the SPI bus frequency using the
 	 * dws->max_mem_freq parameter.
 	 */
-	local_irq_save(flags);
-	preempt_disable();
+	if (!enhanced_spi) {
+		local_irq_save(flags);
+		preempt_disable();
 
-	ret = dw_spi_write_then_read(dws, mem->spi);
+		ret = dw_spi_write_then_read(dws, mem->spi);
 
-	local_irq_restore(flags);
-	preempt_enable();
+		local_irq_restore(flags);
+		preempt_enable();
 
-	/*
-	 * Wait for the operation being finished and check the controller
-	 * status only if there hasn't been any run-time error detected. In the
-	 * former case it's just pointless. In the later one to prevent an
-	 * additional error message printing since any hw error flag being set
-	 * would be due to an error detected on the data transfer.
-	 */
-	if (!ret) {
-		ret = dw_spi_wait_mem_op_done(dws);
-		if (!ret)
-			ret = dw_spi_check_status(dws, true);
+		/*
+		 * Wait for the operation being finished and check the
+		 * controller status only if there hasn't been any
+		 * run-time error detected. In the former case it's
+		 * just pointless. In the later one to prevent an
+		 * additional error message printing since any hw error
+		 * flag being set would be due to an error detected on
+		 * the data transfer.
+		 */
+		if (!ret) {
+			ret = dw_spi_wait_mem_op_done(dws);
+			if (!ret)
+				ret = dw_spi_check_status(dws, true);
+		}
+	} else {
+		/*
+		 * We donot need to disable IRQs as clock stretching will
+		 * be enabled in enhanced mode which will prevent CS
+		 * from being de-assert.
+		 */
+		ret = enhanced_transfer(dws, mem->spi, op);
 	}
 
 	dw_spi_stop_mem_op(dws, mem->spi);
-- 
2.30.2

Re: [PATCH 09/11] spi: dw: prepare the transfer routine for enhanced mode

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:53PM +0100, Sudip Mukherjee wrote:
> The transfer routine of dual/quad/octal is similar to standard SPI mode
> except that we do not need to worry about CS being de-asserted and we
> will be writing the address to a single FIFO location.

Please redesign this patch to having the IRQ-based transfers. For
instance you can just create a new dw_spi_enh_write_then_read() method
which would initialize the IRQs, set up a custom
dw_spi_enh_transfer_handler() method as transfer_handler (or perhaps
re-use the already available dw_spi_transfer_handler() method?) and
initiate the transfer by writing the command and address data to the
Tx FIFO.

Feel free to create some preparation patches if it's needed to reach the
goal.

-Sergey

> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 141 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 125 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 2564a2276572..d6afa75e7023 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -712,6 +712,28 @@ static int dw_spi_wait_mem_op_done(struct dw_spi *dws)
>  	return 0;
>  }
>  
> +static void ext_transfer_delay(struct dw_spi *dws)
> +{
> +	struct spi_delay delay;
> +	unsigned long ns, us;
> +	u32 nents;
> +
> +	nents = dw_readl(dws, DW_SPI_TXFLR);
> +	ns = NSEC_PER_SEC / dws->current_freq * nents;
> +	ns *= dws->n_bytes * BITS_PER_BYTE;
> +	if (ns <= NSEC_PER_USEC) {
> +		delay.unit = SPI_DELAY_UNIT_NSECS;
> +		delay.value = ns;
> +	} else {
> +		us = DIV_ROUND_UP(ns, NSEC_PER_USEC);
> +		delay.unit = SPI_DELAY_UNIT_USECS;
> +		delay.value = clamp_val(us, 0, USHRT_MAX);
> +	}
> +	/* wait until there is some space in TX FIFO */
> +	while (!(dw_readl(dws, DW_SPI_SR) & DW_SPI_SR_TF_NOT_FULL))
> +		spi_delay_exec(&delay, NULL);
> +}
> +
>  static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
>  {
>  	dw_spi_enable_chip(dws, 0);
> @@ -719,6 +741,82 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
>  	dw_spi_enable_chip(dws, 1);
>  }
>  
> +static int enhanced_transfer(struct dw_spi *dws, struct spi_device *spi,
> +			     const struct spi_mem_op *op)
> +{
> +	u32 max, txw = 0, rxw;
> +	bool cs_done = false;
> +	void *buf = dws->tx;
> +	int ret;
> +
> +	/* Send cmd as 32 bit value */
> +	if (buf) {
> +		txw = *(u32 *)(buf);
> +		dw_write_io_reg(dws, DW_SPI_DR, txw);
> +		buf += 4;
> +		dws->tx_len--;
> +		if (op->addr.nbytes) {
> +			/*
> +			 * Send address as 32 bit value if address
> +			 * is present in the instruction.
> +			 */
> +			txw = *(u32 *)(buf);
> +			dw_write_io_reg(dws, DW_SPI_DR, txw);
> +			buf += 4;
> +			dws->tx_len--;
> +		}
> +	}
> +
> +	do {
> +		max = min_t(u32, dws->tx_len, dws->fifo_len -
> +			    dw_readl(dws, DW_SPI_TXFLR));
> +		while (max--) {
> +			if (buf) {
> +				txw = *(u8 *)(buf);
> +				buf += dws->n_bytes;
> +			}
> +			dw_write_io_reg(dws, DW_SPI_DR, txw);
> +			--dws->tx_len;
> +		}
> +		/* Enable CS after filling up FIFO */
> +		if (!cs_done) {
> +			dw_spi_set_cs(spi, false);
> +			cs_done = true;
> +		}
> +		ext_transfer_delay(dws);
> +		if (!dws->tx_len && !dws->rx_len) {
> +			/*
> +			 * We only need to wait for done if there is
> +			 * nothing to receive and there is nothing more
> +			 * to transmit. If we are receiving, then the
> +			 * wait cycles will make sure we wait.
> +			 */
> +			ret = dw_spi_wait_mem_op_done(dws);
> +			if (ret)
> +				return ret;
> +		}
> +	} while (dws->tx_len);
> +
> +	buf = dws->rx;
> +	while (dws->rx_len) {
> +		max = dw_spi_rx_max(dws);
> +
> +		while (max--) {
> +			rxw = dw_read_io_reg(dws, DW_SPI_DR);
> +			if (buf) {
> +				*(u8 *)(buf) = rxw;
> +				buf += dws->n_bytes;
> +			}
> +			--dws->rx_len;
> +		}
> +
> +		ret = dw_spi_check_status(dws, true);
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +
>  static void update_spi_ctrl0(struct dw_spi *dws, const struct spi_mem_op *op, bool enable)
>  {
>  	u32 spi_ctrlr0;
> @@ -846,25 +944,36 @@ static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  	 * manually restricting the SPI bus frequency using the
>  	 * dws->max_mem_freq parameter.
>  	 */
> -	local_irq_save(flags);
> -	preempt_disable();
> +	if (!enhanced_spi) {
> +		local_irq_save(flags);
> +		preempt_disable();
>  
> -	ret = dw_spi_write_then_read(dws, mem->spi);
> +		ret = dw_spi_write_then_read(dws, mem->spi);
>  
> -	local_irq_restore(flags);
> -	preempt_enable();
> +		local_irq_restore(flags);
> +		preempt_enable();
>  
> -	/*
> -	 * Wait for the operation being finished and check the controller
> -	 * status only if there hasn't been any run-time error detected. In the
> -	 * former case it's just pointless. In the later one to prevent an
> -	 * additional error message printing since any hw error flag being set
> -	 * would be due to an error detected on the data transfer.
> -	 */
> -	if (!ret) {
> -		ret = dw_spi_wait_mem_op_done(dws);
> -		if (!ret)
> -			ret = dw_spi_check_status(dws, true);
> +		/*
> +		 * Wait for the operation being finished and check the
> +		 * controller status only if there hasn't been any
> +		 * run-time error detected. In the former case it's
> +		 * just pointless. In the later one to prevent an
> +		 * additional error message printing since any hw error
> +		 * flag being set would be due to an error detected on
> +		 * the data transfer.
> +		 */
> +		if (!ret) {
> +			ret = dw_spi_wait_mem_op_done(dws);
> +			if (!ret)
> +				ret = dw_spi_check_status(dws, true);
> +		}
> +	} else {
> +		/*
> +		 * We donot need to disable IRQs as clock stretching will
> +		 * be enabled in enhanced mode which will prevent CS
> +		 * from being de-assert.
> +		 */
> +		ret = enhanced_transfer(dws, mem->spi, op);
>  	}
>  
>  	dw_spi_stop_mem_op(dws, mem->spi);
> -- 
> 2.30.2
> 

[PATCH 10/11] spi: dw-apb-ssi: add generic 1.03a version

[Sudip Mukherjee]

From: Ben Dooks <ben.dooks@sifive.com>

Add new snps,dw-ssi-1.03a version to the bindings.

Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index 37c3c272407d..35aa04a85813 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -45,6 +45,7 @@ properties:
         enum:
           - snps,dw-apb-ssi
           - snps,dwc-ssi-1.01a
+          - snps,dwc-ssi-1.03a
       - description: Microsemi Ocelot/Jaguar2 SoC SPI Controller
         items:
           - enum:
-- 
2.30.2

Re: [PATCH 10/11] spi: dw-apb-ssi: add generic 1.03a version

[Krzysztof Kozlowski]

On 02/08/2022 19:57, Sudip Mukherjee wrote:
> From: Ben Dooks <ben.dooks@sifive.com>
> 
> Add new snps,dw-ssi-1.03a version to the bindings.

Use subject prefix properly identifying files. I do not have enough of
time to check 200 patches everyday if they touch or do not touch bindings.

> 
> Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index 37c3c272407d..35aa04a85813 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -45,6 +45,7 @@ properties:
>          enum:
>            - snps,dw-apb-ssi
>            - snps,dwc-ssi-1.01a
> +          - snps,dwc-ssi-1.03a

With subject fixes:

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 10/11] spi: dw-apb-ssi: add generic 1.03a version

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:54PM +0100, Sudip Mukherjee wrote:
> From: Ben Dooks <ben.dooks@sifive.com>
> 
> Add new snps,dw-ssi-1.03a version to the bindings.
> 
> Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index 37c3c272407d..35aa04a85813 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -45,6 +45,7 @@ properties:
>          enum:
>            - snps,dw-apb-ssi
>            - snps,dwc-ssi-1.01a

> +          - snps,dwc-ssi-1.03a

There is no point in having the IP-core versions used in the
compatible string. The controller version is retrieved from the device
CSR. Just add new generic compatible string "snps,dw-ahb-ssi" and use
it to identify your device.

-Sergey

>        - description: Microsemi Ocelot/Jaguar2 SoC SPI Controller
>          items:
>            - enum:
> -- 
> 2.30.2
> 

Re: [PATCH 10/11] spi: dw-apb-ssi: add generic 1.03a version

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:54PM +0100, Sudip Mukherjee wrote:
> From: Ben Dooks <ben.dooks@sifive.com>
> 
> Add new snps,dw-ssi-1.03a version to the bindings.
> 
> Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index 37c3c272407d..35aa04a85813 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -45,6 +45,7 @@ properties:
>          enum:
>            - snps,dw-apb-ssi
>            - snps,dwc-ssi-1.01a

> +          - snps,dwc-ssi-1.03a

Just add the "snps,dw-ahb-ssi" compatible string instead.

-Sergey

>        - description: Microsemi Ocelot/Jaguar2 SoC SPI Controller
>          items:
>            - enum:
> -- 
> 2.30.2
> 

[PATCH 11/11] spi: dw: initialize dwc-ssi-1.03a controller

[Sudip Mukherjee]

Define the initialization of dwc-ssi-1.03a controller and mark it with
the capability of enhanced SPI supporting dual/quad/octal modes of
transfer.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/spi/spi-dw-mmio.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 26c40ea6dd12..db80e0645172 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -237,6 +237,15 @@ static int dw_spi_canaan_k210_init(struct platform_device *pdev,
 	return 0;
 }
 
+static int dw_spi_hssi_ext_init(struct platform_device *pdev,
+				struct dw_spi_mmio *dwsmmio)
+{
+	dwsmmio->dws.ip = DW_HSSI_ID;
+	dwsmmio->dws.caps = DW_SPI_CAP_EXT_SPI;
+
+	return 0;
+}
+
 static int dw_spi_mmio_probe(struct platform_device *pdev)
 {
 	int (*init_func)(struct platform_device *pdev,
@@ -352,6 +361,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "intel,thunderbay-ssi", .data = dw_spi_intel_init},
 	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
 	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
+	{ .compatible = "snps,dwc-ssi-1.03a", dw_spi_hssi_ext_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
-- 
2.30.2

Re: [PATCH 11/11] spi: dw: initialize dwc-ssi-1.03a controller

[Serge Semin]

On Tue, Aug 02, 2022 at 06:57:55PM +0100, Sudip Mukherjee wrote:
> Define the initialization of dwc-ssi-1.03a controller and mark it with
> the capability of enhanced SPI supporting dual/quad/octal modes of
> transfer.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-mmio.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 26c40ea6dd12..db80e0645172 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -237,6 +237,15 @@ static int dw_spi_canaan_k210_init(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int dw_spi_hssi_ext_init(struct platform_device *pdev,
> +				struct dw_spi_mmio *dwsmmio)
> +{
> +	dwsmmio->dws.ip = DW_HSSI_ID;

> +	dwsmmio->dws.caps = DW_SPI_CAP_EXT_SPI;

The eSPI capability shall be auto-detectable by checking whether the
CTRLR0.SPI_FRF field is writable. If it is then you'll need to check
whether the clock stretching is also available by setting to the
SPI_CTRLR0.CLK_STRETCH_EN flag. After that you can set the capability
flag.

Since the auto-detection procedure is available you won't need the
dw_spi_hssi_ext_init() method here. So just drop it and use
dw_spi_hssi_init() for the platform-specific initialization. Note it
will also check the DMA-capability for you.

> +
> +	return 0;
> +}
> +
>  static int dw_spi_mmio_probe(struct platform_device *pdev)
>  {
>  	int (*init_func)(struct platform_device *pdev,
> @@ -352,6 +361,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
>  	{ .compatible = "intel,thunderbay-ssi", .data = dw_spi_intel_init},
>  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
>  	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},

> +	{ .compatible = "snps,dwc-ssi-1.03a", dw_spi_hssi_ext_init},

Add "snps,dw-ahb-ssi" with the generic dw_spi_hssi_init() init method.

-Sergey

>  	{ /* end of table */}
>  };
>  MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
> -- 
> 2.30.2
> 

Re: [PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[Serge Semin]

Hi Sudip

On Tue, Aug 02, 2022 at 06:57:44PM +0100, Sudip Mukherjee wrote:
> Some Synopsys SSI controllers support enhanced SPI which includes
> Dual mode, Quad mode and Octal mode. DWC_ssi includes clock stretching
> feature in enhanced SPI modes which can be used to prevent FIFO underflow
> and overflow conditions while transmitting or receiving the data respectively.
> This is only tested on controller version 1.03a.
> 
> Ben Dooks (1):
>   spi: dw-apb-ssi: add generic 1.03a version

Thanks for the patchset. It's always welcome to have a new
functionality support. I'll have it reviewed on the next week. There
are several aspects like using version-based compatible string and new
capability flag I am the most worried about. They need to be
discussed first before proceeding with the rest of the things.

-Sergey

> 
> Sudip Mukherjee (10):
>   spi: dw: define capability for enhanced spi
>   spi: dw: add check for support of dual/quad/octal
>   spi: dw: define spi_frf for dual/quad/octal modes
>   spi: dw: use TMOD_RO to read in enhanced spi modes
>   spi: dw: define SPI_CTRLR0 register and its fields
>   spi: dw: update SPI_CTRLR0 register
>   spi: dw: update NDF while writing in enhanced spi mode
>   spi: dw: update buffer for enhanced spi mode
>   spi: dw: prepare the transfer routine for enhanced mode
>   spi: dw: initialize dwc-ssi-1.03a controller
> 
>  .../bindings/spi/snps,dw-apb-ssi.yaml         |   1 +
>  drivers/spi/spi-dw-core.c                     | 288 ++++++++++++++++--
>  drivers/spi/spi-dw-mmio.c                     |  10 +
>  drivers/spi/spi-dw.h                          |  19 ++
>  4 files changed, 291 insertions(+), 27 deletions(-)
> 
> -- 
> 2.30.2
> 

Re: [PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[Sudip Mukherjee]

On Wed, Aug 3, 2022 at 7:56 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> Hi Sudip
>
> On Tue, Aug 02, 2022 at 06:57:44PM +0100, Sudip Mukherjee wrote:
> > Some Synopsys SSI controllers support enhanced SPI which includes
> > Dual mode, Quad mode and Octal mode. DWC_ssi includes clock stretching
> > feature in enhanced SPI modes which can be used to prevent FIFO underflow
> > and overflow conditions while transmitting or receiving the data respectively.
> > This is only tested on controller version 1.03a.
> >
> > Ben Dooks (1):
> >   spi: dw-apb-ssi: add generic 1.03a version
>
> Thanks for the patchset. It's always welcome to have a new
> functionality support. I'll have it reviewed on the next week.

Thanks Sergey. I will then wait for your review before sending a v2
with the changes Mark has suggested.

--
Regards
Sudip

Re: [PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[Serge Semin]

Hi Sudip

On Tue, Aug 02, 2022 at 06:57:44PM +0100, Sudip Mukherjee wrote:
> Some Synopsys SSI controllers support enhanced SPI which includes
> Dual mode, Quad mode and Octal mode. DWC_ssi includes clock stretching
> feature in enhanced SPI modes which can be used to prevent FIFO underflow
> and overflow conditions while transmitting or receiving the data respectively.
> This is only tested on controller version 1.03a.
> 
> Ben Dooks (1):
>   spi: dw-apb-ssi: add generic 1.03a version
> 
> Sudip Mukherjee (10):
>   spi: dw: define capability for enhanced spi
>   spi: dw: add check for support of dual/quad/octal
>   spi: dw: define spi_frf for dual/quad/octal modes
>   spi: dw: use TMOD_RO to read in enhanced spi modes
>   spi: dw: define SPI_CTRLR0 register and its fields
>   spi: dw: update SPI_CTRLR0 register
>   spi: dw: update NDF while writing in enhanced spi mode
>   spi: dw: update buffer for enhanced spi mode
>   spi: dw: prepare the transfer routine for enhanced mode
>   spi: dw: initialize dwc-ssi-1.03a controller

Thanks for the very useful series. I've started reviewing it and will
share all my comments tomorrow.

-Sergey

> 
>  .../bindings/spi/snps,dw-apb-ssi.yaml         |   1 +
>  drivers/spi/spi-dw-core.c                     | 288 ++++++++++++++++--
>  drivers/spi/spi-dw-mmio.c                     |  10 +
>  drivers/spi/spi-dw.h                          |  19 ++
>  4 files changed, 291 insertions(+), 27 deletions(-)
> 
> -- 
> 2.30.2
> 

Re: [PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[Serge Semin]

Hello Sudip

On Tue, Aug 02, 2022 at 06:57:44PM +0100, Sudip Mukherjee wrote:
> Some Synopsys SSI controllers support enhanced SPI which includes
> Dual mode, Quad mode and Octal mode. DWC_ssi includes clock stretching
> feature in enhanced SPI modes which can be used to prevent FIFO underflow
> and overflow conditions while transmitting or receiving the data respectively.
> This is only tested on controller version 1.03a.

Thank you very much the patchset. As I already said adding new
controller features support is always welcome. Yet there are some
things which need to be fixed before the series would be fully
suitable to be merged in into the kernel. Here is a short summary
of ones:

1. The eSPI capability isn't specific for the DW AHB SSI controller
only. It can be found on the DW APB SSI controllers of v4.x and newer
(though the SPI_FRF field is placed at different offset in CTRL0 CSR).
Thus your patches will need to be fixed so the in-driver infrastructure
would imply that.

2. The mem ops check procedure provided by you doesn't verify whether
the passed cmd, address and dummy data lengths meet the controller
constraints or at least the constraints set by your code. You always
expect the address and command being 4 and 1 bytes long, which is way
not always true. So the implementation provided by you just won't
correctly work for the unsupported cases with no any error returned.

3. From what I see WAIT_CYCLES is specific for the Read-operations
only (see the controller HW manual, the paragraphs like "Write
Operation in Enhanced SPI Modes" or the SPI_CTRL0.WAIT_CYCLES field
description). So any dummy-bytes requested for the Tx operations just
won't be sent. Even though AFAICS the dummy cycles are specific for
the Read SPI NAND/NOR operations it still would be correct to
explicitly refuse the non-Rx transactions with non-zero dummy data
length.

4. I don't really see a reason of adding the address, command and
dummy data length constraints. You can as easily update the
command/address/dummy lengths passed in the SPI mem-op structure
thus providing wider SPI memory devices range support.

5. The main problem I can see in your implementation is that you try
to assimilate the eSPI feature for the current DW SSI EEPROM
read/write infrastructure. Note the SPI MEM ops currently available in
the driver have been specifically created for the platforms with the
native CS'es used to access the EEPROM devices. For such cases I had to
use some not that nice solutions like IRQ-less transfers, local IRQs
disabling and the outbound data collection in a single buffer in order
to bypass the nasty DW SSI controller peculiarities. All of that isn't
required in your case. You can implement a very nice and robust
algorithm.

6. You said your controller supports the clock stretching on Tx and Rx
transfers. This is a very useful feature which can be used to bypass
the main DW SSI controller problem connected with the native CS
auto-toggling when the Tx FIFO gets empty or data loose due to the Rx
FIFO overruns. Thus you won't need to always keep up with the Tx/Rx
FIFO levels and implement the IRQ-based SPI MEM transfers.

7. You unconditionally enable the eSPI support for the generic device
snps,dwc-ssi-1.03a while this is an optional feature which yet can be
disabled for any new controllers (see the SSI_SPI_MODE IP-core
synthesize parameter). What you really need is to simply auto-detect
the eSPI feature availability by checking whether the SPI_FRF field is
writable for the DW APB SSI v4.0a and newer and for any DWC AHB SSI.

8. There is no need in the IP-core version added to the compatible
string because it can be retrieved from the SSI_VERSION_ID CSR. I
would suggest to add a new generic compatible string "snps,dw-ahb-ssi"
for the DW AHB SSI controllers and forget about the compatible strings
versioning.

9. Always study the driver coding convention before updating. In this
particular case should you need to add new methods, macros, etc please
add the vendor-specific prefix as is done for the rest of the driver
entities.

I've deliberately collected all the generic comments here so you'd be
aware of the required changes in total, because I very much doubt all
of them could be fixed at once via a single patchset iteration. But as
soon as all of them are fixed we'll get a very nice and neat solution
for the eSPI feature.

I'll give you some more detailed comments right in the corresponding
patches, but they won't cover all the issues noted above on this
patchset iteration. So feel free to update your series based on your
understanding of the issues (you can ask me if you don't fully get
what I said above). It may reduce the number of the further series
re-submissions.

-Sergey

> 
> Ben Dooks (1):
>   spi: dw-apb-ssi: add generic 1.03a version
> 
> Sudip Mukherjee (10):
>   spi: dw: define capability for enhanced spi
>   spi: dw: add check for support of dual/quad/octal
>   spi: dw: define spi_frf for dual/quad/octal modes
>   spi: dw: use TMOD_RO to read in enhanced spi modes
>   spi: dw: define SPI_CTRLR0 register and its fields
>   spi: dw: update SPI_CTRLR0 register
>   spi: dw: update NDF while writing in enhanced spi mode
>   spi: dw: update buffer for enhanced spi mode
>   spi: dw: prepare the transfer routine for enhanced mode
>   spi: dw: initialize dwc-ssi-1.03a controller
> 
>  .../bindings/spi/snps,dw-apb-ssi.yaml         |   1 +
>  drivers/spi/spi-dw-core.c                     | 288 ++++++++++++++++--
>  drivers/spi/spi-dw-mmio.c                     |  10 +
>  drivers/spi/spi-dw.h                          |  19 ++
>  4 files changed, 291 insertions(+), 27 deletions(-)
> 
> -- 
> 2.30.2
> 

Re: [PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[Sudip Mukherjee]

On Fri, Aug 26, 2022 at 7:03 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> Hello Sudip
>
> On Tue, Aug 02, 2022 at 06:57:44PM +0100, Sudip Mukherjee wrote:
> > Some Synopsys SSI controllers support enhanced SPI which includes
> > Dual mode, Quad mode and Octal mode. DWC_ssi includes clock stretching
> > feature in enhanced SPI modes which can be used to prevent FIFO underflow
> > and overflow conditions while transmitting or receiving the data respectively.
> > This is only tested on controller version 1.03a.
>

<snip>

>
> I've deliberately collected all the generic comments here so you'd be
> aware of the required changes in total, because I very much doubt all
> of them could be fixed at once via a single patchset iteration. But as
> soon as all of them are fixed we'll get a very nice and neat solution
> for the eSPI feature.
>

Thanks a lot for the summary here Sergey. I am sure I will have a few
questions for you after I start with the changes.

-- 
Regards
Sudip

Re: [PATCH 00/11] Add support for enhanced SPI for Designware SPI controllers

[Serge Semin]

On Tue, Aug 30, 2022 at 09:48:35AM +0100, Sudip Mukherjee wrote:
> On Fri, Aug 26, 2022 at 7:03 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> >
> > Hello Sudip
> >
> > On Tue, Aug 02, 2022 at 06:57:44PM +0100, Sudip Mukherjee wrote:
> > > Some Synopsys SSI controllers support enhanced SPI which includes
> > > Dual mode, Quad mode and Octal mode. DWC_ssi includes clock stretching
> > > feature in enhanced SPI modes which can be used to prevent FIFO underflow
> > > and overflow conditions while transmitting or receiving the data respectively.
> > > This is only tested on controller version 1.03a.
> >
> 
> <snip>
> 
> >
> > I've deliberately collected all the generic comments here so you'd be
> > aware of the required changes in total, because I very much doubt all
> > of them could be fixed at once via a single patchset iteration. But as
> > soon as all of them are fixed we'll get a very nice and neat solution
> > for the eSPI feature.
> >
> 

> Thanks a lot for the summary here Sergey. I am sure I will have a few
> questions for you after I start with the changes.

Ok. Don't hesitate to ask.

-Sergey

> 
> -- 
> Regards
> Sudip