diff mbox series

[v6,05/17] spi: add offload TX/RX streaming APIs

Message ID 20241211-dlech-mainline-spi-engine-offload-2-v6-5-88ee574d5d03@baylibre.com (mailing list archive)
State Changes Requested
Headers show
Series spi: axi-spi-engine: add offload support | expand

Commit Message

David Lechner Dec. 11, 2024, 8:54 p.m. UTC
Most configuration of SPI offloads is handled opaquely using the offload
pointer that is passed to the various offload functions. However, there
are some offload features that need to be controlled on a per transfer
basis.

This patch adds a flag field to struct spi_transfer to allow specifying
such features. The first feature to be added is the ability to stream
data to/from a hardware sink/source rather than using a tx or rx buffer.
Additional flags can be added in the future as needed.

A flags field is also added to the offload struct for providers to
indicate which flags are supported. This allows for generic checking of
offload capabilities during __spi_validate() so that each offload
provider doesn't have to implement their own validation.

As a first users of this streaming capability, getter functions are
added to get a DMA channel that is directly connected to the offload.
Peripheral drivers will use this to get a DMA channel and configure it
to suit their needs.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

v6 changes:
* Update for header file split.
* Fix wrong kernel-doc comments.

v5 change:
* Remove incorrect comment about caller needing to release DMA channels.

v4 changes:
* DMA API's now automatically release DMA channels instead of leaving
  it up to the caller.

v3 changes:
* Added spi_offload_{tx,rx}_stream_get_dma_chan() functions.

v2 changes:
* This is also split out from "spi: add core support for controllers with
  offload capabilities".
* In the previous version, we were using (void *)-1 as a sentinel value
  that could be assigned, e.g. to rx_buf. But this was naive since there
  is core code that would try to dereference this pointer. So instead,
  we've added a new flags field to the spi_transfer structure for this
  sort of thing. This also has the advantage of being able to be used in
  the future for other arbitrary features.
---
 drivers/spi/spi-offload.c            | 70 ++++++++++++++++++++++++++++++++++++
 drivers/spi/spi.c                    | 10 ++++++
 include/linux/spi/offload/consumer.h |  5 +++
 include/linux/spi/offload/types.h    | 19 ++++++++++
 include/linux/spi/spi.h              |  3 ++
 5 files changed, 107 insertions(+)

Comments

Jonathan Cameron Dec. 14, 2024, 2:28 p.m. UTC | #1
On Wed, 11 Dec 2024 14:54:42 -0600
David Lechner <dlechner@baylibre.com> wrote:

> Most configuration of SPI offloads is handled opaquely using the offload
> pointer that is passed to the various offload functions. However, there
> are some offload features that need to be controlled on a per transfer
> basis.
> 
> This patch adds a flag field to struct spi_transfer to allow specifying
> such features. The first feature to be added is the ability to stream
> data to/from a hardware sink/source rather than using a tx or rx buffer.
> Additional flags can be added in the future as needed.
> 
> A flags field is also added to the offload struct for providers to
> indicate which flags are supported. This allows for generic checking of
> offload capabilities during __spi_validate() so that each offload
> provider doesn't have to implement their own validation.
> 
> As a first users of this streaming capability, getter functions are
> added to get a DMA channel that is directly connected to the offload.
> Peripheral drivers will use this to get a DMA channel and configure it
> to suit their needs.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Really really minor comment inline.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index ff1add2ecb91f18cf82e6f1e9595584c11adf9d8..4a871db9ee636aba64c866ebdd8bb1dbf82e0f42 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -31,6 +31,7 @@
>  #include <linux/ptp_clock_kernel.h>
>  #include <linux/sched/rt.h>
>  #include <linux/slab.h>
> +#include <linux/spi/offload/types.h>
>  #include <linux/spi/spi.h>
>  #include <linux/spi/spi-mem.h>
>  #include <uapi/linux/sched/types.h>
> @@ -4163,6 +4164,15 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
>  
>  		if (_spi_xfer_word_delay_update(xfer, spi))
>  			return -EINVAL;
> +
> +		/* make sure controller supports required offload features */
Comment syntax seems inconsistent with local code. 

		/* Make sure controller supports required offload features. */

> +		if (xfer->offload_flags) {
> +			if (!message->offload)
> +				return -EINVAL;
> +
> +			if (xfer->offload_flags & ~message->offload->xfer_flags)
> +				return -EINVAL;
> +		}
>  	}
>  
>  	message->status = -EINPROGRESS;
Nuno Sá Dec. 17, 2024, 11:43 a.m. UTC | #2
On Wed, 2024-12-11 at 14:54 -0600, David Lechner wrote:
> Most configuration of SPI offloads is handled opaquely using the offload
> pointer that is passed to the various offload functions. However, there
> are some offload features that need to be controlled on a per transfer
> basis.
> 
> This patch adds a flag field to struct spi_transfer to allow specifying
> such features. The first feature to be added is the ability to stream
> data to/from a hardware sink/source rather than using a tx or rx buffer.
> Additional flags can be added in the future as needed.
> 
> A flags field is also added to the offload struct for providers to
> indicate which flags are supported. This allows for generic checking of
> offload capabilities during __spi_validate() so that each offload
> provider doesn't have to implement their own validation.
> 
> As a first users of this streaming capability, getter functions are
> added to get a DMA channel that is directly connected to the offload.
> Peripheral drivers will use this to get a DMA channel and configure it
> to suit their needs.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---


Still not sure about releasing the channel. But I guess this might be also a
problem today with "plain" IIO DMA buffering. So...

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

> v6 changes:
> * Update for header file split.
> * Fix wrong kernel-doc comments.
> 
> v5 change:
> * Remove incorrect comment about caller needing to release DMA channels.
> 
> v4 changes:
> * DMA API's now automatically release DMA channels instead of leaving
>   it up to the caller.
> 
> v3 changes:
> * Added spi_offload_{tx,rx}_stream_get_dma_chan() functions.
> 
> v2 changes:
> * This is also split out from "spi: add core support for controllers with
>   offload capabilities".
> * In the previous version, we were using (void *)-1 as a sentinel value
>   that could be assigned, e.g. to rx_buf. But this was naive since there
>   is core code that would try to dereference this pointer. So instead,
>   we've added a new flags field to the spi_transfer structure for this
>   sort of thing. This also has the advantage of being able to be used in
>   the future for other arbitrary features.
> ---
>  drivers/spi/spi-offload.c            | 70
> ++++++++++++++++++++++++++++++++++++
>  drivers/spi/spi.c                    | 10 ++++++
>  include/linux/spi/offload/consumer.h |  5 +++
>  include/linux/spi/offload/types.h    | 19 ++++++++++
>  include/linux/spi/spi.h              |  3 ++
>  5 files changed, 107 insertions(+)
> 
> diff --git a/drivers/spi/spi-offload.c b/drivers/spi/spi-offload.c
> index
> 43582e50e279c4b1b958765fec556aaa91180e55..df5e963d5ee29d37833559595536a460c530
> bc81 100644
> --- a/drivers/spi/spi-offload.c
> +++ b/drivers/spi/spi-offload.c
> @@ -18,6 +18,7 @@
>  
>  #include <linux/cleanup.h>
>  #include <linux/device.h>
> +#include <linux/dmaengine.h>
>  #include <linux/export.h>
>  #include <linux/kref.h>
>  #include <linux/list.h>
> @@ -332,6 +333,75 @@ void spi_offload_trigger_disable(struct spi_offload
> *offload,
>  }
>  EXPORT_SYMBOL_GPL(spi_offload_trigger_disable);
>  
> +static void spi_offload_release_dma_chan(void *chan)
> +{
> +	dma_release_channel(chan);
> +}
> +
> +/**
> + * devm_spi_offload_tx_stream_request_dma_chan - Get the DMA channel info for
> the TX stream
> + * @dev: Device for devm purposes.
> + * @offload: Offload instance
> + *
> + * This is the DMA channel that will provide data to transfers that use the
> + * %SPI_OFFLOAD_XFER_TX_STREAM offload flag.
> + *
> + * Return: Pointer to DMA channel info, or negative error code
> + */
> +struct dma_chan
> +*devm_spi_offload_tx_stream_request_dma_chan(struct device *dev,
> +					     struct spi_offload *offload)
> +{
> +	struct dma_chan *chan;
> +	int ret;
> +
> +	if (!offload->ops || !offload->ops->tx_stream_request_dma_chan)
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	chan = offload->ops->tx_stream_request_dma_chan(offload);
> +	if (IS_ERR(chan))
> +		return chan;
> +
> +	ret = devm_add_action_or_reset(dev, spi_offload_release_dma_chan,
> chan);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return chan;
> +}
> +EXPORT_SYMBOL_GPL(devm_spi_offload_tx_stream_request_dma_chan);
> +
> +/**
> + * devm_spi_offload_rx_stream_request_dma_chan - Get the DMA channel info for
> the RX stream
> + * @dev: Device for devm purposes.
> + * @offload: Offload instance
> + *
> + * This is the DMA channel that will receive data from transfers that use the
> + * %SPI_OFFLOAD_XFER_RX_STREAM offload flag.
> + *
> + * Return: Pointer to DMA channel info, or negative error code
> + */
> +struct dma_chan
> +*devm_spi_offload_rx_stream_request_dma_chan(struct device *dev,
> +					     struct spi_offload *offload)
> +{
> +	struct dma_chan *chan;
> +	int ret;
> +
> +	if (!offload->ops || !offload->ops->rx_stream_request_dma_chan)
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	chan = offload->ops->rx_stream_request_dma_chan(offload);
> +	if (IS_ERR(chan))
> +		return chan;
> +
> +	ret = devm_add_action_or_reset(dev, spi_offload_release_dma_chan,
> chan);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return chan;
> +}
> +EXPORT_SYMBOL_GPL(devm_spi_offload_rx_stream_request_dma_chan);
> +
>  /* Triggers providers */
>  
>  static void spi_offload_trigger_unregister(void *data)
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index
> ff1add2ecb91f18cf82e6f1e9595584c11adf9d8..4a871db9ee636aba64c866ebdd8bb1dbf82e
> 0f42 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -31,6 +31,7 @@
>  #include <linux/ptp_clock_kernel.h>
>  #include <linux/sched/rt.h>
>  #include <linux/slab.h>
> +#include <linux/spi/offload/types.h>
>  #include <linux/spi/spi.h>
>  #include <linux/spi/spi-mem.h>
>  #include <uapi/linux/sched/types.h>
> @@ -4163,6 +4164,15 @@ static int __spi_validate(struct spi_device *spi,
> struct spi_message *message)
>  
>  		if (_spi_xfer_word_delay_update(xfer, spi))
>  			return -EINVAL;
> +
> +		/* make sure controller supports required offload features */
> +		if (xfer->offload_flags) {
> +			if (!message->offload)
> +				return -EINVAL;
> +
> +			if (xfer->offload_flags & ~message->offload-
> >xfer_flags)
> +				return -EINVAL;
> +		}
>  	}
>  
>  	message->status = -EINPROGRESS;
> diff --git a/include/linux/spi/offload/consumer.h
> b/include/linux/spi/offload/consumer.h
> index
> 5a0ec5303d600728959594bcdbd0cb2baeba7c77..cd7d5daa21e69b61c16eba6c10c855345a4f
> 3297 100644
> --- a/include/linux/spi/offload/consumer.h
> +++ b/include/linux/spi/offload/consumer.h
> @@ -31,4 +31,9 @@ int spi_offload_trigger_enable(struct spi_offload *offload,
>  void spi_offload_trigger_disable(struct spi_offload *offload,
>  				 struct spi_offload_trigger *trigger);
>  
> +struct dma_chan *devm_spi_offload_tx_stream_request_dma_chan(struct device
> *dev,
> +							     struct
> spi_offload *offload);
> +struct dma_chan *devm_spi_offload_rx_stream_request_dma_chan(struct device
> *dev,
> +							     struct
> spi_offload *offload);
> +
>  #endif /* __LINUX_SPI_OFFLOAD_CONSUMER_H */
> diff --git a/include/linux/spi/offload/types.h
> b/include/linux/spi/offload/types.h
> index
> 7476f2073b02ee0f9edd3ae75e587b075746fa92..86d0e8cb9495bb43e177378b2041067de8ea
> 8786 100644
> --- a/include/linux/spi/offload/types.h
> +++ b/include/linux/spi/offload/types.h
> @@ -11,6 +11,11 @@
>  
>  struct device;
>  
> +/* This is write xfer but TX uses external data stream rather than tx_buf. */
> +#define SPI_OFFLOAD_XFER_TX_STREAM	BIT(0)
> +/* This is read xfer but RX uses external data stream rather than rx_buf. */
> +#define SPI_OFFLOAD_XFER_RX_STREAM	BIT(1)
> +
>  /* 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. */
> @@ -40,6 +45,8 @@ struct spi_offload {
>  	void *priv;
>  	/** @ops: callbacks for offload support */
>  	const struct spi_offload_ops *ops;
> +	/** @xfer_flags: %SPI_OFFLOAD_XFER_* flags supported by provider */
> +	u32 xfer_flags;
>  };
>  
>  enum spi_offload_trigger_type {
> @@ -75,6 +82,18 @@ struct spi_offload_ops {
>  	 * given offload instance.
>  	 */
>  	void (*trigger_disable)(struct spi_offload *offload);
> +	/**
> +	 * @tx_stream_request_dma_chan: Optional callback for controllers
> that
> +	 * have an offload where the TX data stream is connected directly to
> a
> +	 * DMA channel.
> +	 */
> +	struct dma_chan *(*tx_stream_request_dma_chan)(struct spi_offload
> *offload);
> +	/**
> +	 * @rx_stream_request_dma_chan: Optional callback for controllers
> that
> +	 * have an offload where the RX data stream is connected directly to
> a
> +	 * DMA channel.
> +	 */
> +	struct dma_chan *(*rx_stream_request_dma_chan)(struct spi_offload
> *offload);
>  };
>  
>  #endif /* __LINUX_SPI_OFFLOAD_TYPES_H */
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index
> 98bdc8c16c20521c0a94e5f72f5e71c4f6d7d11e..4c087009cf974595f23036b1b7a030a45913
> 420c 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -1093,6 +1093,9 @@ struct spi_transfer {
>  
>  	u32		effective_speed_hz;
>  
> +	/* Use %SPI_OFFLOAD_XFER_* from spi-offload.h */
> +	unsigned int	offload_flags;
> +
>  	unsigned int	ptp_sts_word_pre;
>  	unsigned int	ptp_sts_word_post;
>  
>
diff mbox series

Patch

diff --git a/drivers/spi/spi-offload.c b/drivers/spi/spi-offload.c
index 43582e50e279c4b1b958765fec556aaa91180e55..df5e963d5ee29d37833559595536a460c530bc81 100644
--- a/drivers/spi/spi-offload.c
+++ b/drivers/spi/spi-offload.c
@@ -18,6 +18,7 @@ 
 
 #include <linux/cleanup.h>
 #include <linux/device.h>
+#include <linux/dmaengine.h>
 #include <linux/export.h>
 #include <linux/kref.h>
 #include <linux/list.h>
@@ -332,6 +333,75 @@  void spi_offload_trigger_disable(struct spi_offload *offload,
 }
 EXPORT_SYMBOL_GPL(spi_offload_trigger_disable);
 
+static void spi_offload_release_dma_chan(void *chan)
+{
+	dma_release_channel(chan);
+}
+
+/**
+ * devm_spi_offload_tx_stream_request_dma_chan - Get the DMA channel info for the TX stream
+ * @dev: Device for devm purposes.
+ * @offload: Offload instance
+ *
+ * This is the DMA channel that will provide data to transfers that use the
+ * %SPI_OFFLOAD_XFER_TX_STREAM offload flag.
+ *
+ * Return: Pointer to DMA channel info, or negative error code
+ */
+struct dma_chan
+*devm_spi_offload_tx_stream_request_dma_chan(struct device *dev,
+					     struct spi_offload *offload)
+{
+	struct dma_chan *chan;
+	int ret;
+
+	if (!offload->ops || !offload->ops->tx_stream_request_dma_chan)
+		return ERR_PTR(-EOPNOTSUPP);
+
+	chan = offload->ops->tx_stream_request_dma_chan(offload);
+	if (IS_ERR(chan))
+		return chan;
+
+	ret = devm_add_action_or_reset(dev, spi_offload_release_dma_chan, chan);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return chan;
+}
+EXPORT_SYMBOL_GPL(devm_spi_offload_tx_stream_request_dma_chan);
+
+/**
+ * devm_spi_offload_rx_stream_request_dma_chan - Get the DMA channel info for the RX stream
+ * @dev: Device for devm purposes.
+ * @offload: Offload instance
+ *
+ * This is the DMA channel that will receive data from transfers that use the
+ * %SPI_OFFLOAD_XFER_RX_STREAM offload flag.
+ *
+ * Return: Pointer to DMA channel info, or negative error code
+ */
+struct dma_chan
+*devm_spi_offload_rx_stream_request_dma_chan(struct device *dev,
+					     struct spi_offload *offload)
+{
+	struct dma_chan *chan;
+	int ret;
+
+	if (!offload->ops || !offload->ops->rx_stream_request_dma_chan)
+		return ERR_PTR(-EOPNOTSUPP);
+
+	chan = offload->ops->rx_stream_request_dma_chan(offload);
+	if (IS_ERR(chan))
+		return chan;
+
+	ret = devm_add_action_or_reset(dev, spi_offload_release_dma_chan, chan);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return chan;
+}
+EXPORT_SYMBOL_GPL(devm_spi_offload_rx_stream_request_dma_chan);
+
 /* Triggers providers */
 
 static void spi_offload_trigger_unregister(void *data)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ff1add2ecb91f18cf82e6f1e9595584c11adf9d8..4a871db9ee636aba64c866ebdd8bb1dbf82e0f42 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -31,6 +31,7 @@ 
 #include <linux/ptp_clock_kernel.h>
 #include <linux/sched/rt.h>
 #include <linux/slab.h>
+#include <linux/spi/offload/types.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/spi-mem.h>
 #include <uapi/linux/sched/types.h>
@@ -4163,6 +4164,15 @@  static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 
 		if (_spi_xfer_word_delay_update(xfer, spi))
 			return -EINVAL;
+
+		/* make sure controller supports required offload features */
+		if (xfer->offload_flags) {
+			if (!message->offload)
+				return -EINVAL;
+
+			if (xfer->offload_flags & ~message->offload->xfer_flags)
+				return -EINVAL;
+		}
 	}
 
 	message->status = -EINPROGRESS;
diff --git a/include/linux/spi/offload/consumer.h b/include/linux/spi/offload/consumer.h
index 5a0ec5303d600728959594bcdbd0cb2baeba7c77..cd7d5daa21e69b61c16eba6c10c855345a4f3297 100644
--- a/include/linux/spi/offload/consumer.h
+++ b/include/linux/spi/offload/consumer.h
@@ -31,4 +31,9 @@  int spi_offload_trigger_enable(struct spi_offload *offload,
 void spi_offload_trigger_disable(struct spi_offload *offload,
 				 struct spi_offload_trigger *trigger);
 
+struct dma_chan *devm_spi_offload_tx_stream_request_dma_chan(struct device *dev,
+							     struct spi_offload *offload);
+struct dma_chan *devm_spi_offload_rx_stream_request_dma_chan(struct device *dev,
+							     struct spi_offload *offload);
+
 #endif /* __LINUX_SPI_OFFLOAD_CONSUMER_H */
diff --git a/include/linux/spi/offload/types.h b/include/linux/spi/offload/types.h
index 7476f2073b02ee0f9edd3ae75e587b075746fa92..86d0e8cb9495bb43e177378b2041067de8ea8786 100644
--- a/include/linux/spi/offload/types.h
+++ b/include/linux/spi/offload/types.h
@@ -11,6 +11,11 @@ 
 
 struct device;
 
+/* This is write xfer but TX uses external data stream rather than tx_buf. */
+#define SPI_OFFLOAD_XFER_TX_STREAM	BIT(0)
+/* This is read xfer but RX uses external data stream rather than rx_buf. */
+#define SPI_OFFLOAD_XFER_RX_STREAM	BIT(1)
+
 /* 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. */
@@ -40,6 +45,8 @@  struct spi_offload {
 	void *priv;
 	/** @ops: callbacks for offload support */
 	const struct spi_offload_ops *ops;
+	/** @xfer_flags: %SPI_OFFLOAD_XFER_* flags supported by provider */
+	u32 xfer_flags;
 };
 
 enum spi_offload_trigger_type {
@@ -75,6 +82,18 @@  struct spi_offload_ops {
 	 * given offload instance.
 	 */
 	void (*trigger_disable)(struct spi_offload *offload);
+	/**
+	 * @tx_stream_request_dma_chan: Optional callback for controllers that
+	 * have an offload where the TX data stream is connected directly to a
+	 * DMA channel.
+	 */
+	struct dma_chan *(*tx_stream_request_dma_chan)(struct spi_offload *offload);
+	/**
+	 * @rx_stream_request_dma_chan: Optional callback for controllers that
+	 * have an offload where the RX data stream is connected directly to a
+	 * DMA channel.
+	 */
+	struct dma_chan *(*rx_stream_request_dma_chan)(struct spi_offload *offload);
 };
 
 #endif /* __LINUX_SPI_OFFLOAD_TYPES_H */
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 98bdc8c16c20521c0a94e5f72f5e71c4f6d7d11e..4c087009cf974595f23036b1b7a030a45913420c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1093,6 +1093,9 @@  struct spi_transfer {
 
 	u32		effective_speed_hz;
 
+	/* Use %SPI_OFFLOAD_XFER_* from spi-offload.h */
+	unsigned int	offload_flags;
+
 	unsigned int	ptp_sts_word_pre;
 	unsigned int	ptp_sts_word_post;