2023-07-06 05:31:26

by Praveen Talari

[permalink] [raw]
Subject: [PATCH v4 0/2] spi-geni-qcom: Add SPI device mode support for

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
---
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 | 57 ++++++++++++++++++++++++++++----
include/linux/soc/qcom/geni-se.h | 9 +++++
2 files changed, 60 insertions(+), 6 deletions(-)

--
2.17.1



2023-07-06 05:32:26

by Praveen Talari

[permalink] [raw]
Subject: [PATCH v4 2/2] spi: spi-geni-qcom: Add SPI Device mode support for GENI based QuPv3

Currently spi geni driver supports only master mode operation.

Add spi device mode support to GENI based QuPv3.

Signed-off-by: Praveen Talari <[email protected]>
---
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 | 57 +++++++++++++++++++++++++++++++++----
1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 206cc04bb1ed..4ec4fdee06b5 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,24 @@ struct spi_geni_master {
int cur_xfer_mode;
};

+static struct spi_master *get_spi_master(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct spi_master *spi = platform_get_drvdata(pdev);
+
+ return spi;
+}
+
+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 +162,18 @@ 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) {
+ 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 +570,10 @@ static bool geni_can_dma(struct spi_controller *ctlr,
if (mas->cur_xfer_mode == GENI_GPI_DMA)
return true;

+ /* Set 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 +651,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 = get_spi_master(mas->dev);
struct geni_se *se = &mas->se;
unsigned int proto, major, minor, ver;
u32 spi_tx_cfg, fifo_disable;
@@ -627,7 +660,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 +717,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 +1114,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 +1137,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


2023-07-06 05:38:14

by Praveen Talari

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

Add device mode supported registers and masks.

Signed-off-by: Praveen Talari <[email protected]>
---
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


2023-07-06 06:59:48

by Dmitry Baryshkov

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

On Thu, 6 Jul 2023 at 08:12, Praveen Talari <[email protected]> wrote:
>
> Currently spi geni driver supports only master mode operation.
>
> Add spi device mode support to GENI based QuPv3.
>
> Signed-off-by: Praveen Talari <[email protected]>
> ---
> 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 | 57 +++++++++++++++++++++++++++++++++----
> 1 file changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 206cc04bb1ed..4ec4fdee06b5 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,24 @@ struct spi_geni_master {
> int cur_xfer_mode;
> };
>
> +static struct spi_master *get_spi_master(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct spi_master *spi = platform_get_drvdata(pdev);

You can use dev_get_drvdata() directly, without going through platform_device.

> +
> + return spi;
> +}
> +
> +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 +162,18 @@ 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) {

A comment here would be nice.

> + 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 +570,10 @@ static bool geni_can_dma(struct spi_controller *ctlr,
> if (mas->cur_xfer_mode == GENI_GPI_DMA)
> return true;
>
> + /* Set DMA mode for SPI slave. */

SE DMA mode

> + 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 +651,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 = get_spi_master(mas->dev);
> struct geni_se *se = &mas->se;
> unsigned int proto, major, minor, ver;
> u32 spi_tx_cfg, fifo_disable;
> @@ -627,7 +660,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 +717,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 +1114,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 +1137,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
>


--
With best wishes
Dmitry

2023-07-07 05:27:07

by Praveen Talari

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

Hi,

On 7/6/2023 12:09 PM, Dmitry Baryshkov wrote:
> On Thu, 6 Jul 2023 at 08:12, Praveen Talari <[email protected]> wrote:
>> Currently spi geni driver supports only master mode operation.
>>
>> Add spi device mode support to GENI based QuPv3.
>>
>> Signed-off-by: Praveen Talari <[email protected]>
>> ---
>> 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 | 57 +++++++++++++++++++++++++++++++++----
>> 1 file changed, 51 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
>> index 206cc04bb1ed..4ec4fdee06b5 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,24 @@ struct spi_geni_master {
>> int cur_xfer_mode;
>> };
>>
>> +static struct spi_master *get_spi_master(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct spi_master *spi = platform_get_drvdata(pdev);
> You can use dev_get_drvdata() directly, without going through platform_device.
Done
>
>> +
>> + return spi;
>> +}
>> +
>> +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 +162,18 @@ 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) {
> A comment here would be nice.
Done
>
>> + 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 +570,10 @@ static bool geni_can_dma(struct spi_controller *ctlr,
>> if (mas->cur_xfer_mode == GENI_GPI_DMA)
>> return true;
>>
>> + /* Set DMA mode for SPI slave. */
> SE DMA mode
Done
>
>> + 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 +651,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 = get_spi_master(mas->dev);
>> struct geni_se *se = &mas->se;
>> unsigned int proto, major, minor, ver;
>> u32 spi_tx_cfg, fifo_disable;
>> @@ -627,7 +660,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 +717,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 +1114,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 +1137,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
>>
>