Re: [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