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

[Frieder Schrempf]

Hi Miquèl,

On 31.03.26 11:23, Miquel Raynal wrote:
> 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.

Ah, ok. I didn't know the PHY mode bypasses the usual clock setup.

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

Hm, I've seen it the other way round. In my perspective the DT max
frequency describes the maximum clock frequency supported by the chip as
given in the datasheet (assuming there are no other limitations
introduced by board design, etc.).

The RX sampling delay is a timing parameter specified in the datasheet
and it is relevant in order to interface the chip at maximum frequency
within spec.

Your approach would mean, that we need to manually calculate the maximum
supported clock frequency without RX sampling delay compensation and use
this value in the DT. Then let the driver increase the clock if RX
sampling delay compensation is available.

This would have the advantage, that even before initial detection we
would use the correct clock by default. But it has the downside that the
clock value in DT won't match the datasheet value.

In my approach we assume the driver is able to use the maximum clock
from the DT (same as in datasheet) by using RX sampling delay
compensation and only if it turns out the compensation is not possible
we will lower the clock dynamically.

What if we do the following?

1. Add a parameter in the SPI NAND chip driver that contains the maximum
supported clock frequency as given in the datasheet as this is clearly
the best place to keep this device-specific parameter.

2. Allow to leave spi-max-frequency in DT unset for SPI NANDs in case
there are no board-specific limitations and therefore the maximum
frequency supported by the combination of chip and controller can be used.

3. By default limit the clock frequency for the READ_ID op to a safe
value that works for all chips and controllers, even if RX sampling
delay compensations is not available.

4. In PHY mode, check against the DT max frequency (board limitations)
and chip max frequency before switching to this mode.

I hope this makes at least a bit of sense. I'm really not yet familiar
with all the different use-cases and limitations.

Best regards
Frieder