Re: [PATCH] spi: atmel: Prevent false timeouts on long transfers

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hi Mark,

broonie@xxxxxxxxxx wrote on Fri, 16 Jun 2023 17:43:06 +0100:

> On Fri, Jun 16, 2023 at 06:15:35PM +0200, Miquel Raynal wrote:
> > broonie@xxxxxxxxxx wrote on Fri, 16 Jun 2023 15:20:27 +0100:  
> 
> > > On Fri, Jun 16, 2023 at 04:12:25PM +0200, Miquel Raynal wrote:  
> 
> > > > -#define SPI_DMA_TIMEOUT		(msecs_to_jiffies(1000))
> > > > +#define SPI_DMA_MIN_TIMEOUT	(msecs_to_jiffies(1000))
> > > > +#define SPI_DMA_TIMEOUT_PER_10K	(msecs_to_jiffies(4))    
> 
> > > Given that we know the bus speed can't we just calculate this like other
> > > drivers do (we should probably add a helper TBH)?  
> 
> > I agree we should probably have some kind of easy-to-use helper to
> > derive a decent timeout value. How do sound the heuristics
> > proposed here to you ? That would be:  
> 
> > 	timeout = 1s + 4ms/10k  
> 
> Like I say we should know the transfer speed so we can do better than
> 4ms/10k - we know how long it takes to clock out each byte, we can just
> multiply that by the size of the transfer then add some fudge factor for
> setup/teardown overhead.  1s feels pretty generous too.  The sun6i
> driver for example does 
> 
>    max(tfr->len * 8 * 2 / (tfr->speed_hz / 1000), 100U)
> 
> and just doubles the length based timeout with a minimum of 100ms which
> seems reasonable.

I already had issues with ~0.1s timeouts on NAND controllers, just
because the machine was heavily loaded. I believe we should avoid too
small timeouts, it does not make sense and make things worse under load.

I'll have a look.

Thanks,
Miquèl