Re: [PATCH v3 10/12] spi: offload trigger: add ADI Util Sigma-Delta SPI driver

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, 2025-07-01 at 16:37 -0500, David Lechner wrote:
> Add a new driver for the ADI Util Sigma-Delta SPI FPGA IP core.
> 
> This is used to trigger a SPI offload based on a RDY signal from an ADC
> while masking out other signals on the same line.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  MAINTAINERS                                        |  2 +-
>  drivers/spi/Kconfig                                |  5 ++
>  drivers/spi/Makefile                               |  1 +
>  .../spi/spi-offload-trigger-adi-util-sigma-delta.c | 62
> ++++++++++++++++++++++
>  4 files changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index
> 60ba572be7f5b48c0ab1d0d9724e19e335e8761b..4ed4977deb6ddc545be39b5c1d5e9959e9fe
> 64cf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23357,7 +23357,7 @@ F:	include/linux/mtd/spi-nor.h
>  
>  SPI OFFLOAD
>  R:	David Lechner <dlechner@baylibre.com>
> -F:	drivers/spi/spi-offload-trigger-pwm.c
> +F:	drivers/spi/spi-offload-trigger-*.c
>  F:	drivers/spi/spi-offload.c
>  F:	include/linux/spi/offload/
>  K:	spi_offload
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index
> c51da3fc3604977b05388687e5e64a58370186c4..e69f060d3875c168a2dc701a372e47b8ffd3
> 3268 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -1355,6 +1355,11 @@ if SPI_OFFLOAD
>  
>  comment "SPI Offload triggers"
>  
> +config SPI_OFFLOAD_TRIGGER_ADI_UTIL_SD
> +	tristate "SPI offload trigger using ADI sigma-delta utility"
> +	help
> +	  SPI offload trigger from ADI sigma-delta utility FPGA IP block.
> +
>  config SPI_OFFLOAD_TRIGGER_PWM
>  	tristate "SPI offload trigger using PWM"
>  	depends on PWM
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index
> 4ea89f6fc531625060255ecff237470927e1f041..51f9f16ed734424ff10672a04f2ec166dc63
> 7e0b 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -170,3 +170,4 @@ obj-$(CONFIG_SPI_SLAVE_SYSTEM_CONTROL)	+= spi-slave-
> system-control.o
>  
>  # SPI offload triggers
>  obj-$(CONFIG_SPI_OFFLOAD_TRIGGER_PWM)	+= spi-offload-trigger-pwm.o
> +obj-$(CONFIG_SPI_OFFLOAD_TRIGGER_ADI_UTIL_SD) += spi-offload-trigger-adi-
> util-sigma-delta.o
> diff --git a/drivers/spi/spi-offload-trigger-adi-util-sigma-delta.c
> b/drivers/spi/spi-offload-trigger-adi-util-sigma-delta.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..8468c773713a3d203b2e668f340ee3f477b8
> fb6c
> --- /dev/null
> +++ b/drivers/spi/spi-offload-trigger-adi-util-sigma-delta.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Analog Devices Inc.
> + * Copyright (C) 2025 BayLibre, SAS
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/spi/offload/provider.h>
> +#include <linux/spi/offload/types.h>
> +#include <linux/types.h>
> +
> +static bool adi_util_sigma_delta_match(struct spi_offload_trigger *trigger,
> +				       enum spi_offload_trigger_type type,
> +				       u64 *args, u32 nargs)
> +{
> +	return type == SPI_OFFLOAD_TRIGGER_DATA_READY && nargs == 0;
> +}
> +
> +static const struct spi_offload_trigger_ops adi_util_sigma_delta_ops = {
> +	.match = adi_util_sigma_delta_match,
> +};
> +
> +static int adi_util_sigma_delta_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct spi_offload_trigger_info info = {
> +		.fwnode = dev_fwnode(dev),
> +		.ops = &adi_util_sigma_delta_ops,
> +	};
> +	struct clk *clk;
> +
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk), "Failed to get
> clock\n");
> +

Small nit. Did you consider enabling/disabling the clock on the trigger
enable()/disable() callback? I guess the ref clk will be enabled anyways by
someone else but conceptually kind of makes sense to enable the resource only
when needed.

Not a big deal (at least to me).

- Nuno Sá

> +	return devm_spi_offload_trigger_register(dev, &info);
> +}
> +
> +static const struct of_device_id adi_util_sigma_delta_of_match_table[] = {
> +	{ .compatible = "adi,util-sigma-delta-spi", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, adi_util_sigma_delta_of_match_table);
> +
> +static struct platform_driver adi_util_sigma_delta_driver = {
> +	.probe  = adi_util_sigma_delta_probe,
> +	.driver = {
> +		.name = "adi-util-sigma-delta-spi",
> +		.of_match_table = adi_util_sigma_delta_of_match_table,
> +	},
> +};
> +module_platform_driver(adi_util_sigma_delta_driver);
> +
> +MODULE_AUTHOR("David Lechner <dlechner@baylibre.com>");
> +MODULE_DESCRIPTION("ADI Sigma-Delta SPI offload trigger utility driver");
> +MODULE_LICENSE("GPL");