On Fri, 15 Nov 2024 14:18:40 -0600
David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> Add the basic infrastructure to support SPI offload providers and
> consumers.
>
> SPI offloading is a feature that allows the SPI controller to perform
> transfers without any CPU intervention. This is useful, e.g. for
> high-speed data acquisition.
>
> SPI controllers with offload support need to implement the get_offload
> and put_offload callbacks and can use the devm_spi_offload_alloc() to
> allocate offload instances.
>
> SPI peripheral drivers will call devm_spi_offload_get() to get a
> reference to the matching offload instance. This offload instance can
> then be attached to a SPI message to request offloading that message.
>
> It is expected that SPI controllers with offload support will check for
> the offload instance in the SPI message in the ctlr->optimize_message()
> callback and handle it accordingly.
>
> CONFIG_SPI_OFFLOAD is intended to be a select-only option. Both
> consumer and provider drivers should `select SPI_OFFLOAD` in their
> Kconfig to ensure that the SPI core is built with offload support.
>
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
A few additional comments, but basically fine.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> diff --git a/drivers/spi/spi-offload.c b/drivers/spi/spi-offload.c
> new file mode 100644
> index 000000000000..5ded7aecf9fc
> --- /dev/null
> +++ b/drivers/spi/spi-offload.c
...
> +/**
> + * devm_spi_offload_get() - Get an offload instance
> + * @dev: Device for devm purposes
> + * @spi: SPI device to use for the transfers
> + * @config: Offload configuration
> + *
> + * Peripheral drivers call this function to get an offload instance that meets
> + * the requirements specified in @config. If no suitable offload instance is
> + * available, -ENODEV is returned.
> + *
> + * Return: Offload instance or error on failure.
> + */
> +struct spi_offload *devm_spi_offload_get(struct device *dev,
> + struct spi_device *spi,
> + const struct spi_offload_config *config)
> +{
> + struct spi_controller_and_offload *resource;
> + int ret;
> +
> + if (!spi || !config)
> + return ERR_PTR(-EINVAL);
> +
> + if (!spi->controller->get_offload)
> + return ERR_PTR(-ENODEV);
> +
> + resource = kzalloc(sizeof(*resource), GFP_KERNEL);
> + if (!resource)
> + return ERR_PTR(-ENOMEM);
> +
> + resource->controller = spi->controller;
> + resource->offload = spi->controller->get_offload(spi, config);
> + ret = PTR_ERR_OR_ZERO(resource->offload);
> + if (ret) {
Why not simply
if (IS_ERR(resource->offload) {
kfree(resource);
return resource->offload;
}
> + kfree(resource);
> + return ERR_PTR(ret);
> + }
> +
> + ret = devm_add_action_or_reset(dev, spi_offload_put, resource);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return resource->offload;
> +}
> +EXPORT_SYMBOL_GPL(devm_spi_offload_get);
> diff --git a/include/linux/spi/spi-offload.h b/include/linux/spi/spi-offload.h
> new file mode 100644
> index 000000000000..81b115fc89bf
> --- /dev/null
> +++ b/include/linux/spi/spi-offload.h
> +
> +MODULE_IMPORT_NS(SPI_OFFLOAD);
This is rarely done in headers. (only pwm.h does it I think)
I'd push it down into code that uses this.
It might be worth splitting the header into a spi-offload-provider.h
and spi-offload-consumer.h with a common spi-offload-types.h included
by both.
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index 8497f4747e24..c2b24a0909ea 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -31,6 +31,9 @@ struct spi_transfer;
> struct spi_controller_mem_ops;
> struct spi_controller_mem_caps;
> struct spi_message;
> +struct spi_controller_offload_ops;
Not used so introduce it only when needed (I guess in a later patch).
