[PATCH v1 0/3] Xilinx SPI driver enhancements

[PATCH v1 0/3] Xilinx SPI driver enhancements

[Mayuresh Chitale]

This series makes the Xilinx XPS SPI driver compatible with the MMC_SPI
driver and also includes an enhacement from the corresponding Linux driver.

Mayuresh Chitale (3):
  drivers: xilinx_spi: Use udevice in start_tranfer
  drivers: xilinx_spi: Add xfer callback
  drivers: xilinx_spi: Probe fifo_depth at runtime

 drivers/spi/xilinx_spi.c | 68 ++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 16 deletions(-)

-- 
2.34.1

[PATCH v1 1/3] drivers: xilinx_spi: Use udevice in start_tranfer

[Mayuresh Chitale]

Modify start_transfer and related functions to take a udevice parameter
as input instead of spi_slave. This is needed so that start_transfer can
be used directly via the xfer callback. Also fix a compiler warning.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/spi/xilinx_spi.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index b58a3f632a..d9faceec18 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -67,7 +67,7 @@
 /* SPI Slave Select Register (spissr), [1] p13, [2] p13 */
 #define SPISSR_MASK(cs)		(1 << (cs))
 #define SPISSR_ACT(cs)		~SPISSR_MASK(cs)
-#define SPISSR_OFF		~0UL
+#define SPISSR_OFF		(~0U)
 
 /* SPI Software Reset Register (ssr) */
 #define SPISSR_RESET_VALUE	0x0a
@@ -217,9 +217,9 @@ static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
 	return i;
 }
 
-static int start_transfer(struct spi_slave *spi, const void *dout, void *din, u32 len)
+static int start_transfer(struct udevice *dev, const void *dout, void *din, u32 len)
 {
-	struct udevice *bus = spi->dev->parent;
+	struct udevice *bus = dev->parent;
 	struct xilinx_spi_priv *priv = dev_get_priv(bus);
 	struct xilinx_spi_regs *regs = priv->regs;
 	u32 count, txbytes, rxbytes;
@@ -259,10 +259,9 @@ static int start_transfer(struct spi_slave *spi, const void *dout, void *din, u3
 	return 0;
 }
 
-static void xilinx_spi_startup_block(struct spi_slave *spi)
+static void xilinx_spi_startup_block(struct udevice *dev)
 {
-	struct dm_spi_slave_plat *slave_plat =
-				dev_get_parent_plat(spi->dev);
+	struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
 	unsigned char txp;
 	unsigned char rxp[8];
 
@@ -270,13 +269,13 @@ static void xilinx_spi_startup_block(struct spi_slave *spi)
 	 * Perform a dummy read as a work around for
 	 * the startup block issue.
 	 */
-	spi_cs_activate(spi->dev, slave_plat->cs);
+	spi_cs_activate(dev, slave_plat->cs);
 	txp = 0x9f;
-	start_transfer(spi, (void *)&txp, NULL, 1);
+	start_transfer(dev, (void *)&txp, NULL, 1);
 
-	start_transfer(spi, NULL, (void *)rxp, 6);
+	start_transfer(dev, NULL, (void *)rxp, 6);
 
-	spi_cs_deactivate(spi->dev);
+	spi_cs_deactivate(dev);
 }
 
 static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
@@ -294,14 +293,15 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
 	 * as QSPI provides command. So first command fails.
 	 */
 	if (!startup) {
-		xilinx_spi_startup_block(spi);
+		xilinx_spi_startup_block(spi->dev);
 		startup++;
 	}
 
 	spi_cs_activate(spi->dev, slave_plat->cs);
 
 	if (op->cmd.opcode) {
-		ret = start_transfer(spi, (void *)&op->cmd.opcode, NULL, 1);
+		ret = start_transfer(spi->dev, (void *)&op->cmd.opcode,
+				     NULL, 1);
 		if (ret)
 			goto done;
 	}
@@ -313,7 +313,7 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
 			addr_buf[i] = op->addr.val >>
 			(8 * (op->addr.nbytes - i - 1));
 
-		ret = start_transfer(spi, (void *)addr_buf, NULL,
+		ret = start_transfer(spi->dev, (void *)addr_buf, NULL,
 				     op->addr.nbytes);
 		if (ret)
 			goto done;
@@ -322,16 +322,16 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
 		dummy_len = (op->dummy.nbytes * op->data.buswidth) /
 			     op->dummy.buswidth;
 
-		ret = start_transfer(spi, NULL, NULL, dummy_len);
+		ret = start_transfer(spi->dev, NULL, NULL, dummy_len);
 		if (ret)
 			goto done;
 	}
 	if (op->data.nbytes) {
 		if (op->data.dir == SPI_MEM_DATA_IN) {
-			ret = start_transfer(spi, NULL,
+			ret = start_transfer(spi->dev, NULL,
 					     op->data.buf.in, op->data.nbytes);
 		} else {
-			ret = start_transfer(spi, op->data.buf.out,
+			ret = start_transfer(spi->dev, op->data.buf.out,
 					     NULL, op->data.nbytes);
 		}
 		if (ret)
-- 
2.34.1

[PATCH v1 2/3] drivers: xilinx_spi: Add xfer callback

[Mayuresh Chitale]

Add the xfer callback which is used by the MMC_SPI driver and generally by
the dm_spi_xfer callback.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/spi/xilinx_spi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index d9faceec18..b63cda2091 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -278,6 +278,18 @@ static void xilinx_spi_startup_block(struct udevice *dev)
 	spi_cs_deactivate(dev);
 }
 
+static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
+			   const void *dout, void *din, unsigned long flags)
+{
+	struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
+	int ret;
+
+	spi_cs_activate(dev, slave_plat->cs);
+	ret = start_transfer(dev, dout, din, bitlen / 8);
+	spi_cs_deactivate(dev);
+	return ret;
+}
+
 static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
 				  const struct spi_mem_op *op)
 {
@@ -427,6 +439,7 @@ static const struct spi_controller_mem_ops xilinx_spi_mem_ops = {
 static const struct dm_spi_ops xilinx_spi_ops = {
 	.claim_bus	= xilinx_spi_claim_bus,
 	.release_bus	= xilinx_spi_release_bus,
+	.xfer           = xilinx_spi_xfer,
 	.set_speed	= xilinx_spi_set_speed,
 	.set_mode	= xilinx_spi_set_mode,
 	.mem_ops	= &xilinx_spi_mem_ops,
-- 
2.34.1

[PATCH v1 3/3] drivers: xilinx_spi: Probe fifo_depth at runtime

[Mayuresh Chitale]

If the fifo-size DT parameter is not provided then probe the
controller's fifo depth at runtime. This is ported from a patch
in the Linux Xilinx SPI driver.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Link: https://lore.kernel.org/r/1422029330-10971-5-git-send-email-ricardo.ribalda@gmail.com
---
 drivers/spi/xilinx_spi.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index b63cda2091..99ae5be291 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -109,6 +109,27 @@ struct xilinx_spi_priv {
 	u8 startup;
 };
 
+static int xilinx_spi_find_buffer_size(struct xilinx_spi_regs *regs)
+{
+	u8 sr;
+	int n_words = 0;
+
+	/*
+	 * Before the buffer_size detection we reset the core
+	 * to make sure we start with a clean state.
+	 */
+	writel(SPISSR_RESET_VALUE, &regs->srr);
+
+	/* Fill the Tx FIFO with as many words as possible */
+	do {
+		writel(0, &regs->spidtr);
+		sr = readl(&regs->spisr);
+		n_words++;
+	} while (!(sr & SPISR_TX_FULL));
+
+	return n_words;
+}
+
 static int xilinx_spi_probe(struct udevice *bus)
 {
 	struct xilinx_spi_priv *priv = dev_get_priv(bus);
@@ -116,6 +137,8 @@ static int xilinx_spi_probe(struct udevice *bus)
 
 	regs = priv->regs = dev_read_addr_ptr(bus);
 	priv->fifo_depth = dev_read_u32_default(bus, "fifo-size", 0);
+	if (!priv->fifo_depth)
+		priv->fifo_depth = xilinx_spi_find_buffer_size(regs);
 
 	writel(SPISSR_RESET_VALUE, &regs->srr);
 
-- 
2.34.1

Re: [PATCH v1 3/3] drivers: xilinx_spi: Probe fifo_depth at runtime

[Michal Simek]

On 11/11/23 18:31, Mayuresh Chitale wrote:
> If the fifo-size DT parameter is not provided then probe the
> controller's fifo depth at runtime. This is ported from a patch
> in the Linux Xilinx SPI driver.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Link: https://lore.kernel.org/r/1422029330-10971-5-git-send-email-ricardo.ribalda@gmail.com
> ---
>   drivers/spi/xilinx_spi.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index b63cda2091..99ae5be291 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -109,6 +109,27 @@ struct xilinx_spi_priv {
>   	u8 startup;
>   };
>   
> +static int xilinx_spi_find_buffer_size(struct xilinx_spi_regs *regs)
> +{
> +	u8 sr;
> +	int n_words = 0;
> +
> +	/*
> +	 * Before the buffer_size detection we reset the core
> +	 * to make sure we start with a clean state.


nit: can you please remove "we" above? It should be written in imperative mood.


> +	 */
> +	writel(SPISSR_RESET_VALUE, &regs->srr);
> +
> +	/* Fill the Tx FIFO with as many words as possible */
> +	do {
> +		writel(0, &regs->spidtr);
> +		sr = readl(&regs->spisr);
> +		n_words++;
> +	} while (!(sr & SPISR_TX_FULL));
> +
> +	return n_words;
> +}
> +
>   static int xilinx_spi_probe(struct udevice *bus)
>   {
>   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
> @@ -116,6 +137,8 @@ static int xilinx_spi_probe(struct udevice *bus)
>   
>   	regs = priv->regs = dev_read_addr_ptr(bus);
>   	priv->fifo_depth = dev_read_u32_default(bus, "fifo-size", 0);
> +	if (!priv->fifo_depth)
> +		priv->fifo_depth = xilinx_spi_find_buffer_size(regs);
>   
>   	writel(SPISSR_RESET_VALUE, &regs->srr);
>   

When above fixed feel free to add
Reviewed-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal

Re: [PATCH v1 3/3] drivers: xilinx_spi: Probe fifo_depth at runtime

[mchitale]

On Mon, 2023-11-13 at 10:06 +0100, Michal Simek wrote:
> 
> On 11/11/23 18:31, Mayuresh Chitale wrote:
> > If the fifo-size DT parameter is not provided then probe the
> > controller's fifo depth at runtime. This is ported from a patch
> > in the Linux Xilinx SPI driver.
> > 
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > Link: 
> > https://lore.kernel.org/r/1422029330-10971-5-git-send-email-ricardo.ribalda@gmail.com
> > ---
> >   drivers/spi/xilinx_spi.c | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> > 
> > diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> > index b63cda2091..99ae5be291 100644
> > --- a/drivers/spi/xilinx_spi.c
> > +++ b/drivers/spi/xilinx_spi.c
> > @@ -109,6 +109,27 @@ struct xilinx_spi_priv {
> >   	u8 startup;
> >   };
> >   
> > +static int xilinx_spi_find_buffer_size(struct xilinx_spi_regs
> > *regs)
> > +{
> > +	u8 sr;
> > +	int n_words = 0;
> > +
> > +	/*
> > +	 * Before the buffer_size detection we reset the core
> > +	 * to make sure we start with a clean state.
> 
> nit: can you please remove "we" above? It should be written in
> imperative mood.
Ok.
> 
> 
> > +	 */
> > +	writel(SPISSR_RESET_VALUE, &regs->srr);
> > +
> > +	/* Fill the Tx FIFO with as many words as possible */
> > +	do {
> > +		writel(0, &regs->spidtr);
> > +		sr = readl(&regs->spisr);
> > +		n_words++;
> > +	} while (!(sr & SPISR_TX_FULL));
> > +
> > +	return n_words;
> > +}
> > +
> >   static int xilinx_spi_probe(struct udevice *bus)
> >   {
> >   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
> > @@ -116,6 +137,8 @@ static int xilinx_spi_probe(struct udevice
> > *bus)
> >   
> >   	regs = priv->regs = dev_read_addr_ptr(bus);
> >   	priv->fifo_depth = dev_read_u32_default(bus, "fifo-size", 0);
> > +	if (!priv->fifo_depth)
> > +		priv->fifo_depth = xilinx_spi_find_buffer_size(regs);
> >   
> >   	writel(SPISSR_RESET_VALUE, &regs->srr);
> >   
> 
> When above fixed feel free to add
> Reviewed-by: Michal Simek <michal.simek@amd.com>
Ok.
> 
> Thanks,
> Michal