Re: [PATCH v8 01/17] spi: add basic support for SPI offloading

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Fri, Feb 07, 2025 at 02:08:58PM -0600, David Lechner 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.

(I know that this is now in SPI tree, but still we have time to address something)

> +++ b/include/linux/spi/offload/consumer.h

> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2024 Analog Devices Inc.
> + * Copyright (C) 2024 BayLibre, SAS
> + */
> +
> +#ifndef __LINUX_SPI_OFFLOAD_CONSUMER_H
> +#define __LINUX_SPI_OFFLOAD_CONSUMER_H
> +
> +#include <linux/module.h>
> +#include <linux/spi/offload/types.h>
> +#include <linux/types.h>

> +MODULE_IMPORT_NS("SPI_OFFLOAD");

This diminishes the point of the namespaces. Anybody who includes a (dangling)
header gets namespace imported, which is not good. Same for other globally
visible headers.

(This is the main concern of this patch).

...

> +#ifndef __LINUX_SPI_OFFLOAD_TYPES_H
> +#define __LINUX_SPI_OFFLOAD_TYPES_H


+ linux/bits.h

> +#include <linux/types.h>
> +
> +struct device;
> +
> +/* Offload can be triggered by external hardware event. */
> +#define SPI_OFFLOAD_CAP_TRIGGER			BIT(0)
> +/* Offload can record and then play back TX data when triggered. */
> +#define SPI_OFFLOAD_CAP_TX_STATIC_DATA		BIT(1)
> +/* Offload can get TX data from an external stream source. */
> +#define SPI_OFFLOAD_CAP_TX_STREAM_DMA		BIT(2)
> +/* Offload can send RX data to an external stream sink. */
> +#define SPI_OFFLOAD_CAP_RX_STREAM_DMA		BIT(3)

> +/**
> + * struct spi_offload_config - offload configuration
> + *
> + * This is used to request an offload with specific configuration.
> + */
> +struct spi_offload_config {
> +	/** @capability_flags: required capabilities. See %SPI_OFFLOAD_CAP_* */
> +	u32 capability_flags;
> +};
> +
> +/**
> + * struct spi_offload - offload instance
> + */
> +struct spi_offload {
> +	/** @provider_dev: for get/put reference counting */
> +	struct device *provider_dev;
> +	/** @priv: provider driver private data */
> +	void *priv;
> +};
> +
> +#endif /* __LINUX_SPI_OFFLOAD_TYPES_H */

-- 
With Best Regards,
Andy Shevchenko