Re: [PATCH 3/5] spi: core: allow defining time that cs is deasserted as a multiple of SCK

[kernel@xxxxxxxxxxxxxxxx]

Hi Lukas!

> On 23.02.2019, at 13:40, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> 
> On Sat, Feb 23, 2019 at 08:49:50AM +0000, kernel@xxxxxxxxxxxxxxxx wrote:
>> Support setting a delay between cs assert and deassert as
>> a multiple of spi clock length.
> 
> To what end?
...
> 
> The ability to define a unit seems somewhat over-engineered to me.
> You're introducing lots of branching and integer divisions (both
> comparatively expensive operations) which are executed for every
> single transfer (i.e. in a hot path).  I'd recommend standardizing
> on a single unit.  If 1 usec is too coarse-grained for your use case,
> convert everything to nsec.

Well - a spi_device driver does not actually know what is the
actual spi clock configured/used by the spi-bus-driver.

All it knows is that the device requires X clock cycles of cs high.
So how would you define that so that is valid for all cases without
replicating code in every driver that needs it (everywhere slightly
differently)?

As for hot-path: we are not in the hot path - we need to waste time
when any of those parameters are set!

cs_change is not that commonly used - or cs_delay_usec for that matter.

The corresponding ifs int the hot path are in the code right now - 
worsted case we are jumping a few more cpu instructions further than we
would right now.

Also the code has been moved into a separate function and some of it
is used from multiple locations.

Thus it is up to the compiler to know best if it is worth inlining the 
function or just calling it.

>From a CPU utilization perspective: for most situations we will
(most likely) be busy waiting to pass the required time anyway.
So those divides are just taking time from those busy cycles…

Finally if the computation yields a shorter value than a (conservative)
time-value that would have been given instead, then those computations
(divides)  have helped save cpu cycles and SPI bus bandwidth, so they
are beneficial.

As for the rpi-mailing-list: if this is an issue, then this is a
separate issue that needs to get addressed separately as it is not
really code-related.

Martin