Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Fri, 27 Oct 2023 00:56:43 +0200
From:   Miquel Raynal <miquel.raynal@bootlin.com>
To:     Eberhard Stoll <estl@gmx.net>
Cc:     linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org,
        Mark Brown <broonie@kernel.org>,
        Eberhard Stoll <eberhard.stoll@kontron.de>,
        Frieder Schrempf <frieder.schrempf@kontron.de>,
        Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Krishna Yarlagadda <kyarlagadda@nvidia.com>,
        Leonard =?UTF-8?B?R8O2aHJz?= <l.goehrs@pengutronix.de>,
        Yang Yingliang <yangyingliang@huawei.com>
Subject: Re: [PATCH 1/4] spi: Add parameter for clock to rx delay
Message-ID: <20231027005643.4b95f17e@xps-13>
In-Reply-To: <20231026152316.2729575-2-estl@gmx.net>
References: <20231026152316.2729575-1-estl@gmx.net>
        <20231026152316.2729575-2-estl@gmx.net>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hello,

estl@gmx.net wrote on Thu, 26 Oct 2023 17:23:02 +0200:

> From: Eberhard Stoll <eberhard.stoll@kontron.de>
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> The 'clock to receive data delay' value is often referenced as tCLQV
> in spi device data sheets.
>=20
> Signed-off-by: Eberhard Stoll <eberhard.stoll@kontron.de>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
> ---
>  include/linux/spi/spi.h | 3 +++
>  1 file changed, 3 insertions(+)
>=20
> 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 */
>=20
>  	/* The statistics */
>  	struct spi_statistics __percpu	*pcpu_statistics;
> --
> 2.25.1
>=20


Thanks,
Miqu=C3=A8l