[PATCH 2/2] spi: s3c64xx: update flush_fifo timeout code

[Ben Dooks]

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 at codethink.co.uk>
>> ---
>>   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.

> 
> 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