[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b8a54ec7-5c8f-422b-92cf-ece744073868@gmail.com>
Date: Fri, 25 Oct 2024 18:39:33 +0200 (GMT+02:00)
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
Oct 25, 2024 14:59:20 Nuno Sá <noname.nuno@...il.com>:
> 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)?
>
Actually we should use the flex array or something similar to what IIO does.
PS: just trying some email client from my phone so hopefully this is not too messed up.
- Nuno Sá
Powered by blists - more mailing lists