linux-kernel - Re: [PATCH 3/3] iio: adc: ad7768-1: add support for SPI offload

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aYNTHZmSVdP5_L2T@smile.fi.intel.com>
Date: Wed, 4 Feb 2026 16:09:33 +0200
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Jonathan Santos <Jonathan.Santos@...log.com>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
	Michael.Hennerich@...log.com, lars@...afoo.de, jic23@...nel.org,
	dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org
Subject: Re: [PATCH 3/3] iio: adc: ad7768-1: add support for SPI offload

On Sat, Jan 31, 2026 at 10:35:39PM -0300, Jonathan Santos wrote:
> The AD7768-1 family supports sampling rates up to 1 MSPS, which exceeds
> the capabilities of conventional triggered buffer operations due to SPI
> transaction overhead and interrupt latency.
> 
> Add SPI offload support to enable hardware-accelerated data acquisition
> that bypasses software SPI transactions using continuous data streaming.

...

> +	if (st->offload_en)
> +		return st->oversampling_ratio == 8 ?
> +			AD7768_SCAN_TYPE_OFFLOAD_HIGH_SPEED : AD7768_SCAN_TYPE_OFFLOAD_NORMAL;

Wrong indentation.

...

> +	ret = spi_offload_trigger_enable(st->offload,
> +					 st->offload_trigger,
> +					 &config);

One of the parameters can be moved to the previous line (either 2nd or 3rd).

> +	if (ret) {
> +		spi_unoptimize_message(&st->offload_msg);

> +		return ret;
> +	}

> +	return 0;

	if (ret)
		spi_unoptimize_message(&st->offload_msg);

	return ret;

?

...

> +static bool ad7768_offload_trigger_match(struct spi_offload_trigger *trigger,
> +					 enum spi_offload_trigger_type type,
> +					 u64 *args, u32 nargs)
> +{
> +	if (type != SPI_OFFLOAD_TRIGGER_DATA_READY)
> +		return false;
> +
> +	/* Requires 1 arg to indicate the trigger output signal */

^^^ for the context to below remark.

> +	if (nargs != 1 || args[0] != AD7768_TRIGGER_SOURCE_DRDY)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int ad7768_offload_trigger_request(struct spi_offload_trigger *trigger,
> +					  enum spi_offload_trigger_type type,
> +					  u64 *args, u32 nargs)
> +{
> +	/* Should already be validated by match, but just in case. */

Please, be consistent with a style for one-line comments (pay attention
to the period at the end and capitalisation of the sentence). Choose one
(whatever you prefer or which one is already used) style and follow it
everywhere.

> +	if (nargs != 1)
> +		return -EINVAL;
> +
> +	return 0;
> +}

...

> +static const struct spi_offload_config ad7768_spi_offload_config = {
> +	.capability_flags = SPI_OFFLOAD_CAP_TRIGGER |
> +			    SPI_OFFLOAD_CAP_RX_STREAM_DMA,

It can be one line out of 84 characters.

> +};

...

> +static int ad7768_spi_offload_probe(struct iio_dev *indio_dev,
> +				    struct ad7768_state *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	struct spi_offload_trigger_info trigger_info = {
> +		.fwnode = dev_fwnode(dev),
> +		.ops = &ad7768_offload_trigger_ops,
> +		.priv = st,
> +	};
> +	struct dma_chan *rx_dma;
> +	int ret;
> +
> +	ret = devm_spi_offload_trigger_register(dev, &trigger_info);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to register offload trigger\n");

Can be one line, checkpatch for more than 10 years do not complain on
the long string literals (even in --strict mode). Yes, not all lines
can follow this, I pointed out this one, because it's okay balance between
80 limit and the overhead.

> +
> +	st->offload_trigger = devm_spi_offload_trigger_get(dev, st->offload,
> +		SPI_OFFLOAD_TRIGGER_DATA_READY);

Something wrong with the indentation at least second time. Please, review
the code and make sure it indents accordingly.

> +	if (IS_ERR(st->offload_trigger))
> +		return dev_err_probe(dev, PTR_ERR(st->offload_trigger),
> +				     "failed to get offload trigger\n");
> +
> +	rx_dma = devm_spi_offload_rx_stream_request_dma_chan(dev, st->offload);
> +	if (IS_ERR(rx_dma))
> +		return dev_err_probe(dev, PTR_ERR(rx_dma),
> +				     "failed to get offload RX DMA\n");
> +
> +	ret = devm_iio_dmaengine_buffer_setup_with_handle(dev, indio_dev,
> +		rx_dma, IIO_BUFFER_DIRECTION_IN);

Indentation.

> +	if (ret)
> +		return dev_err_probe(dev, PTR_ERR(rx_dma),
> +				     "failed to setup offload RX DMA\n");
> +
> +	indio_dev->setup_ops = &ad7768_offload_buffer_ops;
> +	st->offload_en = true;
> +
> +	return 0;
> +}

...

> +	st->offload = devm_spi_offload_get(&spi->dev, spi, &ad7768_spi_offload_config);
> +	ret = PTR_ERR_OR_ZERO(st->offload);
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(&spi->dev, ret, "failed to get SPI offload\n");

Add

	struct device *dev = &spi->dev;

at the top of the function and make some lines shorter and easier to follow.

> +	/* If not using SPI offload, fall back to low speed usage. */
> +	if (ret == -ENODEV) {

No need to check for ENODEV twice.

	if (ret == -ENODEV) {
		...
	} else if (ret) {
		return dev_err_probe(...);
	} else {
		...
	}

> +		ret = ad7768_triggered_buffer_alloc(indio_dev);
> +		if (ret)
> +			return ret;
> +	} else {
> +		ret = ad7768_spi_offload_probe(indio_dev, st);
> +		if (ret)
> +			return ret;
> +	}

-- 
With Best Regards,
Andy Shevchenko