Re: [PATCH RFC 6/7] spi: spi-mem: Call spi_set_rx_sampling_point() for each op

[Miquel Raynal]

Hi Frieder,

> The sampling point delay is coupled to the clock frequency. So if the
> clock changes per-op, we also need to change the sampling delay per-op.
>
> In general, if we want to avoid switching the parameters back and forth
> in cases of alternating ops with different max frequencies, we should
> maybe do some "min of the max" calculation and use the resulting
> frequency for all ops.
>
> Is that what you mean?

Not exactly, I am not afraid by the time it would take to switch, I am
afraid by the likelihood that both the PHY tuning series and your series
might want to force a different maximum frequency. For instance with TI
designs, when entering PHY mode, the frequency is 166MHz, period. You
cannot lower it because by design, you bypass the SPI divisors.

> We could even set a threshold to decide between using a common "min of
> the max" frequency or do the switching per-op.
>
> One other possibility would be to somehow cache the per-op frequencies
> and calculated sampling delay values so they can be reused when
> switching without much overhead.
>
> There is one more issue that is not yet covered by this series: Before
> spinand_id_detect() we don't know yet what RX sampling delay value the
> chip requires, but we already use read-status and read-id operations at
> maximum chip clock.

Not exactly "maximum chip clock" as in Santhosh's work, but indeed we
run these at the frequency given in the DT as bein the maximum
frequency. If that's not correct, you must lower it in the DT. That is
why I am in favour of having two maximum frequencies: the standard one,
that just works, which is the one for non optimized settings (the one we
actually use today) and another one, when tuning the bus timings.

> For example on Winbond W25N04KV this leads to detection failures. So we
> should maybe introduce some kind of reduced clock setpoint for the
> initial detection, that is safe to be used without RX sampling delay
> compensation.

That should be the DT max frequency, no?

Thanks,
Miquèl