[PATCH v3 0/5] Add support for Intel Thunder Bay SPI controller

[PATCH v3 0/5] Add support for Intel Thunder Bay SPI controller

[nandhini.srikandan]

From: Nandhini Srikandan <nandhini.srikandan@intel.com>

Hi,

This patch set adds support for Designware SPI version v1.02a (patch 1,2,3).
This patch also enables support for DW SPI on Intel Thunder Bay (patch 4,5).

Patch 1: DW SPI DT bindings for Slave Select Toggle Enable (SSTE).
Patch 2: Adds SSTE support for Designware SPI controller.
Patch 3: Adds master mode support for Designware SPI controller v1.02a.
Patch 4: DW SPI DT bindings for Intel Thunder Bay SoC.
Patch 5: Adds support for Designware SPI on Intel Thunderbay SoC.

The driver is tested on Keem Bay and Thunder Bay evaluation board

Summary:
Changes from v2:
1) SSTE support made using dt and created seperate patches.
2) SPI controller master mode selection made common to all DW SPI controllers.
3) Using a common init function for both keem bay and thunder bay.

Changes from v1:
1) Designware CR0 specific macros are named in a generic way.
2) SPI CAP macros are named in generic way rather than naming project specific.
3) SPI KEEM BAY specific macros are replaced by generic macros.
4) Resued the existing SPI deassert API instead of adding another reset


Changes in patches:

Patch 1 and Patch 2
Changes from v2/v1:
1) Patch is newly introduced in v3 to support SSTE via DTS

Patch 3:
Changes from v2/v1:
1)Newly introduced in v3 to make master mode selection (v1.02a) as seperate patch

Patch 4:
Changes from v2/v1:
1) No change in this patch. 

Patch 5:
Changes from v2/v1:
1) Removed SSTE support from this patch and moved to Patch 2.
2) Removed master mode selection and moved to Patch 3.
3) Init function made common for Keem Bay and Thunder Bay.



Thanks & Regards,
Nandhini



Nandhini Srikandan (5):
  dt-bindings: spi: Add SSTE support for DWC SSI controller
  spi: dw: Add SSTE support for DWC SSI controller
  spi: dw: Add support for master mode selection for DWC SSI controller
  dt-bindings: spi: Add bindings for Intel Thunder Bay SoC
  spi: dw: Add support for Intel Thunder Bay SPI controller

 .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml  |  8 ++++++++
 drivers/spi/spi-dw-core.c                         | 15 +++++++++++++--
 drivers/spi/spi-dw-mmio.c                         |  9 +++++----
 drivers/spi/spi-dw.h                              |  6 ++++--
 4 files changed, 30 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller

[nandhini.srikandan]

From: Nandhini Srikandan <nandhini.srikandan@intel.com>

Add Slave Select Toggle Enable(SSTE) support for DWC SSI controller.

Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index ca91201a9926..866416d01e94 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -149,6 +149,12 @@ patternProperties:
           is an optional feature of the designware controller, and the
           upper limit is also subject to controller configuration.
 
+      snps,sste:
+        description: Slave select line will toggle between consecutive
+          data frames, with the serial clock being held to its default
+          value while slave select line is high.
+        type: boolean
+
 unevaluatedProperties: false
 
 required:
-- 
2.17.1

Re: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller

[Mark Brown]

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

On Thu, Nov 11, 2021 at 02:51:57PM +0800, nandhini.srikandan@intel.com wrote:

> +      snps,sste:
> +        description: Slave select line will toggle between consecutive
> +          data frames, with the serial clock being held to its default
> +          value while slave select line is high.
> +        type: boolean

This is not something that should be configured in the DT, it needs to
be controlled by the client driver.  Changing this without involving the
client driver will lead to data corruption.

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

Re: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller

[Serge Semin]

Hello Nandhini, Mark

On Thu, Nov 11, 2021 at 02:51:57PM +0800, nandhini.srikandan@intel.com wrote:
> From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> 

> Add Slave Select Toggle Enable(SSTE) support for DWC SSI controller.

Nandhini, as Mark said this is no need in this new property since that
feature is supposed to be enabled by the client drivers by means of
setting the SPI_CS_WORD flag in the spi_device->mode field. (See its
usage for reference.)

BTW Mark, why not to have a generic DT-property which would set that
flag automatically by the SPI-core subsystem seeing it's indeed a
client device-property? For instance there can be some property like
"spi-cs-toggle" DT-property which when specified for the particular
SPI-client DT-node will make the SPI-core subsystem to set the
SPI_CS_WORD flag of the device mode? Like it has already been done for
"spi-cs-high"/"spi-lsb-first"/etc.
In this case Nandhini would need to just convert this patch a bit so
to be fixing the Documentation/devicetree/bindings/spi/spi-controller.yaml
bindings instead.

-Sergey

> 
> Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index ca91201a9926..866416d01e94 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -149,6 +149,12 @@ patternProperties:
>            is an optional feature of the designware controller, and the
>            upper limit is also subject to controller configuration.
>  
> +      snps,sste:
> +        description: Slave select line will toggle between consecutive
> +          data frames, with the serial clock being held to its default
> +          value while slave select line is high.
> +        type: boolean
> +
>  unevaluatedProperties: false
>  
>  required:
> -- 
> 2.17.1
> 

Re: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller

[Mark Brown]

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

On Thu, Nov 11, 2021 at 05:31:08PM +0300, Serge Semin wrote:

> BTW Mark, why not to have a generic DT-property which would set that
> flag automatically by the SPI-core subsystem seeing it's indeed a
> client device-property? For instance there can be some property like
> "spi-cs-toggle" DT-property which when specified for the particular
> SPI-client DT-node will make the SPI-core subsystem to set the

Anything like this is fundamentally part of the wire protocol for the
device, there's no need for an extra property on top of the compatible
for the device and the driver really, really needs to know what's going
on to avoid data corruption.  You could also use this feature together
with varying the word size as an optimisation at runtime (eg, do long
sequences of register writes in a single hardware operation by setting
an appropriate word length to cause the controller to bounce chip
select between writes).

> SPI_CS_WORD flag of the device mode? Like it has already been done for
> "spi-cs-high"/"spi-lsb-first"/etc.

I don't think either of those properties was a good idea, there's a
bunch of stuff in the older SPI bindings that don't make much sense.

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

Re: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller

[Serge Semin]

On Thu, Nov 11, 2021 at 03:01:12PM +0000, Mark Brown wrote:
> On Thu, Nov 11, 2021 at 05:31:08PM +0300, Serge Semin wrote:
> 
> > BTW Mark, why not to have a generic DT-property which would set that
> > flag automatically by the SPI-core subsystem seeing it's indeed a
> > client device-property? For instance there can be some property like
> > "spi-cs-toggle" DT-property which when specified for the particular
> > SPI-client DT-node will make the SPI-core subsystem to set the
> 
> Anything like this is fundamentally part of the wire protocol for the
> device, there's no need for an extra property on top of the compatible
> for the device and the driver really, really needs to know what's going
> on to avoid data corruption.  You could also use this feature together
> with varying the word size as an optimisation at runtime (eg, do long
> sequences of register writes in a single hardware operation by setting
> an appropriate word length to cause the controller to bounce chip
> select between writes).
> 
> > SPI_CS_WORD flag of the device mode? Like it has already been done for
> > "spi-cs-high"/"spi-lsb-first"/etc.
> 
> I don't think either of those properties was a good idea, there's a
> bunch of stuff in the older SPI bindings that don't make much sense.

Ok. Thanks for clarification. No new DT-property then.

Nandhini, could you please drop this patch in v4? One more time I'm sorry
for misleading you on v2.

-Sergey

RE: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI controller

[Srikandan, Nandhini]

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Thursday, November 11, 2021 8:37 PM
> To: Mark Brown <broonie@kernel.org>; Srikandan, Nandhini
> <nandhini.srikandan@intel.com>
> Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; robh+dt@kernel.org;
> linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; mgross@linux.intel.com; Pan, Kris
> <kris.pan@intel.com>; Demakkanavar, Kenchappa
> <kenchappa.demakkanavar@intel.com>; Zhou, Furong
> <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@intel.com>; Vaidya, Mahesh R
> <mahesh.r.vaidya@intel.com>; A, Rashmi <rashmi.a@intel.com>
> Subject: Re: [PATCH v3 1/5] dt-bindings: spi: Add SSTE support for DWC SSI
> controller
> 
> On Thu, Nov 11, 2021 at 03:01:12PM +0000, Mark Brown wrote:
> > On Thu, Nov 11, 2021 at 05:31:08PM +0300, Serge Semin wrote:
> >
> > > BTW Mark, why not to have a generic DT-property which would set that
> > > flag automatically by the SPI-core subsystem seeing it's indeed a
> > > client device-property? For instance there can be some property like
> > > "spi-cs-toggle" DT-property which when specified for the particular
> > > SPI-client DT-node will make the SPI-core subsystem to set the
> >
> > Anything like this is fundamentally part of the wire protocol for the
> > device, there's no need for an extra property on top of the compatible
> > for the device and the driver really, really needs to know what's
> > going on to avoid data corruption.  You could also use this feature
> > together with varying the word size as an optimisation at runtime (eg,
> > do long sequences of register writes in a single hardware operation by
> > setting an appropriate word length to cause the controller to bounce
> > chip select between writes).
> >
> > > SPI_CS_WORD flag of the device mode? Like it has already been done
> > > for "spi-cs-high"/"spi-lsb-first"/etc.
> >
> > I don't think either of those properties was a good idea, there's a
> > bunch of stuff in the older SPI bindings that don't make much sense.
> 
> Ok. Thanks for clarification. No new DT-property then.
> 
> Nandhini, could you please drop this patch in v4? One more time I'm sorry
> for misleading you on v2.

Sure, I will make use of SPI_CS_WORD flag and drop this patch in next version. 

> 
> -Sergey

[PATCH v3 2/5] spi: dw: Add SSTE support for DWC SSI controller

[nandhini.srikandan]

From: Nandhini Srikandan <nandhini.srikandan@intel.com>

Add support for Slave Select Toggle Enable (SSTE) in DWC SSI controller
via DTS. The slave select line will toggle between consecutive data frames,
with the serial clock being held to its default value while slave
select line is high.

Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
---
 drivers/spi/spi-dw-core.c | 11 +++++++++++
 drivers/spi/spi-dw.h      |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index a305074c482e..bfa075a4f779 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -27,6 +27,7 @@
 struct chip_data {
 	u32 cr0;
 	u32 rx_sample_dly;	/* RX sample delay */
+	bool sste;		/* Slave select Toggle flag */
 };
 
 #ifdef CONFIG_DEBUG_FS
@@ -269,6 +270,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
 
 static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
 {
+	struct chip_data *chip = spi_get_ctldata(spi);
 	u32 cr0 = 0;
 
 	if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
@@ -285,6 +287,9 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
 
 		/* CTRLR0[11] Shift Register Loop */
 		cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET;
+
+		/* CTRLR0[24] Slave Select Toggle Enable */
+		cr0 |= chip->sste << SPI_SSTE_OFFSET;
 	} else {
 		/* CTRLR0[ 7: 6] Frame Format */
 		cr0 |= SSI_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
@@ -300,6 +305,9 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
 		/* CTRLR0[13] Shift Register Loop */
 		cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DWC_SSI_CTRLR0_SRL_OFFSET;
 
+		/* CTRLR0[14] Slave Select Toggle Enable */
+		cr0 |= chip->sste << DWC_SSI_CTRLR0_SSTE_OFFSET;
+
 		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
 			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
 	}
@@ -789,6 +797,9 @@ static int dw_spi_setup(struct spi_device *spi)
 		chip->rx_sample_dly = DIV_ROUND_CLOSEST(rx_sample_dly_ns,
 							NSEC_PER_SEC /
 							dws->max_freq);
+
+		/* Get slave select toggling feature requirement */
+		chip->sste = device_property_read_bool(&spi->dev, "snps,sste");
 	}
 
 	/*
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index b665e040862c..2ee3f839de39 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -65,8 +65,10 @@
 #define SPI_SLVOE_OFFSET		10
 #define SPI_SRL_OFFSET			11
 #define SPI_CFS_OFFSET			12
+#define SPI_SSTE_OFFSET			24
 
 /* Bit fields in CTRLR0 based on DWC_ssi_databook.pdf v1.01a */
+#define DWC_SSI_CTRLR0_SSTE_OFFSET	14
 #define DWC_SSI_CTRLR0_SRL_OFFSET	13
 #define DWC_SSI_CTRLR0_TMOD_OFFSET	10
 #define DWC_SSI_CTRLR0_TMOD_MASK	GENMASK(11, 10)
-- 
2.17.1

Re: [PATCH v3 2/5] spi: dw: Add SSTE support for DWC SSI controller

[Mark Brown]

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

On Thu, Nov 11, 2021 at 02:51:58PM +0800, nandhini.srikandan@intel.com wrote:
> From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> 
> Add support for Slave Select Toggle Enable (SSTE) in DWC SSI controller
> via DTS. The slave select line will toggle between consecutive data frames,
> with the serial clock being held to its default value while slave
> select line is high.

This should be controlled by SPI_CS_WORD.

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

Re: [PATCH v3 2/5] spi: dw: Add SSTE support for DWC SSI controller

[Serge Semin]

On Thu, Nov 11, 2021 at 01:56:36PM +0000, Mark Brown wrote:
> On Thu, Nov 11, 2021 at 02:51:58PM +0800, nandhini.srikandan@intel.com wrote:
> > From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> > 
> > Add support for Slave Select Toggle Enable (SSTE) in DWC SSI controller
> > via DTS. The slave select line will toggle between consecutive data frames,
> > with the serial clock being held to its default value while slave
> > select line is high.
> 

> This should be controlled by SPI_CS_WORD.

Oh, yeah. I've absolutely forgotten about that flag. Indeed then there
is no need in implementing a separate DT-property. In this case the
patchset will need to be fixed a bit: remove the DT-part of the
sste-feature and alter the dw_spi_setup() method so to take the
SPI_CS_WORD flag into account.

Nandhini, sorry about a wrong advice on v2. It seems v4 will be
required...

-Sergey

Re: [PATCH v3 2/5] spi: dw: Add SSTE support for DWC SSI controller

[Serge Semin]

On Thu, Nov 11, 2021 at 02:51:58PM +0800, nandhini.srikandan@intel.com wrote:
> From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> 
> Add support for Slave Select Toggle Enable (SSTE) in DWC SSI controller
> via DTS. The slave select line will toggle between consecutive data frames,
> with the serial clock being held to its default value while slave
> select line is high.
> 
> Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
> ---
>  drivers/spi/spi-dw-core.c | 11 +++++++++++
>  drivers/spi/spi-dw.h      |  2 ++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index a305074c482e..bfa075a4f779 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -27,6 +27,7 @@
>  struct chip_data {
>  	u32 cr0;
>  	u32 rx_sample_dly;	/* RX sample delay */

> +	bool sste;		/* Slave select Toggle flag */

As Mark said there is no need in the new DT-property thus there is no
need in the sste flag being preserved in the chip-data structure
seeing there is a dedicated flag has been defined for this mode.

>  };
>  
>  #ifdef CONFIG_DEBUG_FS
> @@ -269,6 +270,7 @@ static irqreturn_t dw_spi_irq(int irq, void *dev_id)
>  
>  static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
>  {
> +	struct chip_data *chip = spi_get_ctldata(spi);
>  	u32 cr0 = 0;
>  
>  	if (!(dws->caps & DW_SPI_CAP_DWC_SSI)) {
> @@ -285,6 +287,9 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
>  
>  		/* CTRLR0[11] Shift Register Loop */
>  		cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << SPI_SRL_OFFSET;

> +
> +		/* CTRLR0[24] Slave Select Toggle Enable */
> +		cr0 |= chip->sste << SPI_SSTE_OFFSET;

Just check for the SPI_CS_WORD flag state here directly. Like this:
+ cr0 |= ((spi->mode & SPI_CS_WORD) ? 1 : 0) << SPI_SSTE_OFFSET;

>  	} else {
>  		/* CTRLR0[ 7: 6] Frame Format */
>  		cr0 |= SSI_MOTO_SPI << DWC_SSI_CTRLR0_FRF_OFFSET;
> @@ -300,6 +305,9 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
>  		/* CTRLR0[13] Shift Register Loop */
>  		cr0 |= ((spi->mode & SPI_LOOP) ? 1 : 0) << DWC_SSI_CTRLR0_SRL_OFFSET;
>  

> +		/* CTRLR0[14] Slave Select Toggle Enable */
> +		cr0 |= chip->sste << DWC_SSI_CTRLR0_SSTE_OFFSET;
> +

the same as above.

>  		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
>  			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
>  	}
> @@ -789,6 +797,9 @@ static int dw_spi_setup(struct spi_device *spi)
>  		chip->rx_sample_dly = DIV_ROUND_CLOSEST(rx_sample_dly_ns,
>  							NSEC_PER_SEC /
>  							dws->max_freq);

> +
> +		/* Get slave select toggling feature requirement */
> +		chip->sste = device_property_read_bool(&spi->dev, "snps,sste");

As Mark said there is no need in this new DT-property.

-Sergey

>  	}
>  
>  	/*
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index b665e040862c..2ee3f839de39 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -65,8 +65,10 @@
>  #define SPI_SLVOE_OFFSET		10
>  #define SPI_SRL_OFFSET			11
>  #define SPI_CFS_OFFSET			12
> +#define SPI_SSTE_OFFSET			24
>  
>  /* Bit fields in CTRLR0 based on DWC_ssi_databook.pdf v1.01a */
> +#define DWC_SSI_CTRLR0_SSTE_OFFSET	14
>  #define DWC_SSI_CTRLR0_SRL_OFFSET	13
>  #define DWC_SSI_CTRLR0_TMOD_OFFSET	10
>  #define DWC_SSI_CTRLR0_TMOD_MASK	GENMASK(11, 10)
> -- 
> 2.17.1
> 

[PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[nandhini.srikandan]

From: Nandhini Srikandan <nandhini.srikandan@intel.com>

Add support to select the controller mode as master mode by setting
Bit 31 of CTRLR0 register. This feature is supported for controller
versions above v1.02.

Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
---
 drivers/spi/spi-dw-core.c | 4 ++--
 drivers/spi/spi-dw.h      | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index bfa075a4f779..26d0ef87661d 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -308,8 +308,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
 		/* CTRLR0[14] Slave Select Toggle Enable */
 		cr0 |= chip->sste << DWC_SSI_CTRLR0_SSTE_OFFSET;
 
-		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
-			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
+		/* CTRLR0[31] MST */
+		cr0 |= DWC_SSI_CTRLR0_MST;
 	}
 
 	return cr0;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 2ee3f839de39..d67f62ff79c9 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -78,11 +78,11 @@
 #define DWC_SSI_CTRLR0_DFS_OFFSET	0
 
 /*
- * For Keem Bay, CTRLR0[31] is used to select controller mode.
+ * CTRLR0[31] is used to select controller mode.
  * 0: SSI is slave
  * 1: SSI is master
  */
-#define DWC_SSI_CTRLR0_KEEMBAY_MST	BIT(31)
+#define DWC_SSI_CTRLR0_MST		BIT(31)
 
 /* Bit fields in CTRLR1 */
 #define SPI_NDF_MASK			GENMASK(15, 0)
-- 
2.17.1

Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Mark Brown]

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

On Thu, Nov 11, 2021 at 02:51:59PM +0800, nandhini.srikandan@intel.com wrote:

> Add support to select the controller mode as master mode by setting
> Bit 31 of CTRLR0 register. This feature is supported for controller
> versions above v1.02.

Clearly older versions of the controller can also run in this mode...

> -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> -			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> +		/* CTRLR0[31] MST */
> +		cr0 |= DWC_SSI_CTRLR0_MST;

This makes the configuration unconditional, it's not gated by controller
version checks or any kind of quirk any more meaning that if anything
interprets that bit differently things might break.  If this is really
required to put the controller in master mode it seems that either the
1.02 version is not widespread or this is generally the hardware
default.

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

Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Serge Semin]

Hello Mark,

On Thu, Nov 11, 2021 at 02:16:05PM +0000, Mark Brown wrote:
> On Thu, Nov 11, 2021 at 02:51:59PM +0800, nandhini.srikandan@intel.com wrote:
> 
> > Add support to select the controller mode as master mode by setting
> > Bit 31 of CTRLR0 register. This feature is supported for controller
> > versions above v1.02.
> 

> Clearly older versions of the controller can also run in this mode...

Yes, but the driver doesn't support the slave mode at the moment.
So always enabling the master mode seems natural. (see my next comment
also concerning this matter)

> 
> > -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > -			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> > +		/* CTRLR0[31] MST */
> > +		cr0 |= DWC_SSI_CTRLR0_MST;
> 

> This makes the configuration unconditional, it's not gated by controller
> version checks or any kind of quirk any more meaning that if anything
> interprets that bit differently things might break.  If this is really
> required to put the controller in master mode it seems that either the
> 1.02 version is not widespread or this is generally the hardware
> default.

We have already discussed this feature in v2:
https://patchwork.kernel.org/project/spi-devel-general/patch/20210824085856.12714-3-nandhini.srikandan@intel.com/
Since that bit has been reserved before 1.02a but is no available for
any DWC SSI controller and the driver doesn't support the SPI-slave mode
at the moment I suggested to just always set that flag for the DWC SSI
code. Please see my reply to Nandhini here:
https://patchwork.kernel.org/project/spi-devel-general/patch/20210824085856.12714-3-nandhini.srikandan@intel.com/#24433679

-Sergey

Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Mark Brown]

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

On Thu, Nov 11, 2021 at 05:52:46PM +0300, Serge Semin wrote:
> On Thu, Nov 11, 2021 at 02:16:05PM +0000, Mark Brown wrote:
> > On Thu, Nov 11, 2021 at 02:51:59PM +0800, nandhini.srikandan@intel.com wrote:

> > > Add support to select the controller mode as master mode by setting
> > > Bit 31 of CTRLR0 register. This feature is supported for controller
> > > versions above v1.02.

> > Clearly older versions of the controller can also run in this mode...

> Yes, but the driver doesn't support the slave mode at the moment.
> So always enabling the master mode seems natural. (see my next comment
> also concerning this matter)

The commit message makes it sound like master mode is only supported for
the newer versions.

> > This makes the configuration unconditional, it's not gated by controller
> > version checks or any kind of quirk any more meaning that if anything

> We have already discussed this feature in v2:
> https://patchwork.kernel.org/project/spi-devel-general/patch/20210824085856.12714-3-nandhini.srikandan@intel.com/
> Since that bit has been reserved before 1.02a but is no available for
> any DWC SSI controller and the driver doesn't support the SPI-slave mode
> at the moment I suggested to just always set that flag for the DWC SSI
> code. Please see my reply to Nandhini here:

Given that people seem to frequently customise these IPs when
integrating them I wouldn't trust people not to have added some other
control into that reserved bit doing some magic stuff that's useful in
their system.

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

Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Serge Semin]

On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote:
> On Thu, Nov 11, 2021 at 05:52:46PM +0300, Serge Semin wrote:
> > On Thu, Nov 11, 2021 at 02:16:05PM +0000, Mark Brown wrote:
> > > On Thu, Nov 11, 2021 at 02:51:59PM +0800, nandhini.srikandan@intel.com wrote:
> 
> > > > Add support to select the controller mode as master mode by setting
> > > > Bit 31 of CTRLR0 register. This feature is supported for controller
> > > > versions above v1.02.
> 
> > > Clearly older versions of the controller can also run in this mode...
> 
> > Yes, but the driver doesn't support the slave mode at the moment.
> > So always enabling the master mode seems natural. (see my next comment
> > also concerning this matter)
> 

> The commit message makes it sound like master mode is only supported for
> the newer versions.

I meant it doesn't really matter if the bit has been reserved before
and the driver doesn't support the Slave-mode of the controller
anyway.
Regarding the Master-mode feature availability. Originally Wan added
that flag setting for v1.01a here:
https://patchwork.kernel.org/project/spi-devel-general/patch/20200312113129.8198-8-wan.ahmad.zainie.wan.mohamad@intel.com/
Nandhini said in v2 that both Keem Bay and Thunder Bay uses DWC SSI
v1.02a and the BIT[31] functionality is not Intel-specific, but
generic for DWC SSIs.  So version-wise it's either Wan or Nandhini
ware mistaken at some point.

> 
> > > This makes the configuration unconditional, it's not gated by controller
> > > version checks or any kind of quirk any more meaning that if anything
> 
> > We have already discussed this feature in v2:
> > https://patchwork.kernel.org/project/spi-devel-general/patch/20210824085856.12714-3-nandhini.srikandan@intel.com/
> > Since that bit has been reserved before 1.02a but is no available for
> > any DWC SSI controller and the driver doesn't support the SPI-slave mode
> > at the moment I suggested to just always set that flag for the DWC SSI
> > code. Please see my reply to Nandhini here:
> 

> Given that people seem to frequently customise these IPs when
> integrating them I wouldn't trust people not to have added some other
> control into that reserved bit doing some magic stuff that's useful in
> their system.

In that case the corresponding platform code would have needed to have
that peculiarity properly handled and not to use a generic compatibles
like "snps,dwc-ssi-1.01a" or "snps,dw-apb-ssi", which are supposed to
be utilized for the default IP-core configs only. For the sake of the
code simplification I'd stick to setting that flag for each generic
DWC SSI-compatible device. That will be also helpful for DWC SSIs
which for some reason have the slave-mode enabled by default.

Alternatively the driver could read the IP-core version from the
DW_SPI_VERSION register, parse it (since it's in ASCII) and then use
it in the conditional Master mode activation here. But that could have
been a better solution in case if the older IP-cores would have used
that bit for something special, while Nandhini claims it was reserved.
So in this case I would stick with a simpler approach until we get to
face any problem in this matter, especially seeing we already pocking
the reserved bits of the CTRL0 register in this driver in the
spi_hw_init() method when it comes to the DFS field width detection.

-Sergey

Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Mark Brown]

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

On Thu, Nov 11, 2021 at 07:06:27PM +0300, Serge Semin wrote:
> On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote:

> > Given that people seem to frequently customise these IPs when
> > integrating them I wouldn't trust people not to have added some other
> > control into that reserved bit doing some magic stuff that's useful in
> > their system.

> In that case the corresponding platform code would have needed to have
> that peculiarity properly handled and not to use a generic compatibles
> like "snps,dwc-ssi-1.01a" or "snps,dw-apb-ssi", which are supposed to
> be utilized for the default IP-core configs only. For the sake of the
> code simplification I'd stick to setting that flag for each generic
> DWC SSI-compatible device. That will be also helpful for DWC SSIs
> which for some reason have the slave-mode enabled by default.

That's easier right up until the point where it explodes - I'd prefer to
be more conservative here.  Fixing things up after the fact gets painful
when people end up only finding the bug in released kernels, especially
if it's distro end users or similar rather than developers.

> Alternatively the driver could read the IP-core version from the
> DW_SPI_VERSION register, parse it (since it's in ASCII) and then use
> it in the conditional Master mode activation here. But that could have
> been a better solution in case if the older IP-cores would have used
> that bit for something special, while Nandhini claims it was reserved.
> So in this case I would stick with a simpler approach until we get to
> face any problem in this matter, especially seeing we already pocking
> the reserved bits of the CTRL0 register in this driver in the
> spi_hw_init() method when it comes to the DFS field width detection.

If the device has a version register checking that seems ideal - the
infrastructure will most likely be useful in future anyway.  A bit of a
shame that it's an ASCII string though.

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

Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Serge Semin]

On Thu, Nov 11, 2021 at 04:25:02PM +0000, Mark Brown wrote:
> On Thu, Nov 11, 2021 at 07:06:27PM +0300, Serge Semin wrote:
> > On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote:
> 
> > > Given that people seem to frequently customise these IPs when
> > > integrating them I wouldn't trust people not to have added some other
> > > control into that reserved bit doing some magic stuff that's useful in
> > > their system.
> 
> > In that case the corresponding platform code would have needed to have
> > that peculiarity properly handled and not to use a generic compatibles
> > like "snps,dwc-ssi-1.01a" or "snps,dw-apb-ssi", which are supposed to
> > be utilized for the default IP-core configs only. For the sake of the
> > code simplification I'd stick to setting that flag for each generic
> > DWC SSI-compatible device. That will be also helpful for DWC SSIs
> > which for some reason have the slave-mode enabled by default.
> 

> That's easier right up until the point where it explodes - I'd prefer to
> be more conservative here.  Fixing things up after the fact gets painful
> when people end up only finding the bug in released kernels, especially
> if it's distro end users or similar rather than developers.

Since IP-core and components versions is now supported that will easy
to implement. Thanks for merging the corresponding series in BTW.

> 
> > Alternatively the driver could read the IP-core version from the
> > DW_SPI_VERSION register, parse it (since it's in ASCII) and then use
> > it in the conditional Master mode activation here. But that could have
> > been a better solution in case if the older IP-cores would have used
> > that bit for something special, while Nandhini claims it was reserved.
> > So in this case I would stick with a simpler approach until we get to
> > face any problem in this matter, especially seeing we already pocking
> > the reserved bits of the CTRL0 register in this driver in the
> > spi_hw_init() method when it comes to the DFS field width detection.
> 
> If the device has a version register checking that seems ideal - the
> infrastructure will most likely be useful in future anyway.  A bit of a
> shame that it's an ASCII string though.

That's what the patchset has been implemented for in the first place
https://lore.kernel.org/linux-spi/20211115181917.7521-1-Sergey.Semin@baikalelectronics.ru/

Nandhini, Mark has just merged in the series that adds the IP-core
versions infrastructure support to the DW SSI driver.  So now you can
easily convert this patch to be using that new interface like this:
-               if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
-                       cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
+               /* CTRLR0[31] MST */
+		if (dw_spi_ver_is_ge(dws, HSSI, 102A))
+       	        cr0 |= DWC_HSSI_CTRLR0_MST;

Please don't forget to convert the DWC_SSI_CTRLR0_KEEMBAY_MST name to
something like DWC_HSSI_CTRLR0_MST and place it at the top of the DWC
HSSI CTRLR0 register macros list.

-Sergey

RE: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Srikandan, Nandhini]

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Wednesday, November 17, 2021 12:46 AM
> To: Mark Brown <broonie@kernel.org>; Srikandan, Nandhini
> <nandhini.srikandan@intel.com>
> Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; robh+dt@kernel.org;
> linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; mgross@linux.intel.com; Pan, Kris
> <kris.pan@intel.com>; Demakkanavar, Kenchappa
> <kenchappa.demakkanavar@intel.com>; Zhou, Furong
> <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@intel.com>; Vaidya, Mahesh R
> <mahesh.r.vaidya@intel.com>; A, Rashmi <rashmi.a@intel.com>
> Subject: Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection
> for DWC SSI controller
> 
> On Thu, Nov 11, 2021 at 04:25:02PM +0000, Mark Brown wrote:
> > On Thu, Nov 11, 2021 at 07:06:27PM +0300, Serge Semin wrote:
> > > On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote:
> >
> > > > Given that people seem to frequently customise these IPs when
> > > > integrating them I wouldn't trust people not to have added some
> > > > other control into that reserved bit doing some magic stuff that's
> > > > useful in their system.
> >
> > > In that case the corresponding platform code would have needed to
> > > have that peculiarity properly handled and not to use a generic
> > > compatibles like "snps,dwc-ssi-1.01a" or "snps,dw-apb-ssi", which
> > > are supposed to be utilized for the default IP-core configs only.
> > > For the sake of the code simplification I'd stick to setting that
> > > flag for each generic DWC SSI-compatible device. That will be also
> > > helpful for DWC SSIs which for some reason have the slave-mode
> enabled by default.
> >
> 
> > That's easier right up until the point where it explodes - I'd prefer
> > to be more conservative here.  Fixing things up after the fact gets
> > painful when people end up only finding the bug in released kernels,
> > especially if it's distro end users or similar rather than developers.
> 
> Since IP-core and components versions is now supported that will easy to
> implement. Thanks for merging the corresponding series in BTW.
> 
> >
> > > Alternatively the driver could read the IP-core version from the
> > > DW_SPI_VERSION register, parse it (since it's in ASCII) and then use
> > > it in the conditional Master mode activation here. But that could
> > > have been a better solution in case if the older IP-cores would have
> > > used that bit for something special, while Nandhini claims it was
> reserved.
> > > So in this case I would stick with a simpler approach until we get
> > > to face any problem in this matter, especially seeing we already
> > > pocking the reserved bits of the CTRL0 register in this driver in
> > > the
> > > spi_hw_init() method when it comes to the DFS field width detection.
> >
> > If the device has a version register checking that seems ideal - the
> > infrastructure will most likely be useful in future anyway.  A bit of
> > a shame that it's an ASCII string though.
> 
> That's what the patchset has been implemented for in the first place
> https://lore.kernel.org/linux-spi/20211115181917.7521-1-
> Sergey.Semin@baikalelectronics.ru/
> 
> Nandhini, Mark has just merged in the series that adds the IP-core versions
> infrastructure support to the DW SSI driver.  So now you can easily convert
> this patch to be using that new interface like this:
> -               if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> -                       cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> +               /* CTRLR0[31] MST */
> +		if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> +       	        cr0 |= DWC_HSSI_CTRLR0_MST;
> 
> Please don't forget to convert the DWC_SSI_CTRLR0_KEEMBAY_MST name to
> something like DWC_HSSI_CTRLR0_MST and place it at the top of the DWC
> HSSI CTRLR0 register macros list.
> 
> -Sergey
Sure, I will test this patch set on our hardware and then use the IP-core version infrastructure support 
and make the changes as mentioned for MST bit.

- Nandhini

RE: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Srikandan, Nandhini]

> -----Original Message-----
> From: Srikandan, Nandhini
> Sent: Wednesday, November 17, 2021 5:29 PM
> To: Serge Semin <fancer.lancer@gmail.com>; Mark Brown
> <broonie@kernel.org>
> Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; robh+dt@kernel.org;
> linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; mgross@linux.intel.com; Pan, Kris
> <kris.pan@intel.com>; Demakkanavar, Kenchappa
> <kenchappa.demakkanavar@intel.com>; Zhou, Furong
> <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@intel.com>; Vaidya, Mahesh R
> <mahesh.r.vaidya@intel.com>; A, Rashmi <Rashmi.A@intel.com>
> Subject: RE: [PATCH v3 3/5] spi: dw: Add support for master mode selection
> for DWC SSI controller
> 
> 
> 
> > -----Original Message-----
> > From: Serge Semin <fancer.lancer@gmail.com>
> > Sent: Wednesday, November 17, 2021 12:46 AM
> > To: Mark Brown <broonie@kernel.org>; Srikandan, Nandhini
> > <nandhini.srikandan@intel.com>
> > Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>;
> > robh+dt@kernel.org; linux-spi@vger.kernel.org;
> > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > mgross@linux.intel.com; Pan, Kris <kris.pan@intel.com>; Demakkanavar,
> > Kenchappa <kenchappa.demakkanavar@intel.com>; Zhou, Furong
> > <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa
> > <mallikarjunappa.sangannavar@intel.com>; Vaidya, Mahesh R
> > <mahesh.r.vaidya@intel.com>; A, Rashmi <rashmi.a@intel.com>
> > Subject: Re: [PATCH v3 3/5] spi: dw: Add support for master mode
> > selection for DWC SSI controller
> >
> > On Thu, Nov 11, 2021 at 04:25:02PM +0000, Mark Brown wrote:
> > > On Thu, Nov 11, 2021 at 07:06:27PM +0300, Serge Semin wrote:
> > > > On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote:
> > >
> > > > > Given that people seem to frequently customise these IPs when
> > > > > integrating them I wouldn't trust people not to have added some
> > > > > other control into that reserved bit doing some magic stuff
> > > > > that's useful in their system.
> > >
> > > > In that case the corresponding platform code would have needed to
> > > > have that peculiarity properly handled and not to use a generic
> > > > compatibles like "snps,dwc-ssi-1.01a" or "snps,dw-apb-ssi", which
> > > > are supposed to be utilized for the default IP-core configs only.
> > > > For the sake of the code simplification I'd stick to setting that
> > > > flag for each generic DWC SSI-compatible device. That will be also
> > > > helpful for DWC SSIs which for some reason have the slave-mode
> > enabled by default.
> > >
> >
> > > That's easier right up until the point where it explodes - I'd
> > > prefer to be more conservative here.  Fixing things up after the
> > > fact gets painful when people end up only finding the bug in
> > > released kernels, especially if it's distro end users or similar rather than
> developers.
> >
> > Since IP-core and components versions is now supported that will easy
> > to implement. Thanks for merging the corresponding series in BTW.
> >
> > >
> > > > Alternatively the driver could read the IP-core version from the
> > > > DW_SPI_VERSION register, parse it (since it's in ASCII) and then
> > > > use it in the conditional Master mode activation here. But that
> > > > could have been a better solution in case if the older IP-cores
> > > > would have used that bit for something special, while Nandhini
> > > > claims it was
> > reserved.
> > > > So in this case I would stick with a simpler approach until we get
> > > > to face any problem in this matter, especially seeing we already
> > > > pocking the reserved bits of the CTRL0 register in this driver in
> > > > the
> > > > spi_hw_init() method when it comes to the DFS field width detection.
> > >
> > > If the device has a version register checking that seems ideal - the
> > > infrastructure will most likely be useful in future anyway.  A bit
> > > of a shame that it's an ASCII string though.
> >
> > That's what the patchset has been implemented for in the first place
> > https://lore.kernel.org/linux-spi/20211115181917.7521-1-
> > Sergey.Semin@baikalelectronics.ru/
> >
> > Nandhini, Mark has just merged in the series that adds the IP-core
> > versions infrastructure support to the DW SSI driver.  So now you can
> > easily convert this patch to be using that new interface like this:
> > -               if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > -                       cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> > +               /* CTRLR0[31] MST */
> > +		if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> > +       	        cr0 |= DWC_HSSI_CTRLR0_MST;
> >
> > Please don't forget to convert the DWC_SSI_CTRLR0_KEEMBAY_MST name
> to
> > something like DWC_HSSI_CTRLR0_MST and place it at the top of the DWC
> > HSSI CTRLR0 register macros list.
> >
> > -Sergey
> Sure, I will test this patch set on our hardware and then use the IP-core
> version infrastructure support and make the changes as mentioned for MST
> bit.
> 
> - Nandhini
I have tested the patches on our hardware and it is working fine. When would these patches be available in mainline? So that I can start applying the changes on top of these patches and share for review.
- Nandhini 

Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller

[Serge Semin]

On Wed, Dec 08, 2021 at 11:03:03AM +0000, Srikandan, Nandhini wrote:
> 
> 
> > -----Original Message-----
> > From: Srikandan, Nandhini
> > Sent: Wednesday, November 17, 2021 5:29 PM
> > To: Serge Semin <fancer.lancer@gmail.com>; Mark Brown
> > <broonie@kernel.org>
> > Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; robh+dt@kernel.org;
> > linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org; mgross@linux.intel.com; Pan, Kris
> > <kris.pan@intel.com>; Demakkanavar, Kenchappa
> > <kenchappa.demakkanavar@intel.com>; Zhou, Furong
> > <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa
> > <mallikarjunappa.sangannavar@intel.com>; Vaidya, Mahesh R
> > <mahesh.r.vaidya@intel.com>; A, Rashmi <Rashmi.A@intel.com>
> > Subject: RE: [PATCH v3 3/5] spi: dw: Add support for master mode selection
> > for DWC SSI controller
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Serge Semin <fancer.lancer@gmail.com>
> > > Sent: Wednesday, November 17, 2021 12:46 AM
> > > To: Mark Brown <broonie@kernel.org>; Srikandan, Nandhini
> > > <nandhini.srikandan@intel.com>
> > > Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>;
> > > robh+dt@kernel.org; linux-spi@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; devicetree@vger.kernel.org;
> > > mgross@linux.intel.com; Pan, Kris <kris.pan@intel.com>; Demakkanavar,
> > > Kenchappa <kenchappa.demakkanavar@intel.com>; Zhou, Furong
> > > <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa
> > > <mallikarjunappa.sangannavar@intel.com>; Vaidya, Mahesh R
> > > <mahesh.r.vaidya@intel.com>; A, Rashmi <rashmi.a@intel.com>
> > > Subject: Re: [PATCH v3 3/5] spi: dw: Add support for master mode
> > > selection for DWC SSI controller
> > >
> > > On Thu, Nov 11, 2021 at 04:25:02PM +0000, Mark Brown wrote:
> > > > On Thu, Nov 11, 2021 at 07:06:27PM +0300, Serge Semin wrote:
> > > > > On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote:
> > > >
> > > > > > Given that people seem to frequently customise these IPs when
> > > > > > integrating them I wouldn't trust people not to have added some
> > > > > > other control into that reserved bit doing some magic stuff
> > > > > > that's useful in their system.
> > > >
> > > > > In that case the corresponding platform code would have needed to
> > > > > have that peculiarity properly handled and not to use a generic
> > > > > compatibles like "snps,dwc-ssi-1.01a" or "snps,dw-apb-ssi", which
> > > > > are supposed to be utilized for the default IP-core configs only.
> > > > > For the sake of the code simplification I'd stick to setting that
> > > > > flag for each generic DWC SSI-compatible device. That will be also
> > > > > helpful for DWC SSIs which for some reason have the slave-mode
> > > enabled by default.
> > > >
> > >
> > > > That's easier right up until the point where it explodes - I'd
> > > > prefer to be more conservative here.  Fixing things up after the
> > > > fact gets painful when people end up only finding the bug in
> > > > released kernels, especially if it's distro end users or similar rather than
> > developers.
> > >
> > > Since IP-core and components versions is now supported that will easy
> > > to implement. Thanks for merging the corresponding series in BTW.
> > >
> > > >
> > > > > Alternatively the driver could read the IP-core version from the
> > > > > DW_SPI_VERSION register, parse it (since it's in ASCII) and then
> > > > > use it in the conditional Master mode activation here. But that
> > > > > could have been a better solution in case if the older IP-cores
> > > > > would have used that bit for something special, while Nandhini
> > > > > claims it was
> > > reserved.
> > > > > So in this case I would stick with a simpler approach until we get
> > > > > to face any problem in this matter, especially seeing we already
> > > > > pocking the reserved bits of the CTRL0 register in this driver in
> > > > > the
> > > > > spi_hw_init() method when it comes to the DFS field width detection.
> > > >
> > > > If the device has a version register checking that seems ideal - the
> > > > infrastructure will most likely be useful in future anyway.  A bit
> > > > of a shame that it's an ASCII string though.
> > >
> > > That's what the patchset has been implemented for in the first place
> > > https://lore.kernel.org/linux-spi/20211115181917.7521-1-
> > > Sergey.Semin@baikalelectronics.ru/
> > >
> > > Nandhini, Mark has just merged in the series that adds the IP-core
> > > versions infrastructure support to the DW SSI driver.  So now you can
> > > easily convert this patch to be using that new interface like this:
> > > -               if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > > -                       cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> > > +               /* CTRLR0[31] MST */
> > > +		if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> > > +       	        cr0 |= DWC_HSSI_CTRLR0_MST;
> > >
> > > Please don't forget to convert the DWC_SSI_CTRLR0_KEEMBAY_MST name
> > to
> > > something like DWC_HSSI_CTRLR0_MST and place it at the top of the DWC
> > > HSSI CTRLR0 register macros list.
> > >
> > > -Sergey
> > Sure, I will test this patch set on our hardware and then use the IP-core
> > version infrastructure support and make the changes as mentioned for MST
> > bit.
> > 
> > - Nandhini

> I have tested the patches on our hardware and it is working fine. When would these patches be available in mainline? So that I can start applying the changes on top of these patches and share for review.
> - Nandhini 

These changes have been merged into the Mark' repo in the spi-next
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/log/?h=for-next
The series together with the rest of the SPI-related changed will get
into the mainline during the merge window.
For the time being you can rebase your patches on top of the Mark' repo
and if it works well for you just post it out for review. If it looks
good you'll even be able to have your series merged into the kernel in
the next merge-window.

-Sergey

> 

[PATCH v3 4/5] dt-bindings: spi: Add bindings for Intel Thunder Bay SoC

[nandhini.srikandan]

From: Nandhini Srikandan <nandhini.srikandan@intel.com>

Add documentation for SPI controller in Intel Thunder Bay SoC.

Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index 866416d01e94..5b126a034981 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -61,6 +61,8 @@ properties:
           - const: snps,dw-apb-ssi
       - description: Intel Keem Bay SPI Controller
         const: intel,keembay-ssi
+      - description: Intel Thunder Bay SPI Controller
+        const: intel,thunderbay-ssi
       - description: Baikal-T1 SPI Controller
         const: baikal,bt1-ssi
       - description: Baikal-T1 System Boot SPI Controller
-- 
2.17.1

Re: [PATCH v3 4/5] dt-bindings: spi: Add bindings for Intel Thunder Bay SoC

[Rob Herring]

On Thu, 11 Nov 2021 14:52:00 +0800, nandhini.srikandan@intel.com wrote:
> From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> 
> Add documentation for SPI controller in Intel Thunder Bay SoC.
> 
> Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v3 5/5] spi: dw: Add support for Intel Thunder Bay SPI controller

[nandhini.srikandan]

From: Nandhini Srikandan <nandhini.srikandan@intel.com>

Add support for Intel Thunder Bay SPI controller, which uses DesignWare
DWC_ssi core and also add common init function for both Keem Bay and
Thunder Bay.

Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
---
 drivers/spi/spi-dw-mmio.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 3379720cfcb8..c357680f4aa3 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -214,10 +214,10 @@ static int dw_spi_dwc_ssi_init(struct platform_device *pdev,
 	return 0;
 }
 
-static int dw_spi_keembay_init(struct platform_device *pdev,
-			       struct dw_spi_mmio *dwsmmio)
+static int dw_spi_intel_init(struct platform_device *pdev,
+			     struct dw_spi_mmio *dwsmmio)
 {
-	dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_SSI;
+	dwsmmio->dws.caps = DW_SPI_CAP_DWC_SSI;
 
 	return 0;
 }
@@ -348,7 +348,8 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "amazon,alpine-dw-apb-ssi", .data = dw_spi_alpine_init},
 	{ .compatible = "renesas,rzn1-spi", .data = dw_spi_dw_apb_init},
 	{ .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
-	{ .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
+	{ .compatible = "intel,keembay-ssi", .data = dw_spi_intel_init},
+	{ .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},
 	{ /* end of table */}
-- 
2.17.1