Re: [PATCH 1/4] spi: Add parameter for clock to rx delay

[Miquel Raynal <miquel.raynal@xxxxxxxxxxx>]

Hello,

estl@xxxxxxx wrote on Thu, 26 Oct 2023 17:23:02 +0200:

> From: Eberhard Stoll <eberhard.stoll@xxxxxxxxxx>
> 
> For spi devices the master clock output defines the sampling point
> for receive data input stream (rising or falling edge). The receive
> data stream from the device is delayed in relation to the master
> clock output.
> 
> For some devices this delay is larger than one half clock period,

Can you be more specific? I am wondering how big the need is.

> which is normally the sampling point for receive data. In this case
> receive data is sampled too early and the device fails to operate.
> In consequence the spi clock has to be reduced to match the delay
> characteristics and this reduces performance and is therefore not
> recommended.
> 
> Some spi controllers implement a 'clock to receive data delay'
> compensation which shifts the receive sampling point. So we need
> a property to set this value for each spi device.

What if the spi controller does not support this feature? Shall we add
a capability? Shall we refuse to probe if the controller is not capable
of sampling at the right moment?

> Add a parameter 'rx_sample_delay_ns' to enable setting the clock
> to rx data delay for each spi device separately.
> 
> The 'clock to receive data delay' value is often referenced as tCLQV
> in spi device data sheets.
> 
> Signed-off-by: Eberhard Stoll <eberhard.stoll@xxxxxxxxxx>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
> ---
>  include/linux/spi/spi.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 7f8b478fdeb3..14622d47f44f 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -166,6 +166,7 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg,
>   * @cs_inactive: delay to be introduced by the controller after CS is
>   *	deasserted. If @cs_change_delay is used from @spi_transfer, then the
>   *	two delays will be added up.
> + * @rx_sample_delay_ns: spi clk to spi rx data delay
>   * @pcpu_statistics: statistics for the spi_device
>   *
>   * A @spi_device is used to interchange data between an SPI slave
> @@ -219,6 +220,8 @@ struct spi_device {
>  	struct spi_delay	cs_setup;
>  	struct spi_delay	cs_hold;
>  	struct spi_delay	cs_inactive;
> +	/* Transfer characteristics */
> +	u32			rx_sample_delay_ns; /* Clock to RX data delay */
> 
>  	/* The statistics */
>  	struct spi_statistics __percpu	*pcpu_statistics;
> --
> 2.25.1
> 


Thanks,
Miquèl