| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b5b454acec976a6fe2dece85191cda941d0d0cb3.camel@gmail.com>
Date: Tue, 17 Dec 2024 12:43:20 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Mark Brown <broonie@...nel.org>,
Jonathan Cameron
<jic23@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Nuno
Sá
<nuno.sa@...log.com>
Cc: Uwe Kleine-König <ukleinek@...nel.org>, Michael
Hennerich <Michael.Hennerich@...log.com>, Lars-Peter Clausen
<lars@...afoo.de>, David Jander <david@...tonic.nl>, Martin Sperl
<kernel@...tin.sperl.org>, linux-spi@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-iio@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v6 05/17] spi: add offload TX/RX streaming APIs
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@...libre.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@...log.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;
>
>
Powered by blists - more mailing lists