[PATCH v1 0/2] iio: adc: Add support for TI ADS131M0x ADCs

[PATCH v1 0/2] iio: adc: Add support for TI ADS131M0x ADCs

[Oleksij Rempel]

This series adds support for the Texas Instruments ADS131M0x family of
24-bit, simultaneous-sampling ADCs. The first patch introduces the
DeviceTree binding, and the second adds the driver itself.

These devices are not compatible with the ADS131E0x series despite the
similar naming.

David Jander (1):
  iio: adc: Add TI ADS131M0x ADC driver

Oleksij Rempel (1):
  bindings: iio: adc: Add bindings for TI ADS131M0x ADCs

 .../bindings/iio/adc/ti,ads131m08.yaml        | 162 ++++
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ti-ads131m0x.c                | 886 ++++++++++++++++++
 4 files changed, 1059 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
 create mode 100644 drivers/iio/adc/ti-ads131m0x.c

--
2.47.3

[PATCH v1 1/2] bindings: iio: adc: Add bindings for TI ADS131M0x ADCs

[Oleksij Rempel]

Add device tree bindings documentation for the Texas Instruments
ADS131M0x analog-to-digital converters. This family includes the ADS131M02,
ADS131M03, ADS131M04, ADS131M06, and ADS131M08 variants.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../bindings/iio/adc/ti,ads131m08.yaml        | 162 ++++++++++++++++++
 1 file changed, 162 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
new file mode 100644
index 000000000000..193ac84c41cd
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
@@ -0,0 +1,162 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads131m08.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments ADS131M0x 2-, 3-, 4-, 6- and 8-Channel ADCs
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+  The ADS131M0x are a family of multichannel, simultaneous sampling,
+  24-bit, delta-sigma, analog-to-digital converters (ADCs) with a
+  built-in programmable gain amplifier (PGA) and internal reference.
+  Communication with the ADC chip is via SPI.
+
+  Datasheets:
+  - ADS131M08: https://www.ti.com/lit/ds/symlink/ads131m08.pdf
+  - ADS131M06: https://www.ti.com/lit/ds/symlink/ads131m06.pdf
+  - ADS131M04: https://www.ti.com/lit/ds/symlink/ads131m04.pdf
+  - ADS131M03: https://www.ti.com/lit/ds/symlink/ads131m03.pdf
+  - ADS131M02: https://www.ti.com/lit/ds/symlink/ads131m02.pdf
+
+properties:
+  compatible:
+    enum:
+      - ti,ads131m02
+      - ti,ads131m03
+      - ti,ads131m04
+      - ti,ads131m06
+      - ti,ads131m08
+
+  reg:
+    description: SPI chip select number.
+
+  clocks:
+    description:
+      Phandle to the external clock source required by the ADC's CLKIN pin.
+      The datasheet recommends specific frequencies based on the desired power
+      mode (e.g., 8.192 MHz for High-Resolution mode).
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+patternProperties:
+  "^channel@([0-7])$":
+    type: object
+    $ref: /schemas/iio/adc/adc.yaml#
+    description: |
+      Properties for a single ADC channel. The maximum valid channel number
+      depends on the specific compatible string used (e.g., 0-1 for ads131m02,
+      0-7 for ads131m08).
+
+    properties:
+      reg:
+        description: The channel index (0-7).
+        minimum: 0
+        maximum: 7 # Max channels on ADS131M08
+
+      label: true
+
+    required:
+      - reg
+
+    unevaluatedProperties: false
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,ads131m02
+    then:
+      patternProperties:
+        "^channel@[0-7]$":
+          properties:
+            reg:
+              maximum: 1
+        "^channel@([2-7])$": false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,ads131m03
+    then:
+      patternProperties:
+        "^channel@[0-7]$":
+          properties:
+            reg:
+              maximum: 2
+        "^channel@([3-7])$": false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,ads131m04
+    then:
+      patternProperties:
+        "^channel@[0-7]$":
+          properties:
+            reg:
+              maximum: 3
+        "^channel@([4-7])$": false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,ads131m06
+    then:
+      patternProperties:
+        "^channel@[0-7]$":
+          properties:
+            reg:
+              maximum: 5
+        "^channel@([6-7])$": false
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/stm32mp1-clks.h>
+
+    spi1 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "ti,ads131m02";
+            reg = <0>;
+            spi-max-frequency = <8000000>;
+
+            clocks = <&rcc CK_MCO2>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            channel@0 {
+                reg = <0>;
+                label = "input_voltage";
+            };
+
+            channel@1 {
+                reg = <1>;
+                label = "input_current";
+            };
+        };
+    };
-- 
2.47.3

Re: [PATCH v1 1/2] bindings: iio: adc: Add bindings for TI ADS131M0x ADCs

[Jonathan Cameron]

On Wed, 5 Nov 2025 15:38:13 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Add device tree bindings documentation for the Texas Instruments
> ADS131M0x analog-to-digital converters. This family includes the ADS131M02,
> ADS131M03, ADS131M04, ADS131M06, and ADS131M08 variants.
Hi Olkesij,

Add a clear statement of difference between them that means we can't
use a single fallback compatible. You kind of state it in the binding
(number of channels) but having it here explicitly makes for easier review.


> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../bindings/iio/adc/ti,ads131m08.yaml        | 162 ++++++++++++++++++
>  1 file changed, 162 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
> new file mode 100644
> index 000000000000..193ac84c41cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
> @@ -0,0 +1,162 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads131m08.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS131M0x 2-, 3-, 4-, 6- and 8-Channel ADCs
> +
> +maintainers:
> +  - Oleksij Rempel <o.rempel@pengutronix.de>
> +
> +description: |
> +  The ADS131M0x are a family of multichannel, simultaneous sampling,
> +  24-bit, delta-sigma, analog-to-digital converters (ADCs) with a
> +  built-in programmable gain amplifier (PGA) and internal reference.
> +  Communication with the ADC chip is via SPI.
> +
> +  Datasheets:
> +  - ADS131M08: https://www.ti.com/lit/ds/symlink/ads131m08.pdf
> +  - ADS131M06: https://www.ti.com/lit/ds/symlink/ads131m06.pdf
> +  - ADS131M04: https://www.ti.com/lit/ds/symlink/ads131m04.pdf
> +  - ADS131M03: https://www.ti.com/lit/ds/symlink/ads131m03.pdf
> +  - ADS131M02: https://www.ti.com/lit/ds/symlink/ads131m02.pdf
Trivial but seems a little odd to have these in reverse order of the compatibles.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ads131m02
> +      - ti,ads131m03
> +      - ti,ads131m04
> +      - ti,ads131m06
> +      - ti,ads131m08
> +
> +  reg:
> +    description: SPI chip select number.
> +
> +  clocks:
> +    description:
> +      Phandle to the external clock source required by the ADC's CLKIN pin.
> +      The datasheet recommends specific frequencies based on the desired power
> +      mode (e.g., 8.192 MHz for High-Resolution mode).
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

There should be some supplies here.
Looks like REFIN is optional but AVDD and DVDD are required.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +patternProperties:
> +  "^channel@([0-7])$":
> +    type: object
> +    $ref: /schemas/iio/adc/adc.yaml#
> +    description: |

No need for | on this one as I don't think formatting needs to be controlled.

> +      Properties for a single ADC channel. The maximum valid channel number
> +      depends on the specific compatible string used (e.g., 0-1 for ads131m02,
> +      0-7 for ads131m08).
> +
> +    properties:
> +      reg:
> +        description: The channel index (0-7).
> +        minimum: 0
> +        maximum: 7 # Max channels on ADS131M08
> +
> +      label: true
> +
> +    required:
> +      - reg
> +
> +    unevaluatedProperties: false
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,ads131m02
> +    then:
> +      patternProperties:
> +        "^channel@[0-7]$":
> +          properties:
> +            reg:
> +              maximum: 1
> +        "^channel@([2-7])$": false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,ads131m03
> +    then:
> +      patternProperties:
> +        "^channel@[0-7]$":
> +          properties:
> +            reg:
> +              maximum: 2
> +        "^channel@([3-7])$": false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,ads131m04
> +    then:
> +      patternProperties:
> +        "^channel@[0-7]$":
> +          properties:
> +            reg:
> +              maximum: 3
> +        "^channel@([4-7])$": false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,ads131m06
> +    then:
> +      patternProperties:
> +        "^channel@[0-7]$":
> +          properties:
> +            reg:
> +              maximum: 5
> +        "^channel@([6-7])$": false
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/stm32mp1-clks.h>
> +
> +    spi1 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "ti,ads131m02";
> +            reg = <0>;
> +            spi-max-frequency = <8000000>;
> +
> +            clocks = <&rcc CK_MCO2>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            channel@0 {
> +                reg = <0>;
> +                label = "input_voltage";
> +            };
> +
> +            channel@1 {
> +                reg = <1>;
> +                label = "input_current";
> +            };
> +        };
> +    };

Re: [PATCH v1 1/2] bindings: iio: adc: Add bindings for TI ADS131M0x ADCs

[David Lechner]

On 11/5/25 8:38 AM, Oleksij Rempel wrote:
> Add device tree bindings documentation for the Texas Instruments
> ADS131M0x analog-to-digital converters. This family includes the ADS131M02,
> ADS131M03, ADS131M04, ADS131M06, and ADS131M08 variants.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../bindings/iio/adc/ti,ads131m08.yaml        | 162 ++++++++++++++++++

For consistency, I always try to name the file after the lowest part number.
Since we usually list things from lowest to highest it makes the file name
match the first item in the compatible: list. Not sure how widely that is
done in general though.

>  1 file changed, 162 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
> new file mode 100644
> index 000000000000..193ac84c41cd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads131m08.yaml
> @@ -0,0 +1,162 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads131m08.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ADS131M0x 2-, 3-, 4-, 6- and 8-Channel ADCs

The more interesting thing to me than the number of channels is that
these are simultaneous sampling. But I guess it says that below already.

> +
> +maintainers:
> +  - Oleksij Rempel <o.rempel@pengutronix.de>
> +
> +description: |
> +  The ADS131M0x are a family of multichannel, simultaneous sampling,
> +  24-bit, delta-sigma, analog-to-digital converters (ADCs) with a
> +  built-in programmable gain amplifier (PGA) and internal reference.
> +  Communication with the ADC chip is via SPI.
> +
> +  Datasheets:
> +  - ADS131M08: https://www.ti.com/lit/ds/symlink/ads131m08.pdf
> +  - ADS131M06: https://www.ti.com/lit/ds/symlink/ads131m06.pdf
> +  - ADS131M04: https://www.ti.com/lit/ds/symlink/ads131m04.pdf
> +  - ADS131M03: https://www.ti.com/lit/ds/symlink/ads131m03.pdf
> +  - ADS131M02: https://www.ti.com/lit/ds/symlink/ads131m02.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ads131m02
> +      - ti,ads131m03
> +      - ti,ads131m04
> +      - ti,ads131m06
> +      - ti,ads131m08
> +
> +  reg:
> +    description: SPI chip select number.
> +
> +  clocks:
> +    description:
> +      Phandle to the external clock source required by the ADC's CLKIN pin.
> +      The datasheet recommends specific frequencies based on the desired power
> +      mode (e.g., 8.192 MHz for High-Resolution mode).
> +    maxItems: 1

We probably also need to know what is wired to the clock pin so that the driver
can correctly set XTAL_DIS in the CLOCK register.

clock-names:
  description:
    Indicates if a crystal oscillator (XTAL) or CMOS signal is connected (CLKIN).
  enum: [xtal, clkin]


> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +

In addition to the supplies that Jonathan mentioned, we can also add
an interrupts property for the DRDY output signal.

And a reset-gpios for the reset signal.

These are all trivial bindings we know are going to be correct even if
the driver doesn't use them yet.

> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +patternProperties:
> +  "^channel@([0-7])$":
> +    type: object
> +    $ref: /schemas/iio/adc/adc.yaml#
> +    description: |
> +      Properties for a single ADC channel. The maximum valid channel number
> +      depends on the specific compatible string used (e.g., 0-1 for ads131m02,
> +      0-7 for ads131m08).

I think this description would be better as a comment on the if statements
below so that it isn't so far away from the relevant code.

> +
> +    properties:
> +      reg:
> +        description: The channel index (0-7).
> +        minimum: 0
> +        maximum: 7 # Max channels on ADS131M08
> +
> +      label: true
> +
> +    required:
> +      - reg
> +
> +    unevaluatedProperties: false
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,ads131m02
> +    then:
> +      patternProperties:
> +        "^channel@[0-7]$":

        "^channel@[0-1]$":

Same pattern applies to the similar statements below (that I trimmed from the reply).

> +          properties:
> +            reg:
> +              maximum: 1
> +        "^channel@([2-7])$": false
> +

[PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[Oleksij Rempel]

From: David Jander <david@protonic.nl>

Add a new IIO ADC driver for Texas Instruments ADS131M0x devices
(ADS131M02/03/04/06/08). These are 24-bit, up to 64 kSPS, simultaneous-
sampling delta-sigma ADCs accessed via SPI.

Highlights:
- Supports 2/3/4/6/8-channel variants with per-channel RAW and SCALE.
- Implements device-required full-duplex fixed-frame transfers.
- Handles both input and output CRC; uses a non-reflected CCITT (0x1021)
  implementation because the generic crc_ccitt helper is incompatible.

Note: Despite the almost identical name, this hardware is not
compatible with the ADS131E0x series handled by
drivers/iio/adc/ti-ads131e08.c.

Signed-off-by: David Jander <david@protonic.nl>
Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/iio/adc/Kconfig        |  10 +
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/ti-ads131m0x.c | 886 +++++++++++++++++++++++++++++++++
 3 files changed, 897 insertions(+)
 create mode 100644 drivers/iio/adc/ti-ads131m0x.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58a14e6833f6..c17f8914358c 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1691,6 +1691,16 @@ config TI_ADS131E08
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-ads131e08.
 
+config TI_ADS131M0X
+	tristate "Texas Instruments ADS131M0x"
+	depends on SPI && COMMON_CLK
+	help
+	  Say yes here to get support for Texas Instruments ADS131M02, ADS131M03,
+	  ADS131M04, ADS131M06 and ADS131M08 chips.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called ti-ads131m0x.
+
 config TI_ADS7138
 	tristate "Texas Instruments ADS7128 and ADS7138 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d008f78dc010..c23dae3ddcc7 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -147,6 +147,7 @@ obj-$(CONFIG_TI_ADS1119) += ti-ads1119.o
 obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
 obj-$(CONFIG_TI_ADS1298) += ti-ads1298.o
 obj-$(CONFIG_TI_ADS131E08) += ti-ads131e08.o
+obj-$(CONFIG_TI_ADS131M0X) += ti-ads131m0x.o
 obj-$(CONFIG_TI_ADS7138) += ti-ads7138.o
 obj-$(CONFIG_TI_ADS7924) += ti-ads7924.o
 obj-$(CONFIG_TI_ADS7950) += ti-ads7950.o
diff --git a/drivers/iio/adc/ti-ads131m0x.c b/drivers/iio/adc/ti-ads131m0x.c
new file mode 100644
index 000000000000..d40aacc129ba
--- /dev/null
+++ b/drivers/iio/adc/ti-ads131m0x.c
@@ -0,0 +1,886 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Texas Instruments ADS131M0x family ADC chips.
+ *
+ * Copyright (C) 2024 Protonic Holland
+ * Copyright (C) 2025 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
+ *
+ * Primary Datasheet Reference (used for citations):
+ * ADS131M08 8-Channel, Simultaneously-Sampling, 24-Bit, Delta-Sigma ADC
+ * Document SBAS950B, Revised February 2021
+ * https://www.ti.com/lit/ds/symlink/ads131m08.pdf
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/spi/spi.h>
+
+/* Max channels supported by the largest variant in the family (ADS131M08) */
+#define ADS131M_MAX_CHANNELS		8
+
+/* ADS131M08 tolerates up to 25 MHz SCLK.
+ */
+#define ADS131M_MAX_SCLK_HZ		25000000
+
+/* Section 6.7, t_REGACQ (min time after reset) is 5us */
+#define ADS131M_RESET_DELAY_US		10
+
+/*
+ * SPI Frame word count calculation.
+ * Frame = N channel words + 1 response word + 1 CRC word.
+ * Word size depends on WLENGTH bits in MODE register (Default 24-bit).
+ */
+#define ADS131M_FRAME_WSIZE(nch)	(nch + 2)
+/*
+ * SPI Frame byte size calculation.
+ * Assumes default word size of 24 bits (3 bytes).
+ */
+#define ADS131M_FRAME_BSIZE(nch)	(ADS131M_FRAME_WSIZE(nch) * 3)
+/*
+ * Index calculation for the start byte of channel 'x' data within the RX buffer.
+ * Assumes 24-bit words (3 bytes per word).
+ * The received frame starts with the response word (e.g., STATUS register
+ * content when NULL command was sent), followed by data for channels 0 to N-1,
+ * and finally the output CRC word.
+ * Response = index 0..2, Chan0 = index 3..5, Chan1 = index 6..8, ...
+ * Index for ChanX = 3 (response) + x * 3 (channel data size).
+ */
+#define ADS131M_CHANNEL_INDEX(x)	(x * 3 + 3)
+
+#define ADS131M_CMD_NULL		0x0000
+#define ADS131M_CMD_RESET		0x0011
+
+#define ADS131M_CMD_RREG(a, n) \
+	(0xa000 | ((u16)(a & 0x1f) << 7) | (u16)(n & 0x7f))
+#define ADS131M_CMD_WREG(a, n) \
+	(0x6000 | ((u16)(a & 0x1f) << 7) | (u16)(n & 0x7f))
+
+/*  STATUS Register (0x01h) bit definitions */
+#define ADS131M_STATUS_CRC_ERR		BIT(12) /* Input CRC Error */
+
+#define ADS131M_REG_MODE		0x02
+#define ADS131M_MODE_RX_CRC_EN		BIT(12) /* Enable Input CRC */
+#define ADS131M_MODE_CRC_TYPE_ANSI	BIT(11) /* 0=CCITT, 1=ANSI */
+#define ADS131M_MODE_RESET_FLAG		BIT(10)
+
+struct ads131m_configuration {
+	const struct iio_chan_spec	*channels;
+	u8				num_channels;
+	u16				reset_ack;
+};
+
+enum ads131m_device_id {
+	ADS131M08_ID,
+	ADS131M06_ID,
+	ADS131M04_ID,
+	ADS131M03_ID,
+	ADS131M02_ID,
+};
+
+struct ads131m_priv {
+	struct spi_device *spi;
+	struct clk *clk;
+	struct mutex lock;
+	u8 num_channels;
+	const struct ads131m_configuration *config;
+	u8 tx_buffer[ADS131M_FRAME_BSIZE(ADS131M_MAX_CHANNELS)]
+		__aligned(IIO_DMA_MINALIGN);
+	u8 rx_buffer[ADS131M_FRAME_BSIZE(ADS131M_MAX_CHANNELS)]
+		__aligned(IIO_DMA_MINALIGN);
+	struct spi_transfer xfer[1];
+	struct spi_message msg;
+	unsigned int gain[ADS131M_MAX_CHANNELS];
+};
+
+/**
+ * ads131m_crc_ccitt_byte - Calculate CRC-16-CCITT (Poly 0x1021) for one byte
+ * @crc: The previous 16-bit CRC value
+ * @data: The new byte of data
+ *
+ * Device CRC:
+ * - CRC-16-CCITT, polynomial 0x1021
+ * - MSB-first (non-reflected), init 0xFFFF, no final xor
+ * - Computed over 24-bit words as sent on the wire (MSB-aligned), including
+ *   padded bits of command/data words (See Table 8-7 CRC Types).
+ * Note: This differs from kernel crc_ccitt(), which is a reflected variant.
+ *
+ * Return: The new 16-bit CRC value.
+ */
+static inline u16 ads131m_crc_ccitt_byte(u16 crc, u8 data)
+{
+	int i;
+
+	crc ^= ((u16)data << 8);
+	for (i = 0; i < 8; i++) {
+		if (crc & 0x8000)
+			crc = (crc << 1) ^ 0x1021;
+		else
+			crc = (crc << 1);
+	}
+	return crc & 0xFFFF;
+}
+
+/**
+ * ads131m_crc_calculate - Calculate CRC-16-CCITT over a buffer
+ * @buffer: The data buffer to process
+ * @len: The length of the buffer
+ *
+ * This function processes a buffer with the CCITT algorithm required
+ * by the device, using the 0xFFFF seed.
+ *
+ * Return: The final 16-bit CRC.
+ */
+static u16 ads131m_crc_calculate(const u8 *buffer, size_t len)
+{
+	u16 crc = 0xFFFF;
+	size_t i;
+
+	for (i = 0; i < len; i++)
+		crc = ads131m_crc_ccitt_byte(crc, buffer[i]);
+
+	return crc;
+}
+
+/**
+ * ads131m_tx_frame_unlocked - Sends a command frame with Input CRC
+ * @priv: Device private data structure.
+ * @command: The 16-bit command to send (e.g., NULL, RREG, RESET).
+ *
+ * Assumes the mutex lock is held.
+ * This function sends a command in Word 0, and its calculated 16-bit
+ * CRC in Word 1, as required when Input CRC is enabled.
+ *
+ * Return: 0 on success, or a negative error code from spi_sync.
+ */
+static int ads131m_tx_frame_unlocked(struct ads131m_priv *priv, u32 command)
+{
+	int ret;
+	u16 crc;
+
+	/*
+	 * Zero the entire TX buffer to send a valid frame.
+	 */
+	memset(priv->tx_buffer, 0, ADS131M_FRAME_BSIZE(priv->num_channels));
+
+	/*
+	 * Word 0: 16-bit command, MSB-aligned in 24-bit word.
+	 */
+	put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
+
+	/*
+	 * Word 1: Input CRC
+	 * Calculated over the 3 bytes of Word 0.
+	 */
+	crc = ads131m_crc_calculate(priv->tx_buffer, 3);
+	put_unaligned_be24(crc << 8, &priv->tx_buffer[3]);
+
+	return spi_sync(priv->spi, &priv->msg);
+}
+
+/**
+ * ads131m_rx_frame_unlocked - Receives a full SPI data frame.
+ * @priv: Device private data structure.
+ *
+ * This function sends a NULL command (with its CRC) to clock out a
+ * full SPI frame from the device (e.g., response + channel data + CRC).
+ *
+ * Assumes the mutex lock is held.
+ *
+ * Return: 0 on success, or a negative error code from spi_sync.
+ */
+static int ads131m_rx_frame_unlocked(struct ads131m_priv *priv)
+{
+	return ads131m_tx_frame_unlocked(priv, ADS131M_CMD_NULL);
+}
+
+/**
+ * ads131m_check_status_crc_err - Checks for an Input CRC error.
+ * @priv: Device private data structure.
+ *
+ * Sends a NULL command to fetch the STATUS register and checks the
+ * CRC_ERR bit. This is used to verify the integrity of the previous
+ * command (like RREG or WREG).
+ *
+ * Context: This function assumes the mutex 'lock' is held.
+ * Return: 0 on success, -EIO if CRC_ERR bit is set.
+ */
+static int ads131m_check_status_crc_err(struct ads131m_priv *priv)
+{
+	int ret;
+	u16 status;
+
+	ret = ads131m_rx_frame_unlocked(priv);
+	if (ret < 0) {
+		dev_err(&priv->spi->dev, "SPI error on STATUS read for CRC check\n");
+		return ret;
+	}
+
+	status = get_unaligned_be16(&priv->rx_buffer[0]);
+	if (status & ADS131M_STATUS_CRC_ERR) {
+		dev_err(&priv->spi->dev, "Previous input CRC error, STATUS=0x%04x\n",
+			status);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+/**
+ * ads131m_write_reg_unlocked - Writes a single register and verifies the ACK.
+ * @priv: Device private data structure.
+ * @reg: The 8-bit register address.
+ * @val: The 16-bit value to write.
+ *
+ * This function performs the full 3-cycle WREG operation with Input CRC:
+ * 1. (Cycle 1) Sends WREG command, data, and its calculated CRC.
+ * 2. (Cycle 2) Sends NULL+CRC to retrieve the response from Cycle 1.
+ * 3. Verifies the response is the correct ACK for the WREG.
+ * 4. (Cycle 3) Sends NULL+CRC to retrieve STATUS and check for CRC_ERR.
+ *
+ * Assumes the mutex lock is held.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int ads131m_write_reg_unlocked(struct ads131m_priv *priv, u8 reg,
+				      u16 val)
+{
+	u16 command, expected_ack, response, crc;
+	int ret;
+
+	command = ADS131M_CMD_WREG(reg, 0); /* n=0 for 1 register */
+	/*
+	 * Per Table 8-11, WREG response is: 010a aaaa ammm mmmm
+	 * For 1 reg (n=0 -> m=0): 010a aaaa a000 0000 = 0x4000 | (reg << 7)
+	 */
+	expected_ack = 0x4000 | (reg << 7);
+
+	/*
+	 * Cycle 1: Send WREG Command + Data + Input CRC
+	 */
+
+	/* Zero the entire TX buffer */
+	memset(priv->tx_buffer, 0, ADS131M_FRAME_BSIZE(priv->num_channels));
+
+	/* Word 0: WREG command, 1 reg (n=0), MSB-aligned */
+	put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
+
+	/* Word 1: Data, MSB-aligned */
+	put_unaligned_be24(val << 8, &priv->tx_buffer[3]);
+
+	/*
+	 * Word 2: Input CRC
+	 * Calculated over Word 0 (Cmd) and Word 1 (Data).
+	 */
+	crc = ads131m_crc_calculate(priv->tx_buffer, 6);
+	put_unaligned_be24(crc << 8, &priv->tx_buffer[6]);
+
+	/* We ignore the RX buffer (it's from the *previous* command) */
+	ret = spi_sync(priv->spi, &priv->msg);
+
+	if (ret < 0) {
+		dev_err(&priv->spi->dev, "SPI error on WREG (cycle 1)\n");
+		goto write_err;
+	}
+
+	/*
+	 * Cycle 2: Send NULL Command to get the WREG response
+	 */
+	ret = ads131m_rx_frame_unlocked(priv);
+	if (ret < 0) {
+		dev_err(&priv->spi->dev, "SPI error on WREG ACK (cycle 2)\n");
+		goto write_err;
+	}
+
+	/*
+	 * Response is in the first 2 bytes of the RX buffer
+	 * (MSB-aligned 16-bit response)
+	 */
+	response = get_unaligned_be16(&priv->rx_buffer[0]);
+
+	if (response != expected_ack) {
+		dev_err(&priv->spi->dev,
+			"WREG(0x%02x) failed, expected ACK 0x%04x, got 0x%04x\n",
+			reg, expected_ack, response);
+		ret = -EIO;
+		/*
+		 * Don't unlock yet, still need to do Cycle 3 to clear
+		 * any potential CRC_ERR flag from this failed command.
+		 */
+	} else {
+		dev_dbg(&priv->spi->dev, "WREG(0x%02x) ACK 0x%04x OK\n",
+			reg, response);
+	}
+
+	/*
+	 * Cycle 3: Check STATUS for Input CRC error.
+	 * This is necessary even if ACK was wrong, to clear the CRC_ERR flag.
+	 */
+	if (ads131m_check_status_crc_err(priv) < 0)
+		ret = -EIO;
+
+write_err:
+	return ret;
+}
+
+/**
+ * ads131m_read_reg_unlocked - Reads a single register from the device.
+ * @priv: Device private data structure.
+ * @reg: The 8-bit register address.
+ * @val: Pointer to store the 16-bit register value.
+ *
+ * This function performs the full 3-cycle RREG operation with Input CRC:
+ * 1. (Cycle 1) Sends the RREG command + Input CRC.
+ * 2. (Cycle 2) Sends NULL+CRC to retrieve the register data.
+ * 3. (Cycle 3) Sends NULL+CRC to retrieve STATUS and check for CRC_ERR.
+ *
+ * Assumes the mutex lock is held.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int ads131m_read_reg_unlocked(struct ads131m_priv *priv, u8 reg, u16 *val)
+{
+	u16 command;
+	int ret;
+
+	command = ADS131M_CMD_RREG(reg, 0); /* n=0 for 1 register */
+
+	/*
+	 * Cycle 1: Send RREG Command + Input CRC
+	 * We ignore the RX buffer (it's from the previous command).
+	 */
+	ret = ads131m_tx_frame_unlocked(priv, command);
+	if (ret < 0) {
+		dev_err(&priv->spi->dev, "SPI error on RREG (cycle 1)\n");
+		goto read_err;
+	}
+
+	/*
+	 * Cycle 2: Send NULL Command to get the register data
+	 */
+	ret = ads131m_rx_frame_unlocked(priv);
+	if (ret < 0) {
+		dev_err(&priv->spi->dev, "SPI error on RREG data (cycle 2)\n");
+		goto read_err;
+	}
+
+	/*
+	 * Per datasheet, for a single reg read, the response is the data.
+	 * It's in the first 2 bytes of the RX buffer (MSB-aligned 16-bit).
+	 */
+	*val = get_unaligned_be16(&priv->rx_buffer[0]);
+
+	dev_dbg(&priv->spi->dev, "RREG(0x%02x) = 0x%04x\n", reg, *val);
+
+	/*
+	 * Cycle 3: Check STATUS for Input CRC error.
+	 * The RREG command does not execute if CRC is bad, but we read
+	 * STATUS anyway to clear the flag in case it was set.
+	 */
+	if (ads131m_check_status_crc_err(priv) < 0)
+		ret = -EIO;
+
+read_err:
+	return ret;
+}
+
+/**
+ * ads131m_rmw_reg - Reads, modifies, and writes a single register.
+ * @priv: Device private data structure.
+ * @reg: The 8-bit register address.
+ * @clear: Bitmask of bits to clear.
+ * @set: Bitmask of bits to set.
+ *
+ * This function performs an atomic read-modify-write operation on a register.
+ * It reads the register, applies the clear and set masks, and writes
+ * the new value back if it has changed.
+ *
+ * Context: This function handles its own mutex locking
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int ads131m_rmw_reg(struct ads131m_priv *priv, u8 reg, u16 clear,
+			   u16 set)
+{
+	u16 old_val, new_val;
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+
+	ret = ads131m_read_reg_unlocked(priv, reg, &old_val);
+	if (ret < 0)
+		goto rmw_unlock;
+
+	new_val = (old_val & ~clear) | set;
+
+	if (new_val == old_val)
+		goto rmw_unlock;
+
+	ret = ads131m_write_reg_unlocked(priv, reg, new_val);
+
+rmw_unlock:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+/**
+ * ads131m_verify_output_crc - Verifies the CRC of the received SPI frame.
+ * @priv: Device private data structure.
+ *
+ * This function calculates the CRC-16-CCITT (Poly 0x1021, Seed 0xFFFF) over
+ * the received response and channel data, and compares it to the CRC word
+ * received at the end of the SPI frame.
+ *
+ * Context: Must be called with mutex lock held.
+ *
+ * Return: 0 on success, -EIO on CRC mismatch.
+ */
+static int ads131m_verify_output_crc(struct ads131m_priv *priv)
+{
+	size_t data_len;
+	u16 calculated_crc;
+	u16 received_crc;
+
+	/*
+	 * Frame: [Response][Chan 0]...[Chan N-1][CRC Word]
+	 * Data for CRC: [Response][Chan 0]...[Chan N-1]
+	 * Data length = (N_channels + 1) * 3 bytes (at 24-bit word size)
+	 */
+	data_len = ADS131M_FRAME_BSIZE(priv->num_channels) - 3;
+	calculated_crc = ads131m_crc_calculate(priv->rx_buffer, data_len);
+
+	/*
+	 * The received 16-bit CRC is MSB-aligned in the last 24-bit word.
+	 * We extract it from the first 2 bytes (BE) of that word.
+	 */
+	received_crc = get_unaligned_be16(&priv->rx_buffer[data_len]);
+
+	if (calculated_crc != received_crc) {
+		dev_err_ratelimited(&priv->spi->dev,
+				    "Output CRC error. Got %04x, expected %04x\n",
+				    received_crc, calculated_crc);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+/**
+ * ads131m_adc_read - Reads channel data, checks input and output CRCs.
+ * @priv: Device private data structure.
+ * @channel: The channel number to read.
+ * @val: Pointer to store the raw 24-bit value.
+ *
+ * This function sends a NULL command (with Input CRC) to retrieve data.
+ * It checks the received STATUS word for any Input CRC errors from the
+ * previous command, and then verifies the Output CRC of the current
+ * data frame.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int ads131m_adc_read(struct ads131m_priv *priv, u8 channel, s32 *val)
+{
+	int ret;
+	u8 *buf;
+	u16 status;
+
+	mutex_lock(&priv->lock);
+
+	/* Send NULL command + Input CRC, and receive data frame */
+	ret = ads131m_rx_frame_unlocked(priv);
+	if (ret < 0) {
+		mutex_unlock(&priv->lock);
+		return ret;
+	}
+
+	/*
+	 * Check STATUS word (Word 0) for an Input CRC Error from the
+	 * previous SPI frame.
+	 */
+	status = get_unaligned_be16(&priv->rx_buffer[0]);
+	if (status & ADS131M_STATUS_CRC_ERR) {
+		dev_err_ratelimited(&priv->spi->dev,
+				    "Previous input CRC Error reported in STATUS (0x%04x)\n",
+				    status);
+	}
+
+	/*
+	 * Validate the output CRC on the current data frame to ensure
+	 * data integrity.
+	 */
+	ret = ads131m_verify_output_crc(priv);
+	if (ret < 0) {
+		mutex_unlock(&priv->lock);
+		return ret;
+	}
+
+	buf = &priv->rx_buffer[ADS131M_CHANNEL_INDEX(channel)];
+	*val = sign_extend32(get_unaligned_be24(buf), 23);
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static int ads131m_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *channel, int *val,
+			   int *val2, long mask)
+{
+	struct ads131m_priv *priv = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = ads131m_adc_read(priv, channel->channel, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * Scale = (Vref / Gain) / 2^(Resolution - 1)
+		 * Vref = 1.2V (1200mV) [DS, 8.3.4], Resolution = 24 bits
+		 * LSB = (1.2V / Gain) / 2^23
+		 *
+		 * Using IIO_VAL_FRACTIONAL_LOG2, the unit is millivolts.
+		 * Scale = val * 2^(-val2)
+		 * Scale = 1200 * 2^-(23 + log2(Gain))
+		 *
+		 * priv->gain[] stores log2(Gain) (e.g., 0 for Gain=1).
+		 */
+		*val = 1200; /* 1.2Vref, in millivolts */
+		*val2 = 23 + priv->gain[channel->channel];
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+#define ADS131M_VOLTAGE_CHANNEL(num)	\
+	{ \
+		.type = IIO_VOLTAGE, \
+		.indexed = 1, \
+		.channel = (num), \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+			BIT(IIO_CHAN_INFO_SCALE) \
+	}
+
+static const struct iio_chan_spec ads131m08_channels[] = {
+	ADS131M_VOLTAGE_CHANNEL(0),
+	ADS131M_VOLTAGE_CHANNEL(1),
+	ADS131M_VOLTAGE_CHANNEL(2),
+	ADS131M_VOLTAGE_CHANNEL(3),
+	ADS131M_VOLTAGE_CHANNEL(4),
+	ADS131M_VOLTAGE_CHANNEL(5),
+	ADS131M_VOLTAGE_CHANNEL(6),
+	ADS131M_VOLTAGE_CHANNEL(7),
+};
+
+static const struct iio_chan_spec ads131m06_channels[] = {
+	ADS131M_VOLTAGE_CHANNEL(0),
+	ADS131M_VOLTAGE_CHANNEL(1),
+	ADS131M_VOLTAGE_CHANNEL(2),
+	ADS131M_VOLTAGE_CHANNEL(3),
+	ADS131M_VOLTAGE_CHANNEL(4),
+	ADS131M_VOLTAGE_CHANNEL(5),
+};
+
+static const struct iio_chan_spec ads131m04_channels[] = {
+	ADS131M_VOLTAGE_CHANNEL(0),
+	ADS131M_VOLTAGE_CHANNEL(1),
+	ADS131M_VOLTAGE_CHANNEL(2),
+	ADS131M_VOLTAGE_CHANNEL(3),
+};
+
+static const struct iio_chan_spec ads131m03_channels[] = {
+	ADS131M_VOLTAGE_CHANNEL(0),
+	ADS131M_VOLTAGE_CHANNEL(1),
+	ADS131M_VOLTAGE_CHANNEL(2),
+};
+
+static const struct iio_chan_spec ads131m02_channels[] = {
+	ADS131M_VOLTAGE_CHANNEL(0),
+	ADS131M_VOLTAGE_CHANNEL(1),
+};
+
+static const struct ads131m_configuration ads131m_config[] = {
+	[ADS131M08_ID] = {
+		.channels = ads131m08_channels,
+		.num_channels = ARRAY_SIZE(ads131m08_channels),
+		.reset_ack = 0xFF28,
+	},
+	[ADS131M06_ID] = {
+		.channels = ads131m06_channels,
+		.num_channels = ARRAY_SIZE(ads131m06_channels),
+		.reset_ack = 0xFF26,
+	},
+	[ADS131M04_ID] = {
+		.channels = ads131m04_channels,
+		.num_channels = ARRAY_SIZE(ads131m04_channels),
+		.reset_ack = 0xFF24,
+	},
+	[ADS131M03_ID] = {
+		.channels = ads131m03_channels,
+		.num_channels = ARRAY_SIZE(ads131m03_channels),
+		.reset_ack = 0xFF23,
+	},
+	[ADS131M02_ID] = {
+		.channels = ads131m02_channels,
+		.num_channels = ARRAY_SIZE(ads131m02_channels),
+		.reset_ack = 0xFF22,
+	},
+};
+
+static const struct iio_info ads131m_info = {
+	.read_raw = ads131m_read_raw,
+};
+
+/*
+ * Prepares the reusable SPI message structure for a full-duplex transfer.
+ * The ADS131M requires sending a command frame while simultaneously
+ * receiving the response/data frame from the previous command cycle.
+ *
+ * This message is optimized for the primary data acquisition workflow:
+ * sending a single-word command (like NULL) and receiving a full data
+ * frame (Response + N*Channels + CRC).
+ *
+ * This pre-configured message is NOT suitable for variable-length SPI
+ * transactions (e.g., multi-word WREG or multi-response RREG),
+ * which would require a separate, dynamically-sized spi_message.
+ */
+static void ads131m_prepare_message(struct ads131m_priv *priv)
+{
+	priv->xfer[0].tx_buf = &priv->tx_buffer[0];
+	priv->xfer[0].rx_buf = &priv->rx_buffer[0];
+	priv->xfer[0].len = ADS131M_FRAME_BSIZE(priv->num_channels);
+	spi_message_init_with_transfers(&priv->msg, &priv->xfer[0], 1);
+}
+
+static void ads131m_clk_disable_unprepare(void *arg)
+{
+	struct clk *clk = arg;
+
+	clk_disable_unprepare(clk);
+}
+
+/**
+ * ads131m_soft_reset - Issues a software RESET and verifies ACK.
+ * @priv: Device private data structure.
+ *
+ * This function sends a RESET command (with Input CRC), waits t_REGACQ,
+ * reads back the RESET ACK, and then sends a final NULL to check for
+ * any input CRC errors.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int ads131m_soft_reset(struct ads131m_priv *priv)
+{
+	struct spi_device *spi = priv->spi;
+	u16 response;
+	int ret;
+	u16 expected_ack = priv->config->reset_ack;
+
+	mutex_lock(&priv->lock);
+	dev_dbg(&spi->dev, "Sending RESET command\n");
+	ret = ads131m_tx_frame_unlocked(priv, ADS131M_CMD_RESET);
+	if (ret < 0) {
+		dev_err(&spi->dev, "Failed to send RESET command\n");
+		goto err_unlock;
+	}
+
+	/* Wait t_REGACQ (5us) for device to be ready after reset */
+	usleep_range(ADS131M_RESET_DELAY_US, ADS131M_RESET_DELAY_US + 5);
+
+	/* Cycle 2: Send NULL+CRC to retrieve the response to the RESET */
+	dev_dbg(&spi->dev, "Reading RESET ACK\n");
+	ret = ads131m_rx_frame_unlocked(priv);
+	if (ret < 0) {
+		dev_err(&spi->dev, "Failed to read RESET ACK\n");
+		goto err_unlock;
+	}
+
+	response = get_unaligned_be16(&priv->rx_buffer[0]);
+
+	/* Check against the device-specific ACK value */
+	if (response != expected_ack) {
+		dev_warn(&spi->dev, "RESET ACK mismatch, got 0x%04x, expected 0x%04x\n",
+			 response, expected_ack);
+		ret = -EIO;
+		goto err_unlock;
+	}
+
+	/* Cycle 3: Check STATUS for Input CRC error on the RESET command. */
+	if (ads131m_check_status_crc_err(priv) < 0)
+		ret = -EIO;
+
+err_unlock:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+/**
+ * ads131m_hw_init - Initialize the ADC hardware.
+ * @priv: Device private data structure.
+ *
+ * This function performs the hardware-specific initialization sequence:
+ * 1. Enables the main clock.
+ * 2. Issues a software RESET command to clear FIFOs and defaults.
+ * 3. Configures the MODE register to clear RESET, set CCITT CRC,
+ * and enable Input CRC checking.
+ *
+ * Return: 0 on success, or a negative error code.
+ */
+static int ads131m_hw_init(struct ads131m_priv *priv)
+{
+	struct spi_device *spi = priv->spi;
+	u16 clear_mask, set_mask;
+	int ret;
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(&spi->dev, "clk enable failed: %d\n", ret);
+		return ret;
+	}
+	ret = devm_add_action_or_reset(&spi->dev, ads131m_clk_disable_unprepare,
+				       priv->clk);
+	if (ret) {
+		clk_disable_unprepare(priv->clk);
+		return ret;
+	}
+
+	/*
+	 * Issue a software RESET to ensure device is in a known state.
+	 * This clears the 2-deep FIFO and resets all registers to default.
+	 */
+	ret = ads131m_soft_reset(priv);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * The RESET command sets all registers to default, which means:
+	 * 1. The RESET bit (Bit 10) in MODE is set to '1'.
+	 * 2. The CRC_TYPE bit (Bit 11) in MODE is '0' (CCITT).
+	 * 3. The RX_CRC_EN bit (Bit 12) in MODE is '0' (Disabled).
+	 *
+	 * We must:
+	 * 1. Clear the RESET bit.
+	 * 2. Enable Input CRC (RX_CRC_EN).
+	 * 3. Explicitly clear the ANSI CRC bit (for certainty).
+	 */
+	clear_mask = ADS131M_MODE_CRC_TYPE_ANSI | ADS131M_MODE_RESET_FLAG;
+	set_mask = ADS131M_MODE_RX_CRC_EN;
+
+	ret = ads131m_rmw_reg(priv, ADS131M_REG_MODE, clear_mask, set_mask);
+	if (ret < 0) {
+		dev_err(&spi->dev, "Failed to configure MODE register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ads131m_probe(struct spi_device *spi)
+{
+	const struct ads131m_configuration *config;
+	struct iio_dev *indio_dev;
+	struct ads131m_priv *priv;
+	int ret;
+
+	spi->mode = SPI_MODE_1;
+	spi->bits_per_word = 8;
+
+	if (!spi->max_speed_hz || spi->max_speed_hz > ADS131M_MAX_SCLK_HZ)
+		spi->max_speed_hz = ADS131M_MAX_SCLK_HZ;
+
+	ret = spi_setup(spi);
+	if (ret < 0) {
+		dev_err(&spi->dev, "Error in spi setup\n");
+		return ret;
+	}
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(indio_dev);
+	priv->spi = spi;
+
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &ads131m_info;
+
+	config = device_get_match_data(&spi->dev);
+	if (!config) {
+		const struct spi_device_id *id;
+
+		id = spi_get_device_id(spi);
+		if (!id)
+			return -ENODEV;
+
+		config = (const void *)id->driver_data;
+	}
+	priv->config = config;
+
+	indio_dev->channels = config->channels;
+	indio_dev->num_channels = config->num_channels;
+	priv->num_channels = config->num_channels;
+
+	/* Get the external clock source connected to the CLKIN pin */
+	priv->clk = devm_clk_get(&spi->dev, NULL);
+	if (IS_ERR(priv->clk)) {
+		ret = PTR_ERR(priv->clk);
+		dev_err(&spi->dev, "clk get failed: %d\n", ret);
+		return ret;
+	}
+
+	mutex_init(&priv->lock);
+	/* Setup the reusable SPI message structure */
+	ads131m_prepare_message(priv);
+
+	/*
+	 * Perform all hardware-specific initialization.
+	 */
+	ret = ads131m_hw_init(priv);
+	if (ret < 0)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id ads131m_of_match[] = {
+	{ .compatible = "ti,ads131m08", .data = &ads131m_config[ADS131M08_ID] },
+	{ .compatible = "ti,ads131m06", .data = &ads131m_config[ADS131M06_ID] },
+	{ .compatible = "ti,ads131m04", .data = &ads131m_config[ADS131M04_ID] },
+	{ .compatible = "ti,ads131m03", .data = &ads131m_config[ADS131M03_ID] },
+	{ .compatible = "ti,ads131m02", .data = &ads131m_config[ADS131M02_ID] },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ads131m_of_match);
+
+static const struct spi_device_id ads131m_id[] = {
+	{ "ads131m08", (kernel_ulong_t)&ads131m_config[ADS131M08_ID] },
+	{ "ads131m06", (kernel_ulong_t)&ads131m_config[ADS131M06_ID] },
+	{ "ads131m04", (kernel_ulong_t)&ads131m_config[ADS131M04_ID] },
+	{ "ads131m03", (kernel_ulong_t)&ads131m_config[ADS131M03_ID] },
+	{ "ads131m02", (kernel_ulong_t)&ads131m_config[ADS131M02_ID] },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ads131m_id);
+
+static struct spi_driver ads131m_driver = {
+	.driver = {
+		.name = "ads131m0x",
+		.of_match_table = of_match_ptr(ads131m_of_match),
+	},
+	.probe = ads131m_probe,
+	.id_table = ads131m_id,
+};
+module_spi_driver(ads131m_driver);
+
+MODULE_AUTHOR("David Jander <david@protonic.nl>");
+MODULE_DESCRIPTION("Texas Instruments ADS131M0x ADC driver");
+MODULE_LICENSE("GPL");
-- 
2.47.3

Re: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]

On 05.11.2025 15:38:14, Oleksij Rempel wrote:
> +static int ads131m_probe(struct spi_device *spi)
> +{
> +	const struct ads131m_configuration *config;
> +	struct iio_dev *indio_dev;
> +	struct ads131m_priv *priv;
> +	int ret;
> +
> +	spi->mode = SPI_MODE_1;
> +	spi->bits_per_word = 8;
> +
> +	if (!spi->max_speed_hz || spi->max_speed_hz > ADS131M_MAX_SCLK_HZ)
> +		spi->max_speed_hz = ADS131M_MAX_SCLK_HZ;
> +
> +	ret = spi_setup(spi);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Error in spi setup\n");
> +		return ret;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	priv = iio_priv(indio_dev);
> +	priv->spi = spi;
> +
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &ads131m_info;
> +
> +	config = device_get_match_data(&spi->dev);
> +	if (!config) {
> +		const struct spi_device_id *id;
> +
> +		id = spi_get_device_id(spi);
> +		if (!id)
> +			return -ENODEV;
> +
> +		config = (const void *)id->driver_data;
> +	}
> +	priv->config = config;
> +
> +	indio_dev->channels = config->channels;
> +	indio_dev->num_channels = config->num_channels;
> +	priv->num_channels = config->num_channels;
> +
> +	/* Get the external clock source connected to the CLKIN pin */
> +	priv->clk = devm_clk_get(&spi->dev, NULL);

Can you use devm_clk_get_prepared() here? This simplifies the
ads131m_hw_init() function.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[Jonathan Cameron]

On Wed, 5 Nov 2025 15:38:14 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> From: David Jander <david@protonic.nl>
> 
> Add a new IIO ADC driver for Texas Instruments ADS131M0x devices
> (ADS131M02/03/04/06/08). These are 24-bit, up to 64 kSPS, simultaneous-
> sampling delta-sigma ADCs accessed via SPI.
> 
> Highlights:
> - Supports 2/3/4/6/8-channel variants with per-channel RAW and SCALE.
> - Implements device-required full-duplex fixed-frame transfers.
> - Handles both input and output CRC; uses a non-reflected CCITT (0x1021)
>   implementation because the generic crc_ccitt helper is incompatible.
> 
> Note: Despite the almost identical name, this hardware is not
> compatible with the ADS131E0x series handled by
> drivers/iio/adc/ti-ads131e08.c.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Hi Oleksij, David,

Various comments inline.

Jonathan

> diff --git a/drivers/iio/adc/ti-ads131m0x.c b/drivers/iio/adc/ti-ads131m0x.c
> new file mode 100644
> index 000000000000..d40aacc129ba
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads131m0x.c
> @@ -0,0 +1,886 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Texas Instruments ADS131M0x family ADC chips.
> + *
> + * Copyright (C) 2024 Protonic Holland
> + * Copyright (C) 2025 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
> + *
> + * Primary Datasheet Reference (used for citations):
> + * ADS131M08 8-Channel, Simultaneously-Sampling, 24-Bit, Delta-Sigma ADC
> + * Document SBAS950B, Revised February 2021
> + * https://www.ti.com/lit/ds/symlink/ads131m08.pdf
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>

I guess here for of_match_ptr() which I suggest you drop. Then this include
isn't needed.

> +#include <linux/property.h>
> +#include <linux/spi/spi.h>
> +
> +/* Max channels supported by the largest variant in the family (ADS131M08) */
> +#define ADS131M_MAX_CHANNELS		8
> +
> +/* ADS131M08 tolerates up to 25 MHz SCLK.
> + */
> +#define ADS131M_MAX_SCLK_HZ		25000000
> +
> +/* Section 6.7, t_REGACQ (min time after reset) is 5us */
> +#define ADS131M_RESET_DELAY_US		10
> +
> +/*
> + * SPI Frame word count calculation.
> + * Frame = N channel words + 1 response word + 1 CRC word.
> + * Word size depends on WLENGTH bits in MODE register (Default 24-bit).
> + */
> +#define ADS131M_FRAME_WSIZE(nch)	(nch + 2)
Similar to case below ((nch) + 2) prefererd.

> +/*
> + * Index calculation for the start byte of channel 'x' data within the RX buffer.
> + * Assumes 24-bit words (3 bytes per word).
> + * The received frame starts with the response word (e.g., STATUS register
> + * content when NULL command was sent), followed by data for channels 0 to N-1,
> + * and finally the output CRC word.
> + * Response = index 0..2, Chan0 = index 3..5, Chan1 = index 6..8, ...
> + * Index for ChanX = 3 (response) + x * 3 (channel data size).
> + */
> +#define ADS131M_CHANNEL_INDEX(x)	(x * 3 + 3)
((x) * 3 + 3)
to avoid having to check carefully for odd things being passed as x and
precedence issues that might result.



> +enum ads131m_device_id {
> +	ADS131M08_ID,
> +	ADS131M06_ID,
> +	ADS131M04_ID,
> +	ADS131M03_ID,
> +	ADS131M02_ID,
reverse order.
> +};
> +
> +struct ads131m_priv {
> +	struct spi_device *spi;
> +	struct clk *clk;
> +	struct mutex lock;

Lock should have a commment describing what data it protects.
Looks like internal state during rmw sequences. Maybe other stuff?

> +	u8 num_channels;
> +	const struct ads131m_configuration *config;
> +	u8 tx_buffer[ADS131M_FRAME_BSIZE(ADS131M_MAX_CHANNELS)]
> +		__aligned(IIO_DMA_MINALIGN);
> +	u8 rx_buffer[ADS131M_FRAME_BSIZE(ADS131M_MAX_CHANNELS)]
> +		__aligned(IIO_DMA_MINALIGN);
> +	struct spi_transfer xfer[1];
> +	struct spi_message msg;
> +	unsigned int gain[ADS131M_MAX_CHANNELS];
> +};

> +
> +/**
> + * ads131m_tx_frame_unlocked - Sends a command frame with Input CRC
> + * @priv: Device private data structure.
> + * @command: The 16-bit command to send (e.g., NULL, RREG, RESET).
> + *
> + * Assumes the mutex lock is held.
> + * This function sends a command in Word 0, and its calculated 16-bit
> + * CRC in Word 1, as required when Input CRC is enabled.
> + *
> + * Return: 0 on success, or a negative error code from spi_sync.
> + */
> +static int ads131m_tx_frame_unlocked(struct ads131m_priv *priv, u32 command)
> +{
> +	int ret;
> +	u16 crc;
> +
> +	/*
> +	 * Zero the entire TX buffer to send a valid frame.

Single line comments where it fits give shorter overall driver
and more on a screen at time (important for those of us who have
less than perfect eyesight!)

> +	 */
> +	memset(priv->tx_buffer, 0, ADS131M_FRAME_BSIZE(priv->num_channels));
> +
> +	/*
> +	 * Word 0: 16-bit command, MSB-aligned in 24-bit word.
> +	 */
> +	put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
Similar to below. Smells like it should be a 16bit write to the correct location.

> +
> +	/*
> +	 * Word 1: Input CRC
> +	 * Calculated over the 3 bytes of Word 0.
> +	 */
> +	crc = ads131m_crc_calculate(priv->tx_buffer, 3);
> +	put_unaligned_be24(crc << 8, &priv->tx_buffer[3]);
likewise.

> +
> +	return spi_sync(priv->spi, &priv->msg);
> +}
> +
> +/**
> + * ads131m_rx_frame_unlocked - Receives a full SPI data frame.
> + * @priv: Device private data structure.
> + *
> + * This function sends a NULL command (with its CRC) to clock out a
> + * full SPI frame from the device (e.g., response + channel data + CRC).
> + *
> + * Assumes the mutex lock is held.

Can use a lockdep assert to make this explicit in code and drop
the comment.

> + *
> + * Return: 0 on success, or a negative error code from spi_sync.
> + */
> +static int ads131m_rx_frame_unlocked(struct ads131m_priv *priv)
> +{
> +	return ads131m_tx_frame_unlocked(priv, ADS131M_CMD_NULL);
> +}
> +
> +/**
> + * ads131m_check_status_crc_err - Checks for an Input CRC error.
> + * @priv: Device private data structure.
> + *
> + * Sends a NULL command to fetch the STATUS register and checks the
> + * CRC_ERR bit. This is used to verify the integrity of the previous
> + * command (like RREG or WREG).
> + *
> + * Context: This function assumes the mutex 'lock' is held.
> + * Return: 0 on success, -EIO if CRC_ERR bit is set.
> + */
> +static int ads131m_check_status_crc_err(struct ads131m_priv *priv)
> +{
> +	int ret;
> +	u16 status;
> +
> +	ret = ads131m_rx_frame_unlocked(priv);
> +	if (ret < 0) {
> +		dev_err(&priv->spi->dev, "SPI error on STATUS read for CRC check\n");

Local dev variable useful here.

> +		return ret;
> +	}
> +
> +	status = get_unaligned_be16(&priv->rx_buffer[0]);
> +	if (status & ADS131M_STATUS_CRC_ERR) {
> +		dev_err(&priv->spi->dev, "Previous input CRC error, STATUS=0x%04x\n",
> +			status);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * ads131m_write_reg_unlocked - Writes a single register and verifies the ACK.
> + * @priv: Device private data structure.
> + * @reg: The 8-bit register address.
> + * @val: The 16-bit value to write.
> + *
> + * This function performs the full 3-cycle WREG operation with Input CRC:
> + * 1. (Cycle 1) Sends WREG command, data, and its calculated CRC.
> + * 2. (Cycle 2) Sends NULL+CRC to retrieve the response from Cycle 1.
> + * 3. Verifies the response is the correct ACK for the WREG.
> + * 4. (Cycle 3) Sends NULL+CRC to retrieve STATUS and check for CRC_ERR.
> + *
> + * Assumes the mutex lock is held.
> + *
> + * Return: 0 on success, or a negative error code.
> + */
> +static int ads131m_write_reg_unlocked(struct ads131m_priv *priv, u8 reg,
> +				      u16 val)
> +{
> +	u16 command, expected_ack, response, crc;
> +	int ret;
> +
> +	command = ADS131M_CMD_WREG(reg, 0); /* n=0 for 1 register */
n = 0
> +	/*
> +	 * Per Table 8-11, WREG response is: 010a aaaa ammm mmmm
> +	 * For 1 reg (n=0 -> m=0): 010a aaaa a000 0000 = 0x4000 | (reg << 7)

n = 0 -> m = 0
Slightly long line is fine.
I'd format all these comments along those lines.


> +	 */
> +	expected_ack = 0x4000 | (reg << 7);
> +
> +	/*
> +	 * Cycle 1: Send WREG Command + Data + Input CRC
> +	 */
> +
> +	/* Zero the entire TX buffer */
> +	memset(priv->tx_buffer, 0, ADS131M_FRAME_BSIZE(priv->num_channels));
> +
> +	/* Word 0: WREG command, 1 reg (n=0), MSB-aligned */
> +	put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
That shift makes me thing this is better represented as
	put_unaligned_be16(command, &priv->tx_buffer[0]);
	//gap but it's zero so no problem.
	put_unaligned_be16(val, &priv->tx_buffer[3]);
	//gap here as well, also zero currently.
> +
> +	/* Word 1: Data, MSB-aligned */
The msb aligned seems obvious.
> +	put_unaligned_be24(val << 8, &priv->tx_buffer[3]);
> +
> +	/*
> +	 * Word 2: Input CRC
> +	 * Calculated over Word 0 (Cmd) and Word 1 (Data).
> +	 */
> +	crc = ads131m_crc_calculate(priv->tx_buffer, 6);
> +	put_unaligned_be24(crc << 8, &priv->tx_buffer[6]);
	put_unaligned_be16(crc, &priv->tx_buffer[6]);
and another gap after this.

> +
> +	/* We ignore the RX buffer (it's from the *previous* command) */
Prefer imperative for documentation.
	/* Ignore the RX buffer  ...

> +	ret = spi_sync(priv->spi, &priv->msg);
> +

Drop this blank line.

> +	if (ret < 0) {
> +		dev_err(&priv->spi->dev, "SPI error on WREG (cycle 1)\n");

Worth a local variable for this function.
	struct device *dev = &priv->spi->dev;

> +		goto write_err;
> +	}
> +
> +	/*
> +	 * Cycle 2: Send NULL Command to get the WREG response
> +	 */
> +	ret = ads131m_rx_frame_unlocked(priv);
> +	if (ret < 0) {
> +		dev_err(&priv->spi->dev, "SPI error on WREG ACK (cycle 2)\n");
> +		goto write_err;
> +	}
> +
> +	/*
> +	 * Response is in the first 2 bytes of the RX buffer
> +	 * (MSB-aligned 16-bit response)
> +	 */
> +	response = get_unaligned_be16(&priv->rx_buffer[0]);
> +
> +	if (response != expected_ack) {
> +		dev_err(&priv->spi->dev,
> +			"WREG(0x%02x) failed, expected ACK 0x%04x, got 0x%04x\n",
> +			reg, expected_ack, response);
> +		ret = -EIO;
> +		/*
> +		 * Don't unlock yet, still need to do Cycle 3 to clear
> +		 * any potential CRC_ERR flag from this failed command.
> +		 */
> +	} else {
> +		dev_dbg(&priv->spi->dev, "WREG(0x%02x) ACK 0x%04x OK\n",
> +			reg, response);
> +	}
> +
> +	/*
> +	 * Cycle 3: Check STATUS for Input CRC error.
> +	 * This is necessary even if ACK was wrong, to clear the CRC_ERR flag.
> +	 */
> +	if (ads131m_check_status_crc_err(priv) < 0)
> +		ret = -EIO;
> +
> +write_err:
As below. Early return preferred. It's easier to read in simple functions like
this as we can immediately see no cleanup is happening in those error paths
(so nothing to check!)

> +	return ret;
> +}
> +
> +/**
> + * ads131m_read_reg_unlocked - Reads a single register from the device.
> + * @priv: Device private data structure.
> + * @reg: The 8-bit register address.
> + * @val: Pointer to store the 16-bit register value.
> + *
> + * This function performs the full 3-cycle RREG operation with Input CRC:
> + * 1. (Cycle 1) Sends the RREG command + Input CRC.
> + * 2. (Cycle 2) Sends NULL+CRC to retrieve the register data.
> + * 3. (Cycle 3) Sends NULL+CRC to retrieve STATUS and check for CRC_ERR.
> + *
> + * Assumes the mutex lock is held.
> + *
> + * Return: 0 on success, or a negative error code.
> + */
> +static int ads131m_read_reg_unlocked(struct ads131m_priv *priv, u8 reg, u16 *val)
> +{
> +	u16 command;
> +	int ret;
> +
> +	command = ADS131M_CMD_RREG(reg, 0); /* n=0 for 1 register */
n = 0
Might as well keep comments to kernel coding style too as helps readability.

> +
> +	/*
> +	 * Cycle 1: Send RREG Command + Input CRC
> +	 * We ignore the RX buffer (it's from the previous command).
> +	 */
> +	ret = ads131m_tx_frame_unlocked(priv, command);
> +	if (ret < 0) {
> +		dev_err(&priv->spi->dev, "SPI error on RREG (cycle 1)\n");
> +		goto read_err;
> +	}
> +
> +	/*
> +	 * Cycle 2: Send NULL Command to get the register data
> +	 */
> +	ret = ads131m_rx_frame_unlocked(priv);
> +	if (ret < 0) {
> +		dev_err(&priv->spi->dev, "SPI error on RREG data (cycle 2)\n");
> +		goto read_err;
> +	}
> +
> +	/*
> +	 * Per datasheet, for a single reg read, the response is the data.
> +	 * It's in the first 2 bytes of the RX buffer (MSB-aligned 16-bit).
> +	 */
> +	*val = get_unaligned_be16(&priv->rx_buffer[0]);
> +
> +	dev_dbg(&priv->spi->dev, "RREG(0x%02x) = 0x%04x\n", reg, *val);
> +
> +	/*
> +	 * Cycle 3: Check STATUS for Input CRC error.
> +	 * The RREG command does not execute if CRC is bad, but we read
> +	 * STATUS anyway to clear the flag in case it was set.
> +	 */
> +	if (ads131m_check_status_crc_err(priv) < 0)
> +		ret = -EIO;
> +
> +read_err:
> +	return ret;

Return early on error and drop this (or at least make it return 0 always)

> +}
> +
> +/**
> + * ads131m_rmw_reg - Reads, modifies, and writes a single register.
> + * @priv: Device private data structure.
> + * @reg: The 8-bit register address.
> + * @clear: Bitmask of bits to clear.
> + * @set: Bitmask of bits to set.
> + *
> + * This function performs an atomic read-modify-write operation on a register.
> + * It reads the register, applies the clear and set masks, and writes
> + * the new value back if it has changed.
> + *
> + * Context: This function handles its own mutex locking
> + *
> + * Return: 0 on success, or a negative error code.
> + */
> +static int ads131m_rmw_reg(struct ads131m_priv *priv, u8 reg, u16 clear,
> +			   u16 set)
> +{
> +	u16 old_val, new_val;
> +	int ret = 0;
Always set before use - don't init.
> +
> +	mutex_lock(&priv->lock);

As below. Use guard() and early returns on error.


> +
> +	ret = ads131m_read_reg_unlocked(priv, reg, &old_val);
> +	if (ret < 0)
> +		goto rmw_unlock;
> +
> +	new_val = (old_val & ~clear) | set;
> +
> +	if (new_val == old_val)
> +		goto rmw_unlock;
> +
> +	ret = ads131m_write_reg_unlocked(priv, reg, new_val);
> +
> +rmw_unlock:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}

> +
> +/**
> + * ads131m_adc_read - Reads channel data, checks input and output CRCs.
> + * @priv: Device private data structure.
> + * @channel: The channel number to read.
> + * @val: Pointer to store the raw 24-bit value.
> + *
> + * This function sends a NULL command (with Input CRC) to retrieve data.
> + * It checks the received STATUS word for any Input CRC errors from the
> + * previous command, and then verifies the Output CRC of the current
> + * data frame.
> + *
> + * Return: 0 on success, or a negative error code.
> + */
> +static int ads131m_adc_read(struct ads131m_priv *priv, u8 channel, s32 *val)
> +{
> +	int ret;
> +	u8 *buf;
> +	u16 status;
> +
> +	mutex_lock(&priv->lock);

guard(mutex)(&priv->lock); Then no need to unlock in any paths.

> +
> +	/* Send NULL command + Input CRC, and receive data frame */
> +	ret = ads131m_rx_frame_unlocked(priv);
> +	if (ret < 0) {
> +		mutex_unlock(&priv->lock);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Check STATUS word (Word 0) for an Input CRC Error from the
> +	 * previous SPI frame.
> +	 */
> +	status = get_unaligned_be16(&priv->rx_buffer[0]);
> +	if (status & ADS131M_STATUS_CRC_ERR) {
> +		dev_err_ratelimited(&priv->spi->dev,
> +				    "Previous input CRC Error reported in STATUS (0x%04x)\n",
> +				    status);
> +	}
> +
> +	/*
> +	 * Validate the output CRC on the current data frame to ensure
> +	 * data integrity.
> +	 */
> +	ret = ads131m_verify_output_crc(priv);
> +	if (ret < 0) {
> +		mutex_unlock(&priv->lock);
> +		return ret;
> +	}
> +
> +	buf = &priv->rx_buffer[ADS131M_CHANNEL_INDEX(channel)];
> +	*val = sign_extend32(get_unaligned_be24(buf), 23);
> +
> +	mutex_unlock(&priv->lock);
> +
> +	return 0;
> +}


> +
> +static const struct ads131m_configuration ads131m_config[] = {
> +	[ADS131M08_ID] = {
We used to do this enum and array approach a lot, but it doesn't scale
particularly well and leaves an enum around people tend to abuse to
do thing sin code that belong as data.

As such, modern preference in IIO is separate named structures
static const struct ads131m_configuration ads131m08_config = {
};

etc

> +		.channels = ads131m08_channels,
> +		.num_channels = ARRAY_SIZE(ads131m08_channels),
> +		.reset_ack = 0xFF28,
> +	},
> +	[ADS131M06_ID] = {
> +		.channels = ads131m06_channels,
> +		.num_channels = ARRAY_SIZE(ads131m06_channels),
> +		.reset_ack = 0xFF26,
> +	},
> +	[ADS131M04_ID] = {
> +		.channels = ads131m04_channels,
> +		.num_channels = ARRAY_SIZE(ads131m04_channels),
> +		.reset_ack = 0xFF24,
> +	},
> +	[ADS131M03_ID] = {
> +		.channels = ads131m03_channels,
> +		.num_channels = ARRAY_SIZE(ads131m03_channels),
> +		.reset_ack = 0xFF23,
> +	},
> +	[ADS131M02_ID] = {
> +		.channels = ads131m02_channels,
> +		.num_channels = ARRAY_SIZE(ads131m02_channels),
> +		.reset_ack = 0xFF22,
> +	},
> +};

> +/*
> + * Prepares the reusable SPI message structure for a full-duplex transfer.
> + * The ADS131M requires sending a command frame while simultaneously
> + * receiving the response/data frame from the previous command cycle.
> + *
> + * This message is optimized for the primary data acquisition workflow:
> + * sending a single-word command (like NULL) and receiving a full data
> + * frame (Response + N*Channels + CRC).
> + *
> + * This pre-configured message is NOT suitable for variable-length SPI
> + * transactions (e.g., multi-word WREG or multi-response RREG),
> + * which would require a separate, dynamically-sized spi_message.
> + */
> +static void ads131m_prepare_message(struct ads131m_priv *priv)
> +{
> +	priv->xfer[0].tx_buf = &priv->tx_buffer[0];
> +	priv->xfer[0].rx_buf = &priv->rx_buffer[0];
> +	priv->xfer[0].len = ADS131M_FRAME_BSIZE(priv->num_channels);
> +	spi_message_init_with_transfers(&priv->msg, &priv->xfer[0], 1);

Consider doing devm_spi_optimize_message() to reduce the overheads of
this particular message further.

> +}

> +
> +/**
> + * ads131m_soft_reset - Issues a software RESET and verifies ACK.
> + * @priv: Device private data structure.
> + *
> + * This function sends a RESET command (with Input CRC), waits t_REGACQ,
> + * reads back the RESET ACK, and then sends a final NULL to check for
> + * any input CRC errors.
> + *
> + * Return: 0 on success, or a negative error code.
> + */
> +static int ads131m_soft_reset(struct ads131m_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	u16 response;
> +	int ret;
> +	u16 expected_ack = priv->config->reset_ack;
Pick a consistent ordering for declarations when there is no dependency
between them. I'm not spotting one here. Something like reverse xmas tree
is easy to follow.

> +
> +	mutex_lock(&priv->lock);

Include cleanup.h and
	guard(mutex)(&priv->lock);
then no need for got he got err_unlock or any manual unlocking.

> +	dev_dbg(&spi->dev, "Sending RESET command\n");

Probably not appropriate to keep this level of debug print in a
production driver.

> +	ret = ads131m_tx_frame_unlocked(priv, ADS131M_CMD_RESET);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Failed to send RESET command\n");
> +		goto err_unlock;

Only in probe so
		return dev_err_probe() is appropriate even in here.
same for all other cases in this function.

> +	}
> +
> +	/* Wait t_REGACQ (5us) for device to be ready after reset */
> +	usleep_range(ADS131M_RESET_DELAY_US, ADS131M_RESET_DELAY_US + 5);

fsleep(). That provides a standard 'slack' on a sleep like this.

> +
> +	/* Cycle 2: Send NULL+CRC to retrieve the response to the RESET */
> +	dev_dbg(&spi->dev, "Reading RESET ACK\n");
> +	ret = ads131m_rx_frame_unlocked(priv);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Failed to read RESET ACK\n");
> +		goto err_unlock;
> +	}
> +
> +	response = get_unaligned_be16(&priv->rx_buffer[0]);
> +
> +	/* Check against the device-specific ACK value */
> +	if (response != expected_ack) {
> +		dev_warn(&spi->dev, "RESET ACK mismatch, got 0x%04x, expected 0x%04x\n",
> +			 response, expected_ack);
> +		ret = -EIO;
> +		goto err_unlock;
> +	}
> +
> +	/* Cycle 3: Check STATUS for Input CRC error on the RESET command. */
> +	if (ads131m_check_status_crc_err(priv) < 0)
> +		ret = -EIO;
> +
> +err_unlock:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}
> +
> +/**
> + * ads131m_hw_init - Initialize the ADC hardware.
> + * @priv: Device private data structure.
> + *
> + * This function performs the hardware-specific initialization sequence:
> + * 1. Enables the main clock.
> + * 2. Issues a software RESET command to clear FIFOs and defaults.
> + * 3. Configures the MODE register to clear RESET, set CCITT CRC,
> + * and enable Input CRC checking.
> + *
> + * Return: 0 on success, or a negative error code.
> + */
> +static int ads131m_hw_init(struct ads131m_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	u16 clear_mask, set_mask;
> +	int ret;
> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(&spi->dev, "clk enable failed: %d\n", ret);
> +		return ret;
> +	}
> +	ret = devm_add_action_or_reset(&spi->dev, ads131m_clk_disable_unprepare,
> +				       priv->clk);

As Marc pointed out just use the enabled form when getting the clock
in the first place.

> +	if (ret) {
> +		clk_disable_unprepare(priv->clk);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Issue a software RESET to ensure device is in a known state.
> +	 * This clears the 2-deep FIFO and resets all registers to default.
> +	 */
> +	ret = ads131m_soft_reset(priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * The RESET command sets all registers to default, which means:
> +	 * 1. The RESET bit (Bit 10) in MODE is set to '1'.
> +	 * 2. The CRC_TYPE bit (Bit 11) in MODE is '0' (CCITT).
> +	 * 3. The RX_CRC_EN bit (Bit 12) in MODE is '0' (Disabled).
> +	 *
> +	 * We must:
> +	 * 1. Clear the RESET bit.
> +	 * 2. Enable Input CRC (RX_CRC_EN).
> +	 * 3. Explicitly clear the ANSI CRC bit (for certainty).
> +	 */
> +	clear_mask = ADS131M_MODE_CRC_TYPE_ANSI | ADS131M_MODE_RESET_FLAG;
> +	set_mask = ADS131M_MODE_RX_CRC_EN;
> +
> +	ret = ads131m_rmw_reg(priv, ADS131M_REG_MODE, clear_mask, set_mask);

As below - I'd pass the iio_dev around to avoid need to duplicate any data
that is in there in priv.

> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Failed to configure MODE register\n");
> +		return ret;
Only called in probe, so 
		return dev_err_probe()

> +	}
> +
> +	return 0;
> +}
> +
> +static int ads131m_probe(struct spi_device *spi)
> +{
> +	const struct ads131m_configuration *config;
> +	struct iio_dev *indio_dev;
> +	struct ads131m_priv *priv;
> +	int ret;
> +
> +	spi->mode = SPI_MODE_1;

Should come from the firmware, not be specified in driver.
cpha should be set in the dt binding.

> +	spi->bits_per_word = 8;
IIRC that's the default.

> +
> +	if (!spi->max_speed_hz || spi->max_speed_hz > ADS131M_MAX_SCLK_HZ)
> +		spi->max_speed_hz = ADS131M_MAX_SCLK_HZ;

If this isn't variable, normal assumption is it is a problem for firmware
so drivers tend to not enforce max frequencies.
> +
> +	ret = spi_setup(spi);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Error in spi setup\n");
> +		return ret;

return dev_err_probe();

> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	priv = iio_priv(indio_dev);
> +	priv->spi = spi;
> +
> +	indio_dev->name = spi_get_device_id(spi)->name;

Put the string in your config structure.  This path tends to end up
fragile as it depends on exactly how the firmware match was done.

> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &ads131m_info;
> +
> +	config = device_get_match_data(&spi->dev);
> +	if (!config) {
> +		const struct spi_device_id *id;
> +
> +		id = spi_get_device_id(spi);
> +		if (!id)
> +			return -ENODEV;
> +
> +		config = (const void *)id->driver_data;

spi_get_device_match_data()

> +	}
> +	priv->config = config;
> +
> +	indio_dev->channels = config->channels;
> +	indio_dev->num_channels = config->num_channels;
> +	priv->num_channels = config->num_channels;

Why do you need another copy?  I'd just pass indio_dev
into the functions that need this and use iio_priv() to get to
the prv structure.

> +
> +	/* Get the external clock source connected to the CLKIN pin */
> +	priv->clk = devm_clk_get(&spi->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		ret = PTR_ERR(priv->clk);
> +		dev_err(&spi->dev, "clk get failed: %d\n", ret);
> +		return ret;

		return dev_err_probe();

For all error reporting in probe.  Pretty prints the error value and
gives shorter code.  Also possible this particular call might defer
if the clock chip driver hasn't probed yet.


> +	}
> +
> +	mutex_init(&priv->lock);
	ret = devm_mutex_init(&priv->lock);
	if (ret)
		return ret;

Very small advantage for debug, but also now easy to do so we might as well.

> +	/* Setup the reusable SPI message structure */
Seems fairly obvious from function name. Probably drop this comment.

> +	ads131m_prepare_message(priv);
> +
> +	/*
> +	 * Perform all hardware-specific initialization.
> +	 */

The function name feels sufficient to convey that it is hardware init
so drop the comment.

> +	ret = ads131m_hw_init(priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
> +static const struct of_device_id ads131m_of_match[] = {
> +	{ .compatible = "ti,ads131m08", .data = &ads131m_config[ADS131M08_ID] },
As below, reverse the ordering.

> +	{ .compatible = "ti,ads131m06", .data = &ads131m_config[ADS131M06_ID] },
> +	{ .compatible = "ti,ads131m04", .data = &ads131m_config[ADS131M04_ID] },
> +	{ .compatible = "ti,ads131m03", .data = &ads131m_config[ADS131M03_ID] },
> +	{ .compatible = "ti,ads131m02", .data = &ads131m_config[ADS131M02_ID] },
> +	{ /* sentinel */ },
	{ }
So no comma the sentinel.
> +};
> +MODULE_DEVICE_TABLE(of, ads131m_of_match);
> +
> +static const struct spi_device_id ads131m_id[] = {
> +	{ "ads131m08", (kernel_ulong_t)&ads131m_config[ADS131M08_ID] },

Normally do these in opposite order. Doesn't really matter but nice to have
a universal style so I'd prefer it that way up.

> +	{ "ads131m06", (kernel_ulong_t)&ads131m_config[ADS131M06_ID] },
> +	{ "ads131m04", (kernel_ulong_t)&ads131m_config[ADS131M04_ID] },
> +	{ "ads131m03", (kernel_ulong_t)&ads131m_config[ADS131M03_ID] },
> +	{ "ads131m02", (kernel_ulong_t)&ads131m_config[ADS131M02_ID] },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, ads131m_id);
> +
> +static struct spi_driver ads131m_driver = {
> +	.driver = {
> +		.name = "ads131m0x",
> +		.of_match_table = of_match_ptr(ads131m_of_match),

Drop of_match_ptr(). It prevents other types of firmware - e.g. the
magic ACPI ID that says use the dt binding, from working and provide
no real benefit.

> +	},
> +	.probe = ads131m_probe,
> +	.id_table = ads131m_id,
> +};
> +module_spi_driver(ads131m_driver);
> +
> +MODULE_AUTHOR("David Jander <david@protonic.nl>");
> +MODULE_DESCRIPTION("Texas Instruments ADS131M0x ADC driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[kernel test robot]

Hi Oleksij,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.18-rc4 next-20251106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/bindings-iio-adc-Add-bindings-for-TI-ADS131M0x-ADCs/20251105-224149
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20251105143814.1807444-3-o.rempel%40pengutronix.de
patch subject: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20251106/202511061504.VlK7FeXK-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251106/202511061504.VlK7FeXK-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511061504.VlK7FeXK-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/iio/adc/ti-ads131m0x.c: In function 'ads131m_tx_frame_unlocked':
   drivers/iio/adc/ti-ads131m0x.c:174:9: error: implicit declaration of function 'put_unaligned_be24' [-Wimplicit-function-declaration]
     174 |         put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
         |         ^~~~~~~~~~~~~~~~~~
>> drivers/iio/adc/ti-ads131m0x.c:163:13: warning: unused variable 'ret' [-Wunused-variable]
     163 |         int ret;
         |             ^~~
   drivers/iio/adc/ti-ads131m0x.c: In function 'ads131m_check_status_crc_err':
   drivers/iio/adc/ti-ads131m0x.c:224:18: error: implicit declaration of function 'get_unaligned_be16' [-Wimplicit-function-declaration]
     224 |         status = get_unaligned_be16(&priv->rx_buffer[0]);
         |                  ^~~~~~~~~~~~~~~~~~
   drivers/iio/adc/ti-ads131m0x.c: In function 'ads131m_adc_read':
   drivers/iio/adc/ti-ads131m0x.c:523:30: error: implicit declaration of function 'get_unaligned_be24' [-Wimplicit-function-declaration]
     523 |         *val = sign_extend32(get_unaligned_be24(buf), 23);
         |                              ^~~~~~~~~~~~~~~~~~


vim +/ret +163 drivers/iio/adc/ti-ads131m0x.c

   149	
   150	/**
   151	 * ads131m_tx_frame_unlocked - Sends a command frame with Input CRC
   152	 * @priv: Device private data structure.
   153	 * @command: The 16-bit command to send (e.g., NULL, RREG, RESET).
   154	 *
   155	 * Assumes the mutex lock is held.
   156	 * This function sends a command in Word 0, and its calculated 16-bit
   157	 * CRC in Word 1, as required when Input CRC is enabled.
   158	 *
   159	 * Return: 0 on success, or a negative error code from spi_sync.
   160	 */
   161	static int ads131m_tx_frame_unlocked(struct ads131m_priv *priv, u32 command)
   162	{
 > 163		int ret;
   164		u16 crc;
   165	
   166		/*
   167		 * Zero the entire TX buffer to send a valid frame.
   168		 */
   169		memset(priv->tx_buffer, 0, ADS131M_FRAME_BSIZE(priv->num_channels));
   170	
   171		/*
   172		 * Word 0: 16-bit command, MSB-aligned in 24-bit word.
   173		 */
   174		put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
   175	
   176		/*
   177		 * Word 1: Input CRC
   178		 * Calculated over the 3 bytes of Word 0.
   179		 */
   180		crc = ads131m_crc_calculate(priv->tx_buffer, 3);
   181		put_unaligned_be24(crc << 8, &priv->tx_buffer[3]);
   182	
   183		return spi_sync(priv->spi, &priv->msg);
   184	}
   185	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[kernel test robot]

Hi Oleksij,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.18-rc4 next-20251106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/bindings-iio-adc-Add-bindings-for-TI-ADS131M0x-ADCs/20251105-224149
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20251105143814.1807444-3-o.rempel%40pengutronix.de
patch subject: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20251106/202511061518.r106EK1T-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251106/202511061518.r106EK1T-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511061518.r106EK1T-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/iio/adc/ti-ads131m0x.c: In function 'ads131m_tx_frame_unlocked':
>> drivers/iio/adc/ti-ads131m0x.c:174:9: error: implicit declaration of function 'put_unaligned_be24' [-Wimplicit-function-declaration]
     174 |         put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
         |         ^~~~~~~~~~~~~~~~~~
   drivers/iio/adc/ti-ads131m0x.c:163:13: warning: unused variable 'ret' [-Wunused-variable]
     163 |         int ret;
         |             ^~~
   drivers/iio/adc/ti-ads131m0x.c: In function 'ads131m_check_status_crc_err':
>> drivers/iio/adc/ti-ads131m0x.c:224:18: error: implicit declaration of function 'get_unaligned_be16' [-Wimplicit-function-declaration]
     224 |         status = get_unaligned_be16(&priv->rx_buffer[0]);
         |                  ^~~~~~~~~~~~~~~~~~
   drivers/iio/adc/ti-ads131m0x.c: In function 'ads131m_adc_read':
>> drivers/iio/adc/ti-ads131m0x.c:523:30: error: implicit declaration of function 'get_unaligned_be24' [-Wimplicit-function-declaration]
     523 |         *val = sign_extend32(get_unaligned_be24(buf), 23);
         |                              ^~~~~~~~~~~~~~~~~~


vim +/put_unaligned_be24 +174 drivers/iio/adc/ti-ads131m0x.c

   149	
   150	/**
   151	 * ads131m_tx_frame_unlocked - Sends a command frame with Input CRC
   152	 * @priv: Device private data structure.
   153	 * @command: The 16-bit command to send (e.g., NULL, RREG, RESET).
   154	 *
   155	 * Assumes the mutex lock is held.
   156	 * This function sends a command in Word 0, and its calculated 16-bit
   157	 * CRC in Word 1, as required when Input CRC is enabled.
   158	 *
   159	 * Return: 0 on success, or a negative error code from spi_sync.
   160	 */
   161	static int ads131m_tx_frame_unlocked(struct ads131m_priv *priv, u32 command)
   162	{
   163		int ret;
   164		u16 crc;
   165	
   166		/*
   167		 * Zero the entire TX buffer to send a valid frame.
   168		 */
   169		memset(priv->tx_buffer, 0, ADS131M_FRAME_BSIZE(priv->num_channels));
   170	
   171		/*
   172		 * Word 0: 16-bit command, MSB-aligned in 24-bit word.
   173		 */
 > 174		put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
   175	
   176		/*
   177		 * Word 1: Input CRC
   178		 * Calculated over the 3 bytes of Word 0.
   179		 */
   180		crc = ads131m_crc_calculate(priv->tx_buffer, 3);
   181		put_unaligned_be24(crc << 8, &priv->tx_buffer[3]);
   182	
   183		return spi_sync(priv->spi, &priv->msg);
   184	}
   185	
   186	/**
   187	 * ads131m_rx_frame_unlocked - Receives a full SPI data frame.
   188	 * @priv: Device private data structure.
   189	 *
   190	 * This function sends a NULL command (with its CRC) to clock out a
   191	 * full SPI frame from the device (e.g., response + channel data + CRC).
   192	 *
   193	 * Assumes the mutex lock is held.
   194	 *
   195	 * Return: 0 on success, or a negative error code from spi_sync.
   196	 */
   197	static int ads131m_rx_frame_unlocked(struct ads131m_priv *priv)
   198	{
   199		return ads131m_tx_frame_unlocked(priv, ADS131M_CMD_NULL);
   200	}
   201	
   202	/**
   203	 * ads131m_check_status_crc_err - Checks for an Input CRC error.
   204	 * @priv: Device private data structure.
   205	 *
   206	 * Sends a NULL command to fetch the STATUS register and checks the
   207	 * CRC_ERR bit. This is used to verify the integrity of the previous
   208	 * command (like RREG or WREG).
   209	 *
   210	 * Context: This function assumes the mutex 'lock' is held.
   211	 * Return: 0 on success, -EIO if CRC_ERR bit is set.
   212	 */
   213	static int ads131m_check_status_crc_err(struct ads131m_priv *priv)
   214	{
   215		int ret;
   216		u16 status;
   217	
   218		ret = ads131m_rx_frame_unlocked(priv);
   219		if (ret < 0) {
   220			dev_err(&priv->spi->dev, "SPI error on STATUS read for CRC check\n");
   221			return ret;
   222		}
   223	
 > 224		status = get_unaligned_be16(&priv->rx_buffer[0]);
   225		if (status & ADS131M_STATUS_CRC_ERR) {
   226			dev_err(&priv->spi->dev, "Previous input CRC error, STATUS=0x%04x\n",
   227				status);
   228			return -EIO;
   229		}
   230	
   231		return 0;
   232	}
   233	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[kernel test robot]

Hi Oleksij,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.18-rc4 next-20251106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/bindings-iio-adc-Add-bindings-for-TI-ADS131M0x-ADCs/20251105-224149
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20251105143814.1807444-3-o.rempel%40pengutronix.de
patch subject: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver
config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20251106/202511061850.kYeqflcU-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251106/202511061850.kYeqflcU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511061850.kYeqflcU-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/adc/ti-ads131m0x.c:174:2: error: call to undeclared function 'put_unaligned_be24'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     174 |         put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
         |         ^
   drivers/iio/adc/ti-ads131m0x.c:163:6: warning: unused variable 'ret' [-Wunused-variable]
     163 |         int ret;
         |             ^~~
>> drivers/iio/adc/ti-ads131m0x.c:224:11: error: call to undeclared function 'get_unaligned_be16'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     224 |         status = get_unaligned_be16(&priv->rx_buffer[0]);
         |                  ^
   drivers/iio/adc/ti-ads131m0x.c:271:2: error: call to undeclared function 'put_unaligned_be24'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     271 |         put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
         |         ^
   drivers/iio/adc/ti-ads131m0x.c:304:13: error: call to undeclared function 'get_unaligned_be16'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     304 |         response = get_unaligned_be16(&priv->rx_buffer[0]);
         |                    ^
   drivers/iio/adc/ti-ads131m0x.c:376:9: error: call to undeclared function 'get_unaligned_be16'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     376 |         *val = get_unaligned_be16(&priv->rx_buffer[0]);
         |                ^
   drivers/iio/adc/ti-ads131m0x.c:461:17: error: call to undeclared function 'get_unaligned_be16'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     461 |         received_crc = get_unaligned_be16(&priv->rx_buffer[data_len]);
         |                        ^
   drivers/iio/adc/ti-ads131m0x.c:505:11: error: call to undeclared function 'get_unaligned_be16'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     505 |         status = get_unaligned_be16(&priv->rx_buffer[0]);
         |                  ^
>> drivers/iio/adc/ti-ads131m0x.c:523:23: error: call to undeclared function 'get_unaligned_be24'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     523 |         *val = sign_extend32(get_unaligned_be24(buf), 23);
         |                              ^
   drivers/iio/adc/ti-ads131m0x.c:706:13: error: call to undeclared function 'get_unaligned_be16'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     706 |         response = get_unaligned_be16(&priv->rx_buffer[0]);
         |                    ^
   1 warning and 9 errors generated.


vim +/put_unaligned_be24 +174 drivers/iio/adc/ti-ads131m0x.c

   149	
   150	/**
   151	 * ads131m_tx_frame_unlocked - Sends a command frame with Input CRC
   152	 * @priv: Device private data structure.
   153	 * @command: The 16-bit command to send (e.g., NULL, RREG, RESET).
   154	 *
   155	 * Assumes the mutex lock is held.
   156	 * This function sends a command in Word 0, and its calculated 16-bit
   157	 * CRC in Word 1, as required when Input CRC is enabled.
   158	 *
   159	 * Return: 0 on success, or a negative error code from spi_sync.
   160	 */
   161	static int ads131m_tx_frame_unlocked(struct ads131m_priv *priv, u32 command)
   162	{
   163		int ret;
   164		u16 crc;
   165	
   166		/*
   167		 * Zero the entire TX buffer to send a valid frame.
   168		 */
   169		memset(priv->tx_buffer, 0, ADS131M_FRAME_BSIZE(priv->num_channels));
   170	
   171		/*
   172		 * Word 0: 16-bit command, MSB-aligned in 24-bit word.
   173		 */
 > 174		put_unaligned_be24(command << 8, &priv->tx_buffer[0]);
   175	
   176		/*
   177		 * Word 1: Input CRC
   178		 * Calculated over the 3 bytes of Word 0.
   179		 */
   180		crc = ads131m_crc_calculate(priv->tx_buffer, 3);
   181		put_unaligned_be24(crc << 8, &priv->tx_buffer[3]);
   182	
   183		return spi_sync(priv->spi, &priv->msg);
   184	}
   185	
   186	/**
   187	 * ads131m_rx_frame_unlocked - Receives a full SPI data frame.
   188	 * @priv: Device private data structure.
   189	 *
   190	 * This function sends a NULL command (with its CRC) to clock out a
   191	 * full SPI frame from the device (e.g., response + channel data + CRC).
   192	 *
   193	 * Assumes the mutex lock is held.
   194	 *
   195	 * Return: 0 on success, or a negative error code from spi_sync.
   196	 */
   197	static int ads131m_rx_frame_unlocked(struct ads131m_priv *priv)
   198	{
   199		return ads131m_tx_frame_unlocked(priv, ADS131M_CMD_NULL);
   200	}
   201	
   202	/**
   203	 * ads131m_check_status_crc_err - Checks for an Input CRC error.
   204	 * @priv: Device private data structure.
   205	 *
   206	 * Sends a NULL command to fetch the STATUS register and checks the
   207	 * CRC_ERR bit. This is used to verify the integrity of the previous
   208	 * command (like RREG or WREG).
   209	 *
   210	 * Context: This function assumes the mutex 'lock' is held.
   211	 * Return: 0 on success, -EIO if CRC_ERR bit is set.
   212	 */
   213	static int ads131m_check_status_crc_err(struct ads131m_priv *priv)
   214	{
   215		int ret;
   216		u16 status;
   217	
   218		ret = ads131m_rx_frame_unlocked(priv);
   219		if (ret < 0) {
   220			dev_err(&priv->spi->dev, "SPI error on STATUS read for CRC check\n");
   221			return ret;
   222		}
   223	
 > 224		status = get_unaligned_be16(&priv->rx_buffer[0]);
   225		if (status & ADS131M_STATUS_CRC_ERR) {
   226			dev_err(&priv->spi->dev, "Previous input CRC error, STATUS=0x%04x\n",
   227				status);
   228			return -EIO;
   229		}
   230	
   231		return 0;
   232	}
   233	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[David Lechner]

On 11/5/25 8:38 AM, Oleksij Rempel wrote:
> From: David Jander <david@protonic.nl>
> 
> Add a new IIO ADC driver for Texas Instruments ADS131M0x devices
> (ADS131M02/03/04/06/08). These are 24-bit, up to 64 kSPS, simultaneous-
> sampling delta-sigma ADCs accessed via SPI.
> 
> Highlights:
> - Supports 2/3/4/6/8-channel variants with per-channel RAW and SCALE.
> - Implements device-required full-duplex fixed-frame transfers.
> - Handles both input and output CRC; uses a non-reflected CCITT (0x1021)
>   implementation because the generic crc_ccitt helper is incompatible.
> 
> Note: Despite the almost identical name, this hardware is not
> compatible with the ADS131E0x series handled by
> drivers/iio/adc/ti-ads131e08.c.
> 
> Signed-off-by: David Jander <david@protonic.nl>
> Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/iio/adc/Kconfig        |  10 +
>  drivers/iio/adc/Makefile       |   1 +
>  drivers/iio/adc/ti-ads131m0x.c | 886 +++++++++++++++++++++++++++++++++
>  3 files changed, 897 insertions(+)
>  create mode 100644 drivers/iio/adc/ti-ads131m0x.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 58a14e6833f6..c17f8914358c 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1691,6 +1691,16 @@ config TI_ADS131E08
>  	  This driver can also be built as a module. If so, the module will be
>  	  called ti-ads131e08.
>  
> +config TI_ADS131M0X
> +	tristate "Texas Instruments ADS131M0x"
> +	depends on SPI && COMMON_CLK
> +	help
> +	  Say yes here to get support for Texas Instruments ADS131M02, ADS131M03,
> +	  ADS131M04, ADS131M06 and ADS131M08 chips.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called ti-ads131m0x.
> +
>  config TI_ADS7138
>  	tristate "Texas Instruments ADS7128 and ADS7138 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d008f78dc010..c23dae3ddcc7 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -147,6 +147,7 @@ obj-$(CONFIG_TI_ADS1119) += ti-ads1119.o
>  obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
>  obj-$(CONFIG_TI_ADS1298) += ti-ads1298.o
>  obj-$(CONFIG_TI_ADS131E08) += ti-ads131e08.o
> +obj-$(CONFIG_TI_ADS131M0X) += ti-ads131m0x.o

No x in the name please. As in my other review, I would make it ads131m02.

>  obj-$(CONFIG_TI_ADS7138) += ti-ads7138.o
>  obj-$(CONFIG_TI_ADS7924) += ti-ads7924.o
>  obj-$(CONFIG_TI_ADS7950) += ti-ads7950.o
> diff --git a/drivers/iio/adc/ti-ads131m0x.c b/drivers/iio/adc/ti-ads131m0x.c
> new file mode 100644
> index 000000000000..d40aacc129ba
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads131m0x.c
> @@ -0,0 +1,886 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for Texas Instruments ADS131M0x family ADC chips.
> + *
> + * Copyright (C) 2024 Protonic Holland
> + * Copyright (C) 2025 Oleksij Rempel <kernel@pengutronix.de>, Pengutronix
> + *
> + * Primary Datasheet Reference (used for citations):
> + * ADS131M08 8-Channel, Simultaneously-Sampling, 24-Bit, Delta-Sigma ADC
> + * Document SBAS950B, Revised February 2021
> + * https://www.ti.com/lit/ds/symlink/ads131m08.pdf
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/property.h>
> +#include <linux/spi/spi.h>
> +
> +/* Max channels supported by the largest variant in the family (ADS131M08) */
> +#define ADS131M_MAX_CHANNELS		8
> +
> +/* ADS131M08 tolerates up to 25 MHz SCLK.
> + */
> +#define ADS131M_MAX_SCLK_HZ		25000000

25 * MEGA, but this should rather be in the devicetree bindings
as spi-max-frequency.

> +
> +/* Section 6.7, t_REGACQ (min time after reset) is 5us */
> +#define ADS131M_RESET_DELAY_US		10
Why not make this 5 instead of 10?

> +
> +/*
> + * SPI Frame word count calculation.
> + * Frame = N channel words + 1 response word + 1 CRC word.
> + * Word size depends on WLENGTH bits in MODE register (Default 24-bit).
> + */
> +#define ADS131M_FRAME_WSIZE(nch)	(nch + 2)
> +/*
> + * SPI Frame byte size calculation.
> + * Assumes default word size of 24 bits (3 bytes).
> + */
> +#define ADS131M_FRAME_BSIZE(nch)	(ADS131M_FRAME_WSIZE(nch) * 3)

Would be easier to understand at the call site if we spell out
WORD_SIZE and BYTE_SIZE.

> +/*
> + * Index calculation for the start byte of channel 'x' data within the RX buffer.
> + * Assumes 24-bit words (3 bytes per word).
> + * The received frame starts with the response word (e.g., STATUS register
> + * content when NULL command was sent), followed by data for channels 0 to N-1,
> + * and finally the output CRC word.
> + * Response = index 0..2, Chan0 = index 3..5, Chan1 = index 6..8, ...
> + * Index for ChanX = 3 (response) + x * 3 (channel data size).
> + */
> +#define ADS131M_CHANNEL_INDEX(x)	(x * 3 + 3)
> +
> +#define ADS131M_CMD_NULL		0x0000
> +#define ADS131M_CMD_RESET		0x0011
> +
> +#define ADS131M_CMD_RREG(a, n) \
> +	(0xa000 | ((u16)(a & 0x1f) << 7) | (u16)(n & 0x7f))
> +#define ADS131M_CMD_WREG(a, n) \
> +	(0x6000 | ((u16)(a & 0x1f) << 7) | (u16)(n & 0x7f))
> +
> +/*  STATUS Register (0x01h) bit definitions */
> +#define ADS131M_STATUS_CRC_ERR		BIT(12) /* Input CRC Error */
> +
> +#define ADS131M_REG_MODE		0x02
> +#define ADS131M_MODE_RX_CRC_EN		BIT(12) /* Enable Input CRC */
> +#define ADS131M_MODE_CRC_TYPE_ANSI	BIT(11) /* 0=CCITT, 1=ANSI */
> +#define ADS131M_MODE_RESET_FLAG		BIT(10)
> +
> +struct ads131m_configuration {
> +	const struct iio_chan_spec	*channels;
> +	u8				num_channels;
> +	u16				reset_ack;
> +};
> +
> +enum ads131m_device_id {
> +	ADS131M08_ID,
> +	ADS131M06_ID,
> +	ADS131M04_ID,
> +	ADS131M03_ID,
> +	ADS131M02_ID,
> +};
> +
> +struct ads131m_priv {
> +	struct spi_device *spi;
> +	struct clk *clk;
> +	struct mutex lock;
> +	u8 num_channels;
> +	const struct ads131m_configuration *config;
> +	u8 tx_buffer[ADS131M_FRAME_BSIZE(ADS131M_MAX_CHANNELS)]
> +		__aligned(IIO_DMA_MINALIGN);
> +	u8 rx_buffer[ADS131M_FRAME_BSIZE(ADS131M_MAX_CHANNELS)]
> +		__aligned(IIO_DMA_MINALIGN);

The second __aligned(IIO_DMA_MINALIGN) is not needed. And everything
below here needs to be move above the first __aligned(IIO_DMA_MINALIGN)
Othewise the items below can be corrupted.

> +	struct spi_transfer xfer[1];

Not much sense in an array with length one. Can drop [1].

> +	struct spi_message msg;
> +	unsigned int gain[ADS131M_MAX_CHANNELS];

It looks like this is never assigned (always all 0s). Maybe we should
save it for a later patch that implements variable gain?

> +};
> +
...

> +static int ads131m_check_status_crc_err(struct ads131m_priv *priv)
> +{
> +	int ret;
> +	u16 status;
> +
> +	ret = ads131m_rx_frame_unlocked(priv);
> +	if (ret < 0) {
> +		dev_err(&priv->spi->dev, "SPI error on STATUS read for CRC check\n");
> +		return ret;
> +	}
> +
> +	status = get_unaligned_be16(&priv->rx_buffer[0]);
> +	if (status & ADS131M_STATUS_CRC_ERR) {
> +		dev_err(&priv->spi->dev, "Previous input CRC error, STATUS=0x%04x\n",
> +			status);

Should this be rate-limited? If things get really bad, we could get an
error on every xfer which could flood the logs.

> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +

...

> +/**
> + * ads131m_rmw_reg - Reads, modifies, and writes a single register.

Any reason we couldn't turn the read/write into a regmap and avoid
implementing extras like this?

> + * @priv: Device private data structure.
> + * @reg: The 8-bit register address.
> + * @clear: Bitmask of bits to clear.
> + * @set: Bitmask of bits to set.
> + *
> + * This function performs an atomic read-modify-write operation on a register.
> + * It reads the register, applies the clear and set masks, and writes
> + * the new value back if it has changed.
> + *
> + * Context: This function handles its own mutex locking
> + *
> + * Return: 0 on success, or a negative error code.
> + */

...

> +static int ads131m_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *channel, int *val,
> +			   int *val2, long mask)
> +{
> +	struct ads131m_priv *priv = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = ads131m_adc_read(priv, channel->channel, val);
> +		if (ret)
> +			return ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		/*
> +		 * Scale = (Vref / Gain) / 2^(Resolution - 1)
> +		 * Vref = 1.2V (1200mV) [DS, 8.3.4], Resolution = 24 bits
> +		 * LSB = (1.2V / Gain) / 2^23
> +		 *
> +		 * Using IIO_VAL_FRACTIONAL_LOG2, the unit is millivolts.
> +		 * Scale = val * 2^(-val2)
> +		 * Scale = 1200 * 2^-(23 + log2(Gain))

I would write it as vref_mv / 2^(23 + log2(gain)). The multiplication
by negative exponent is a bit subtile when we are talking about a
fractional value.

> +		 *
> +		 * priv->gain[] stores log2(Gain) (e.g., 0 for Gain=1).
> +		 */
> +		*val = 1200; /* 1.2Vref, in millivolts */
> +		*val2 = 23 + priv->gain[channel->channel];

s/gain/gain_log2/ for the variable name to make it obious
what kind of value it holds.

> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +/**
> + * ads131m_hw_init - Initialize the ADC hardware.
> + * @priv: Device private data structure.
> + *
> + * This function performs the hardware-specific initialization sequence:
> + * 1. Enables the main clock.
> + * 2. Issues a software RESET command to clear FIFOs and defaults.
> + * 3. Configures the MODE register to clear RESET, set CCITT CRC,
> + * and enable Input CRC checking.
> + *
> + * Return: 0 on success, or a negative error code.
> + */
> +static int ads131m_hw_init(struct ads131m_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	u16 clear_mask, set_mask;
> +	int ret;
> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(&spi->dev, "clk enable failed: %d\n", ret);
> +		return ret;
> +	}
> +	ret = devm_add_action_or_reset(&spi->dev, ads131m_clk_disable_unprepare,
> +				       priv->clk);
> +	if (ret) {
> +		clk_disable_unprepare(priv->clk);
> +		return ret;
> +	}
> +

Would be trivial to implement optional gpio hardware reset here as well.

> +	/*
> +	 * Issue a software RESET to ensure device is in a known state.
> +	 * This clears the 2-deep FIFO and resets all registers to default.
> +	 */
> +	ret = ads131m_soft_reset(priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * The RESET command sets all registers to default, which means:
> +	 * 1. The RESET bit (Bit 10) in MODE is set to '1'.
> +	 * 2. The CRC_TYPE bit (Bit 11) in MODE is '0' (CCITT).
> +	 * 3. The RX_CRC_EN bit (Bit 12) in MODE is '0' (Disabled).
> +	 *
> +	 * We must:
> +	 * 1. Clear the RESET bit.
> +	 * 2. Enable Input CRC (RX_CRC_EN).
> +	 * 3. Explicitly clear the ANSI CRC bit (for certainty).
> +	 */
> +	clear_mask = ADS131M_MODE_CRC_TYPE_ANSI | ADS131M_MODE_RESET_FLAG;
> +	set_mask = ADS131M_MODE_RX_CRC_EN;
> +
> +	ret = ads131m_rmw_reg(priv, ADS131M_REG_MODE, clear_mask, set_mask);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Failed to configure MODE register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ads131m_probe(struct spi_device *spi)
> +{
> +	const struct ads131m_configuration *config;
> +	struct iio_dev *indio_dev;
> +	struct ads131m_priv *priv;
> +	int ret;
> +
> +	spi->mode = SPI_MODE_1;
> +	spi->bits_per_word = 8;
> +
> +	if (!spi->max_speed_hz || spi->max_speed_hz > ADS131M_MAX_SCLK_HZ)
> +		spi->max_speed_hz = ADS131M_MAX_SCLK_HZ;
> +
> +	ret = spi_setup(spi);

As Jonathan mentioned, we should be able to avoid calling spi_setup()
here completly.

> +	if (ret < 0) {
> +		dev_err(&spi->dev, "Error in spi setup\n");
> +		return ret;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*priv));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	priv = iio_priv(indio_dev);
> +	priv->spi = spi;
> +
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &ads131m_info;
> +
> +	config = device_get_match_data(&spi->dev);
> +	if (!config) {
> +		const struct spi_device_id *id;
> +
> +		id = spi_get_device_id(spi);
> +		if (!id)
> +			return -ENODEV;
> +
> +		config = (const void *)id->driver_data;
> +	}
> +	priv->config = config;
> +
> +	indio_dev->channels = config->channels;
> +	indio_dev->num_channels = config->num_channels;
> +	priv->num_channels = config->num_channels;

We already have priv->config->num_channels, so this extra copy seems
unnecessary.

> +
> +	/* Get the external clock source connected to the CLKIN pin */
> +	priv->clk = devm_clk_get(&spi->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		ret = PTR_ERR(priv->clk);
> +		dev_err(&spi->dev, "clk get failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	mutex_init(&priv->lock);
> +	/* Setup the reusable SPI message structure */
> +	ads131m_prepare_message(priv);
> +
> +	/*
> +	 * Perform all hardware-specific initialization.
> +	 */
> +	ret = ads131m_hw_init(priv);
> +	if (ret < 0)
> +		return ret;
> +
> +	return devm_iio_device_register(&spi->dev, indio_dev);
> +}
> +
...

Re: [PATCH v1 2/2] iio: adc: Add TI ADS131M0x ADC driver

[Oleksij Rempel]

Hi David,

On Thu, Nov 06, 2025 at 10:57:26AM -0600, David Lechner wrote:
> ...
> 
> > +/**
> > + * ads131m_rmw_reg - Reads, modifies, and writes a single register.
> 
> Any reason we couldn't turn the read/write into a regmap and avoid
> implementing extras like this?

I thought about regmap, but it is a poor fit for this chip.

The problem is the device protocol. It is not a simple register-based device;
it is a frame-based protocol that uses opcodes.

-  Hot Path (Data Read): The main data read (in read_raw) does not access
registers. It sends a NULL opcode frame to read all channel data at once.

-  Cold Path (Setup): Register access (RREG/WREG) is a complex, stateful
3-cycle operation. It is only used in probe for setup.

This leaves two (bad?) options for regmap:

- Mixed Access: Use regmap only for the cold path (probe) and use raw
for the hot path (read_raw). This is messy because we mix two access methods.

- Virtual Registers: Try to model all opcodes (NULL, RREG, WREG) as virtual
registers. This is a very unnatural abstraction for this chip.

Using the regmap dependency just to replace one rmw function that runs only
once at probe seemed like the overkill.

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |