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

[Frieder Schrempf]

On 31.03.26 17:26, Miquel Raynal wrote:
> Hello,
> 
>>>> 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.).
> 
> I believe that what is in the datasheet should instead be statically
> described in the SPI NAND vendor file.
> 
> The max SPI frequency DT property is here to tell at which speed to use
> the device, and this depends mostly on the routing.

Ok, agree.

> 
>> 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.
> 
> If it's not design dependent, then we should handle it "statically".

The RX delay introduced by the chip is not design-dependent, only
chip-dependent. There might be additional delays introduced by the board
layout, but that's out of scope currently and I don't know if we even
need this.

> 
>> 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.
> 
> It never does. Winbond chips can run at 166MHz. I know no design capable
> of that natively (ie. without finer tuning, like what Santhosh
> proposes).

Ok, right.

> 
>> 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.
> 
> I've so far avoided doing it, but yes, this is something we can do. It
> is going to be simple enough to implement as all the infrastructure is
> already there. You can provide variants with speed limitations (look
> into winbond.c). I was writing "0" in the fields which didn't need a
> limitation that is something else than the maximum speed the chip can
> sustain, as anyway the DT property *will* tell us what is this speed, if
> we are ever able to reach it.

Ok.

> 
>> 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.
> 
> In many cases, the limitations are board specific. Anyway, this is
> already the case, you may just not put any value in this property.

Ok.

> 
>> 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.
> 
> I am sorry this is not going to work out. There is no such harmonized
> speed, differences can be an order (or two) of magnitude different, we
> do not want to pay the penalty of a very slow identification on all
> designs. We currently do the read several times with different
> parameters. What if we were trying one more time by cutting the speed by
> 2 (completely random proposal)?

If we try with reduced clock as a fallback after the other attempts,
there would be a slight chance of one of the earlier attempts with
normal clock rate returning some invalid data that matches an existing
chip ID, no? Isn't this a bit flaky?

Let's say for a worst case scenario a chip has an RX delay of 20ns (the
highest I've seen in datasheets so far is 8ns). In that case the maximum
clock we could safely use for reading the ID would be 1/(2*20e-9) =
25MHz. Do you think it really makes much of a difference if we read the
ID (only a handful of bytes) at 25MHz or full speed (e. g. 104 MHz)? I
mean this should be fast enough either way, no? But maybe I'm misjudging
this.

> 
>> 4. In PHY mode, check against the DT max frequency (board limitations)
>> and chip max frequency before switching to this mode.
> 
> There is not such thing :-) the max frequency in DT currently would be
> set to the platform limitation. We need somehow another parameter
> indicating what is the maximum speed if we can fine tune the timings.

So we can exceed the platform limitations using fine-tuned timings? I
guess I need to read and understand that tuning RFC series.

> 
>> I hope this makes at least a bit of sense. I'm really not yet familiar
>> with all the different use-cases and limitations.
> 
> If I get back to your own issue. Is there any chance we could avoid
> tweaking the DT for handling it? Would there be a configuration of your
> controller that would allow reading the ID of all the chips with enough
> delays? Currently, the spi core doesn't care much about other parameters
> than the frequency, but there is perhaps room for improvement.

We could use RX delay compensation (one half clock cycle) in the
controller by default (as currently done by the STM32 driver). But that
would break if we use a very low clock (for whatever reason) because
then the RX sampling delay gets neglectable compared to the clock period
and sampling is triggered at the very end of the clock cycle when the
data might already be invalid. At least that's what I suspect based on
my test results.
And it would also break if the chip actually requires more than one half
clock cycle of RX sampling delay.

The RX sampling delay setting must match the used clock frequency. In
most cases there is a lot of room for tolerance before things start to
fail, but it's not endless and especially at very high clock rates we
need to take it into account.

So if we don't cap the speed for reading the ID by default, we somehow
need to know what value to use for the RX sampling delay or we need to
try different values. And if the controller does not support or
implement the RX sampling delay compensation, we need to cap the speed
anyway.