| 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: <225da1bc0f0b9407c3f7b3374cbbbf6cc6b43aa6.camel@gmail.com>
Date: Tue, 17 Dec 2024 12:30:37 +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, Jonathan Cameron
<Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v6 02/17] spi: offload: add support for hardware triggers
On Wed, 2024-12-11 at 14:54 -0600, David Lechner wrote:
> Extend SPI offloading to support hardware triggers.
>
> This allows an arbitrary hardware trigger to be used to start a SPI
> transfer that was previously set up with spi_optimize_message().
>
> A new struct spi_offload_trigger is introduced that can be used to
> configure any type of trigger. It has a type discriminator and a union
> to allow it to be extended in the future. Two trigger types are defined
> to start with. One is a trigger that indicates that the SPI peripheral
> is ready to read or write data. The other is a periodic trigger to
> repeat a SPI message at a fixed rate.
>
> There is also a spi_offload_hw_trigger_validate() function that works
> similar to clk_round_rate(). It basically asks the question of if we
> enabled the hardware trigger what would the actual parameters be. This
> can be used to test if the requested trigger type is actually supported
> by the hardware and for periodic triggers, it can be used to find the
> actual rate that the hardware is capable of.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
>
One minor comment (and another suggestion) inline...
Reviewed-by: Nuno Sa <nuno.sa@...log.com>
> v6 changes:
> * Updated for header file split.
>
> v5 changes:
> * Use struct kref instead of struct dev for trigger lifetime management.
> * Don't use __free() for args.fwnode.
> * Pass *trigger instead of *priv to all callbacks.
> * Add new *spi_offload_trigger_get_priv() function.
> * Use ops instead of priv for "provider is gone" flag.
> * Combine devm_spi_offload_trigger_alloc() and
> devm_spi_offload_trigger_register() into one function.
> * Add kernel-doc comments for public functions.
>
> v4 changes:
> * Added new struct spi_offload_trigger that is a generic struct for any
> hardware trigger rather than returning a struct clk.
> * Added new spi_offload_hw_trigger_validate() function.
> * Dropped extra locking since it was too restrictive.
>
> v3 changes:
> * renamed enable/disable functions to spi_offload_hw_trigger_*mode*_...
> * added spi_offload_hw_trigger_get_clk() function
> * fixed missing EXPORT_SYMBOL_GPL
>
> v2 changes:
> * This is split out from "spi: add core support for controllers with
> offload capabilities".
> * Added locking for offload trigger to claim exclusive use of the SPI
> bus.
> ---
> drivers/spi/spi-offload.c | 281
> +++++++++++++++++++++++++++++++++++
> include/linux/spi/offload/consumer.h | 12 ++
> include/linux/spi/offload/provider.h | 28 ++++
> include/linux/spi/offload/types.h | 37 +++++
> 4 files changed, 358 insertions(+)
>
> diff --git a/drivers/spi/spi-offload.c b/drivers/spi/spi-offload.c
> index
> 3a40ef30debf09c6fd7b2c14526f3e5976e2b21f..43582e50e279c4b1b958765fec556aaa9118
> 0e55 100644
> --- a/drivers/spi/spi-offload.c
> +++ b/drivers/spi/spi-offload.c
> @@ -19,7 +19,11 @@
> #include <linux/cleanup.h>
> #include <linux/device.h>
> #include <linux/export.h>
> +#include <linux/kref.h>
> +#include <linux/list.h>
> #include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/property.h>
> #include <linux/spi/offload/consumer.h>
> #include <linux/spi/offload/provider.h>
> #include <linux/spi/offload/types.h>
> @@ -31,6 +35,23 @@ struct spi_controller_and_offload {
> struct spi_offload *offload;
> };
>
> +struct spi_offload_trigger {
> + struct list_head list;
> + struct kref ref;
> + struct fwnode_handle *fwnode;
> + /* synchronizes calling ops and driver registration */
> + struct mutex lock;
> + /*
> + * If the provider goes away while the consumer still has a
> reference,
> + * ops and priv will be set to NULL and all calls will fail with -
> ENODEV.
> + */
> + const struct spi_offload_trigger_ops *ops;
> + void *priv;
> +};
> +
> +static LIST_HEAD(spi_offload_triggers);
> +static DEFINE_MUTEX(spi_offload_triggers_lock);
> +
> /**
> * devm_spi_offload_alloc() - Allocate offload instance
> * @dev: Device for devm purposes and assigned to &struct
> spi_offload.provider_dev
> @@ -112,3 +133,263 @@ struct spi_offload *devm_spi_offload_get(struct device
> *dev,
> return resource->offload;
> }
> EXPORT_SYMBOL_GPL(devm_spi_offload_get);
> +
> +static void spi_offload_trigger_free(struct kref *ref)
> +{
> + struct spi_offload_trigger *trigger =
> + container_of(ref, struct spi_offload_trigger, ref);
> +
> + mutex_destroy(&trigger->lock);
> + fwnode_handle_put(trigger->fwnode);
> + kfree(trigger);
> +}
> +
> +static void spi_offload_trigger_put(void *data)
> +{
> + struct spi_offload_trigger *trigger = data;
> +
> + scoped_guard(mutex, &trigger->lock)
> + if (trigger->ops && trigger->ops->release)
> + trigger->ops->release(trigger);
> +
> + kref_put(&trigger->ref, spi_offload_trigger_free);
> +}
> +
> +static struct spi_offload_trigger
> +*spi_offload_trigger_get(enum spi_offload_trigger_type type,
> + struct fwnode_reference_args *args)
> +{
> + struct spi_offload_trigger *trigger;
> + bool match = false;
> + int ret;
> +
> + guard(mutex)(&spi_offload_triggers_lock);
> +
> + list_for_each_entry(trigger, &spi_offload_triggers, list) {
> + if (trigger->fwnode != args->fwnode)
> + continue;
> +
> + match = trigger->ops->match(trigger, type, args->args, args-
> >nargs);
> + if (match)
> + break;
> + }
> +
> + if (!match)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + guard(mutex)(&trigger->lock);
> +
> + if (!trigger->ops)
> + return ERR_PTR(-ENODEV);
> +
> + if (trigger->ops->request) {
> + ret = trigger->ops->request(trigger, type, args->args, args-
> >nargs);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + kref_get(&trigger->ref);
maybe try_module_get() would also make sense...
> +
> + return trigger;
> +}
> +
> +/**
> + * devm_spi_offload_trigger_get() - Get an offload trigger instance
> + * @dev: Device for devm purposes.
> + * @offload: Offload instance connected to a trigger.
> + * @type: Trigger type to get.
> + *
> + * Return: Offload trigger instance or error on failure.
> + */
> +struct spi_offload_trigger
> +*devm_spi_offload_trigger_get(struct device *dev,
> + struct spi_offload *offload,
> + enum spi_offload_trigger_type type)
> +{
> + struct spi_offload_trigger *trigger;
> + struct fwnode_reference_args args;
> + int ret;
> +
> + ret = fwnode_property_get_reference_args(dev_fwnode(offload-
> >provider_dev),
> + "trigger-sources",
> + "#trigger-source-cells", 0,
> 0,
> + &args);
I guess at some point we can add these to fwlinks?
- Nuno Sá
Powered by blists - more mailing lists