[PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller

[PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller

[nandhini.srikandan]

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

Hi,

This patch enables support for DW SPI on Intel Thunder Bay (patch 1,2).
This patch set also enables master mode for latest Designware SPI versions (patch 3).

Patch 1: DW SPI DT bindings for Intel Thunder Bay SoC.
Patch 2: Adds support for Designware SPI on Intel Thunderbay SoC.
Patch 3: Adds master mode support for Designware SPI controller.

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

Summary:
Changes from v3:
1) Dropped SSTE support in this patch.
2) Rebased to the latest code.

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: 
Changes from v3/v2/v1:
1) No change in this patch

Patch 2:
Changes from v3:
1) No changes.

Changes from v2:
1) Init function is made common for Keem Bay and Thunder Bay.
 
Patch 3:
Changes from v3:
1) Corrected dw_spi_ip_is macro with the missing underscore.
2) Setting CTRLR0 BIT31 without any condition check as in older version of 
   DW SPI controller this bit is reserved.

Changes from v2/v1:
1)Newly introduced in v3 to make master mode selection as seperate patch

Thanks & Regards,
Nandhini


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

 .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml          | 2 ++
 drivers/spi/spi-dw-core.c                                 | 4 ++--
 drivers/spi/spi-dw-mmio.c                                 | 8 ++++----
 drivers/spi/spi-dw.h                                      | 7 +++----
 4 files changed, 11 insertions(+), 10 deletions(-)

-- 
2.17.1

[PATCH v4 1/3] 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 d7e08b03e204..5ecd996ebf33 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

[PATCH v4 2/3] 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 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 5101c4c6017b..26c40ea6dd12 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -214,11 +214,10 @@ static int dw_spi_hssi_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.ip = DW_HSSI_ID;
-	dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST;
 
 	return 0;
 }
@@ -349,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_pssi_init},
 	{ .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_hssi_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

[PATCH v4 3/3] 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      | 7 +++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index ecea471ff42c..68bfdf2c4dc7 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -307,8 +307,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
 		if (spi->mode & SPI_LOOP)
 			cr0 |= DW_HSSI_CTRLR0_SRL;
 
-		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
-			cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
+		/* CTRLR0[31] MST */
+		cr0 |= DW_HSSI_CTRLR0_MST;
 	}
 
 	return cr0;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index d5ee5130601e..2583b7314c41 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -23,7 +23,7 @@
 	((_dws)->ip == DW_ ## _ip ## _ID)
 
 #define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
-	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
+	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ ## _ver)
 
 #define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws, _ip, _ver, ==)
 
@@ -31,8 +31,7 @@
 
 /* DW SPI controller capabilities */
 #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
-#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
-#define DW_SPI_CAP_DFS32		BIT(2)
+#define DW_SPI_CAP_DFS32		BIT(1)
 
 /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
 #define DW_SPI_CTRLR0			0x00
@@ -100,7 +99,7 @@
  * 0: SSI is slave
  * 1: SSI is master
  */
-#define DW_HSSI_CTRLR0_KEEMBAY_MST		BIT(31)
+#define DW_HSSI_CTRLR0_MST			BIT(31)
 
 /* Bit fields in CTRLR1 */
 #define DW_SPI_NDF_MASK				GENMASK(15, 0)
-- 
2.17.1

Re: [PATCH v4 1/3] dt-bindings: spi: Add bindings for Intel Thunder Bay SoC

[Rob Herring]

On Tue, 08 Mar 2022 18:33:29 +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>

RE: [PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller

[Srikandan, Nandhini]

Hi all,

Kindly help to review the patch set "Add support for Intel Thunder Bay SPI controller".

Regards,
Nandhini 

> -----Original Message-----
> From: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> Sent: Tuesday, March 8, 2022 4:03 PM
> To: fancer.lancer@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: 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>; Srikandan, Nandhini
> <nandhini.srikandan@intel.com>; A, Rashmi <rashmi.a@intel.com>
> Subject: [PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller
> 
> From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> 
> Hi,
> 
> This patch enables support for DW SPI on Intel Thunder Bay (patch 1,2).
> This patch set also enables master mode for latest Designware SPI versions
> (patch 3).
> 
> Patch 1: DW SPI DT bindings for Intel Thunder Bay SoC.
> Patch 2: Adds support for Designware SPI on Intel Thunderbay SoC.
> Patch 3: Adds master mode support for Designware SPI controller.
> 
> The driver is tested on Keem Bay and Thunder Bay evaluation board
> 
> Summary:
> Changes from v3:
> 1) Dropped SSTE support in this patch.
> 2) Rebased to the latest code.
> 
> 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:
> Changes from v3/v2/v1:
> 1) No change in this patch
> 
> Patch 2:
> Changes from v3:
> 1) No changes.
> 
> Changes from v2:
> 1) Init function is made common for Keem Bay and Thunder Bay.
> 
> Patch 3:
> Changes from v3:
> 1) Corrected dw_spi_ip_is macro with the missing underscore.
> 2) Setting CTRLR0 BIT31 without any condition check as in older version of
>    DW SPI controller this bit is reserved.
> 
> Changes from v2/v1:
> 1)Newly introduced in v3 to make master mode selection as seperate patch
> 
> Thanks & Regards,
> Nandhini
> 
> 
> Nandhini Srikandan (3):
>   dt-bindings: spi: Add bindings for Intel Thunder Bay SoC
>   spi: dw: Add support for Intel Thunder Bay SPI controller
>   spi: dw: Add support for master mode selection for DWC SSI controller
> 
>  .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml          | 2 ++
>  drivers/spi/spi-dw-core.c                                 | 4 ++--
>  drivers/spi/spi-dw-mmio.c                                 | 8 ++++----
>  drivers/spi/spi-dw.h                                      | 7 +++----
>  4 files changed, 11 insertions(+), 10 deletions(-)
> 
> --
> 2.17.1

Re: [PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller

[Serge Semin]

Hello Nandhini

On Mon, Apr 04, 2022 at 11:51:14AM +0000, Srikandan, Nandhini wrote:
> Hi all,
> 
> Kindly help to review the patch set "Add support for Intel Thunder Bay SPI controller".

Thanks for reminding about your series. It has kind of emailed away
from my sight due to other incoming messages. I'll give you my
comments within two days.

Regards
-Sergey

> 
> Regards,
> Nandhini 
> 
> > -----Original Message-----
> > From: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> > Sent: Tuesday, March 8, 2022 4:03 PM
> > To: fancer.lancer@gmail.com; broonie@kernel.org; robh+dt@kernel.org;
> > linux-spi@vger.kernel.org; linux-kernel@vger.kernel.org
> > Cc: 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>; Srikandan, Nandhini
> > <nandhini.srikandan@intel.com>; A, Rashmi <rashmi.a@intel.com>
> > Subject: [PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller
> > 
> > From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> > 
> > Hi,
> > 
> > This patch enables support for DW SPI on Intel Thunder Bay (patch 1,2).
> > This patch set also enables master mode for latest Designware SPI versions
> > (patch 3).
> > 
> > Patch 1: DW SPI DT bindings for Intel Thunder Bay SoC.
> > Patch 2: Adds support for Designware SPI on Intel Thunderbay SoC.
> > Patch 3: Adds master mode support for Designware SPI controller.
> > 
> > The driver is tested on Keem Bay and Thunder Bay evaluation board
> > 
> > Summary:
> > Changes from v3:
> > 1) Dropped SSTE support in this patch.
> > 2) Rebased to the latest code.
> > 
> > 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:
> > Changes from v3/v2/v1:
> > 1) No change in this patch
> > 
> > Patch 2:
> > Changes from v3:
> > 1) No changes.
> > 
> > Changes from v2:
> > 1) Init function is made common for Keem Bay and Thunder Bay.
> > 
> > Patch 3:
> > Changes from v3:
> > 1) Corrected dw_spi_ip_is macro with the missing underscore.
> > 2) Setting CTRLR0 BIT31 without any condition check as in older version of
> >    DW SPI controller this bit is reserved.
> > 
> > Changes from v2/v1:
> > 1)Newly introduced in v3 to make master mode selection as seperate patch
> > 
> > Thanks & Regards,
> > Nandhini
> > 
> > 
> > Nandhini Srikandan (3):
> >   dt-bindings: spi: Add bindings for Intel Thunder Bay SoC
> >   spi: dw: Add support for Intel Thunder Bay SPI controller
> >   spi: dw: Add support for master mode selection for DWC SSI controller
> > 
> >  .../devicetree/bindings/spi/snps,dw-apb-ssi.yaml          | 2 ++
> >  drivers/spi/spi-dw-core.c                                 | 4 ++--
> >  drivers/spi/spi-dw-mmio.c                                 | 8 ++++----
> >  drivers/spi/spi-dw.h                                      | 7 +++----
> >  4 files changed, 11 insertions(+), 10 deletions(-)
> > 
> > --
> > 2.17.1
> 

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

[Serge Semin]

Hello Nandhini

AFAICS this patch should go before
[PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI
controller Thus you'd perform the DWC AHB SSI Master mode conversion
first, then introduce the new controller support. Otherwise without
this patch applied the DW SPI driver is most likely left broken for
the Intel SPI controllers since you drop the DW_SPI_CAP_KEEMBAY_MST
macro usage in [PATCH 2/3] while the new DW AHB SSI Master
functionality is introduced in the next patch [PATCH 3/3]. So please
convert the series to the harmless configuration on each git image
state.

On Tue, Mar 08, 2022 at 06:33:31PM +0800, nandhini.srikandan@intel.com wrote:
> 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      | 7 +++----
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index ecea471ff42c..68bfdf2c4dc7 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -307,8 +307,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
>  		if (spi->mode & SPI_LOOP)
>  			cr0 |= DW_HSSI_CTRLR0_SRL;
>  

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

Could you please conditionally set that flag here? That's what we
agreed to do in v3:
https://lore.kernel.org/linux-spi/20211116191542.vc42cxvflzn66ien@mobilestation/
like this:
+	/* CTRLR0[31] MST */
+	if (dw_spi_ver_is_ge(dws, HSSI, 102A))
+		cr0 |= DWC_HSSI_CTRLR0_MST;

>  	}
>  
>  	return cr0;
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index d5ee5130601e..2583b7314c41 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -23,7 +23,7 @@
>  	((_dws)->ip == DW_ ## _ip ## _ID)
>  

>  #define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
> -	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
> +	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ ## _ver)

Nice catch. My mistake. Could you please move this change into a
dedicated patch with the next fixes tag?
Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions interface")

>  
>  #define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws, _ip, _ver, ==)
>  
> @@ -31,8 +31,7 @@
>  
>  /* DW SPI controller capabilities */
>  #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
> -#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
> -#define DW_SPI_CAP_DFS32		BIT(2)
> +#define DW_SPI_CAP_DFS32		BIT(1)
>  
>  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
>  #define DW_SPI_CTRLR0			0x00
> @@ -100,7 +99,7 @@

>   * 0: SSI is slave
>   * 1: SSI is master
>   */
> -#define DW_HSSI_CTRLR0_KEEMBAY_MST		BIT(31)
> +#define DW_HSSI_CTRLR0_MST			BIT(31)

Could you please drop the redundant comment above and join the
macro with the DW_HSSI_* macros group?

-Sergey

>  
>  /* Bit fields in CTRLR1 */
>  #define DW_SPI_NDF_MASK				GENMASK(15, 0)
> -- 
> 2.17.1
> 

Re: [PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI controller

[Serge Semin]

On Tue, Mar 08, 2022 at 06:33:30PM +0800, nandhini.srikandan@intel.com wrote:
> 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.

Please see my comments to the patch
[PATCH v4 3/3] spi: dw: Add support for master mode selection for DWC SSI controller
of this series. I think this patch should go after that one so to
introduce a safer conversion of the DWC AHB SSI functionality.

-Sergey

> 
> Signed-off-by: Nandhini Srikandan <nandhini.srikandan@intel.com>
> ---
>  drivers/spi/spi-dw-mmio.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 5101c4c6017b..26c40ea6dd12 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -214,11 +214,10 @@ static int dw_spi_hssi_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.ip = DW_HSSI_ID;
> -	dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST;
>  
>  	return 0;
>  }
> @@ -349,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_pssi_init},
>  	{ .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_hssi_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
> 

RE: [PATCH v4 3/3] 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, April 13, 2022 6:33 PM
> To: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; broonie@kernel.org;
> 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 v4 3/3] spi: dw: Add support for master mode selection
> for DWC SSI controller
> 
> Hello Nandhini
> 
> AFAICS this patch should go before
> [PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI controller
> Thus you'd perform the DWC AHB SSI Master mode conversion first, then
> introduce the new controller support. Otherwise without this patch applied
> the DW SPI driver is most likely left broken for the Intel SPI controllers since
> you drop the DW_SPI_CAP_KEEMBAY_MST macro usage in [PATCH 2/3] while
> the new DW AHB SSI Master functionality is introduced in the next patch
> [PATCH 3/3]. So please convert the series to the harmless configuration on
> each git image state.
> 
Sure, I will reorder patch 2/3 and 3/3 so that the master mode conversion happens first followed by new controller support.

> On Tue, Mar 08, 2022 at 06:33:31PM +0800, nandhini.srikandan@intel.com
> wrote:
> > 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      | 7 +++----
> >  2 files changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index ecea471ff42c..68bfdf2c4dc7 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -307,8 +307,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws,
> struct spi_device *spi)
> >  		if (spi->mode & SPI_LOOP)
> >  			cr0 |= DW_HSSI_CTRLR0_SRL;
> >
> 
> > -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > -			cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> > +		/* CTRLR0[31] MST */
> > +		cr0 |= DW_HSSI_CTRLR0_MST;
> 
> Could you please conditionally set that flag here? That's what we agreed to
> do in v3:
> https://lore.kernel.org/linux-
> spi/20211116191542.vc42cxvflzn66ien@mobilestation/
> like this:
> +	/* CTRLR0[31] MST */
> +	if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> +		cr0 |= DWC_HSSI_CTRLR0_MST;
> 
In case of Keem Bay, though the version of SPI controller is shown as 1.01a from the HW register, it still needs the MST BIT31 to be set in order for controller to work in master mode.
Also since the older versions of the controller which do not need the BIT31 to be set, the bit was reserved. Hence there is no impact by setting this BIT31 for older versions. 
So, the condition check was removed.

> >  	}
> >
> >  	return cr0;
> > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index
> > d5ee5130601e..2583b7314c41 100644
> > --- a/drivers/spi/spi-dw.h
> > +++ b/drivers/spi/spi-dw.h
> > @@ -23,7 +23,7 @@
> >  	((_dws)->ip == DW_ ## _ip ## _ID)
> >
> 
> >  #define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
> > -	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
> > +	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ ##
> _ver)
> 
> Nice catch. My mistake. Could you please move this change into a dedicated
> patch with the next fixes tag?
> Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions
> interface")
> 
Sure, I will convert this to a dedicated patch. Just for confirmation, the patch should be a separate patch with this title "Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions interface")" 
and not part of the current patch set series.
> >
> >  #define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws, _ip,
> > _ver, ==)
> >
> > @@ -31,8 +31,7 @@
> >
> >  /* DW SPI controller capabilities */
> >  #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
> > -#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
> > -#define DW_SPI_CAP_DFS32		BIT(2)
> > +#define DW_SPI_CAP_DFS32		BIT(1)
> >
> >  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
> >  #define DW_SPI_CTRLR0			0x00
> > @@ -100,7 +99,7 @@
> 
> >   * 0: SSI is slave
> >   * 1: SSI is master
> >   */
> > -#define DW_HSSI_CTRLR0_KEEMBAY_MST		BIT(31)
> > +#define DW_HSSI_CTRLR0_MST			BIT(31)
> 
> Could you please drop the redundant comment above and join the macro
> with the DW_HSSI_* macros group?
> 
Sure, I will remove the comment and group the macros. 
> -Sergey
> 
> >
> >  /* Bit fields in CTRLR1 */
> >  #define DW_SPI_NDF_MASK				GENMASK(15, 0)
> > --
> > 2.17.1
> >

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

[Serge Semin]

On Wed, Apr 27, 2022 at 09:51:47AM +0000, Srikandan, Nandhini wrote:
> 
> 
> > -----Original Message-----
> > From: Serge Semin <fancer.lancer@gmail.com>
> > Sent: Wednesday, April 13, 2022 6:33 PM
> > To: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> > Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; broonie@kernel.org;
> > 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 v4 3/3] spi: dw: Add support for master mode selection
> > for DWC SSI controller
> > 
> > Hello Nandhini
> > 
> > AFAICS this patch should go before
> > [PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI controller
> > Thus you'd perform the DWC AHB SSI Master mode conversion first, then
> > introduce the new controller support. Otherwise without this patch applied
> > the DW SPI driver is most likely left broken for the Intel SPI controllers since
> > you drop the DW_SPI_CAP_KEEMBAY_MST macro usage in [PATCH 2/3] while
> > the new DW AHB SSI Master functionality is introduced in the next patch
> > [PATCH 3/3]. So please convert the series to the harmless configuration on
> > each git image state.
> > 
> Sure, I will reorder patch 2/3 and 3/3 so that the master mode conversion happens first followed by new controller support.
> 
> > On Tue, Mar 08, 2022 at 06:33:31PM +0800, nandhini.srikandan@intel.com
> > wrote:
> > > 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      | 7 +++----
> > >  2 files changed, 5 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > > index ecea471ff42c..68bfdf2c4dc7 100644
> > > --- a/drivers/spi/spi-dw-core.c
> > > +++ b/drivers/spi/spi-dw-core.c
> > > @@ -307,8 +307,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws,
> > struct spi_device *spi)
> > >  		if (spi->mode & SPI_LOOP)
> > >  			cr0 |= DW_HSSI_CTRLR0_SRL;
> > >
> > 
> > > -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > > -			cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> > > +		/* CTRLR0[31] MST */
> > > +		cr0 |= DW_HSSI_CTRLR0_MST;
> > 
> > Could you please conditionally set that flag here? That's what we agreed to
> > do in v3:
> > https://lore.kernel.org/linux-
> > spi/20211116191542.vc42cxvflzn66ien@mobilestation/
> > like this:
> > +	/* CTRLR0[31] MST */
> > +	if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> > +		cr0 |= DWC_HSSI_CTRLR0_MST;
> > 

> In case of Keem Bay, though the version of SPI controller is shown as 1.01a from the HW register, it still needs the MST BIT31 to be set in order for controller to work in master mode.
> Also since the older versions of the controller which do not need the BIT31 to be set, the bit was reserved. Hence there is no impact by setting this BIT31 for older versions. 
> So, the condition check was removed.

I am completely confused. Earlier you said that both Keem Bay and
Thunder bay had v1.02a DW AHB SSI IP-core:
https://patchwork.kernel.org/project/spi-devel-general/patch/20210824085856.12714-3-nandhini.srikandan@intel.com/
Now you say they are based on the different versions of the core.
Please clarify.

-Sergey

> 
> > >  	}
> > >
> > >  	return cr0;
> > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index
> > > d5ee5130601e..2583b7314c41 100644
> > > --- a/drivers/spi/spi-dw.h
> > > +++ b/drivers/spi/spi-dw.h
> > > @@ -23,7 +23,7 @@
> > >  	((_dws)->ip == DW_ ## _ip ## _ID)
> > >
> > 
> > >  #define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
> > > -	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
> > > +	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ ##
> > _ver)
> > 
> > Nice catch. My mistake. Could you please move this change into a dedicated
> > patch with the next fixes tag?
> > Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions
> > interface")
> > 

> Sure, I will convert this to a dedicated patch. Just for confirmation, the patch should be a separate patch with this title "Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions interface")" 
> and not part of the current patch set series.

You can add that patch to this series (better to the head of it). The
title can be something like: "spi: dw: Fix IP-core versions macro".
The tag needs to be added in the commit log above the Signed-off-by
tag.

-Sergey

> > >
> > >  #define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws, _ip,
> > > _ver, ==)
> > >
> > > @@ -31,8 +31,7 @@
> > >
> > >  /* DW SPI controller capabilities */
> > >  #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
> > > -#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
> > > -#define DW_SPI_CAP_DFS32		BIT(2)
> > > +#define DW_SPI_CAP_DFS32		BIT(1)
> > >
> > >  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
> > >  #define DW_SPI_CTRLR0			0x00
> > > @@ -100,7 +99,7 @@
> > 
> > >   * 0: SSI is slave
> > >   * 1: SSI is master
> > >   */
> > > -#define DW_HSSI_CTRLR0_KEEMBAY_MST		BIT(31)
> > > +#define DW_HSSI_CTRLR0_MST			BIT(31)
> > 
> > Could you please drop the redundant comment above and join the macro
> > with the DW_HSSI_* macros group?
> > 
> Sure, I will remove the comment and group the macros. 
> > -Sergey
> > 
> > >
> > >  /* Bit fields in CTRLR1 */
> > >  #define DW_SPI_NDF_MASK				GENMASK(15, 0)
> > > --
> > > 2.17.1
> > >

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

[Srikandan, Nandhini]

> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Thursday, April 28, 2022 8:06 PM
> To: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; broonie@kernel.org;
> 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 v4 3/3] spi: dw: Add support for master mode selection
> for DWC SSI controller
> 
> On Wed, Apr 27, 2022 at 09:51:47AM +0000, Srikandan, Nandhini wrote:
> >
> >
> > > -----Original Message-----
> > > From: Serge Semin <fancer.lancer@gmail.com>
> > > Sent: Wednesday, April 13, 2022 6:33 PM
> > > To: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> > > Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>;
> > > broonie@kernel.org;
> > > 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 v4 3/3] spi: dw: Add support for master mode
> > > selection for DWC SSI controller
> > >
> > > Hello Nandhini
> > >
> > > AFAICS this patch should go before
> > > [PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI
> > > controller Thus you'd perform the DWC AHB SSI Master mode conversion
> > > first, then introduce the new controller support. Otherwise without
> > > this patch applied the DW SPI driver is most likely left broken for
> > > the Intel SPI controllers since you drop the DW_SPI_CAP_KEEMBAY_MST
> > > macro usage in [PATCH 2/3] while the new DW AHB SSI Master
> > > functionality is introduced in the next patch [PATCH 3/3]. So please
> > > convert the series to the harmless configuration on each git image state.
> > >
> > Sure, I will reorder patch 2/3 and 3/3 so that the master mode conversion
> happens first followed by new controller support.
> >
> > > On Tue, Mar 08, 2022 at 06:33:31PM +0800,
> > > nandhini.srikandan@intel.com
> > > wrote:
> > > > 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      | 7 +++----
> > > >  2 files changed, 5 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > > > index ecea471ff42c..68bfdf2c4dc7 100644
> > > > --- a/drivers/spi/spi-dw-core.c
> > > > +++ b/drivers/spi/spi-dw-core.c
> > > > @@ -307,8 +307,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi
> > > > *dws,
> > > struct spi_device *spi)
> > > >  		if (spi->mode & SPI_LOOP)
> > > >  			cr0 |= DW_HSSI_CTRLR0_SRL;
> > > >
> > >
> > > > -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > > > -			cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> > > > +		/* CTRLR0[31] MST */
> > > > +		cr0 |= DW_HSSI_CTRLR0_MST;
> > >
> > > Could you please conditionally set that flag here? That's what we
> > > agreed to do in v3:
> > > https://lore.kernel.org/linux-
> > > spi/20211116191542.vc42cxvflzn66ien@mobilestation/
> > > like this:
> > > +	/* CTRLR0[31] MST */
> > > +	if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> > > +		cr0 |= DWC_HSSI_CTRLR0_MST;
> > >
> 
> > In case of Keem Bay, though the version of SPI controller is shown as 1.01a
> from the HW register, it still needs the MST BIT31 to be set in order for
> controller to work in master mode.
> > Also since the older versions of the controller which do not need the BIT31
> to be set, the bit was reserved. Hence there is no impact by setting this BIT31
> for older versions.
> > So, the condition check was removed.
> 
> I am completely confused. Earlier you said that both Keem Bay and Thunder
> bay had v1.02a DW AHB SSI IP-core:
> https://patchwork.kernel.org/project/spi-devel-
> general/patch/20210824085856.12714-3-nandhini.srikandan@intel.com/
> Now you say they are based on the different versions of the core.
> Please clarify.
> 
> -Sergey
> 
HW IP team informed us that the version used by Keem Bay SPI controller is 1.02a. But in the IP version register it is updated as 1.01a. We are checking with the HW IP team regarding this mismatch and this will be taken care of internally. Apologies for the confusions. I will add the conditional check as per your suggestion.
> >
> > > >  	}
> > > >
> > > >  	return cr0;
> > > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index
> > > > d5ee5130601e..2583b7314c41 100644
> > > > --- a/drivers/spi/spi-dw.h
> > > > +++ b/drivers/spi/spi-dw.h
> > > > @@ -23,7 +23,7 @@
> > > >  	((_dws)->ip == DW_ ## _ip ## _ID)
> > > >
> > >
> > > >  #define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
> > > > -	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
> > > > +	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ ##
> > > _ver)
> > >
> > > Nice catch. My mistake. Could you please move this change into a
> > > dedicated patch with the next fixes tag?
> > > Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions
> > > interface")
> > >
> 
> > Sure, I will convert this to a dedicated patch. Just for confirmation, the
> patch should be a separate patch with this title "Fixes: 2cc8d9227bbb ("spi:
> dw: Introduce Synopsys IP-core versions interface")"
> > and not part of the current patch set series.
> 
> You can add that patch to this series (better to the head of it). The title can
> be something like: "spi: dw: Fix IP-core versions macro".
> The tag needs to be added in the commit log above the Signed-off-by tag.
> 
Sure, I will add the patch to the head of this series and add the tag.
> -Sergey
> 
> > > >
> > > >  #define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws,
> > > > _ip, _ver, ==)
> > > >
> > > > @@ -31,8 +31,7 @@
> > > >
> > > >  /* DW SPI controller capabilities */
> > > >  #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
> > > > -#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
> > > > -#define DW_SPI_CAP_DFS32		BIT(2)
> > > > +#define DW_SPI_CAP_DFS32		BIT(1)
> > > >
> > > >  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-
> cores) */
> > > >  #define DW_SPI_CTRLR0			0x00
> > > > @@ -100,7 +99,7 @@
> > >
> > > >   * 0: SSI is slave
> > > >   * 1: SSI is master
> > > >   */
> > > > -#define DW_HSSI_CTRLR0_KEEMBAY_MST		BIT(31)
> > > > +#define DW_HSSI_CTRLR0_MST			BIT(31)
> > >
> > > Could you please drop the redundant comment above and join the macro
> > > with the DW_HSSI_* macros group?
> > >
> > Sure, I will remove the comment and group the macros.
> > > -Sergey
> > >
> > > >
> > > >  /* Bit fields in CTRLR1 */
> > > >  #define DW_SPI_NDF_MASK				GENMASK(15,
> 0)
> > > > --
> > > > 2.17.1
> > > >

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

[Serge Semin]

On Mon, May 02, 2022 at 08:51:06AM +0000, Srikandan, Nandhini wrote:
> 
> 
> > -----Original Message-----
> > From: Serge Semin <fancer.lancer@gmail.com>
> > Sent: Thursday, April 28, 2022 8:06 PM
> > To: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> > Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; broonie@kernel.org;
> > 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 v4 3/3] spi: dw: Add support for master mode selection
> > for DWC SSI controller
> > 
> > On Wed, Apr 27, 2022 at 09:51:47AM +0000, Srikandan, Nandhini wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Serge Semin <fancer.lancer@gmail.com>
> > > > Sent: Wednesday, April 13, 2022 6:33 PM
> > > > To: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> > > > Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>;
> > > > broonie@kernel.org;
> > > > 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 v4 3/3] spi: dw: Add support for master mode
> > > > selection for DWC SSI controller
> > > >
> > > > Hello Nandhini
> > > >
> > > > AFAICS this patch should go before
> > > > [PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI
> > > > controller Thus you'd perform the DWC AHB SSI Master mode conversion
> > > > first, then introduce the new controller support. Otherwise without
> > > > this patch applied the DW SPI driver is most likely left broken for
> > > > the Intel SPI controllers since you drop the DW_SPI_CAP_KEEMBAY_MST
> > > > macro usage in [PATCH 2/3] while the new DW AHB SSI Master
> > > > functionality is introduced in the next patch [PATCH 3/3]. So please
> > > > convert the series to the harmless configuration on each git image state.
> > > >
> > > Sure, I will reorder patch 2/3 and 3/3 so that the master mode conversion
> > happens first followed by new controller support.
> > >
> > > > On Tue, Mar 08, 2022 at 06:33:31PM +0800,
> > > > nandhini.srikandan@intel.com
> > > > wrote:
> > > > > 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      | 7 +++----
> > > > >  2 files changed, 5 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > > > > index ecea471ff42c..68bfdf2c4dc7 100644
> > > > > --- a/drivers/spi/spi-dw-core.c
> > > > > +++ b/drivers/spi/spi-dw-core.c
> > > > > @@ -307,8 +307,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi
> > > > > *dws,
> > > > struct spi_device *spi)
> > > > >  		if (spi->mode & SPI_LOOP)
> > > > >  			cr0 |= DW_HSSI_CTRLR0_SRL;
> > > > >
> > > >
> > > > > -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > > > > -			cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> > > > > +		/* CTRLR0[31] MST */
> > > > > +		cr0 |= DW_HSSI_CTRLR0_MST;
> > > >
> > > > Could you please conditionally set that flag here? That's what we
> > > > agreed to do in v3:
> > > > https://lore.kernel.org/linux-
> > > > spi/20211116191542.vc42cxvflzn66ien@mobilestation/
> > > > like this:
> > > > +	/* CTRLR0[31] MST */
> > > > +	if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> > > > +		cr0 |= DWC_HSSI_CTRLR0_MST;
> > > >
> > 
> > > In case of Keem Bay, though the version of SPI controller is shown as 1.01a
> > from the HW register, it still needs the MST BIT31 to be set in order for
> > controller to work in master mode.
> > > Also since the older versions of the controller which do not need the BIT31
> > to be set, the bit was reserved. Hence there is no impact by setting this BIT31
> > for older versions.
> > > So, the condition check was removed.
> > 
> > I am completely confused. Earlier you said that both Keem Bay and Thunder
> > bay had v1.02a DW AHB SSI IP-core:
> > https://patchwork.kernel.org/project/spi-devel-
> > general/patch/20210824085856.12714-3-nandhini.srikandan@intel.com/
> > Now you say they are based on the different versions of the core.
> > Please clarify.
> > 
> > -Sergey
> > 

> HW IP team informed us that the version used by Keem Bay SPI controller is 1.02a. But in the IP version register it is updated as 1.01a. We are checking with the HW IP team regarding this mismatch and this will be taken care of internally. Apologies for the confusions. I will add the conditional check as per your suggestion.

Ok. Waiting for v5 then.

-Sergey

> > >
> > > > >  	}
> > > > >
> > > > >  	return cr0;
> > > > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index
> > > > > d5ee5130601e..2583b7314c41 100644
> > > > > --- a/drivers/spi/spi-dw.h
> > > > > +++ b/drivers/spi/spi-dw.h
> > > > > @@ -23,7 +23,7 @@
> > > > >  	((_dws)->ip == DW_ ## _ip ## _ID)
> > > > >
> > > >
> > > > >  #define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
> > > > > -	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
> > > > > +	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ ##
> > > > _ver)
> > > >
> > > > Nice catch. My mistake. Could you please move this change into a
> > > > dedicated patch with the next fixes tag?
> > > > Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions
> > > > interface")
> > > >
> > 
> > > Sure, I will convert this to a dedicated patch. Just for confirmation, the
> > patch should be a separate patch with this title "Fixes: 2cc8d9227bbb ("spi:
> > dw: Introduce Synopsys IP-core versions interface")"
> > > and not part of the current patch set series.
> > 
> > You can add that patch to this series (better to the head of it). The title can
> > be something like: "spi: dw: Fix IP-core versions macro".
> > The tag needs to be added in the commit log above the Signed-off-by tag.
> > 
> Sure, I will add the patch to the head of this series and add the tag.
> > -Sergey
> > 
> > > > >
> > > > >  #define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws,
> > > > > _ip, _ver, ==)
> > > > >
> > > > > @@ -31,8 +31,7 @@
> > > > >
> > > > >  /* DW SPI controller capabilities */
> > > > >  #define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
> > > > > -#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
> > > > > -#define DW_SPI_CAP_DFS32		BIT(2)
> > > > > +#define DW_SPI_CAP_DFS32		BIT(1)
> > > > >
> > > > >  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-
> > cores) */
> > > > >  #define DW_SPI_CTRLR0			0x00
> > > > > @@ -100,7 +99,7 @@
> > > >
> > > > >   * 0: SSI is slave
> > > > >   * 1: SSI is master
> > > > >   */
> > > > > -#define DW_HSSI_CTRLR0_KEEMBAY_MST		BIT(31)
> > > > > +#define DW_HSSI_CTRLR0_MST			BIT(31)
> > > >
> > > > Could you please drop the redundant comment above and join the macro
> > > > with the DW_HSSI_* macros group?
> > > >
> > > Sure, I will remove the comment and group the macros.
> > > > -Sergey
> > > >
> > > > >
> > > > >  /* Bit fields in CTRLR1 */
> > > > >  #define DW_SPI_NDF_MASK				GENMASK(15,
> > 0)
> > > > > --
> > > > > 2.17.1
> > > > >