| 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: <2cec766bb77ef245f34e213f081b5e27b489aff7.camel@gmail.com>
Date: Fri, 25 Oct 2024 14:59:20 +0200
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>, Uwe
Kleine-König <ukleinek@...nel.org>
Cc: 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 RFC v4 02/15] spi: add basic support for SPI offloading
On Wed, 2024-10-23 at 15:59 -0500, 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
> callback 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 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@...libre.com>
> ---
>
> v4 changes:
> * SPI offload functions moved to a separate file instead of spi.c
> (spi.c is already too long).
> * struct spi_offload and devm_spi_offload_get() are back, similar to
> but improved over v1. This avoids having to pass the function ID
> string to every function call and re-lookup the offload instance.
> * offload message prepare/unprepare functions are removed. Instead the
> existing optimize/unoptimize functions should be used. Setting
> spi_message::offload pointer is used as a flag to differentiate
> between an offloaded message and a regular message.
>
> v3 changes:
> * Minor changes to doc comments.
> * Changed to use phandle array for spi-offloads.
> * Changed id to string to make use of spi-offload-names.
>
> v2 changes:
> * This is a rework of "spi: add core support for controllers with offload
> capabilities" from v1.
> * The spi_offload_get() function that Nuno didn't like is gone. Instead,
> there is now a mapping callback that uses the new generic devicetree
> binding to request resources automatically when a SPI device is probed.
> * The spi_offload_enable/disable() functions for dealing with hardware
> triggers are deferred to a separate patch.
> * This leaves adding spi_offload_prepare/unprepare() which have been
> reworked to be a bit more robust.
> ---
> drivers/spi/Kconfig | 3 ++
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-offload.c | 104 ++++++++++++++++++++++++++++++++++++++++
> include/linux/spi/spi-offload.h | 64 +++++++++++++++++++++++++
> include/linux/spi/spi.h | 16 +++++++
> 5 files changed, 188 insertions(+)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 823797217404..d65074b85f62 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -55,6 +55,9 @@ config SPI_MEM
> This extension is meant to simplify interaction with SPI memories
> by providing a high-level interface to send memory-like commands.
>
> +config SPI_OFFLOAD
> + bool
> +
> comment "SPI Master Controller Drivers"
>
> config SPI_AIROHA_SNFI
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index a9b1bc259b68..6a470eb475a2 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -10,6 +10,7 @@ ccflags-$(CONFIG_SPI_DEBUG) := -DDEBUG
> obj-$(CONFIG_SPI_MASTER) += spi.o
> obj-$(CONFIG_SPI_MEM) += spi-mem.o
> obj-$(CONFIG_SPI_MUX) += spi-mux.o
> +obj-$(CONFIG_SPI_OFFLOAD) += spi-offload.o
> obj-$(CONFIG_SPI_SPIDEV) += spidev.o
> obj-$(CONFIG_SPI_LOOPBACK_TEST) += spi-loopback-test.o
>
> diff --git a/drivers/spi/spi-offload.c b/drivers/spi/spi-offload.c
> new file mode 100644
> index 000000000000..c344cbf50bdb
> --- /dev/null
> +++ b/drivers/spi/spi-offload.c
> @@ -0,0 +1,104 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 Analog Devices Inc.
> + * Copyright (C) 2024 BayLibre, SAS
> + */
> +
> +#define DEFAULT_SYMBOL_NAMESPACE SPI_OFFLOAD
> +
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/export.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/spi/spi-offload.h>
> +#include <linux/spi/spi.h>
> +#include <linux/types.h>
> +
> +/**
> + * devm_spi_offload_alloc() - Allocate offload instances
> + * @dev: Device for devm purposes
> + * @num_offloads: Number of offloads to allocate
> + * @priv_size: Size of private data to allocate for each offload
> + *
> + * Offload providers should use this to allocate offload instances.
> + *
> + * Return: Pointer to array of offloads or error on failure.
> + */
> +struct spi_offload *devm_spi_offload_alloc(struct device *dev,
> + size_t num_offloads,
> + size_t priv_size)
> +{
> + struct spi_offload *offloads;
> + void *privs;
> + size_t i;
> +
> + offloads = devm_kcalloc(dev, num_offloads, sizeof(*offloads) + priv_size,
> + GFP_KERNEL);
> + if (!offloads)
> + return ERR_PTR(-ENOMEM);
> +
> + privs = (void *)(offloads + num_offloads);
> +
> + for (i = 0; i < num_offloads; i++) {
> + struct spi_offload *offload = offloads + i;
> + void *priv = privs + i * priv_size;
> +
> + offload->provider_dev = dev;
> + offload->priv = priv;
> + }
Hmm looking at the spi_engine implementation, got me thinking about this. I think
like this we might mess up with natural alignments which can be pretty nasty. Maybe
do something like devres [1] (I guess we do not need the flex array though)?
Now that I also look at this better, I would not do it like this. I would keep it
simple and just allocate one spi_offload object and be done with it. In the future
and when we actually support more than one instance you could introduce a
devm_spi_offload_alloc_array() variant and I'm still not sure if it's that useful.
Anyways this is just personal preference I guess...
[1]: https://elixir.bootlin.com/linux/v6.12-rc4/source/drivers/base/devres.c#L35
- Nuno Sá
Powered by blists - more mailing lists