diff mbox series

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

Message ID 20211111065201.10249-3-nandhini.srikandan@intel.com (mailing list archive)
State New, archived
Headers show
Series Add support for Intel Thunder Bay SPI controller | expand

Commit Message

Srikandan, Nandhini Nov. 11, 2021, 6:51 a.m. UTC 
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(+)

Comments

Mark Brown Nov. 11, 2021, 1:56 p.m. UTC | #1 
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.
Serge Semin Nov. 11, 2021, 2:16 p.m. UTC | #2 
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
Serge Semin Nov. 11, 2021, 2:42 p.m. UTC | #3 
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
>
diff mbox series

Patch

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)