[PATCH v7 0/2] spi-geni-qcom: Add SPI device mode support for GENI based QuPv3

[PATCH v7 0/2] spi-geni-qcom: Add SPI device mode support for GENI based QuPv3

[Praveen Talari]

This series adds spi device mode functionality to geni based Qupv3.
The common header file contains spi slave related registers and masks.

Praveen Talari (2):
  soc: qcom: geni-se: Add SPI Device mode support for GENI based QuPv3
  spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3
---
v6 -> v7:
- Corrected author mail

v5 -> v6:
- Added code comments
- Dropped get_spi_master api

v4 -> v5:
- Addressed review comments in driver

v3 -> v4:
- Used existing property spi-slave
- Hence dropped dt-binding changes

v2 -> v3:
- Modified commit message
- Addressed comment on dt-binding

v1 -> v2:
- Added dt-binding change for spi slave
- Modified commit message
- Addressed review comments in driver

 drivers/spi/spi-geni-qcom.c      | 53 ++++++++++++++++++++++++++++----
 include/linux/soc/qcom/geni-se.h |  9 ++++++
 2 files changed, 56 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH v7 1/2] soc: qcom: geni-se: Add SPI Device mode support for GENI based QuPv3

[Praveen Talari]

Add device mode supported registers and masks.

Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
---
v6 -> v7:
- corrected author mail

v2 -> v3:
- modified commit message to use device mode instead of slave mode

v1 -> v2:
- modified commit message
---
 include/linux/soc/qcom/geni-se.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
index 821a19135bb6..29e06905bc1f 100644
--- a/include/linux/soc/qcom/geni-se.h
+++ b/include/linux/soc/qcom/geni-se.h
@@ -35,6 +35,7 @@ enum geni_se_protocol_type {
 	GENI_SE_UART,
 	GENI_SE_I2C,
 	GENI_SE_I3C,
+	GENI_SE_SPI_SLAVE,
 };
 
 struct geni_wrapper;
@@ -73,12 +74,14 @@ struct geni_se {
 
 /* Common SE registers */
 #define GENI_FORCE_DEFAULT_REG		0x20
+#define GENI_OUTPUT_CTRL		0x24
 #define SE_GENI_STATUS			0x40
 #define GENI_SER_M_CLK_CFG		0x48
 #define GENI_SER_S_CLK_CFG		0x4c
 #define GENI_IF_DISABLE_RO		0x64
 #define GENI_FW_REVISION_RO		0x68
 #define SE_GENI_CLK_SEL			0x7c
+#define SE_GENI_CFG_SEQ_START		0x84
 #define SE_GENI_DMA_MODE_EN		0x258
 #define SE_GENI_M_CMD0			0x600
 #define SE_GENI_M_CMD_CTRL_REG		0x604
@@ -111,6 +114,9 @@ struct geni_se {
 /* GENI_FORCE_DEFAULT_REG fields */
 #define FORCE_DEFAULT	BIT(0)
 
+/* GENI_OUTPUT_CTRL fields */
+#define GENI_IO_MUX_0_EN		BIT(0)
+
 /* GENI_STATUS fields */
 #define M_GENI_CMD_ACTIVE		BIT(0)
 #define S_GENI_CMD_ACTIVE		BIT(12)
@@ -130,6 +136,9 @@ struct geni_se {
 /* GENI_CLK_SEL fields */
 #define CLK_SEL_MSK			GENMASK(2, 0)
 
+/* SE_GENI_CFG_SEQ_START fields */
+#define START_TRIGGER			BIT(0)
+
 /* SE_GENI_DMA_MODE_EN */
 #define GENI_DMA_MODE_EN		BIT(0)
 
-- 
2.17.1

Re: [PATCH v7 1/2] soc: qcom: geni-se: Add SPI Device mode support for GENI based QuPv3

[Mark Brown]

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

On Fri, Jul 14, 2023 at 09:52:02AM +0530, Praveen Talari wrote:
> Add device mode supported registers and masks.

Does it make sense for me to take this one via spi given how trivial it
is?

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

Re: [PATCH v7 1/2] soc: qcom: geni-se: Add SPI Device mode support for GENI based QuPv3

[Bjorn Andersson]

On Thu, Jul 20, 2023 at 02:11:42PM +0100, Mark Brown wrote:
> On Fri, Jul 14, 2023 at 09:52:02AM +0530, Praveen Talari wrote:
> > Add device mode supported registers and masks.
> 
> Does it make sense for me to take this one via spi given how trivial it
> is?

Sounds good to me.

Acked-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

[PATCH v7 2/2] spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3

[Praveen Talari]

Currently spi geni driver supports only master mode operation.

Add spi device mode support to GENI based QuPv3.

Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
---
v6 -> v7:
- corrected author mail

v4 -> v5:
- added code comments
- dropped get_spi_master api.

v3 -> v4:
- Used existing property spi-slave.

v2 -> v3:
- modified commit message to use device mode instead of slave mode

v1 -> v2
- modified the commit message
- added the code changes for code comments
---
 drivers/spi/spi-geni-qcom.c | 53 ++++++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 206cc04bb1ed..06b114f3f21a 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -12,6 +12,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/soc/qcom/geni-se.h>
 #include <linux/spi/spi.h>
 #include <linux/spinlock.h>
@@ -52,6 +53,9 @@
 #define SPI_CS_CLK_DELAY_MSK		GENMASK(19, 10)
 #define SPI_CS_CLK_DELAY_SHFT		10
 
+#define SE_SPI_SLAVE_EN				(0x2BC)
+#define SPI_SLAVE_EN				BIT(0)
+
 /* M_CMD OP codes for SPI */
 #define SPI_TX_ONLY		1
 #define SPI_RX_ONLY		2
@@ -99,6 +103,16 @@ struct spi_geni_master {
 	int cur_xfer_mode;
 };
 
+static void spi_slv_setup(struct spi_geni_master *mas)
+{
+	struct geni_se *se = &mas->se;
+
+	writel(SPI_SLAVE_EN, se->base + SE_SPI_SLAVE_EN);
+	writel(GENI_IO_MUX_0_EN, se->base + GENI_OUTPUT_CTRL);
+	writel(START_TRIGGER, se->base + SE_GENI_CFG_SEQ_START);
+	dev_dbg(mas->dev, "spi slave setup done\n");
+}
+
 static int get_spi_clk_cfg(unsigned int speed_hz,
 			struct spi_geni_master *mas,
 			unsigned int *clk_idx,
@@ -140,12 +154,22 @@ static void handle_se_timeout(struct spi_master *spi,
 	const struct spi_transfer *xfer;
 
 	spin_lock_irq(&mas->lock);
-	reinit_completion(&mas->cancel_done);
 	if (mas->cur_xfer_mode == GENI_SE_FIFO)
 		writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
 
 	xfer = mas->cur_xfer;
 	mas->cur_xfer = NULL;
+
+	if (spi->slave) {
+		/*
+		 * skip CMD Cancel sequnece since spi slave
+		 * doesn`t support CMD Cancel sequnece
+		 */
+		spin_unlock_irq(&mas->lock);
+		goto unmap_if_dma;
+	}
+
+	reinit_completion(&mas->cancel_done);
 	geni_se_cancel_m_cmd(se);
 	spin_unlock_irq(&mas->lock);
 
@@ -542,6 +566,10 @@ static bool geni_can_dma(struct spi_controller *ctlr,
 	if (mas->cur_xfer_mode == GENI_GPI_DMA)
 		return true;
 
+	/* Set SE DMA mode for SPI slave. */
+	if (ctlr->slave)
+		return true;
+
 	len = get_xfer_len_in_words(xfer, mas);
 	fifo_size = mas->tx_fifo_depth * mas->fifo_width_bits / mas->cur_bits_per_word;
 
@@ -619,6 +647,7 @@ static void spi_geni_release_dma_chan(struct spi_geni_master *mas)
 
 static int spi_geni_init(struct spi_geni_master *mas)
 {
+	struct spi_master *spi = dev_get_drvdata(mas->dev);
 	struct geni_se *se = &mas->se;
 	unsigned int proto, major, minor, ver;
 	u32 spi_tx_cfg, fifo_disable;
@@ -627,7 +656,14 @@ static int spi_geni_init(struct spi_geni_master *mas)
 	pm_runtime_get_sync(mas->dev);
 
 	proto = geni_se_read_proto(se);
-	if (proto != GENI_SE_SPI) {
+
+	if (spi->slave) {
+		if (proto != GENI_SE_SPI_SLAVE) {
+			dev_err(mas->dev, "Invalid proto %d\n", proto);
+			goto out_pm;
+		}
+		spi_slv_setup(mas);
+	} else if (proto != GENI_SE_SPI) {
 		dev_err(mas->dev, "Invalid proto %d\n", proto);
 		goto out_pm;
 	}
@@ -677,9 +713,11 @@ static int spi_geni_init(struct spi_geni_master *mas)
 	}
 
 	/* We always control CS manually */
-	spi_tx_cfg = readl(se->base + SE_SPI_TRANS_CFG);
-	spi_tx_cfg &= ~CS_TOGGLE;
-	writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);
+	if (!spi->slave) {
+		spi_tx_cfg = readl(se->base + SE_SPI_TRANS_CFG);
+		spi_tx_cfg &= ~CS_TOGGLE;
+		writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);
+	}
 
 out_pm:
 	pm_runtime_put(mas->dev);
@@ -1072,6 +1110,9 @@ static int spi_geni_probe(struct platform_device *pdev)
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
 	pm_runtime_enable(dev);
 
+	if (device_property_read_bool(&pdev->dev, "spi-slave"))
+		spi->slave = true;
+
 	ret = geni_icc_get(&mas->se, NULL);
 	if (ret)
 		goto spi_geni_probe_runtime_disable;
@@ -1092,7 +1133,7 @@ static int spi_geni_probe(struct platform_device *pdev)
 	 * for dma (gsi) mode, the gsi will set cs based on params passed in
 	 * TRE
 	 */
-	if (mas->cur_xfer_mode == GENI_SE_FIFO)
+	if (!spi->slave && mas->cur_xfer_mode == GENI_SE_FIFO)
 		spi->set_cs = spi_geni_set_cs;
 
 	ret = request_irq(mas->irq, geni_spi_isr, 0, dev_name(dev), spi);
-- 
2.17.1

Re: [PATCH v7 2/2] spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3

[Vijaya Krishna Nivarthi]

Hi,

With no knowledge of spi slave setup, I should say this looks good to me.

A few nits below which can be added in if there is going to be a v8, 
otherwise can be done later...



On 7/14/2023 9:52 AM, Praveen Talari wrote:
> Currently spi geni driver supports only master mode operation.
>
> Add spi device mode support to GENI based QuPv3.
>
> Signed-off-by: Praveen Talari <quic_ptalari@quicinc.com>
> ---
> v6 -> v7:
> - corrected author mail
>
> v4 -> v5:
> - added code comments
> - dropped get_spi_master api.
>
> v3 -> v4:
> - Used existing property spi-slave.
>
> v2 -> v3:
> - modified commit message to use device mode instead of slave mode
>
> v1 -> v2
> - modified the commit message
> - added the code changes for code comments
> ---
>   drivers/spi/spi-geni-qcom.c | 53 ++++++++++++++++++++++++++++++++-----
>   1 file changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 206cc04bb1ed..06b114f3f21a 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -12,6 +12,7 @@
>   #include <linux/platform_device.h>
>   #include <linux/pm_opp.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/property.h>
>   #include <linux/soc/qcom/geni-se.h>
>   #include <linux/spi/spi.h>
>   #include <linux/spinlock.h>
> @@ -52,6 +53,9 @@
>   #define SPI_CS_CLK_DELAY_MSK		GENMASK(19, 10)
>   #define SPI_CS_CLK_DELAY_SHFT		10
>   
> +#define SE_SPI_SLAVE_EN				(0x2BC)
> +#define SPI_SLAVE_EN				BIT(0)
> +

perhaps correct indentation here?

please check in an editor which uses correct number of spaces for a tab

>   /* M_CMD OP codes for SPI */
>   #define SPI_TX_ONLY		1
>   #define SPI_RX_ONLY		2
> @@ -99,6 +103,16 @@ struct spi_geni_master {
>   	int cur_xfer_mode;
>   };
>   
> +static void spi_slv_setup(struct spi_geni_master *mas)
> +{
> +	struct geni_se *se = &mas->se;
> +
> +	writel(SPI_SLAVE_EN, se->base + SE_SPI_SLAVE_EN);
> +	writel(GENI_IO_MUX_0_EN, se->base + GENI_OUTPUT_CTRL);
> +	writel(START_TRIGGER, se->base + SE_GENI_CFG_SEQ_START);
> +	dev_dbg(mas->dev, "spi slave setup done\n");
> +}
> +
>   static int get_spi_clk_cfg(unsigned int speed_hz,
>   			struct spi_geni_master *mas,
>   			unsigned int *clk_idx,
> @@ -140,12 +154,22 @@ static void handle_se_timeout(struct spi_master *spi,
>   	const struct spi_transfer *xfer;
>   
>   	spin_lock_irq(&mas->lock);
> -	reinit_completion(&mas->cancel_done);

good to see reinit moved to after newly added goto

>   	if (mas->cur_xfer_mode == GENI_SE_FIFO)
>   		writel(0, se->base + SE_GENI_TX_WATERMARK_REG);
>   
>   	xfer = mas->cur_xfer;
>   	mas->cur_xfer = NULL;
> +
> +	if (spi->slave) {
> +		/*
> +		 * skip CMD Cancel sequnece since spi slave
> +		 * doesn`t support CMD Cancel sequnece

typo for sequence in 2 lines.

Or better, a more concise comment...

/* spi slave doesnt support CMD_CANCEL sequence */

> +		 */
> +		spin_unlock_irq(&mas->lock);
> +		goto unmap_if_dma;
> +	}
> +
> +	reinit_completion(&mas->cancel_done);
>   	geni_se_cancel_m_cmd(se);
>   	spin_unlock_irq(&mas->lock);
>   
> @@ -542,6 +566,10 @@ static bool geni_can_dma(struct spi_controller *ctlr,
>   	if (mas->cur_xfer_mode == GENI_GPI_DMA)
>   		return true;
>   
> +	/* Set SE DMA mode for SPI slave. */
> +	if (ctlr->slave)
> +		return true;
> +
>   	len = get_xfer_len_in_words(xfer, mas);
>   	fifo_size = mas->tx_fifo_depth * mas->fifo_width_bits / mas->cur_bits_per_word;
>   
> @@ -619,6 +647,7 @@ static void spi_geni_release_dma_chan(struct spi_geni_master *mas)
>   
>   static int spi_geni_init(struct spi_geni_master *mas)
>   {
> +	struct spi_master *spi = dev_get_drvdata(mas->dev);
>   	struct geni_se *se = &mas->se;
>   	unsigned int proto, major, minor, ver;
>   	u32 spi_tx_cfg, fifo_disable;
> @@ -627,7 +656,14 @@ static int spi_geni_init(struct spi_geni_master *mas)
>   	pm_runtime_get_sync(mas->dev);
>   
>   	proto = geni_se_read_proto(se);
> -	if (proto != GENI_SE_SPI) {
> +
> +	if (spi->slave) {
> +		if (proto != GENI_SE_SPI_SLAVE) {
> +			dev_err(mas->dev, "Invalid proto %d\n", proto);
> +			goto out_pm;
> +		}
> +		spi_slv_setup(mas);
> +	} else if (proto != GENI_SE_SPI) {
>   		dev_err(mas->dev, "Invalid proto %d\n", proto);
>   		goto out_pm;
>   	}
> @@ -677,9 +713,11 @@ static int spi_geni_init(struct spi_geni_master *mas)
>   	}
>   
>   	/* We always control CS manually */
> -	spi_tx_cfg = readl(se->base + SE_SPI_TRANS_CFG);
> -	spi_tx_cfg &= ~CS_TOGGLE;
> -	writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);
> +	if (!spi->slave) {
> +		spi_tx_cfg = readl(se->base + SE_SPI_TRANS_CFG);
> +		spi_tx_cfg &= ~CS_TOGGLE;
> +		writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);
> +	}
>   
>   out_pm:
>   	pm_runtime_put(mas->dev);
> @@ -1072,6 +1110,9 @@ static int spi_geni_probe(struct platform_device *pdev)
>   	pm_runtime_set_autosuspend_delay(&pdev->dev, 250);
>   	pm_runtime_enable(dev);
>   
> +	if (device_property_read_bool(&pdev->dev, "spi-slave"))
> +		spi->slave = true;
> +
>   	ret = geni_icc_get(&mas->se, NULL);
>   	if (ret)
>   		goto spi_geni_probe_runtime_disable;
> @@ -1092,7 +1133,7 @@ static int spi_geni_probe(struct platform_device *pdev)
>   	 * for dma (gsi) mode, the gsi will set cs based on params passed in
>   	 * TRE
>   	 */
> -	if (mas->cur_xfer_mode == GENI_SE_FIFO)
> +	if (!spi->slave && mas->cur_xfer_mode == GENI_SE_FIFO)
>   		spi->set_cs = spi_geni_set_cs;
>   
>   	ret = request_irq(mas->irq, geni_spi_isr, 0, dev_name(dev), spi);

Thank you...

-Vijay/

Re: [PATCH v7 0/2] spi-geni-qcom: Add SPI device mode support for GENI based QuPv3

[Vijaya Krishna Nivarthi]

Hi,

LGTM...


On 7/14/2023 9:52 AM, Praveen Talari wrote:
> This series adds spi device mode functionality to geni based Qupv3.
> The common header file contains spi slave related registers and masks.
>
> Praveen Talari (2):
>    soc: qcom: geni-se: Add SPI Device mode support for GENI based QuPv3
>    spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3
> ---
> v6 -> v7:
> - Corrected author mail
>
> v5 -> v6:
> - Added code comments
> - Dropped get_spi_master api
>
> v4 -> v5:
> - Addressed review comments in driver
>
> v3 -> v4:
> - Used existing property spi-slave
> - Hence dropped dt-binding changes
>
> v2 -> v3:
> - Modified commit message
> - Addressed comment on dt-binding
>
> v1 -> v2:
> - Added dt-binding change for spi slave
> - Modified commit message
> - Addressed review comments in driver
>
>   drivers/spi/spi-geni-qcom.c      | 53 ++++++++++++++++++++++++++++----
>   include/linux/soc/qcom/geni-se.h |  9 ++++++
>   2 files changed, 56 insertions(+), 6 deletions(-)


Reviewed-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com>

Re: [PATCH v7 0/2] spi-geni-qcom: Add SPI device mode support for GENI based QuPv3

[Mark Brown]

On Fri, 14 Jul 2023 09:52:01 +0530, Praveen Talari wrote:
> This series adds spi device mode functionality to geni based Qupv3.
> The common header file contains spi slave related registers and masks.
> 
> Praveen Talari (2):
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/2] soc: qcom: geni-se: Add SPI Device mode support for GENI based QuPv3
      commit: 59bbe86bb212b618ec2b50434f54bb4cfc704f44
[2/2] spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3
      commit: d7f74cc31a89a45d4c7deaa5f759661a07a183d6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark