[PATCH 0/3] spi: spi-cadence-quadspi: Add Rx tuning support for DTR mode

[Michael Walle]

Am 2023-02-07 07:48, schrieb Tudor Ambarus:
> On 2/7/23 06:09, Sai Krishna Potthuri wrote:
>> Enable PHY and DQS required for Xilinx Versal Octal SPI to operate in 
>> DTR
>> protocol.
>> Add and update device_id field in spi_mem structure with flash id
>> information. Xilinx Versal Octal SPI driver requires the device id
>> information to perform the Rx tuning operation. Since there is no 
>> common
>> Tuning Data Pattern defined across all vendors, controllers like 
>> Xilinx
>> Versal Octal SPI which requires Rx tuning to find out the optimal 
>> sampling
>> point for data lines, this device id information will be used as a 
>> golden
>> data.
> 
> Using only 6 bytes as golden pattern seems fragile, but you are aware 
> of
> that, as I see that you chose to read the ID 10 times to make the
> decision whether the tap is valid or not. Other option (which is not
> perfect) is to use SFDP data as golden pattern. If I remember
> correctly, JESD216 suggests to use the Read SFDP cmd at 50 MHz, so it
> won't help you much. In practice SPI NOR uses the Read SFDP command at
> the flash's maximum speed and we haven't seen problems. But better 
> would
> be to use some flash OTP data maybe? I remember Pratyush has submitted 
> a
> phy calibration series in the past, I haven't had the chance to read 
> his
> proposal. Did you? How's your proposal different than his?

And its not 6 bytes.. it's usually only three. The last three bytes will
probably be undefined. So the might return ff or just wrap around and
return the first three bytes again.

Is there a datasheet where you can read how the calibration is done? Is 
this
the same for all i/o pads or individual per i/o pad?

I cannot see where the op to read the id is coming from. Are you relying
on the fact that a RDID is the first command which gets executed. If so,
please don't.

Do you calibrate only one pad? RDID (9f) is  single bit i/o, right? And
I guess you are calibrating with the highest frequency, are
we sure that RDID will work with any frequency (on any flash).


>> The reason behind choosing this approach instead of reading the ID 
>> again
>> in the controller driver is to make it generic solution.
>> - Other controller drivers which want to use similar tuning process, 
>> they
>> will make use of this ID instead of reading the ID again in the 
>> driver.

Honestly, I'm not sure this is the way to go. Pratyush proposed solution
to have a dedicated memory area within the flash array with a know 
pattern
seems to make more sense, because you are calibrating on the thing you 
are going
to use later, that is quad/ocal read with the fastest frequency.

>> - Also, we can avoid hardcoding the command information and initiating 
>> the
>> transfer in the controller driver as this should happen from spi-nor.

So how you know that this is a RDID instruction?

-michael