Received-SPF: pass (google.com: domain of linux-kernel+bounces-63267-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <92e7e0acf6d8746a07729924982acbfea777c468.camel@gmail.com>
Subject: Re: [PATCH 1/5] spi: add spi_optimize_message() APIs
From: Nuno =?ISO-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
To: David Lechner <dlechner@baylibre.com>, Mark Brown <broonie@kernel.org>
Cc: Martin Sperl <kernel@martin.sperl.org>, David Jander
 <david@protonic.nl>,  Jonathan Cameron <jic23@kernel.org>, Michael
 Hennerich <michael.hennerich@analog.com>, Nuno =?ISO-8859-1?Q?S=E1?=
 <nuno.sa@analog.com>, Alain Volmat <alain.volmat@foss.st.com>, Maxime
 Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue
 <alexandre.torgue@foss.st.com>,  linux-spi@vger.kernel.org,
 linux-kernel@vger.kernel.org,  linux-stm32@st-md-mailman.stormreply.com,
 linux-arm-kernel@lists.infradead.org,  linux-iio@vger.kernel.org
Date: Tue, 13 Feb 2024 10:53:56 +0100
In-Reply-To: <20240212-mainline-spi-precook-message-v1-1-a2373cd72d36@baylibre.com>
References: 
	<20240212-mainline-spi-precook-message-v1-0-a2373cd72d36@baylibre.com>
	 <20240212-mainline-spi-precook-message-v1-1-a2373cd72d36@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 
Precedence: bulk
MIME-Version: 1.0

On Mon, 2024-02-12 at 17:26 -0600, David Lechner wrote:
> This adds a new spi_optimize_message() function that can be used to
> optimize SPI messages that are used more than once. Peripheral drivers
> that use the same message multiple times can use this API to perform SPI
> message validation and controller-specific optimizations once and then
> reuse the message while avoiding the overhead of revalidating the
> message on each spi_(a)sync() call.
>=20
> Internally, the SPI core will also call this function for each message
> if the peripheral driver did not explicitly call it. This is done to so
> that controller drivers don't have to have multiple code paths for
> optimized and non-optimized messages.
>=20
> A hook is provided for controller drivers to perform controller-specific
> optimizations.
>=20
> Suggested-by: Martin Sperl <kernel@martin.sperl.org>
> Link:
> https://lore.kernel.org/linux-spi/39DEC004-10A1-47EF-9D77-276188D2580C@ma=
rtin.sperl.org/
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> =C2=A0drivers/spi/spi.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 145 +++++++=
+++++++++++++++++++++++++++++++++++++++-
> -
> =C2=A0include/linux/spi/spi.h |=C2=A0 19 +++++++
> =C2=A02 files changed, 160 insertions(+), 4 deletions(-)
>=20
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index c2b10e2c75f0..5bac215d7009 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2106,6 +2106,41 @@ struct spi_message *spi_get_next_queued_message(st=
ruct
> spi_controller *ctlr)
> =C2=A0}
> =C2=A0EXPORT_SYMBOL_GPL(spi_get_next_queued_message);
> =C2=A0
> +/**
> + * __spi_unoptimize_message - shared implementation of
> spi_unoptimize_message()
> + *=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 and spi_maybe_unoptimize_message()
> + * @msg: the message to unoptimize
> + *
> + * Periperhal drivers should use spi_unoptimize_message() and callers in=
side
> + * core should use spi_maybe_unoptimize_message() rather than calling th=
is
> + * function directly.
> + *
> + * It is not valid to call this on a message that is not currently optim=
ized.
> + */
> +static void __spi_unoptimize_message(struct spi_message *msg)
> +{
> +	struct spi_controller *ctlr =3D msg->spi->controller;
> +
> +	if (ctlr->unoptimize_message)
> +		ctlr->unoptimize_message(msg);
> +
> +	msg->optimized =3D false;
> +	msg->opt_state =3D NULL;
> +}
> +
> +/**
> + * spi_maybe_unoptimize_message - unoptimize msg not managed by a periph=
eral
> + * @msg: the message to unoptimize
> + *
> + * This function is used to unoptimize a message if and only if it was
> + * optimized by the core (via spi_maybe_optimize_message()).
> + */
> +static void spi_maybe_unoptimize_message(struct spi_message *msg)
> +{
> +	if (!msg->pre_optimized && msg->optimized)
> +		__spi_unoptimize_message(msg);
> +}
> +
> =C2=A0/**
> =C2=A0 * spi_finalize_current_message() - the current message is complete
> =C2=A0 * @ctlr: the controller to return the message to
> @@ -2153,6 +2188,8 @@ void spi_finalize_current_message(struct spi_contro=
ller
> *ctlr)
> =C2=A0
> =C2=A0	mesg->prepared =3D false;
> =C2=A0
> +	spi_maybe_unoptimize_message(mesg);
> +
> =C2=A0	WRITE_ONCE(ctlr->cur_msg_incomplete, false);
> =C2=A0	smp_mb(); /* See __spi_pump_transfer_message()... */
> =C2=A0	if (READ_ONCE(ctlr->cur_msg_need_completion))
> @@ -4194,6 +4231,99 @@ static int __spi_validate(struct spi_device *spi,
> struct spi_message *message)
> =C2=A0	return 0;
> =C2=A0}
> =C2=A0
> +/**
> + * __spi_optimize_message - shared implementation for spi_optimize_messa=
ge()
> + *=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 and spi_maybe_optimize_message()
> + * @spi: the device that will be used for the message
> + * @msg: the message to optimize
> + * @pre_optimized: whether the message is considered pre-optimized or no=
t
> + *
> + * Peripheral drivers will call spi_optimize_message() and the spi core =
will
> + * call spi_maybe_optimize_message() instead of calling this directly.
> + *
> + * It is not valid to call this on a message that has already been optim=
ized.
> + *
> + * Return: zero on success, else a negative error code
> + */
> +static int __spi_optimize_message(struct spi_device *spi,
> +				=C2=A0 struct spi_message *msg,
> +				=C2=A0 bool pre_optimized)
> +{
> +	struct spi_controller *ctlr =3D spi->controller;
> +	int ret;
> +
> +	ret =3D __spi_validate(spi, msg);
> +	if (ret)
> +		return ret;
> +
> +	if (ctlr->optimize_message) {
> +		ret =3D ctlr->optimize_message(msg);
> +		if (ret)
> +			return ret;
> +	}

Not really sure what are the spi core guarantees or what controllers should=
 be
expecting but I'll still ask :). Do we need to care about locking in here?
Mainly on the controller callback? For spi device related data I guess it's=
 up
to the peripheral driver not to do anything weird or to properly protect th=
e spi
message?

- Nuno S=C3=A1