Received-SPF: pass (google.com: domain of linux-kernel+bounces-63821-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240212-mainline-spi-precook-message-v1-0-a2373cd72d36@baylibre.com>
 <20240212-mainline-spi-precook-message-v1-1-a2373cd72d36@baylibre.com> <92e7e0acf6d8746a07729924982acbfea777c468.camel@gmail.com>
In-Reply-To: <92e7e0acf6d8746a07729924982acbfea777c468.camel@gmail.com>
From: David Lechner <dlechner@baylibre.com>
Date: Tue, 13 Feb 2024 09:38:29 -0600
Message-ID: <CAMknhBG0LY+xiaK6qXcRj5_UAnTUAk0h36qO1qYWYR-MN5ajzA@mail.gmail.com>
Subject: Re: [PATCH 1/5] spi: add spi_optimize_message() APIs
To: =?UTF-8?B?TnVubyBTw6E=?= <noname.nuno@gmail.com>
Cc: Mark Brown <broonie@kernel.org>, Martin Sperl <kernel@martin.sperl.org>, 
	David Jander <david@protonic.nl>, Jonathan Cameron <jic23@kernel.org>, 
	Michael Hennerich <michael.hennerich@analog.com>, =?UTF-8?B?TnVubyBTw6E=?= <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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 13, 2024 at 3:50=E2=80=AFAM Nuno S=C3=A1 <noname.nuno@gmail.com=
> wrote:
>
> 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 SP=
I
> > 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.
> >
> > 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.
> >
> > A hook is provided for controller drivers to perform controller-specifi=
c
> > optimizations.
> >
> > Suggested-by: Martin Sperl <kernel@martin.sperl.org>
> > Link:
> > https://lore.kernel.org/linux-spi/39DEC004-10A1-47EF-9D77-276188D2580C@=
martin.sperl.org/
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> >  drivers/spi/spi.c       | 145 ++++++++++++++++++++++++++++++++++++++++=
++++++-
> > -
> >  include/linux/spi/spi.h |  19 +++++++
> >  2 files changed, 160 insertions(+), 4 deletions(-)
> >
> > 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(=
struct
> > spi_controller *ctlr)
> >  }
> >  EXPORT_SYMBOL_GPL(spi_get_next_queued_message);
> >
> > +/**
> > + * __spi_unoptimize_message - shared implementation of
> > spi_unoptimize_message()
> > + *                            and spi_maybe_unoptimize_message()
> > + * @msg: the message to unoptimize
> > + *
> > + * Periperhal drivers should use spi_unoptimize_message() and callers =
inside
> > + * core should use spi_maybe_unoptimize_message() rather than calling =
this
> > + * function directly.
> > + *
> > + * It is not valid to call this on a message that is not currently opt=
imized.
> > + */
> > +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 peri=
pheral
> > + * @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);
> > +}
> > +
> >  /**
> >   * spi_finalize_current_message() - the current message is complete
> >   * @ctlr: the controller to return the message to
> > @@ -2153,6 +2188,8 @@ void spi_finalize_current_message(struct spi_cont=
roller
> > *ctlr)
> >
> >       mesg->prepared =3D false;
> >
> > +     spi_maybe_unoptimize_message(mesg);
> > +
> >       WRITE_ONCE(ctlr->cur_msg_incomplete, false);
> >       smp_mb(); /* See __spi_pump_transfer_message()... */
> >       if (READ_ONCE(ctlr->cur_msg_need_completion))
> > @@ -4194,6 +4231,99 @@ static int __spi_validate(struct spi_device *spi=
,
> > struct spi_message *message)
> >       return 0;
> >  }
> >
> > +/**
> > + * __spi_optimize_message - shared implementation for spi_optimize_mes=
sage()
> > + *                          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 =
not
> > + *
> > + * Peripheral drivers will call spi_optimize_message() and the spi cor=
e 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 opt=
imized.
> > + *
> > + * Return: zero on success, else a negative error code
> > + */
> > +static int __spi_optimize_message(struct spi_device *spi,
> > +                               struct spi_message *msg,
> > +                               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 shou=
ld 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 =
the spi
> message?
>

Currently, it is expected that this operates only on the message
struct and doesn't poke any hardware so no locking is currently
required. And, yes, it is up to peripheral drivers that opt in to
pre-optimization to follow the rules of not touching the message while
it is in the optimized state. For peripheral drivers that don't call
spi_optimized_message(), nothing has really changed.