[PATCH sunxi/next] spi: sunxi: use XCH status to detect in-progress transfer

[Icenowy Zheng]

在 2022-07-11星期一的 00:03 +0100，Andre Przywara写道：
> On Tue, 28 Jun 2022 14:49:24 +0800
> Icenowy Zheng <uwu at icenowy.me> wrote:
> 
> Hi Icenowy,
> 
> > The current detection of RX FIFO depth seems to be not reliable,
> > and
> > XCH will self-clear when a transfer is done.
> 
> many thanks for sending this, indeed what I put in -next is broken,
> probably for everything except the F1C100 ;-)
> 
> Digging a bit deeper this gets more interesting, though:
> I chased the issue down to my very first commit, that is (now
> properly!)
> setting the SPI bus frequency in the SPI controller's CCR register.
> It
> turns out that there are more issues in this driver, which lead to an
> actual frequency limit of 1 MHz[1]. So my commit now actually
> programs
> this value, and apparently it's too slow(?) for the code? Raising the
> default 1 to 4 MHz makes it work again (even without your patch). The
> previous timeout is generous, though, but by looking at the FIFO
> status
> register it just seemed to be stuck after clocking out one byte only,
> with the RX buf staying at 0. Reading FSR after your new loop reveals
> that the condition holds (RX FIFO level == nbytes), so there is
> something quite weird going on. Without your patch, but with some
> udelay(1000) after(!) the loop it also works, interestingly.
> 
> As for the actual code change: looking at the XCH bit is probably
> indeed the most robust and clever method of checking for the end of a
> transfer, so I am tempted to take this change. However there are more
> things broken, apparently, and I would like to get to the bottom of
> those issues, before trying to paper over them.
> 
> Cheers,
> Andre
> 
> [1] The driver (as most U-Boot SPI drivers, actually) tries to read
> spi-max-frequency from the SPI's *controller* DT node, although this
> is
> a SPI *slave* property. It (correctly) doesn't find anything in
> there,
> so falls back to some (assumed) safe value of 1 MHz.

Ooops... It sounds like the SPI framework is broken?

> 
> > Check XCH bit when polling for transfer finish.
> 
> 
> > 
> > Signed-off-by: Icenowy Zheng <uwu at icenowy.me>
> > ---
> >  drivers/spi/spi-sunxi.c | 14 +++++---------
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
> > index 2f33337725..a424c6a98e 100644
> > --- a/drivers/spi/spi-sunxi.c
> > +++ b/drivers/spi/spi-sunxi.c
> > @@ -83,7 +83,7 @@ DECLARE_GLOBAL_DATA_PTR;
> >  #endif
> >  #define SUN4I_SPI_MIN_RATE             3000
> >  #define SUN4I_SPI_DEFAULT_RATE         1000000
> > -#define SUN4I_SPI_TIMEOUT_US           1000000
> > +#define SUN4I_SPI_TIMEOUT_MS           1000
> >  
> >  #define SPI_REG(priv, reg)             ((priv)->base + \
> >                                         (priv)->variant->regs[reg])
> > @@ -326,7 +326,6 @@ static int sun4i_spi_xfer(struct udevice *dev,
> > unsigned int bitlen,
> >         struct dm_spi_slave_plat *slave_plat =
> > dev_get_parent_plat(dev);
> >  
> >         u32 len = bitlen / 8;
> > -       u32 rx_fifocnt;
> >         u8 nbytes;
> >         int ret;
> >  
> > @@ -364,13 +363,10 @@ static int sun4i_spi_xfer(struct udevice
> > *dev, unsigned int bitlen,
> >                 setbits_le32(SPI_REG(priv, SPI_TCR),
> >                              SPI_BIT(priv, SPI_TCR_XCH));
> >  
> > -               /* Wait till RX FIFO to be empty */
> > -               ret = readl_poll_timeout(SPI_REG(priv, SPI_FSR),
> > -                                        rx_fifocnt,
> > -                                        (((rx_fifocnt &
> > -                                        SPI_BIT(priv,
> > SPI_FSR_RF_CNT_MASK)) >>
> > -                                       
> > SUN4I_FIFO_STA_RF_CNT_BITS) >= nbytes),
> > -                                        SUN4I_SPI_TIMEOUT_US);
> > +               /* Wait for the transfer to be done */
> > +               ret = wait_for_bit_le32((const void *)SPI_REG(priv,
> > SPI_TCR),
> > +                                       SPI_BIT(priv, SPI_TCR_XCH),
> > +                                       false,
> > SUN4I_SPI_TIMEOUT_MS, false);
> >                 if (ret < 0) {
> >                         printf("ERROR: sun4i_spi: Timeout
> > transferring data\n");
> >                         sun4i_spi_set_cs(bus, slave_plat->cs,
> > false);
>