commit "dm: spi: Avoid setting the speed with every transfer"

[Rasmus Villemoes]

On 28/12/2019 03.27, Simon Glass wrote:
> Hi Rasmus,
> 
> On Sat, 14 Dec 2019 at 16:27, Rasmus Villemoes
> <rasmus.villemoes at prevas.dk> wrote:
>>
>> Hi
>>
>> I'm wondering how commit 60e2809a84 (dm: spi: Avoid setting the speed
>> with every transfer) works. AFAIU, the currently selected speed is a
>> property of the SPI master, so suppose we have two slaves, A which has
>> max_hz = 10MHz and B which has max_hz = 20MHz. Now suppose we do
>> transfers to A, then B, then A again. The third time we still compute
>> speed = 10MHz, but since that matches what we cached in slave->speed, we
>> don't call spi_set_speed_mode(), hence we proceed to do the xfer with a
>> too large speed?
>>
>> I'm probably missing something obvious here since the commit is more
>> than 4 years old.
> 
> I think you are right.
> 
> Possible we could store the 'current' bus speed in dm_spi_bus and
> check that against what the slave wants?

Maybe. But I'm wondering if its better left to the individual drivers.
Some anyway have a somewhat limited range of dividers or whatnot, so
speeds of 10 MHz and 15 MHz might both in practice result in 8 MHz - and
we can leave it to the driver to avoid updating registers etc. if
there's nothing to actually change. But clearly that can't be done from
the core.

But sure, if we know the last set_speed for a given master was with an
identical speed as the one we now set (which is probably a very common
case), we can avoid the driver doing the somewhat expensive integer
arithmetic (usually a couple of divisions with non-constant
denominator). So I don't think there's anything wrong with the caching,
it just has to live with the master.

Rasmus