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.
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-specific
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 optimized.
+ */
+static void __spi_unoptimize_message(struct spi_message *msg)
+{
+ struct spi_controller *ctlr = msg->spi->controller;
+
+ if (ctlr->unoptimize_message)
+ ctlr->unoptimize_message(msg);
+
+ msg->optimized = false;
+ msg->opt_state = NULL;
+}
+
+/**
+ * spi_maybe_unoptimize_message - unoptimize msg not managed by a peripheral
+ * @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_controller *ctlr)
mesg->prepared = 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_message()
+ * 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 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 optimized.
+ *
+ * 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 = spi->controller;
+ int ret;
+
+ ret = __spi_validate(spi, msg);
+ if (ret)
+ return ret;
+
+ if (ctlr->optimize_message) {
+ ret = ctlr->optimize_message(msg);
+ if (ret)
+ return ret;
+ }
+
+ msg->pre_optimized = pre_optimized;
+ msg->optimized = true;
+
+ return 0;
+}
+
+/**
+ * spi_maybe_optimize_message - optimize message if it isn't already pre-optimized
+ * @spi: the device that will be used for the message
+ * @msg: the message to optimize
+ * Return: zero on success, else a negative error code
+ */
+static int spi_maybe_optimize_message(struct spi_device *spi,
+ struct spi_message *msg)
+{
+ if (msg->pre_optimized)
+ return 0;
+
+ return __spi_optimize_message(spi, msg, false);
+}
+
+/**
+ * spi_optimize_message - do any one-time validation and setup for a SPI message
+ * @spi: the device that will be used for the message
+ * @msg: the message to optimize
+ *
+ * Peripheral drivers that reuse the same message repeatedly may call this to
+ * perform as much message prep as possible once, rather than repeating it each
+ * time a message transfer is performed to improve throughput and reduce CPU
+ * usage.
+ *
+ * Once a message has been optimized, it cannot be modified with the exception
+ * of updating the contents of any xfer->tx_buf (the pointer can't be changed,
+ * only the data in the memory it points to).
+ *
+ * Calls to this function must be balanced with calls to spi_unoptimize_message()
+ * to avoid leaking resources.
+ *
+ * Context: can sleep
+ * Return: zero on success, else a negative error code
+ */
+int spi_optimize_message(struct spi_device *spi, struct spi_message *msg)
+{
+ return __spi_optimize_message(spi, msg, true);
+}
+EXPORT_SYMBOL_GPL(spi_optimize_message);
+
+/**
+ * spi_unoptimize_message - releases any resources allocated by spi_optimize_message()
+ * @msg: the message to unoptimize
+ *
+ * Calls to this function must be balanced with calls to spi_optimize_message().
+ *
+ * Context: can sleep
+ */
+void spi_unoptimize_message(struct spi_message *msg)
+{
+ __spi_unoptimize_message(msg);
+ msg->pre_optimized = false;
+}
+EXPORT_SYMBOL_GPL(spi_unoptimize_message);
+
static int __spi_async(struct spi_device *spi, struct spi_message *message)
{
struct spi_controller *ctlr = spi->controller;
@@ -4258,8 +4388,8 @@ int spi_async(struct spi_device *spi, struct spi_message *message)
int ret;
unsigned long flags;
- ret = __spi_validate(spi, message);
- if (ret != 0)
+ ret = spi_maybe_optimize_message(spi, message);
+ if (ret)
return ret;
spin_lock_irqsave(&ctlr->bus_lock_spinlock, flags);
@@ -4271,6 +4401,8 @@ int spi_async(struct spi_device *spi, struct spi_message *message)
spin_unlock_irqrestore(&ctlr->bus_lock_spinlock, flags);
+ spi_maybe_unoptimize_message(message);
+
return ret;
}
EXPORT_SYMBOL_GPL(spi_async);
@@ -4331,10 +4463,15 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
return -ESHUTDOWN;
}
- status = __spi_validate(spi, message);
- if (status != 0)
+ status = spi_maybe_optimize_message(spi, message);
+ if (status)
return status;
+ /*
+ * NB: all return paths after this point must ensure that
+ * spi_finalize_current_message() is called to avoid leaking resources.
+ */
+
SPI_STATISTICS_INCREMENT_FIELD(ctlr->pcpu_statistics, spi_sync);
SPI_STATISTICS_INCREMENT_FIELD(spi->pcpu_statistics, spi_sync);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 2b8e2746769a..f7a269f4956b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -475,6 +475,8 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch
*
* @set_cs: set the logic level of the chip select line. May be called
* from interrupt context.
+ * @optimize_message: optimize the message for reuse
+ * @unoptimize_message: release resources allocated by optimize_message
* @prepare_message: set up the controller to transfer a single message,
* for example doing DMA mapping. Called from threaded
* context.
@@ -715,6 +717,8 @@ struct spi_controller {
struct completion xfer_completion;
size_t max_dma_len;
+ int (*optimize_message)(struct spi_message *msg);
+ int (*unoptimize_message)(struct spi_message *msg);
int (*prepare_transfer_hardware)(struct spi_controller *ctlr);
int (*transfer_one_message)(struct spi_controller *ctlr,
struct spi_message *mesg);
@@ -1111,6 +1115,7 @@ struct spi_transfer {
* @spi: SPI device to which the transaction is queued
* @is_dma_mapped: if true, the caller provided both DMA and CPU virtual
* addresses for each transfer buffer
+ * @optimized: spi_optimize_message was called for the this message
* @prepared: spi_prepare_message was called for the this message
* @status: zero for success, else negative errno
* @complete: called to report transaction completions
@@ -1120,6 +1125,7 @@ struct spi_transfer {
* successful segments
* @queue: for use by whichever driver currently owns the message
* @state: for use by whichever driver currently owns the message
+ * @opt_state: for use by whichever driver currently owns the message
* @resources: for resource management when the SPI message is processed
*
* A @spi_message is used to execute an atomic sequence of data transfers,
@@ -1143,6 +1149,11 @@ struct spi_message {
unsigned is_dma_mapped:1;
+ /* spi_optimize_message() was called for this message */
+ bool pre_optimized;
+ /* __spi_optimize_message() was called for this message */
+ bool optimized;
+
/* spi_prepare_message() was called for this message */
bool prepared;
@@ -1172,6 +1183,11 @@ struct spi_message {
*/
struct list_head queue;
void *state;
+ /*
+ * Optional state for use by controller driver between calls to
+ * spi_optimize_message() and spi_unoptimize_message().
+ */
+ void *opt_state;
/* List of spi_res resources when the SPI message is processed */
struct list_head resources;
@@ -1255,6 +1271,9 @@ static inline void spi_message_free(struct spi_message *m)
kfree(m);
}
+extern int spi_optimize_message(struct spi_device *spi, struct spi_message *msg);
+extern void spi_unoptimize_message(struct spi_message *msg);
+
extern int spi_setup(struct spi_device *spi);
extern int spi_async(struct spi_device *spi, struct spi_message *message);
extern int spi_slave_abort(struct spi_device *spi);
--
2.43.0
On Mon, Feb 12, 2024 at 05:26:41PM -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. This looks basically fine. Some small comments: > +/** > + * __spi_unoptimize_message - shared implementation of spi_unoptimize_message() > + * and spi_maybe_unoptimize_message() > + * @msg: the message to unoptimize There's no need for kerneldoc for internal only functions and it can make the generated documentation a bit confusing for users. Just skip the /** for /*. > +static int __spi_optimize_message(struct spi_device *spi, > + struct spi_message *msg, > + bool pre_optimized) > +{ > + struct spi_controller *ctlr = spi->controller; > + int ret; > + > + ret = __spi_validate(spi, msg); > + if (ret) > + return ret; > + > + if (ctlr->optimize_message) { > + ret = ctlr->optimize_message(msg); > + if (ret) > + return ret; > + } > + > + msg->pre_optimized = pre_optimized; It would probably be clearer to name the parameter pre_optimising rather than pre_optimized, as it is the logic is a bit confusing. Either that or some comments. A similar issue applies on the cleanup path.
On Tue, Feb 13, 2024 at 12:55 PM Mark Brown <broonie@kernel.org> wrote: > > On Mon, Feb 12, 2024 at 05:26:41PM -0600, David Lechner wrote: > > > +static int __spi_optimize_message(struct spi_device *spi, > > + struct spi_message *msg, > > + bool pre_optimized) > > +{ > > + struct spi_controller *ctlr = spi->controller; > > + int ret; > > + > > + ret = __spi_validate(spi, msg); > > + if (ret) > > + return ret; > > + > > + if (ctlr->optimize_message) { > > + ret = ctlr->optimize_message(msg); > > + if (ret) > > + return ret; > > + } > > + > > + msg->pre_optimized = pre_optimized; > > It would probably be clearer to name the parameter pre_optimising rather > than pre_optimized, as it is the logic is a bit confusing. Either that > or some comments. A similar issue applies on the cleanup path. Per Jonathan's suggestion, I plan to remove the parameter from this function and handle this flag at the call site instead.
On Tue, Feb 13, 2024 at 01:26:02PM -0600, David Lechner wrote: > On Tue, Feb 13, 2024 at 12:55 PM Mark Brown <broonie@kernel.org> wrote: > > It would probably be clearer to name the parameter pre_optimising rather > > than pre_optimized, as it is the logic is a bit confusing. Either that > > or some comments. A similar issue applies on the cleanup path. > Per Jonathan's suggestion, I plan to remove the parameter from this > function and handle this flag at the call site instead. That works too.
On Mon, 12 Feb 2024 17:26:41 -0600 David Lechner <dlechner@baylibre.com> 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. > > 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-specific > 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> A few trivial things inline but looks good to me in general. I thought about suggesting splitting this into an initial patch that just does the bits without the controller callbacks. Maybe it would work better that way with that introduced after the validate and splitting of transfers (so most of patches 1 and 2) as a patch 3 prior to the stm32 additions? > --- > 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 optimized. > + */ > +static void __spi_unoptimize_message(struct spi_message *msg) > +{ > + struct spi_controller *ctlr = msg->spi->controller; > + > + if (ctlr->unoptimize_message) > + ctlr->unoptimize_message(msg); > + > + msg->optimized = false; > + msg->opt_state = NULL; > +} Seems misbalanced that this doesn't take a pre_optimized flag in but __spi_optimize does. I'd move handling that to outside the call in both cases. > spin_lock_irqsave(&ctlr->bus_lock_spinlock, flags); > @@ -4271,6 +4401,8 @@ int spi_async(struct spi_device *spi, struct spi_message *message) > > spin_unlock_irqrestore(&ctlr->bus_lock_spinlock, flags); > > + spi_maybe_unoptimize_message(message); > + > return ret; > } > EXPORT_SYMBOL_GPL(spi_async); > @@ -4331,10 +4463,15 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message) > return -ESHUTDOWN; > } > > - status = __spi_validate(spi, message); > - if (status != 0) > + status = spi_maybe_optimize_message(spi, message); > + if (status) > return status; > > + /* > + * NB: all return paths after this point must ensure that > + * spi_finalize_current_message() is called to avoid leaking resources. I'm not sure a catch all like that makes sense. Not sufficient to call the finer grained spi_maybe_unoptimize_message() ? > + */ > + > SPI_STATISTICS_INCREMENT_FIELD(ctlr->pcpu_statistics, spi_sync); > SPI_STATISTICS_INCREMENT_FIELD(spi->pcpu_statistics, spi_sync);
On Tue, Feb 13, 2024 at 11:25 AM Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > > I thought about suggesting splitting this into an initial patch that just does > the bits without the controller callbacks. Maybe it would work better that way > with that introduced after the validate and splitting of transfers (so most > of patches 1 and 2) as a patch 3 prior to the stm32 additions? Unless anyone else feels the same way, I'm inclined to avoid the extra work of splitting it up. > > +static void __spi_unoptimize_message(struct spi_message *msg) > > +{ > > + struct spi_controller *ctlr = msg->spi->controller; > > + > > + if (ctlr->unoptimize_message) > > + ctlr->unoptimize_message(msg); > > + > > + msg->optimized = false; > > + msg->opt_state = NULL; > > +} > > Seems misbalanced that this doesn't take a pre_optimized flag in but > __spi_optimize does. I'd move handling that to outside the call in both cases. > > Agreed. > > @@ -4331,10 +4463,15 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message) > > return -ESHUTDOWN; > > } > > > > - status = __spi_validate(spi, message); > > - if (status != 0) > > + status = spi_maybe_optimize_message(spi, message); > > + if (status) > > return status; > > > > + /* > > + * NB: all return paths after this point must ensure that > > + * spi_finalize_current_message() is called to avoid leaking resources. > > I'm not sure a catch all like that makes sense. Not sufficient to call > the finer grained spi_maybe_unoptimize_message() ? Hmm... this is my bias from a previous fix showing through. Maybe this comment doesn't belong in this patch. The short answer to your question is "it's complicated".
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. > > 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-specific > 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 optimized. > + */ > +static void __spi_unoptimize_message(struct spi_message *msg) > +{ > + struct spi_controller *ctlr = msg->spi->controller; > + > + if (ctlr->unoptimize_message) > + ctlr->unoptimize_message(msg); > + > + msg->optimized = false; > + msg->opt_state = NULL; > +} > + > +/** > + * spi_maybe_unoptimize_message - unoptimize msg not managed by a peripheral > + * @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_controller > *ctlr) > > mesg->prepared = 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_message() > + * 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 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 optimized. > + * > + * 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 = spi->controller; > + int ret; > + > + ret = __spi_validate(spi, msg); > + if (ret) > + return ret; > + > + if (ctlr->optimize_message) { > + ret = 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 the spi message? - Nuno Sá
On Tue, Feb 13, 2024 at 10:53:56AM +0100, Nuno Sá 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 SPI > > message validation and controller-specific optimizations once and then > > reuse the message while avoiding the overhead of revalidating the Please delete unneeded context from mails when replying. Doing this makes it much easier to find your reply in the message, helping ensure it won't be missed by people scrolling through the irrelevant quoted material.
On Tue, Feb 13, 2024 at 3:50 AM Nuno Sá <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 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. > > > > 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-specific > > 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 optimized. > > + */ > > +static void __spi_unoptimize_message(struct spi_message *msg) > > +{ > > + struct spi_controller *ctlr = msg->spi->controller; > > + > > + if (ctlr->unoptimize_message) > > + ctlr->unoptimize_message(msg); > > + > > + msg->optimized = false; > > + msg->opt_state = NULL; > > +} > > + > > +/** > > + * spi_maybe_unoptimize_message - unoptimize msg not managed by a peripheral > > + * @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_controller > > *ctlr) > > > > mesg->prepared = 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_message() > > + * 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 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 optimized. > > + * > > + * 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 = spi->controller; > > + int ret; > > + > > + ret = __spi_validate(spi, msg); > > + if (ret) > > + return ret; > > + > > + if (ctlr->optimize_message) { > > + ret = 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 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.
© 2016 - 2024 Red Hat, Inc.