Hi Ben, On Tue, Oct 01, 2024 at 08:01:48AM GMT, Ben Dooks wrote: > On 30/09/2024 23:51, Andi Shyti wrote: > > On Tue, Sep 24, 2024 at 02:40:09PM GMT, Ben Dooks wrote: > > > The code that checks for loops in the s3c6xx_flush_fifo() checks > > > for loops being non-zero as a timeout, however the code /could/ > > > finish with loops being zero and the fifo being flushed... > > > > what is the possibility of this case? > > Not sure, currently we're trying to debug a customer's setup where > we're seeing some weird issues with SPI. This was found during a > look into the code awaiting hardware access. > > The flush count was simply an inspection and it seemed like a good > idea to fix the initial issue and then if there was an issue to > print something more useful than a simple error message. > > > > Also, it would be useful to know what is left in the fifo for this > > > error case, so update the checks to see what is left, and then also > > > print the number of entries. > > > > > > Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> > > > --- > > > drivers/spi/spi-s3c64xx.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > > > index 6ab416a33966..7b244e1fd58a 100644 > > > --- a/drivers/spi/spi-s3c64xx.c > > > +++ b/drivers/spi/spi-s3c64xx.c > > > @@ -247,8 +247,8 @@ static void s3c64xx_flush_fifo(struct s3c64xx_spi_driver_data *sdd) > > > val = readl(regs + S3C64XX_SPI_STATUS); > > > } while (TX_FIFO_LVL(val, sdd) && --loops); > > > - if (loops == 0) > > > - dev_warn(&sdd->pdev->dev, "Timed out flushing TX FIFO\n"); > > > + if (TX_FIFO_LVL(val, sdd)) > > > + dev_warn(&sdd->pdev->dev, "Timed out flushing TX FIFO (%d left)\n", TX_FIFO_LVL(val, sdd)); > > > /* Flush RxFIFO*/ > > > loops = msecs_to_loops(1); > > > @@ -260,8 +260,8 @@ static void s3c64xx_flush_fifo(struct s3c64xx_spi_driver_data *sdd) > > > break; > > > } while (--loops); > > > - if (loops == 0) > > > - dev_warn(&sdd->pdev->dev, "Timed out flushing RX FIFO\n"); > > > + if (RX_FIFO_LVL(val, sdd)) > > > + dev_warn(&sdd->pdev->dev, "Timed out flushing RX FIFO (%d left)\n", RX_FIFO_LVL(val, sdd)); > > > > This change doesn't super excite me, but it's fine. Please add a > > comment explaining the case when loops is '0' and the FIFO is > > flushed. > > > > With the comment given, you can have my r-b. > > Ok, will look at sending a second version later this week. I actually think that these two patches can be squashed into a single one, I don't see much reason for having it double. Even better, I think the first patch is not needed at all with this new one. Right? Andi > > > > Thanks, > > Andi > > > > > val = readl(regs + S3C64XX_SPI_CH_CFG); > > > val &= ~S3C64XX_SPI_CH_SW_RST; > > > -- > > > 2.37.2.352.g3c44437643 > > > > > > > > -- > Ben Dooks http://www.codethink.co.uk/ > Senior Engineer Codethink - Providing Genius > > https://www.codethink.co.uk/privacy.html
