Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Marcelo Schmitt" <marcelo.schmitt@analog.com>
Cc: conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 5/5] iio: adc: ltc2378: Enable triggered buffer data capture
Date: Wed, 17 Jun 2026 02:18:17 +0000	[thread overview]
Message-ID: <20260617021817.A5E9B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <22248e8ba646fbe8edfc87e2b9be527632d5b48c.1781661028.git.marcelo.schmitt@analog.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Triggered buffer setup fallback is unreachable when SPI offload support is compiled in, causing driver probe to fail on systems without SPI offload hardware.
- [High] The triggered buffer pushes misaligned data on big-endian architectures, yielding constant zero readings for 16-bit parts.
- [Low] The newly added file `ltc2378-triggered-buffer.c` uses `#include <ltc2378.h>` instead of `#include "ltc2378.h"`, causing a compile error.
--

commit 975300646e8809a19fe7c482edd698d128476317
Author: Marcelo Schmitt <marcelo.schmitt@analog.com>

iio: adc: ltc2378: Enable triggered buffer data capture

Enable users to run triggered data captures with LTC2378 and similar ADCs.

> diff --git a/drivers/iio/adc/ltc2378-lib-core.c b/drivers/iio/adc/ltc2378-lib-core.c
> index 1160f4324d019..ec83e9f2ae81c 100644
> --- a/drivers/iio/adc/ltc2378-lib-core.c
> +++ b/drivers/iio/adc/ltc2378-lib-core.c
> @@ -18,7 +18,7 @@ int ltc2378_lib_buffer_setup(struct iio_dev *indio_dev, struct ltc2378_state *st
>  
>  	ret = __ltc2378_set_offload_ops(st);
>  	if (ret == -EOPNOTSUPP)
> -		return 0; /* Let device setup complete without buffer support */
> +		goto trigger_buf_setup;
>  
>  	if (!ret)
>  		ret = st->ops->buffer_setup(indio_dev, st);
>  
>  	if (ret)
>  		return dev_err_probe(dev, ret, "error on SPI offload setup\n");

[Severity: High]
If CONFIG_LTC2378_LIB_OFFLOAD_BUFFER is enabled, does __ltc2378_set_offload_ops()
unconditionally return 0?

If so, the fallback condition if (ret == -EOPNOTSUPP) is never met. The driver
then calls st->ops->buffer_setup(), which maps to ltc2378_offload_buffer_setup.

If the device lacks an SPI offload engine in the device tree, 
devm_spi_offload_get() will return -ENODEV, failing the probe entirely.

Does this code prevent the driver from gracefully falling back to triggered
buffers on non-offload setups?

[ ... ]
> diff --git a/drivers/iio/adc/ltc2378-triggered-buffer.c b/drivers/iio/adc/ltc2378-triggered-buffer.c
> new file mode 100644
> index 0000000000000..d1d788fb5cb4f
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2378-triggered-buffer.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2026 Analog Devices, Inc.
> + * Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
> + */
> +
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#include <ltc2378.h>

[Severity: Low]
Does this include directive cause a compile error because it uses angle
brackets?

In C, #include <...> instructs the preprocessor to search standard system
directories. Because ltc2378.h resides in the local subsystem directory
drivers/iio/adc/ and the IIO Makefile does not add -I$(src), compilation
fails with a missing file error.

> +static irqreturn_t ltc2378_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct ltc2378_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = ltc2378_convert_and_acquire(st);
> +	if (ret < 0)
> +		goto err_out;
> +
> +	iio_push_to_buffers_with_ts(indio_dev, &st->scan, sizeof(st->scan),
> +				    pf->timestamp);

[Severity: High]
Does this code push misaligned data on big-endian architectures, resulting
in constant zero readings for 16-bit parts?

The IIO channel specifies storagebits = 32 and shift = 0, so consumers expect
a 32-bit native-endian value with data in the lowest 16 bits.

During ltc2378_trigger_handler(), spi_sync_transfer() with bits_per_word = 16
stores the 16-bit sample in CPU byte order at the start of the st->scan.data
union (sample_buf16).

On little-endian systems, this natively aligns with the lower 16 bits of the
32-bit container. However, on big-endian systems, the 16-bit sample occupies
the most significant bytes of the 32-bit word.

When the IIO core processes this as a 32-bit word, the data sits in the upper
16 bits while the lower 16 bits remain zero. Since scan_type.shift is 0,
masking the lower 16 bits results in the consumer incorrectly receiving zero
for every sample.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781661028.git.marcelo.schmitt@analog.com?part=5

      reply	other threads:[~2026-06-17  2:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-17  2:02 [PATCH v3 0/5] iio: adc: Add support for LTC2378 and similar ADCs Marcelo Schmitt
2026-06-17  2:03 ` [PATCH v3 1/5] dt-bindings: iio: adc: Add ltc2378 Marcelo Schmitt
2026-06-17  2:03 ` [PATCH v3 2/5] iio: adc: ltc2378: Add support for LTC2378-20 and similar ADCs Marcelo Schmitt
2026-06-17  2:16   ` sashiko-bot
2026-06-17  2:03 ` [RFC PATCH v3 3/5] iio: buffer: Extend DMAengine buffer interfaces to take extra sysfs attributes Marcelo Schmitt
2026-06-17  2:13   ` sashiko-bot
2026-06-17  2:04 ` [PATCH v3 4/5] iio: adc: ltc2378: Enable high-speed data capture Marcelo Schmitt
2026-06-17  2:17   ` sashiko-bot
2026-06-17  2:04 ` [PATCH v3 5/5] iio: adc: ltc2378: Enable triggered buffer " Marcelo Schmitt
2026-06-17  2:18   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260617021817.A5E9B1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=marcelo.schmitt@analog.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox