[U-Boot] [PATCH] arm: timer: sunxi: fix a64 spurious timeout issues

[Jagan Teki]

On Fri, Mar 22, 2019 at 1:55 AM Oskari Lemmelä <oskari at lemmela.net> wrote:
>
> On 3/17/19 11:14 PM, André Przywara wrote:
> > On 17/03/2019 18:41, Oskari Lemmelä wrote:
> >> On 3/17/19 6:04 PM, Peter Robinson wrote:
> >>> On Sun, Mar 17, 2019 at 2:56 PM Oskari Lemmela <oskari at lemmela.net>
> >>> wrote:
> >>>> Fixes spurious timeouts which have been seen during testing
> >>>> SPI_SUNXI driver. The false timeouts disappear when number of
> >>>> bits reduced to 10 in workaround.
> > So in general I am fine with this patch, if it fixes things for you, but
> > still scratching my head about this.
> > AFAIK Samuel has never seen less than 11 identical bits in his testing,
> > and I am using the new SPI driver for some weeks now (without the patch)
> > and never had any issues. I am on the Pine64 LTS (with that "R18" SoC).
> >
> > So Oskari, can you share how exactly you triggered this problem?
> > I'd rather know what's going on before papering over the issue,
> > especially if that leaves the much more important Linux kernel still at
> > risk.
>
> Actually now when I'm trying to retrigger it seems to be hard to catch.
> I was running following loop couple of days without any issues
>
> setenv read_loop 'setenv i 0; while sf read ${kernel_addr_r} 0; do
> setexpr i ${i} + 1; echo run ${i}; done'
>
> Then I changed wait loop code in spi driver to original one.

W/o this timer patch change?

>
> diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
> index dbfeac77ee..6aaa79ddd9 100644
> --- a/drivers/spi/spi-sunxi.c
> +++ b/drivers/spi/spi-sunxi.c
> @@ -377,14 +377,11 @@ 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);
> -               if (ret < 0) {
> +               /* Wait transfer to complete */
> +               u32 *tcr = SPI_REG(priv, SPI_TCR);
> +               ret = wait_for_bit_le32(tcr, 0x80000000,

This look strange, to me since we are relying on poll transfer where
the existing code is checking fifo count which seems valid.

> +                                       false, SUN4I_SPI_TIMEOUT_US, false);
> +               if (ret) {
>                          printf("ERROR: sun4i_spi: Timeout transferring
> data\n");
>                          sun4i_spi_set_cs(bus, slave_plat->cs, false);
>                          return ret;
>
> Don't know it that was difference but then only one of my boards trigger
> it two times in row.
>
> => setenv read_loop 'setenv i 0; while sf read ${kernel_addr_r} 0; do
> setexpr i ${i} + 1; echo run ${i}; done'

let me test it.