diff mbox series

[v2,01/15] spi: dw: Introduce spi_frf and STD_SPI

Message ID 20221212180732.79167-2-sudip.mukherjee@sifive.com (mailing list archive)
State New, archived
Headers show
Series Add support for enhanced SPI for Designware SPI controllers | expand

Commit Message

Sudip Mukherjee Dec. 12, 2022, 6:07 p.m. UTC
The DW APB SSI controllers of v4.x and newer and DW AHB SSI controllers
supports enhanced SPI modes which can be defined from SPI_FRF of
DW_SPI_CTRLR0 register. Without enhanced mode, these controllers will
work in the standard spi mode.

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

Comments

Serge Semin Jan. 9, 2023, 4:43 p.m. UTC | #1
On Mon, Dec 12, 2022 at 06:07:18PM +0000, Sudip Mukherjee wrote:
> The DW APB SSI controllers of v4.x and newer and DW AHB SSI controllers
> supports enhanced SPI modes which can be defined from SPI_FRF of
> DW_SPI_CTRLR0 register. Without enhanced mode, these controllers will
> work in the standard spi mode.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/spi/spi-dw-core.c | 13 ++++++++++++-
>  drivers/spi/spi-dw.h      |  6 ++++++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 99edddf9958b9..77c23772bb3d9 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -333,6 +333,16 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
>  		/* CTRLR0[11:10] Transfer Mode */
>  		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
>  

> +	if (dw_spi_ver_is_ge(dws, HSSI, 103A)) {

eSPI has been available most likely since 1.00a (at least 1.01a
has that feature).

> +		cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK;

No need in masking that field because the cr0 variable is
pre-initialized with the device-specific value anyway.

> +		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK,

> +				  cfg->spi_frf);

The HW-manual defines that field as SPI_FRF, but the SPI_ prefix looks
vague because it doesn't differentiate it from just "frf" field. I'd
suggest to use the "enh_frf" name instead.

> +	} else if (dw_spi_ver_is_ge(dws, PSSI, 400A)) {

> +		cr0 &= ~DW_PSSI_CTRLR0_SPI_FRF_MASK;
> +		cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_SPI_FRF_MASK,
> +				  cfg->spi_frf);

The same comments as above.

> +	}
> +
>  	dw_writel(dws, DW_SPI_CTRLR0, cr0);
>  
>  	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
> @@ -422,6 +432,7 @@ static int dw_spi_transfer_one(struct spi_controller *master,
                                                 <--------+
>  		.tmode = DW_SPI_CTRLR0_TMOD_TR,           |
>  		.dfs = transfer->bits_per_word,           |
>  		.freq = transfer->speed_hz,               |
                                                          |
> +		.spi_frf = DW_SPI_CTRLR0_SPI_FRF_STD_SPI, +

You also forgot to update the spi-dw-bt1.c driver.

>  	};
>  	int ret;
>  
> @@ -664,7 +675,7 @@ static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
>  static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
>  {
>  	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);

> -	struct dw_spi_cfg cfg;
> +	struct dw_spi_cfg cfg = {0};

Please explicitly initialize the enh_frf field in the method below in
the same way as it's done for the rest of the fields.

>  	unsigned long flags;
>  	int ret;
>  
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 9e8eb2b52d5c7..414a415deb42a 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -17,6 +17,8 @@
>  
>  /* Synopsys DW SSI component versions (FourCC sequence) */
                                                  <-+
>  #define DW_HSSI_102A			0x3130322a  |
> +#define DW_HSSI_103A			0x3130332a  |
                                                    |
> +#define DW_PSSI_400A			0x3430302a -+

Please define the PSSI-macros above the HSSI ones.

>  
>  /* DW SSI IP-core ID and version check helpers */
>  #define dw_spi_ip_is(_dws, _ip) \
> @@ -94,6 +96,9 @@
>  #define DW_HSSI_CTRLR0_TMOD_MASK		GENMASK(11, 10)
>  #define DW_HSSI_CTRLR0_SRL			BIT(13)
                                                       <---------+
>  #define DW_HSSI_CTRLR0_MST			BIT(31)          |
                                                                 |
> +#define DW_HSSI_CTRLR0_SPI_FRF_MASK		GENMASK(23, 22) -+

This macro should be placed above the DW_HSSI_CTRLR0_MST one. Also
rename SPI_FRF to ENH_FRF.

> +#define DW_PSSI_CTRLR0_SPI_FRF_MASK		GENMASK(22, 21)
> +#define DW_SPI_CTRLR0_SPI_FRF_STD_SPI		0x0

1. Move these macros to the DW APB SSI group of the CSR fields macros.
2. Drop the SPI suffix from the DW_SPI_CTRLR0_SPI_FRF_STD_SPI macro.
3. Replace SPI_FRF with ENH_FRF name.

>  
>  /* Bit fields in CTRLR1 */
>  #define DW_SPI_NDF_MASK				GENMASK(15, 0)
> @@ -135,6 +140,7 @@ struct dw_spi_cfg {
>  	u8 dfs;
>  	u32 ndf;
>  	u32 freq;

> +	u8 spi_frf;

Please move it to the head of the structure and rename to "enh_frf".

-Serge(y)

>  };
>  
>  struct dw_spi;
> -- 
> 2.30.2
>
diff mbox series

Patch

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 99edddf9958b9..77c23772bb3d9 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -333,6 +333,16 @@  void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
 		/* CTRLR0[11:10] Transfer Mode */
 		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_TMOD_MASK, cfg->tmode);
 
+	if (dw_spi_ver_is_ge(dws, HSSI, 103A)) {
+		cr0 &= ~DW_HSSI_CTRLR0_SPI_FRF_MASK;
+		cr0 |= FIELD_PREP(DW_HSSI_CTRLR0_SPI_FRF_MASK,
+				  cfg->spi_frf);
+	} else if (dw_spi_ver_is_ge(dws, PSSI, 400A)) {
+		cr0 &= ~DW_PSSI_CTRLR0_SPI_FRF_MASK;
+		cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_SPI_FRF_MASK,
+				  cfg->spi_frf);
+	}
+
 	dw_writel(dws, DW_SPI_CTRLR0, cr0);
 
 	if (cfg->tmode == DW_SPI_CTRLR0_TMOD_EPROMREAD ||
@@ -422,6 +432,7 @@  static int dw_spi_transfer_one(struct spi_controller *master,
 		.tmode = DW_SPI_CTRLR0_TMOD_TR,
 		.dfs = transfer->bits_per_word,
 		.freq = transfer->speed_hz,
+		.spi_frf = DW_SPI_CTRLR0_SPI_FRF_STD_SPI,
 	};
 	int ret;
 
@@ -664,7 +675,7 @@  static void dw_spi_stop_mem_op(struct dw_spi *dws, struct spi_device *spi)
 static int dw_spi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
 {
 	struct dw_spi *dws = spi_controller_get_devdata(mem->spi->controller);
-	struct dw_spi_cfg cfg;
+	struct dw_spi_cfg cfg = {0};
 	unsigned long flags;
 	int ret;
 
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 9e8eb2b52d5c7..414a415deb42a 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -17,6 +17,8 @@ 
 
 /* Synopsys DW SSI component versions (FourCC sequence) */
 #define DW_HSSI_102A			0x3130322a
+#define DW_HSSI_103A			0x3130332a
+#define DW_PSSI_400A			0x3430302a
 
 /* DW SSI IP-core ID and version check helpers */
 #define dw_spi_ip_is(_dws, _ip) \
@@ -94,6 +96,9 @@ 
 #define DW_HSSI_CTRLR0_TMOD_MASK		GENMASK(11, 10)
 #define DW_HSSI_CTRLR0_SRL			BIT(13)
 #define DW_HSSI_CTRLR0_MST			BIT(31)
+#define DW_HSSI_CTRLR0_SPI_FRF_MASK		GENMASK(23, 22)
+#define DW_PSSI_CTRLR0_SPI_FRF_MASK		GENMASK(22, 21)
+#define DW_SPI_CTRLR0_SPI_FRF_STD_SPI		0x0
 
 /* Bit fields in CTRLR1 */
 #define DW_SPI_NDF_MASK				GENMASK(15, 0)
@@ -135,6 +140,7 @@  struct dw_spi_cfg {
 	u8 dfs;
 	u32 ndf;
 	u32 freq;
+	u8 spi_frf;
 };
 
 struct dw_spi;