[PATCH] drivers: xilinx_spi: Fixes for MMC_SPI
Michal Simek
michal.simek at amd.com
Thu Nov 2 09:37:08 CET 2023
On 11/2/23 09:23, Mayuresh Chitale wrote:
> Add the xfer callback which is used by the MMC_SPI driver and generally by
> the dm_spi_xfer callback. Also probe the fifo_depth during init as is
> done in the linux spi-xilinx driver and fix a compiler warning while at it.
They are independent things that's why please separate them by that topics.
Also update subject to describe issue there.
>
> Signed-off-by: Mayuresh Chitale <mchitale at ventanamicro.com>
> ---
> drivers/spi/xilinx_spi.c | 70 ++++++++++++++++++++++++++++++----------
> 1 file changed, 53 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index b58a3f632a..0d4ff25600 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
> @@ -109,6 +109,27 @@ struct xilinx_spi_priv {
> u8 startup;
> };
>
> +/* Detect fifo depth. Ported from linux sha: 4c9a761402d78 */
don't add linux sha1 here.
Add it to commit message or I normally just provide a link to that patch.
https://lore.kernel.org/r/1422029330-10971-5-git-send-email-ricardo.ribalda@gmail.com
> +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, ®s->srr);
> +
> + /* Fill the Tx FIFO with as many words as possible */
> + do {
> + writel(0, ®s->spidtr);
> + sr = readl(®s->spisr);
> + n_words++;
> + } while (!(sr & SPISR_TX_FULL));
> +
> + return n_words;
> +}
newline here.
> 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, ®s->srr);
>
> @@ -197,7 +220,7 @@ static u32 xilinx_spi_fill_txfifo(struct udevice *bus, const u8 *txp,
> return i;
> }
>
> -static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
> +static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, int rxbytes)
Unrelated. Or do we have a case where we ask for reading negative number of bytes?
> {
> struct xilinx_spi_priv *priv = dev_get_priv(bus);
> struct xilinx_spi_regs *regs = priv->regs;
> @@ -217,9 +240,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)
All these spi -> dev changes should be in separate patch. I expect they are
preparation for xfer but still better to do it separately.
> {
> - 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 +282,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 +292,25 @@ 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_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));
no need for () around
> + spi_cs_deactivate(dev);
> + return ret;
> }
>
> static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
> @@ -294,14 +328,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 +348,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 +357,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)
> @@ -427,6 +462,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,
M
More information about the U-Boot
mailing list