[PATCH] drivers: xilinx_spi: Fixes for MMC_SPI
mchitale at ventanamicro.com
mchitale at ventanamicro.com
Sat Nov 11 14:21:15 CET 2023
Hi Michal,
On Thu, 2023-11-02 at 09:37 +0100, Michal Simek wrote:
> 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.
Ok.
>
> > 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
Ok.
>
>
>
> > +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.
Ok.
>
> > 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?
I will revert the change.
>
> > {
> > 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.
Ok.
>
> > {
> > - 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
Ok.
>
> > + 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