| Message ID | 20240219-mainline-spi-precook-message-v2-2-4a762c6701b9@baylibre.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | spi: add support for pre-cooking messages | expand |
On Mon, 2024-02-19 at 16:33 -0600, David Lechner wrote: > Splitting transfers is an expensive operation so we can potentially > optimize it by doing it only once per optimization of the message > instead of repeating each time the message is transferred. > > The transfer splitting functions are currently the only user of > spi_res_alloc() so spi_res_release() can be safely moved at this time > from spi_finalize_current_message() to spi_unoptimize_message(). > > The doc comments of the public functions for splitting transfers are > also updated so that callers will know when it is safe to call them > to ensure proper resource management. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: David Lechner <dlechner@baylibre.com> > --- Acked-by: Nuno Sa <nuno.sa@analog.com> > > v2 changes: > - Changed line break for multiline if condition > - Removed kernel doc inclusion (/** -> /*) from static members > - Picked up Jonathan's Reviewed-by > > drivers/spi/spi.c | 110 +++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 68 insertions(+), 42 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index f68d92b57543..ba4d3fde2054 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -1747,38 +1747,6 @@ static int __spi_pump_transfer_message(struct spi_controller > *ctlr, > > trace_spi_message_start(msg); > > - /* > - * If an SPI controller does not support toggling the CS line on each > - * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO > - * for the CS line, we can emulate the CS-per-word hardware function by > - * splitting transfers into one-word transfers and ensuring that > - * cs_change is set for each transfer. > - */ > - if ((msg->spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits & SPI_CS_WORD) || > - spi_is_csgpiod(msg->spi))) { > - ret = spi_split_transfers_maxwords(ctlr, msg, 1); > - if (ret) { > - msg->status = ret; > - spi_finalize_current_message(ctlr); > - return ret; > - } > - > - list_for_each_entry(xfer, &msg->transfers, transfer_list) { > - /* Don't change cs_change on the last entry in the list */ > - if (list_is_last(&xfer->transfer_list, &msg->transfers)) > - break; > - xfer->cs_change = 1; > - } > - } else { > - ret = spi_split_transfers_maxsize(ctlr, msg, > - spi_max_transfer_size(msg- > >spi)); > - if (ret) { > - msg->status = ret; > - spi_finalize_current_message(ctlr); > - return ret; > - } > - } > - > if (ctlr->prepare_message) { > ret = ctlr->prepare_message(ctlr, msg); > if (ret) { > @@ -2124,6 +2092,8 @@ static void __spi_unoptimize_message(struct spi_message *msg) > if (ctlr->unoptimize_message) > ctlr->unoptimize_message(msg); > > + spi_res_release(ctlr, msg); > + > msg->optimized = false; > msg->opt_state = NULL; > } > @@ -2169,15 +2139,6 @@ void spi_finalize_current_message(struct spi_controller > *ctlr) > > spi_unmap_msg(ctlr, mesg); > > - /* > - * In the prepare_messages callback the SPI bus has the opportunity > - * to split a transfer to smaller chunks. > - * > - * Release the split transfers here since spi_map_msg() is done on > - * the split transfers. > - */ > - spi_res_release(ctlr, mesg); > - > if (mesg->prepared && ctlr->unprepare_message) { > ret = ctlr->unprepare_message(ctlr, mesg); > if (ret) { > @@ -3819,6 +3780,10 @@ static int __spi_split_transfer_maxsize(struct > spi_controller *ctlr, > * @msg: the @spi_message to transform > * @maxsize: the maximum when to apply this > * > + * This function allocates resources that are automatically freed during the > + * spi message unoptimize phase so this function should only be called from > + * optimize_message callbacks. > + * > * Return: status of transformation > */ > int spi_split_transfers_maxsize(struct spi_controller *ctlr, > @@ -3857,6 +3822,10 @@ EXPORT_SYMBOL_GPL(spi_split_transfers_maxsize); > * @msg: the @spi_message to transform > * @maxwords: the number of words to limit each transfer to > * > + * This function allocates resources that are automatically freed during the > + * spi message unoptimize phase so this function should only be called from > + * optimize_message callbacks. > + * > * Return: status of transformation > */ > int spi_split_transfers_maxwords(struct spi_controller *ctlr, > @@ -4231,6 +4200,57 @@ static int __spi_validate(struct spi_device *spi, struct > spi_message *message) > return 0; > } > > +/* > + * spi_split_transfers - generic handling of transfer splitting > + * @msg: the message to split > + * > + * Under certain conditions, a SPI controller may not support arbitrary > + * transfer sizes or other features required by a peripheral. This function > + * will split the transfers in the message into smaller transfers that are > + * supported by the controller. > + * > + * Controllers with special requirements not covered here can also split > + * transfers in the optimize_message() callback. > + * > + * Context: can sleep > + * Return: zero on success, else a negative error code > + */ > +static int spi_split_transfers(struct spi_message *msg) > +{ > + struct spi_controller *ctlr = msg->spi->controller; > + struct spi_transfer *xfer; > + int ret; > + > + /* > + * If an SPI controller does not support toggling the CS line on each > + * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO > + * for the CS line, we can emulate the CS-per-word hardware function by > + * splitting transfers into one-word transfers and ensuring that > + * cs_change is set for each transfer. > + */ > + if ((msg->spi->mode & SPI_CS_WORD) && > + (!(ctlr->mode_bits & SPI_CS_WORD) || spi_is_csgpiod(msg->spi))) { > + ret = spi_split_transfers_maxwords(ctlr, msg, 1); > + if (ret) > + return ret; > + > + list_for_each_entry(xfer, &msg->transfers, transfer_list) { > + /* Don't change cs_change on the last entry in the list */ > + if (list_is_last(&xfer->transfer_list, &msg->transfers)) > + break; > + > + xfer->cs_change = 1; > + } > + } else { > + ret = spi_split_transfers_maxsize(ctlr, msg, > + spi_max_transfer_size(msg- > >spi)); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > /* > * __spi_optimize_message - shared implementation for spi_optimize_message() > * and spi_maybe_optimize_message() > @@ -4254,10 +4274,16 @@ static int __spi_optimize_message(struct spi_device *spi, > if (ret) > return ret; > > + ret = spi_split_transfers(msg); > + if (ret) > + return ret; > + > if (ctlr->optimize_message) { > ret = ctlr->optimize_message(msg); > - if (ret) > + if (ret) { > + spi_res_release(ctlr, msg); > return ret; > + } > } > > msg->optimized = true; >
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index f68d92b57543..ba4d3fde2054 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1747,38 +1747,6 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr, trace_spi_message_start(msg); - /* - * If an SPI controller does not support toggling the CS line on each - * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO - * for the CS line, we can emulate the CS-per-word hardware function by - * splitting transfers into one-word transfers and ensuring that - * cs_change is set for each transfer. - */ - if ((msg->spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits & SPI_CS_WORD) || - spi_is_csgpiod(msg->spi))) { - ret = spi_split_transfers_maxwords(ctlr, msg, 1); - if (ret) { - msg->status = ret; - spi_finalize_current_message(ctlr); - return ret; - } - - list_for_each_entry(xfer, &msg->transfers, transfer_list) { - /* Don't change cs_change on the last entry in the list */ - if (list_is_last(&xfer->transfer_list, &msg->transfers)) - break; - xfer->cs_change = 1; - } - } else { - ret = spi_split_transfers_maxsize(ctlr, msg, - spi_max_transfer_size(msg->spi)); - if (ret) { - msg->status = ret; - spi_finalize_current_message(ctlr); - return ret; - } - } - if (ctlr->prepare_message) { ret = ctlr->prepare_message(ctlr, msg); if (ret) { @@ -2124,6 +2092,8 @@ static void __spi_unoptimize_message(struct spi_message *msg) if (ctlr->unoptimize_message) ctlr->unoptimize_message(msg); + spi_res_release(ctlr, msg); + msg->optimized = false; msg->opt_state = NULL; } @@ -2169,15 +2139,6 @@ void spi_finalize_current_message(struct spi_controller *ctlr) spi_unmap_msg(ctlr, mesg); - /* - * In the prepare_messages callback the SPI bus has the opportunity - * to split a transfer to smaller chunks. - * - * Release the split transfers here since spi_map_msg() is done on - * the split transfers. - */ - spi_res_release(ctlr, mesg); - if (mesg->prepared && ctlr->unprepare_message) { ret = ctlr->unprepare_message(ctlr, mesg); if (ret) { @@ -3819,6 +3780,10 @@ static int __spi_split_transfer_maxsize(struct spi_controller *ctlr, * @msg: the @spi_message to transform * @maxsize: the maximum when to apply this * + * This function allocates resources that are automatically freed during the + * spi message unoptimize phase so this function should only be called from + * optimize_message callbacks. + * * Return: status of transformation */ int spi_split_transfers_maxsize(struct spi_controller *ctlr, @@ -3857,6 +3822,10 @@ EXPORT_SYMBOL_GPL(spi_split_transfers_maxsize); * @msg: the @spi_message to transform * @maxwords: the number of words to limit each transfer to * + * This function allocates resources that are automatically freed during the + * spi message unoptimize phase so this function should only be called from + * optimize_message callbacks. + * * Return: status of transformation */ int spi_split_transfers_maxwords(struct spi_controller *ctlr, @@ -4231,6 +4200,57 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) return 0; } +/* + * spi_split_transfers - generic handling of transfer splitting + * @msg: the message to split + * + * Under certain conditions, a SPI controller may not support arbitrary + * transfer sizes or other features required by a peripheral. This function + * will split the transfers in the message into smaller transfers that are + * supported by the controller. + * + * Controllers with special requirements not covered here can also split + * transfers in the optimize_message() callback. + * + * Context: can sleep + * Return: zero on success, else a negative error code + */ +static int spi_split_transfers(struct spi_message *msg) +{ + struct spi_controller *ctlr = msg->spi->controller; + struct spi_transfer *xfer; + int ret; + + /* + * If an SPI controller does not support toggling the CS line on each + * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO + * for the CS line, we can emulate the CS-per-word hardware function by + * splitting transfers into one-word transfers and ensuring that + * cs_change is set for each transfer. + */ + if ((msg->spi->mode & SPI_CS_WORD) && + (!(ctlr->mode_bits & SPI_CS_WORD) || spi_is_csgpiod(msg->spi))) { + ret = spi_split_transfers_maxwords(ctlr, msg, 1); + if (ret) + return ret; + + list_for_each_entry(xfer, &msg->transfers, transfer_list) { + /* Don't change cs_change on the last entry in the list */ + if (list_is_last(&xfer->transfer_list, &msg->transfers)) + break; + + xfer->cs_change = 1; + } + } else { + ret = spi_split_transfers_maxsize(ctlr, msg, + spi_max_transfer_size(msg->spi)); + if (ret) + return ret; + } + + return 0; +} + /* * __spi_optimize_message - shared implementation for spi_optimize_message() * and spi_maybe_optimize_message() @@ -4254,10 +4274,16 @@ static int __spi_optimize_message(struct spi_device *spi, if (ret) return ret; + ret = spi_split_transfers(msg); + if (ret) + return ret; + if (ctlr->optimize_message) { ret = ctlr->optimize_message(msg); - if (ret) + if (ret) { + spi_res_release(ctlr, msg); return ret; + } } msg->optimized = true;