在 2022-07-11星期一的 00:03 +0100,Andre Przywara写道: > On Tue, 28 Jun 2022 14:49:24 +0800 > Icenowy Zheng <u...@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 <u...@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); >