Re: [PATCH] dt-bindings: iio: adc: Add schema for AD7816/7/8 digital temperature sensor

Hi,

Thank you for the detailed analysis and for pointing out these issues.

I would like to clarify that these are pre-existing issues in the
driver and were not introduced by my documentation patch. My current
goal is limited to adding the missing Device Tree bindings to resolve
the checkpatch.pl warnings.

I acknowledge that the driver has issues regarding concurrency
protection, DMA mapping, and polling loops. I intend to focus on those
in a separate, subsequent patch series after this one is merged, as I
believe refactoring the driver logic requires a broader scope.

Thank you for your patience and for reviewing my contribution.

Best regards, Taha Narimani


On Sat, May 30, 2026 at 2:41 PM Taha Narimani
[off-list ref] wrote:

Hi,

Thank you for the detailed analysis and for pointing out these issues.

I would like to clarify that these are pre-existing issues in the driver and were not introduced by my documentation patch. My current goal is limited to adding the missing Device Tree bindings to resolve the checkpatch.pl warnings.

I acknowledge that the driver has issues regarding concurrency protection, DMA mapping, and polling loops. I intend to focus on those in a separate, subsequent patch series after this one is merged, as I believe refactoring the driver logic requires a broader scope.

Thank you for your patience and for reviewing my contribution.

Best regards, Taha Narimani


On Sat, May 30, 2026 at 2:36 PM [off-list ref] wrote:

quoted

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing concurrency protection during sysfs accesses.
- [High] Stack-allocated buffers are passed directly to `spi_read()` and `spi_write()`, violating DMA mapping rules.
- [High] Unbounded polling loop for the BUSY pin can cause a hard CPU lockup.
--

commit cb7e57cc9cdde2668abda84c0f0d3e60ebb9a962
Author: Taha Narimani [off-list ref]

dt-bindings: iio: adc: Add schema for AD7816/7/8 digital temperature sensor

This commit adds a new YAML device tree binding schema for the Analog
Devices AD7816, AD7817, and AD7818 digital temperature sensors. It
documents the compatible strings, SPI configuration, interrupts, and
required GPIO pins.

quoted

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
new file mode 100644
index 0000000000000..74008c14d40c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml

@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad7816.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#

[ ... ]

quoted

+properties:
+  compatible:
+    enum:
+      - adi,ad7816
+      - adi,ad7817
+      - adi,ad7818

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing lock to
protect concurrent sysfs accesses?

Looking at ad7816_show_value() in drivers/staging/iio/adc/ad7816.c:

drivers/staging/iio/adc/ad7816.c:ad7816_show_value() {
    ...
    ret = ad7816_spi_read(chip, &data);
    ...
}

Since this is called for world-readable sysfs attributes, can multiple
userspace processes trigger this concurrently?

If so, could concurrent execution of ad7816_spi_read() interleave GPIO pin
toggling and SPI transfers, corrupting the hardware state?


[Severity: High]
This isn't a bug introduced by this patch, but does this code pass
stack-allocated buffers to SPI transfers?

Looking at drivers/staging/iio/adc/ad7816.c:

drivers/staging/iio/adc/ad7816.c:ad7816_spi_read() {
    ...
    ret = spi_read(spi_dev, &buf, sizeof(*data));
    ...
}

And also in ad7816_spi_write():

drivers/staging/iio/adc/ad7816.c:ad7816_spi_write() {
    ...
    ret = spi_write(spi_dev, &data, sizeof(data));
    ...
}

Since the SPI subsystem often maps these buffers for DMA, will passing
variables from the stack cause mapping failures or crashes on architectures
with virtually mapped kernel stacks?


[Severity: High]
This isn't a bug introduced by this patch, but can this unbounded polling
loop cause a CPU lockup?

Looking at ad7816_spi_read() in drivers/staging/iio/adc/ad7816.c:

drivers/staging/iio/adc/ad7816.c:ad7816_spi_read() {
    ...
    while (gpiod_get_value(chip->busy_pin))
        cpu_relax();
    ...
}

If the external device malfunctions or the GPIO line gets permanently stuck
high, will this loop block indefinitely and cause a hard lockup?

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260530135302.54688-1-tahanarimani3443@gmail.com?part=1