[PATCH v2 0/6] iio: amplifiers: ad8366: driver update and dt support

[PATCH v2 0/6] iio: amplifiers: ad8366: driver update and dt support

[Rodrigo Alencar via B4 Relay]

This patch series updates the AD8366 amplifier/attenuator driver with
modern device resource management, including support for additional
device variants.

Adds support for multiple digital step attenuators:
- ADRF5720: 0.5 dB LSB, 6-Bit, Digital Attenuator, 9 kHz to 40 GHz
- ADRF5730: 0.5 dB LSB, 6-Bit, Digital Attenuator, 100 MHz to 40 GHz
- ADRF5731: 2 dB LSB, 4-Bit, Digital Attenuator, 100 MHz to 40 GHz
- HMC271A: 1 dB LSB, 5-bit, Digital Attenuator, 0.7 - 3.7 GHz
- HMC1018A: 1.0 dB LSB GaAs MMIC 5-BIT DIGITAL ATTENUATOR, 0.1 - 30 GHz
- HMC1019A: 0.5 dB LSB GaAs MMIC 5-BIT DIGITAL ATTENUATOR, 0.1 - 30 GHz

Device Tree Support:
- Adds device tree binding documentation
- Adds device tree compatible strings for all supported devices
- Favor chip info tables over device type.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Changes in v2:
- Reorganize commits with cleanups before features.
- Link to v1: https://lore.kernel.org/r/20260119-iio-ad8366-update-v1-0-8044e23e964a@analog.com

---
Rodrigo Alencar (6):
      MAINTAINERS: Add missing maintainer entry for AD8366 driver
      dt-bindings: iio: amplifiers: Add AD8366 support
      iio: amplifiers: ad8366: refactor device resource management
      iio: amplifiers: ad8366: add device tree support
      iio: amplifiers: ad8366: consume enable gpio
      iio: amplifiers: ad8366: Update device support

 .../bindings/iio/amplifiers/adi,ad8366.yaml        |  68 ++++
 MAINTAINERS                                        |   9 +
 drivers/iio/amplifiers/Kconfig                     |   6 +
 drivers/iio/amplifiers/ad8366.c                    | 385 +++++++++++----------
 4 files changed, 278 insertions(+), 190 deletions(-)
---
base-commit: f52690c50893ef1504990199c8a2dfbb869f38c6
change-id: 20251121-iio-ad8366-update-56abac58bbca

Best regards,
-- 
Rodrigo Alencar <rodrigo.alencar@analog.com>

[PATCH v2 1/6] MAINTAINERS: Add missing maintainer entry for AD8366 driver

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add maintainers entry for drivers/iio/amplifiers/ad8366.c

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 70163f939602..8b5bb7030fa6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1583,6 +1583,14 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad7780.yaml
 F:	drivers/iio/adc/ad7780.c
 
+ANALOG DEVICES INC AD8366 DRIVER
+M:	Michael Hennerich <Michael.Hennerich@analog.com>
+M:	Rodrigo Alencar <rodrigo.alencar@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	drivers/iio/amplifiers/ad8366.c
+
 ANALOG DEVICES INC AD9467 DRIVER
 M:	Michael Hennerich <Michael.Hennerich@analog.com>
 M:	Nuno Sa <nuno.sa@analog.com>

-- 
2.43.0

[PATCH v2 2/6] dt-bindings: iio: amplifiers: Add AD8366 support

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add device tree binding documentation for amplifiers and digital
attenuators. This covers different device variants with similar
SPI control.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 .../bindings/iio/amplifiers/adi,ad8366.yaml        | 68 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 69 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml b/Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml
new file mode 100644
index 000000000000..02f790fd73d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/amplifiers/adi,ad8366.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AD8366 and similar Gain Amplifiers and Digital Attenuators
+
+maintainers:
+  - Michael Hennerich <michael.hennerich@analog.com>
+  - Rodrigo Alencar <rodrigo.alencar@analog.com>
+
+description:
+  Digital Variable Gain Amplifiers (VGAs) and Digital Attenuators with
+  SPI interface.
+
+properties:
+  compatible:
+    enum:
+      - adi,ad8366
+      - adi,ada4961
+      - adi,adl5240
+      - adi,adrf5720
+      - adi,adrf5730
+      - adi,adrf5731
+      - adi,hmc271a
+      - adi,hmc792a
+      - adi,hmc1018a
+      - adi,hmc1019a
+      - adi,hmc1119
+
+  reg:
+    maxItems: 1
+
+  vcc-supply:
+    description: Regulator that provides power to the device.
+
+  reset-gpios:
+    maxItems: 1
+
+  enable-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - vcc-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      amplifier@0 {
+        compatible = "adi,ad8366";
+        reg = <0>;
+        spi-max-frequency = <1000000>;
+        vcc-supply = <&vcc_3v3>;
+      };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 8b5bb7030fa6..c3403ae478b7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1589,6 +1589,7 @@ M:	Rodrigo Alencar <rodrigo.alencar@analog.com>
 L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml
 F:	drivers/iio/amplifiers/ad8366.c
 
 ANALOG DEVICES INC AD9467 DRIVER

-- 
2.43.0

Re: [PATCH v2 2/6] dt-bindings: iio: amplifiers: Add AD8366 support

[Conor Dooley]

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

On Mon, Jan 26, 2026 at 01:51:03PM +0000, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Add device tree binding documentation for amplifiers and digital
> attenuators. This covers different device variants with similar
> SPI control.
> 
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
>  .../bindings/iio/amplifiers/adi,ad8366.yaml        | 68 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 69 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml b/Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml
> new file mode 100644
> index 000000000000..02f790fd73d7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/amplifiers/adi,ad8366.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AD8366 and similar Gain Amplifiers and Digital Attenuators
> +
> +maintainers:
> +  - Michael Hennerich <michael.hennerich@analog.com>
> +  - Rodrigo Alencar <rodrigo.alencar@analog.com>
> +
> +description:
> +  Digital Variable Gain Amplifiers (VGAs) and Digital Attenuators with
> +  SPI interface.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad8366
> +      - adi,ada4961
> +      - adi,adl5240
> +      - adi,adrf5720
> +      - adi,adrf5730
> +      - adi,adrf5731
> +      - adi,hmc271a
> +      - adi,hmc792a
> +      - adi,hmc1018a
> +      - adi,hmc1019a
> +      - adi,hmc1119

Why do none of these devices use fallback compatibles? Please put the
rationale in the commit message.

> +
> +  reg:
> +    maxItems: 1
> +
> +  vcc-supply:
> +    description: Regulator that provides power to the device.
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  enable-gpios:
> +    maxItems: 1

How come enable-gpios is optional? Is it optional on all devices?
Do all devices support enable-gpios and/or reset-gpios?

Cheers,
Conor.

> +
> +required:
> +  - compatible
> +  - reg
> +  - vcc-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      amplifier@0 {
> +        compatible = "adi,ad8366";
> +        reg = <0>;
> +        spi-max-frequency = <1000000>;
> +        vcc-supply = <&vcc_3v3>;
> +      };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8b5bb7030fa6..c3403ae478b7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1589,6 +1589,7 @@ M:	Rodrigo Alencar <rodrigo.alencar@analog.com>
>  L:	linux-iio@vger.kernel.org
>  S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/amplifiers/adi,ad8366.yaml
>  F:	drivers/iio/amplifiers/ad8366.c
>  
>  ANALOG DEVICES INC AD9467 DRIVER
> 
> -- 
> 2.43.0
> 
> 

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

Re: [PATCH v2 2/6] dt-bindings: iio: amplifiers: Add AD8366 support

[Rodrigo Alencar]

On 26/01/26 08:11PM, Conor Dooley wrote:
> On Mon, Jan 26, 2026 at 01:51:03PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > 
> > Add device tree binding documentation for amplifiers and digital
> > attenuators. This covers different device variants with similar
> > SPI control.

...

> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ad8366
> > +      - adi,ada4961
> > +      - adi,adl5240
> > +      - adi,adrf5720
> > +      - adi,adrf5730
> > +      - adi,adrf5731
> > +      - adi,hmc271a
> > +      - adi,hmc792a
> > +      - adi,hmc1018a
> > +      - adi,hmc1019a
> > +      - adi,hmc1119
> 
> Why do none of these devices use fallback compatibles? Please put the
> rationale in the commit message.

Will do. Each device has their own gain range/step. 

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  vcc-supply:
> > +    description: Regulator that provides power to the device.
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +
> > +  enable-gpios:
> > +    maxItems: 1
> 
> How come enable-gpios is optional? Is it optional on all devices?
> Do all devices support enable-gpios and/or reset-gpios?

Board designs often hardwire powerup or serial mode enable signals
to high voltage level, so there will not be a reason to add the
enable-gpio.
I went over the device datasheets and I could not find the
reason for the reset gpio. I left it there because it was being used
in the current driver implementation, and I would not like to
invalidate designs that might be currently using it. I will ask around.

-- 
Kind regards,

Rodrigo Alencar

Re: [PATCH v2 2/6] dt-bindings: iio: amplifiers: Add AD8366 support

[Conor Dooley]

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

On Tue, Jan 27, 2026 at 11:37:52AM +0000, Rodrigo Alencar wrote:
> On 26/01/26 08:11PM, Conor Dooley wrote:
> > On Mon, Jan 26, 2026 at 01:51:03PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > 
> > > Add device tree binding documentation for amplifiers and digital
> > > attenuators. This covers different device variants with similar
> > > SPI control.
> 
> ...
> 
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,ad8366
> > > +      - adi,ada4961
> > > +      - adi,adl5240
> > > +      - adi,adrf5720
> > > +      - adi,adrf5730
> > > +      - adi,adrf5731
> > > +      - adi,hmc271a
> > > +      - adi,hmc792a
> > > +      - adi,hmc1018a
> > > +      - adi,hmc1019a
> > > +      - adi,hmc1119
> > 
> > Why do none of these devices use fallback compatibles? Please put the
> > rationale in the commit message.
> 
> Will do. Each device has their own gain range/step. 
> 
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  vcc-supply:
> > > +    description: Regulator that provides power to the device.
> > > +
> > > +  reset-gpios:
> > > +    maxItems: 1
> > > +
> > > +  enable-gpios:
> > > +    maxItems: 1
> > 
> > How come enable-gpios is optional? Is it optional on all devices?
> > Do all devices support enable-gpios and/or reset-gpios?
> 
> Board designs often hardwire powerup or serial mode enable signals
> to high voltage level, so there will not be a reason to add the
> enable-gpio.

I don't see anything about all devices supporting enable-gpios, adl5240
doesn't appear to have one? I'm not going to check all of the datasheets
to see about the others, but you should disallow the property on devices
that don't have an enable pin.

> I went over the device datasheets and I could not find the
> reason for the reset gpio. I left it there because it was being used
> in the current driver implementation, and I would not like to
> invalidate designs that might be currently using it. I will ask around.

If none of the devices have a reset pin, then you should delete the
property from the binding and the driver. Not like you're going to break
something if none of the supported devices even have the pin!

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

Re: [PATCH v2 2/6] dt-bindings: iio: amplifiers: Add AD8366 support

[Rodrigo Alencar]

On 26/01/27 07:38PM, Conor Dooley wrote:
> On Tue, Jan 27, 2026 at 11:37:52AM +0000, Rodrigo Alencar wrote:
> > On 26/01/26 08:11PM, Conor Dooley wrote:
> > > On Mon, Jan 26, 2026 at 01:51:03PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > 
> > > > Add device tree binding documentation for amplifiers and digital
> > > > attenuators. This covers different device variants with similar
> > > > SPI control.
> > 
> > ...
> > 
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - adi,ad8366
> > > > +      - adi,ada4961
> > > > +      - adi,adl5240
> > > > +      - adi,adrf5720
> > > > +      - adi,adrf5730
> > > > +      - adi,adrf5731
> > > > +      - adi,hmc271a
> > > > +      - adi,hmc792a
> > > > +      - adi,hmc1018a
> > > > +      - adi,hmc1019a
> > > > +      - adi,hmc1119
> > > 
> > > Why do none of these devices use fallback compatibles? Please put the
> > > rationale in the commit message.
> > 
> > Will do. Each device has their own gain range/step. 
> > 
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  vcc-supply:
> > > > +    description: Regulator that provides power to the device.
> > > > +
> > > > +  reset-gpios:
> > > > +    maxItems: 1
> > > > +
> > > > +  enable-gpios:
> > > > +    maxItems: 1
> > > 
> > > How come enable-gpios is optional? Is it optional on all devices?
> > > Do all devices support enable-gpios and/or reset-gpios?
> > 
> > Board designs often hardwire powerup or serial mode enable signals
> > to high voltage level, so there will not be a reason to add the
> > enable-gpio.
> 
> I don't see anything about all devices supporting enable-gpios, adl5240
> doesn't appear to have one? I'm not going to check all of the datasheets
> to see about the others, but you should disallow the property on devices
> that don't have an enable pin.

Understood. I will prepare the following for v3:

allOf:
  - $ref: /schemas/spi/spi-peripheral-props.yaml#
  - if:
      properties:
        compatible:
          contains:
            const: adi,hmc271a
    then:
      properties:
        reset-gpios:
          maxItems: 1
  - if:
      properties:
        compatible:
          contains:
            anyOf:
              - const: adi,ad8366
              - const: adi,ada4961
              - const: adi,adrf5720
              - const: adi,adrf5730
              - const: adi,adrf5731
              - const: adi,hmc792a
              - const: adi,hmc1018a
              - const: adi,hmc1019a
              - const: adi,hmc1119
    then:
      properties:
        enable-gpios:
          maxItems: 1
          description: Power-up or Serial Mode Enable GPIO.

> > I went over the device datasheets and I could not find the
> > reason for the reset gpio. I left it there because it was being used
> > in the current driver implementation, and I would not like to
> > invalidate designs that might be currently using it. I will ask around.
> 
> If none of the devices have a reset pin, then you should delete the
> property from the binding and the driver. Not like you're going to break
> something if none of the supported devices even have the pin!

The pin might have been there due to an old revision of the ada4961 device
(the reset_gpio was added to the code when ada4961 support was included)
which I can't find related documentation. However, it turns out that
hmc271a contains a reset pin, as you can see above.

-- 
Kind regards,

Rodrigo Alencar

Re: [PATCH v2 2/6] dt-bindings: iio: amplifiers: Add AD8366 support

[Conor Dooley]

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

On Wed, Jan 28, 2026 at 09:39:12AM +0000, Rodrigo Alencar wrote:
> On 26/01/27 07:38PM, Conor Dooley wrote:
> > On Tue, Jan 27, 2026 at 11:37:52AM +0000, Rodrigo Alencar wrote:
> > > On 26/01/26 08:11PM, Conor Dooley wrote:
> > > > On Mon, Jan 26, 2026 at 01:51:03PM +0000, Rodrigo Alencar via B4 Relay wrote:
> > > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > > 
> > > > > Add device tree binding documentation for amplifiers and digital
> > > > > attenuators. This covers different device variants with similar
> > > > > SPI control.
> > > 
> > > ...
> > > 
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    enum:
> > > > > +      - adi,ad8366
> > > > > +      - adi,ada4961
> > > > > +      - adi,adl5240
> > > > > +      - adi,adrf5720
> > > > > +      - adi,adrf5730
> > > > > +      - adi,adrf5731
> > > > > +      - adi,hmc271a
> > > > > +      - adi,hmc792a
> > > > > +      - adi,hmc1018a
> > > > > +      - adi,hmc1019a
> > > > > +      - adi,hmc1119
> > > > 
> > > > Why do none of these devices use fallback compatibles? Please put the
> > > > rationale in the commit message.
> > > 
> > > Will do. Each device has their own gain range/step. 
> > > 
> > > > > +
> > > > > +  reg:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  vcc-supply:
> > > > > +    description: Regulator that provides power to the device.
> > > > > +
> > > > > +  reset-gpios:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  enable-gpios:
> > > > > +    maxItems: 1
> > > > 
> > > > How come enable-gpios is optional? Is it optional on all devices?
> > > > Do all devices support enable-gpios and/or reset-gpios?
> > > 
> > > Board designs often hardwire powerup or serial mode enable signals
> > > to high voltage level, so there will not be a reason to add the
> > > enable-gpio.
> > 
> > I don't see anything about all devices supporting enable-gpios, adl5240
> > doesn't appear to have one? I'm not going to check all of the datasheets
> > to see about the others, but you should disallow the property on devices
> > that don't have an enable pin.
> 
> Understood. I will prepare the following for v3:
> 
> allOf:
>   - $ref: /schemas/spi/spi-peripheral-props.yaml#
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: adi,hmc271a
>     then:
>       properties:
>         reset-gpios:
>           maxItems: 1

This doesn't look right. The stuff you currently have in the patch
should remain, and this should become
if:
  properties:
    compatible:
      not:
        contains:
          const: adi,hmc271a
then:
  properties:
    reset-gpios: false

>   - if:
>       properties:
>         compatible:
>           contains:
>             anyOf:
>               - const: adi,ad8366
>               - const: adi,ada4961
>               - const: adi,adrf5720
>               - const: adi,adrf5730
>               - const: adi,adrf5731
>               - const: adi,hmc792a
>               - const: adi,hmc1018a
>               - const: adi,hmc1019a
>               - const: adi,hmc1119
>     then:
>       properties:
>         enable-gpios:
>           maxItems: 1
>           description: Power-up or Serial Mode Enable GPIO.

Same for this.

Thanks,
Conor.

> 
> > > I went over the device datasheets and I could not find the
> > > reason for the reset gpio. I left it there because it was being used
> > > in the current driver implementation, and I would not like to
> > > invalidate designs that might be currently using it. I will ask around.
> > 
> > If none of the devices have a reset pin, then you should delete the
> > property from the binding and the driver. Not like you're going to break
> > something if none of the supported devices even have the pin!
> 
> The pin might have been there due to an old revision of the ada4961 device
> (the reset_gpio was added to the code when ada4961 support was included)
> which I can't find related documentation. However, it turns out that
> hmc271a contains a reset pin, as you can see above.
> 
> -- 
> Kind regards,
> 
> Rodrigo Alencar

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

Re: [PATCH v2 2/6] dt-bindings: iio: amplifiers: Add AD8366 support

[David Lechner]

On 1/26/26 7:51 AM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Add device tree binding documentation for amplifiers and digital
> attenuators. This covers different device variants with similar
> SPI control.
> 

...

> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>

This header isn't needed for this example.

> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      amplifier@0 {
> +        compatible = "adi,ad8366";
> +        reg = <0>;
> +        spi-max-frequency = <1000000>;
> +        vcc-supply = <&vcc_3v3>;
> +      };
> +    };

[PATCH v2 3/6] iio: amplifiers: ad8366: refactor device resource management

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Adhere modern device resource management with the following:
- Voltage regulator managed and enabled internally;
- Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init();
- IIO device registration handled with devm_iio_device_register();
- removal of goto's from the probe function;
- ad8366_remove() removed as it is not needed anymore;

Also, dev_err_probe() is used to report probe errors with created local
device pointer.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/amplifiers/ad8366.c | 60 ++++++++++++-----------------------------
 1 file changed, 17 insertions(+), 43 deletions(-)

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index d06ac786501c..0f3fc79ebbf3 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -40,7 +40,6 @@ struct ad8366_info {
 
 struct ad8366_state {
 	struct spi_device	*spi;
-	struct regulator	*reg;
 	struct mutex            lock; /* protect sensor state */
 	struct gpio_desc	*reset_gpio;
 	unsigned char		ch[2];
@@ -245,25 +244,25 @@ static const struct iio_chan_spec ada4961_channels[] = {
 
 static int ad8366_probe(struct spi_device *spi)
 {
+	struct device *dev = &spi->dev;
 	struct iio_dev *indio_dev;
 	struct ad8366_state *st;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (indio_dev == NULL)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
 
-	st->reg = devm_regulator_get(&spi->dev, "vcc");
-	if (!IS_ERR(st->reg)) {
-		ret = regulator_enable(st->reg);
-		if (ret)
-			return ret;
-	}
+	ret = devm_regulator_get_enable(dev, "vcc");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get regulator\n");
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
 
-	spi_set_drvdata(spi, indio_dev);
-	mutex_init(&st->lock);
 	st->spi = spi;
 	st->type = spi_get_device_id(spi)->driver_data;
 
@@ -276,18 +275,16 @@ static int ad8366_probe(struct spi_device *spi)
 	case ID_ADL5240:
 	case ID_HMC792:
 	case ID_HMC1119:
-		st->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_HIGH);
-		if (IS_ERR(st->reset_gpio)) {
-			ret = PTR_ERR(st->reset_gpio);
-			goto error_disable_reg;
-		}
+		st->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+		if (IS_ERR(st->reset_gpio))
+			return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
+					     "Failed to get reset GPIO\n");
+
 		indio_dev->channels = ada4961_channels;
 		indio_dev->num_channels = ARRAY_SIZE(ada4961_channels);
 		break;
 	default:
-		dev_err(&spi->dev, "Invalid device ID\n");
-		ret = -EINVAL;
-		goto error_disable_reg;
+		return dev_err_probe(dev, -EINVAL, "Invalid device ID\n");
 	}
 
 	st->info = &ad8366_infos[st->type];
@@ -297,31 +294,9 @@ static int ad8366_probe(struct spi_device *spi)
 
 	ret = ad8366_write(indio_dev, 0, 0);
 	if (ret < 0)
-		goto error_disable_reg;
+		return dev_err_probe(dev, ret, "failed to write initial gain\n");
 
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto error_disable_reg;
-
-	return 0;
-
-error_disable_reg:
-	if (!IS_ERR(st->reg))
-		regulator_disable(st->reg);
-
-	return ret;
-}
-
-static void ad8366_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct ad8366_state *st = iio_priv(indio_dev);
-	struct regulator *reg = st->reg;
-
-	iio_device_unregister(indio_dev);
-
-	if (!IS_ERR(reg))
-		regulator_disable(reg);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
 static const struct spi_device_id ad8366_id[] = {
@@ -339,7 +314,6 @@ static struct spi_driver ad8366_driver = {
 		.name	= KBUILD_MODNAME,
 	},
 	.probe		= ad8366_probe,
-	.remove		= ad8366_remove,
 	.id_table	= ad8366_id,
 };
 

-- 
2.43.0

Re: [PATCH v2 3/6] iio: amplifiers: ad8366: refactor device resource management

[Andy Shevchenko]

On Mon, Jan 26, 2026 at 01:51:04PM +0000, Rodrigo Alencar via B4 Relay wrote:

> Adhere modern device resource management with the following:
> - Voltage regulator managed and enabled internally;
> - Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init();
> - IIO device registration handled with devm_iio_device_register();
> - removal of goto's from the probe function;
> - ad8366_remove() removed as it is not needed anymore;
> 
> Also, dev_err_probe() is used to report probe errors with created local
> device pointer.

And also it uses the temporary dev variable in the cases that are not covered
by the above.

So, three changes in one patch...

Dunno if Jonathan is okay with this. I would rather split it.
Code wise LGTM.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/6] iio: amplifiers: ad8366: refactor device resource management

[Rodrigo Alencar]

On 26/01/27 11:12PM, Andy Shevchenko wrote:
> On Mon, Jan 26, 2026 at 01:51:04PM +0000, Rodrigo Alencar via B4 Relay wrote:
> 
> > Adhere modern device resource management with the following:
> > - Voltage regulator managed and enabled internally;
> > - Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init();
> > - IIO device registration handled with devm_iio_device_register();
> > - removal of goto's from the probe function;
> > - ad8366_remove() removed as it is not needed anymore;
> > 
> > Also, dev_err_probe() is used to report probe errors with created local
> > device pointer.
> 
> And also it uses the temporary dev variable in the cases that are not covered
> by the above.
> 
> So, three changes in one patch...

I can split. Using proper devm functions allowed me to drop goto's and
ad8366_remove(). Dropping the goto's allowed for the usage of
dev_err_probe().

I could place the temporary dev variable in a separate patch. 

> 
> Dunno if Jonathan is okay with this. I would rather split it.
> Code wise LGTM.

Yeah I can now see it coming. Thanks for the review.

-- 
Kind regards,

Rodrigo Alencar

Re: [PATCH v2 3/6] iio: amplifiers: ad8366: refactor device resource management

[Jonathan Cameron]

On Wed, 28 Jan 2026 09:31:10 +0000
Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:

> On 26/01/27 11:12PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 01:51:04PM +0000, Rodrigo Alencar via B4 Relay wrote:
> >   
> > > Adhere modern device resource management with the following:
> > > - Voltage regulator managed and enabled internally;
> > > - Proper mutex lifecycle with devm_mutex_init(), replacing mutex_init();
> > > - IIO device registration handled with devm_iio_device_register();
> > > - removal of goto's from the probe function;
> > > - ad8366_remove() removed as it is not needed anymore;
> > > 
> > > Also, dev_err_probe() is used to report probe errors with created local
> > > device pointer.  
> > 
> > And also it uses the temporary dev variable in the cases that are not covered
> > by the above.
> > 
> > So, three changes in one patch...  
> 
> I can split. Using proper devm functions allowed me to drop goto's and
> ad8366_remove(). Dropping the goto's allowed for the usage of
> dev_err_probe().

As general rule I prefer one patch one change, though just occasionally
I'll take the expedient view if a series is otherwise ready to merge
and one change is really really small.

J
> 
> I could place the temporary dev variable in a separate patch. 
> 
> > 
> > Dunno if Jonathan is okay with this. I would rather split it.
> > Code wise LGTM.  
> 
> Yeah I can now see it coming. Thanks for the review.
> 

[PATCH v2 4/6] iio: amplifiers: ad8366: add device tree support

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add device-tree support by dropping the enum ID in favor of extended
chip info table, containing:
- gain_step, indicating with sign the start of the code range;
- num_channels, to indicate the number IIO channels;
- pack_code() function to describe how SPI buffer is populated;

With this, switch cases on the device type were dropped:
- probe() function adjusted accordingly;
- Simplified read_raw() and write_raw() callbacks;
- mutex_lock()/mutex_unlock() replaced for guard(mutex)() to allow
  moving to early returns;

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/amplifiers/ad8366.c | 259 +++++++++++++++++-----------------------
 1 file changed, 107 insertions(+), 152 deletions(-)

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index 0f3fc79ebbf3..134188db2e15 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -11,6 +11,7 @@
  * Copyright 2012-2019 Analog Devices Inc.
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/slab.h>
@@ -20,22 +21,20 @@
 #include <linux/gpio/consumer.h>
 #include <linux/err.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/bitrev.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-enum ad8366_type {
-	ID_AD8366,
-	ID_ADA4961,
-	ID_ADL5240,
-	ID_HMC792,
-	ID_HMC1119,
-};
+struct ad8366_state;
 
 struct ad8366_info {
 	int gain_min;
 	int gain_max;
+	int gain_step;
+	int num_channels;
+	size_t (*pack_code)(struct ad8366_state *st);
 };
 
 struct ad8366_state {
@@ -43,7 +42,6 @@ struct ad8366_state {
 	struct mutex            lock; /* protect sensor state */
 	struct gpio_desc	*reset_gpio;
 	unsigned char		ch[2];
-	enum ad8366_type	type;
 	const struct ad8366_info *info;
 	/*
 	 * DMA (thus cache coherency maintenance) may require the
@@ -52,108 +50,91 @@ struct ad8366_state {
 	unsigned char		data[2] __aligned(IIO_DMA_MINALIGN);
 };
 
-static const struct ad8366_info ad8366_infos[] = {
-	[ID_AD8366] = {
-		.gain_min = 4500,
-		.gain_max = 20500,
-	},
-	[ID_ADA4961] = {
-		.gain_min = -6000,
-		.gain_max = 15000,
-	},
-	[ID_ADL5240] = {
-		.gain_min = -11500,
-		.gain_max = 20000,
-	},
-	[ID_HMC792] = {
-		.gain_min = -15750,
-		.gain_max = 0,
-	},
-	[ID_HMC1119] = {
-		.gain_min = -31750,
-		.gain_max = 0,
-	},
+static size_t ad8366_pack_code(struct ad8366_state *st)
+{
+	u8 ch_a = bitrev8(st->ch[0] & 0x3F);
+	u8 ch_b = bitrev8(st->ch[1] & 0x3F);
+
+	st->data[0] = ch_b >> 4;
+	st->data[1] = (ch_b << 4) | (ch_a >> 2);
+	return 2;
+}
+
+static size_t simple_pack_code(struct ad8366_state *st)
+{
+	st->data[0] = st->ch[0];
+	return 1;
+}
+
+static const struct ad8366_info ad8366_chip_info = {
+	.gain_min = 4500,
+	.gain_max = 20500,
+	.gain_step = 253,
+	.num_channels = 2,
+	.pack_code = ad8366_pack_code,
 };
 
-static int ad8366_write(struct iio_dev *indio_dev,
-			unsigned char ch_a, unsigned char ch_b)
+static const struct ad8366_info ada4961_chip_info = {
+	.gain_min = -6000,
+	.gain_max = 15000,
+	.gain_step = -1000,
+	.num_channels = 1,
+	.pack_code = simple_pack_code,
+};
+
+static const struct ad8366_info adl5240_chip_info = {
+	.gain_min = -11500,
+	.gain_max = 20000,
+	.gain_step = 500,
+	.num_channels = 1,
+	.pack_code = simple_pack_code,
+};
+
+static const struct ad8366_info hmc792_chip_info = {
+	.gain_min = -15750,
+	.gain_max = 0,
+	.gain_step = 250,
+	.num_channels = 1,
+	.pack_code = simple_pack_code,
+};
+
+static const struct ad8366_info hmc1119_chip_info = {
+	.gain_min = -31750,
+	.gain_max = 0,
+	.gain_step = -250,
+	.num_channels = 1,
+	.pack_code = simple_pack_code,
+};
+
+static int ad8366_write_code(struct ad8366_state *st)
 {
-	struct ad8366_state *st = iio_priv(indio_dev);
-	int ret;
+	const struct ad8366_info *inf = st->info;
 
-	switch (st->type) {
-	case ID_AD8366:
-		ch_a = bitrev8(ch_a & 0x3F);
-		ch_b = bitrev8(ch_b & 0x3F);
-
-		st->data[0] = ch_b >> 4;
-		st->data[1] = (ch_b << 4) | (ch_a >> 2);
-		break;
-	case ID_ADA4961:
-		st->data[0] = ch_a & 0x1F;
-		break;
-	case ID_ADL5240:
-		st->data[0] = (ch_a & 0x3F);
-		break;
-	case ID_HMC792:
-	case ID_HMC1119:
-		st->data[0] = ch_a;
-		break;
-	}
-
-	ret = spi_write(st->spi, st->data, indio_dev->num_channels);
-	if (ret < 0)
-		dev_err(&indio_dev->dev, "write failed (%d)", ret);
-
-	return ret;
+	return spi_write(st->spi, st->data, inf->pack_code(st));
 }
 
 static int ad8366_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val,
 			   int *val2,
-			   long m)
+			   long mask)
 {
 	struct ad8366_state *st = iio_priv(indio_dev);
-	int ret;
-	int code, gain = 0;
+	const struct ad8366_info *inf = st->info;
+	int gain = inf->gain_step > 0 ? inf->gain_min : inf->gain_max;
 
-	mutex_lock(&st->lock);
-	switch (m) {
+	guard(mutex)(&st->lock);
+
+	switch (mask) {
 	case IIO_CHAN_INFO_HARDWAREGAIN:
-		code = st->ch[chan->channel];
-
-		switch (st->type) {
-		case ID_AD8366:
-			gain = code * 253 + 4500;
-			break;
-		case ID_ADA4961:
-			gain = 15000 - code * 1000;
-			break;
-		case ID_ADL5240:
-			gain = 20000 - 31500 + code * 500;
-			break;
-		case ID_HMC792:
-			gain = -1 * code * 500;
-			break;
-		case ID_HMC1119:
-			gain = -1 * code * 250;
-			break;
-		}
-
-		/* Values in dB */
+		gain += inf->gain_step * st->ch[chan->channel];
 		*val = gain / 1000;
 		*val2 = (gain % 1000) * 1000;
-
-		ret = IIO_VAL_INT_PLUS_MICRO_DB;
-		break;
+		return IIO_VAL_INT_PLUS_MICRO_DB;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-	mutex_unlock(&st->lock);
-
-	return ret;
-};
+}
 
 static int ad8366_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
@@ -163,10 +144,8 @@ static int ad8366_write_raw(struct iio_dev *indio_dev,
 {
 	struct ad8366_state *st = iio_priv(indio_dev);
 	const struct ad8366_info *inf = st->info;
-	int code = 0, gain;
-	int ret;
+	int code, gain, gain_base;
 
-	/* Values in dB */
 	if (val < 0)
 		gain = (val * 1000) - (val2 / 1000);
 	else
@@ -175,36 +154,18 @@ static int ad8366_write_raw(struct iio_dev *indio_dev,
 	if (gain > inf->gain_max || gain < inf->gain_min)
 		return -EINVAL;
 
-	switch (st->type) {
-	case ID_AD8366:
-		code = (gain - 4500) / 253;
-		break;
-	case ID_ADA4961:
-		code = (15000 - gain) / 1000;
-		break;
-	case ID_ADL5240:
-		code = ((gain - 500 - 20000) / 500) & 0x3F;
-		break;
-	case ID_HMC792:
-		code = (abs(gain) / 500) & 0x3F;
-		break;
-	case ID_HMC1119:
-		code = (abs(gain) / 250) & 0x7F;
-		break;
-	}
+	gain_base = inf->gain_step > 0 ? inf->gain_min : inf->gain_max;
+	code = DIV_ROUND_CLOSEST(gain - gain_base, inf->gain_step);
+
+	guard(mutex)(&st->lock);
 
-	mutex_lock(&st->lock);
 	switch (mask) {
 	case IIO_CHAN_INFO_HARDWAREGAIN:
 		st->ch[chan->channel] = code;
-		ret = ad8366_write(indio_dev, st->ch[0], st->ch[1]);
-		break;
+		return ad8366_write_code(st);
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-	mutex_unlock(&st->lock);
-
-	return ret;
 }
 
 static int ad8366_write_raw_get_fmt(struct iio_dev *indio_dev,
@@ -238,10 +199,6 @@ static const struct iio_chan_spec ad8366_channels[] = {
 	AD8366_CHAN(1),
 };
 
-static const struct iio_chan_spec ada4961_channels[] = {
-	AD8366_CHAN(0),
-};
-
 static int ad8366_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
@@ -264,35 +221,22 @@ static int ad8366_probe(struct spi_device *spi)
 		return ret;
 
 	st->spi = spi;
-	st->type = spi_get_device_id(spi)->driver_data;
+	st->info = spi_get_device_match_data(spi);
+	if (!st->info)
+		return dev_err_probe(dev, -EINVAL, "Invalid device info\n");
 
-	switch (st->type) {
-	case ID_AD8366:
-		indio_dev->channels = ad8366_channels;
-		indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
-		break;
-	case ID_ADA4961:
-	case ID_ADL5240:
-	case ID_HMC792:
-	case ID_HMC1119:
-		st->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
-		if (IS_ERR(st->reset_gpio))
-			return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
-					     "Failed to get reset GPIO\n");
+	st->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(st->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
+				     "Failed to get reset GPIO\n");
 
-		indio_dev->channels = ada4961_channels;
-		indio_dev->num_channels = ARRAY_SIZE(ada4961_channels);
-		break;
-	default:
-		return dev_err_probe(dev, -EINVAL, "Invalid device ID\n");
-	}
-
-	st->info = &ad8366_infos[st->type];
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->info = &ad8366_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ad8366_channels;
+	indio_dev->num_channels = st->info->num_channels;
 
-	ret = ad8366_write(indio_dev, 0, 0);
+	ret = ad8366_write_code(st);
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "failed to write initial gain\n");
 
@@ -300,18 +244,29 @@ static int ad8366_probe(struct spi_device *spi)
 }
 
 static const struct spi_device_id ad8366_id[] = {
-	{"ad8366",  ID_AD8366},
-	{"ada4961", ID_ADA4961},
-	{"adl5240", ID_ADL5240},
-	{"hmc792a", ID_HMC792},
-	{"hmc1119", ID_HMC1119},
+	{"ad8366", (kernel_ulong_t)&ad8366_chip_info},
+	{"ada4961", (kernel_ulong_t)&ada4961_chip_info},
+	{"adl5240", (kernel_ulong_t)&adl5240_chip_info},
+	{"hmc792a", (kernel_ulong_t)&hmc792_chip_info},
+	{"hmc1119", (kernel_ulong_t)&hmc1119_chip_info},
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ad8366_id);
 
+static const struct of_device_id ad8366_of_match[] = {
+	{ .compatible = "adi,ad8366", .data = &ad8366_chip_info },
+	{ .compatible = "adi,ada4961", .data = &ada4961_chip_info },
+	{ .compatible = "adi,adl5240", .data = &adl5240_chip_info },
+	{ .compatible = "adi,hmc792a", .data = &hmc792_chip_info },
+	{ .compatible = "adi,hmc1119", .data = &hmc1119_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad8366_of_match);
+
 static struct spi_driver ad8366_driver = {
 	.driver = {
-		.name	= KBUILD_MODNAME,
+		.name		= KBUILD_MODNAME,
+		.of_match_table	= ad8366_of_match,
 	},
 	.probe		= ad8366_probe,
 	.id_table	= ad8366_id,

-- 
2.43.0

Re: [PATCH v2 4/6] iio: amplifiers: ad8366: add device tree support

[Andy Shevchenko]

On Mon, Jan 26, 2026 at 01:51:05PM +0000, Rodrigo Alencar via B4 Relay wrote:

> Add device-tree support by dropping the enum ID in favor of extended
> chip info table, containing:
> - gain_step, indicating with sign the start of the code range;
> - num_channels, to indicate the number IIO channels;
> - pack_code() function to describe how SPI buffer is populated;
> 
> With this, switch cases on the device type were dropped:
> - probe() function adjusted accordingly;
> - Simplified read_raw() and write_raw() callbacks;

> - mutex_lock()/mutex_unlock() replaced for guard(mutex)() to allow
>   moving to early returns;

Shouldn't this be in a separate change? I dunno. Let Jonathan to decide.

...

> +static size_t ad8366_pack_code(struct ad8366_state *st)
> +{
> +	u8 ch_a = bitrev8(st->ch[0] & 0x3F);
> +	u8 ch_b = bitrev8(st->ch[1] & 0x3F);

GENMASK() in both cases? But I don't see why ch_a needs this at all,
isn't the 2 LSBs are not used anyway?

Also missed header inclusion for this? And also perhaps sorting headers first
to see what's there and what needs to be updated (ideally another patch to move
to IWYU principle).

> +	st->data[0] = ch_b >> 4;
> +	st->data[1] = (ch_b << 4) | (ch_a >> 2);
> +	return 2;
> +}

...

>  static int ad8366_read_raw(struct iio_dev *indio_dev,
>  			   struct iio_chan_spec const *chan,
>  			   int *val,
>  			   int *val2,
> -			   long m)
> +			   long mask)

Seems like unrelated change.

...

> -	/* Values in dB */

Do you think this comment is useless? To me looks like a stray change.

>  	if (val < 0)
>  		gain = (val * 1000) - (val2 / 1000);

...

> +	st->info = spi_get_device_match_data(spi);

> +	if (!st->info)
> +		return dev_err_probe(dev, -EINVAL, "Invalid device info\n");

Only useful for the developer, dead code in production.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/6] iio: amplifiers: ad8366: add device tree support

[Rodrigo Alencar]

On 26/01/27 11:21PM, Andy Shevchenko wrote:
> On Mon, Jan 26, 2026 at 01:51:05PM +0000, Rodrigo Alencar via B4 Relay wrote:
> 
> > Add device-tree support by dropping the enum ID in favor of extended
> > chip info table, containing:
> > - gain_step, indicating with sign the start of the code range;
> > - num_channels, to indicate the number IIO channels;
> > - pack_code() function to describe how SPI buffer is populated;
> > 
> > With this, switch cases on the device type were dropped:
> > - probe() function adjusted accordingly;
> > - Simplified read_raw() and write_raw() callbacks;
> 
> > - mutex_lock()/mutex_unlock() replaced for guard(mutex)() to allow
> >   moving to early returns;
> 
> Shouldn't this be in a separate change? I dunno. Let Jonathan to decide.
> 

As read_raw() and write_raw() were refactored, I thought it would not be
a problem. I can drop the change... as it is not a function with many
complicated returns.

> 
> > +static size_t ad8366_pack_code(struct ad8366_state *st)
> > +{
> > +	u8 ch_a = bitrev8(st->ch[0] & 0x3F);
> > +	u8 ch_b = bitrev8(st->ch[1] & 0x3F);
> 
> GENMASK() in both cases? But I don't see why ch_a needs this at all,
> isn't the 2 LSBs are not used anyway?

Yes, I can adjust with:

u8 ch_a = bitrev8(st->ch[0]) >> 2;
u8 ch_b = bitrev8(st->ch[1]) >> 2;

st->data[0] = ch_b >> 2;
st->data[1] = (ch_b << 6) | ch_a;

so no need for masking both.

> Also missed header inclusion for this? And also perhaps sorting headers first
> to see what's there and what needs to be updated (ideally another patch to move
> to IWYU principle).

linux/bitrev.h is there, but indeed header includes are not sorted.
I will create a separate patch for that.

> 
> > +	st->data[0] = ch_b >> 4;
> > +	st->data[1] = (ch_b << 4) | (ch_a >> 2);
> > +	return 2;
> > +}

-- 
Kind regards,

Rodrigo Alencar

Re: [PATCH v2 4/6] iio: amplifiers: ad8366: add device tree support

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 09:55:16AM +0000, Rodrigo Alencar wrote:
> On 26/01/27 11:21PM, Andy Shevchenko wrote:
> > On Mon, Jan 26, 2026 at 01:51:05PM +0000, Rodrigo Alencar via B4 Relay wrote:

...

> > > +static size_t ad8366_pack_code(struct ad8366_state *st)
> > > +{
> > > +	u8 ch_a = bitrev8(st->ch[0] & 0x3F);
> > > +	u8 ch_b = bitrev8(st->ch[1] & 0x3F);
> > 
> > GENMASK() in both cases? But I don't see why ch_a needs this at all,
> > isn't the 2 LSBs are not used anyway?
> 
> Yes, I can adjust with:
> 
> u8 ch_a = bitrev8(st->ch[0]) >> 2;
> u8 ch_b = bitrev8(st->ch[1]) >> 2;
> 
> st->data[0] = ch_b >> 2;
> st->data[1] = (ch_b << 6) | ch_a;
> 
> so no need for masking both.

This is better, but let's think a bit more. The data we put seems to be
__be12 (yeah, we don't have the exact type for that) and can be put slightly
differently.

So, something like

	put_unaligned_be16((ch_b << 6) | ch_a, &st->deta[0]);

should be better, no? (Note, you would need linux/unaligned.h).

> > Also missed header inclusion for this? And also perhaps sorting headers first
> > to see what's there and what needs to be updated (ideally another patch to move
> > to IWYU principle).
> 
> linux/bitrev.h is there, but indeed header includes are not sorted.
> I will create a separate patch for that.

Ah, good!

> > > +	st->data[0] = ch_b >> 4;
> > > +	st->data[1] = (ch_b << 4) | (ch_a >> 2);
> > > +	return 2;
> > > +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/6] iio: amplifiers: ad8366: add device tree support

[Rodrigo Alencar]

On 26/01/28 12:09PM, Andy Shevchenko wrote:
> On Wed, Jan 28, 2026 at 09:55:16AM +0000, Rodrigo Alencar wrote:
> > On 26/01/27 11:21PM, Andy Shevchenko wrote:
> > > On Mon, Jan 26, 2026 at 01:51:05PM +0000, Rodrigo Alencar via B4 Relay wrote:
> 
> ...
> 
> > > > +static size_t ad8366_pack_code(struct ad8366_state *st)
> > > > +{
> > > > +	u8 ch_a = bitrev8(st->ch[0] & 0x3F);
> > > > +	u8 ch_b = bitrev8(st->ch[1] & 0x3F);
> > > 
> > > GENMASK() in both cases? But I don't see why ch_a needs this at all,
> > > isn't the 2 LSBs are not used anyway?
> > 
> > Yes, I can adjust with:
> > 
> > u8 ch_a = bitrev8(st->ch[0]) >> 2;
> > u8 ch_b = bitrev8(st->ch[1]) >> 2;
> > 
> > st->data[0] = ch_b >> 2;
> > st->data[1] = (ch_b << 6) | ch_a;
> > 
> > so no need for masking both.
> 
> This is better, but let's think a bit more. The data we put seems to be
> __be12 (yeah, we don't have the exact type for that) and can be put slightly
> differently.
> 
> So, something like
> 
> 	put_unaligned_be16((ch_b << 6) | ch_a, &st->deta[0]);
> 
> should be better, no? (Note, you would need linux/unaligned.h).

ok, ch_b would have to be u16 and ch_a could be too (for consistency)

-- 
Kind regards,

Rodrigo Alencar

Re: [PATCH v2 4/6] iio: amplifiers: ad8366: add device tree support

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 10:23:58AM +0000, Rodrigo Alencar wrote:
> On 26/01/28 12:09PM, Andy Shevchenko wrote:
> > On Wed, Jan 28, 2026 at 09:55:16AM +0000, Rodrigo Alencar wrote:

...

> > So, something like
> > 
> > 	put_unaligned_be16((ch_b << 6) | ch_a, &st->deta[0]);
> > 
> > should be better, no? (Note, you would need linux/unaligned.h).
> 
> ok, ch_b would have to be u16 and ch_a could be too (for consistency)

Why? u8 should suffice.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/6] iio: amplifiers: ad8366: add device tree support

[David Lechner]

On 1/26/26 7:51 AM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Add device-tree support by dropping the enum ID in favor of extended
> chip info table, containing:
> - gain_step, indicating with sign the start of the code range;
> - num_channels, to indicate the number IIO channels;
> - pack_code() function to describe how SPI buffer is populated;
> 
> With this, switch cases on the device type were dropped:
> - probe() function adjusted accordingly;
> - Simplified read_raw() and write_raw() callbacks;
> - mutex_lock()/mutex_unlock() replaced for guard(mutex)() to allow
>   moving to early returns;
> 

...

> +static size_t simple_pack_code(struct ad8366_state *st)

This name is a bit generic. I would call it e.g. hmc792_pack_code()
so that it clearly belongs to this driver.

Or drop this function and make the logic elsewhere:

	if (info->pack_code)
		return info->pack_code(st);

	st->data[0] = st->ch[0];
	return 1;

Then the info struct definitions would be less verbose.

> +{
> +	st->data[0] = st->ch[0];
> +	return 1;
> +}
> +

...

>  static int ad8366_probe(struct spi_device *spi)
>  {
>  	struct device *dev = &spi->dev;
> @@ -264,35 +221,22 @@ static int ad8366_probe(struct spi_device *spi)
>  		return ret;
>  
>  	st->spi = spi;
> -	st->type = spi_get_device_id(spi)->driver_data;
> +	st->info = spi_get_device_match_data(spi);
> +	if (!st->info)
> +		return dev_err_probe(dev, -EINVAL, "Invalid device info\n");
>  
> -	switch (st->type) {
> -	case ID_AD8366:
> -		indio_dev->channels = ad8366_channels;
> -		indio_dev->num_channels = ARRAY_SIZE(ad8366_channels);
> -		break;
> -	case ID_ADA4961:
> -	case ID_ADL5240:
> -	case ID_HMC792:
> -	case ID_HMC1119:
> -		st->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> -		if (IS_ERR(st->reset_gpio))
> -			return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
> -					     "Failed to get reset GPIO\n");

As a precursor cleanup, st->reset_gpio can be removed and turned into a
local variable. It isn't used anywhere else.

Also, this could use a comment to explain that previously the driver
specifically had the reset gpio for some chips that don't actually
have a reset pin. It could have been wired up to the power/enable pin
instead, so some users might be relying on this to turn the chip on
rather than reset it.

> +	st->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
> +				     "Failed to get reset GPIO\n");
>  
> -		indio_dev->channels = ada4961_channels;
> -		indio_dev->num_channels = ARRAY_SIZE(ada4961_channels);
> -		break;
> -	default:
> -		return dev_err_probe(dev, -EINVAL, "Invalid device ID\n");
> -	}
> -
> -	st->info = &ad8366_infos[st->type];
>  	indio_dev->name = spi_get_device_id(spi)->name;
>  	indio_dev->info = &ad8366_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ad8366_channels;
> +	indio_dev->num_channels = st->info->num_channels;
>  
> -	ret = ad8366_write(indio_dev, 0, 0);
> +	ret = ad8366_write_code(st);
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "failed to write initial gain\n");
>  

[PATCH v2 5/6] iio: amplifiers: ad8366: consume enable gpio

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Some parts may consume enable GPIO to enable serial mode
(HMC1119's and HMC792A P/S pin) or powerup the device
(e.g. ADA4961's PWUP pin).

Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/amplifiers/ad8366.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index 134188db2e15..9ad6a628376c 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -41,6 +41,7 @@ struct ad8366_state {
 	struct spi_device	*spi;
 	struct mutex            lock; /* protect sensor state */
 	struct gpio_desc	*reset_gpio;
+	struct gpio_desc	*enable_gpio;
 	unsigned char		ch[2];
 	const struct ad8366_info *info;
 	/*
@@ -230,6 +231,11 @@ static int ad8366_probe(struct spi_device *spi)
 		return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
 				     "Failed to get reset GPIO\n");
 
+	st->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(st->enable_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->enable_gpio),
+				     "Failed to get enable GPIO\n");
+
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->info = &ad8366_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;

-- 
2.43.0

Re: [PATCH v2 5/6] iio: amplifiers: ad8366: consume enable gpio

[David Lechner]

On 1/26/26 7:51 AM, Rodrigo Alencar via B4 Relay wrote:
> From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> 
> Some parts may consume enable GPIO to enable serial mode
> (HMC1119's and HMC792A P/S pin) or powerup the device
> (e.g. ADA4961's PWUP pin).
> 
> Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> ---
>  drivers/iio/amplifiers/ad8366.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
> index 134188db2e15..9ad6a628376c 100644
> --- a/drivers/iio/amplifiers/ad8366.c
> +++ b/drivers/iio/amplifiers/ad8366.c
> @@ -41,6 +41,7 @@ struct ad8366_state {
>  	struct spi_device	*spi;
>  	struct mutex            lock; /* protect sensor state */
>  	struct gpio_desc	*reset_gpio;
> +	struct gpio_desc	*enable_gpio;

This isn't needed since it is never used outside of the probe function.
It can be a local variable.

>  	unsigned char		ch[2];
>  	const struct ad8366_info *info;
>  	/*
> @@ -230,6 +231,11 @@ static int ad8366_probe(struct spi_device *spi)
>  		return dev_err_probe(dev, PTR_ERR(st->reset_gpio),
>  				     "Failed to get reset GPIO\n");
>  
> +	st->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->enable_gpio))
> +		return dev_err_probe(dev, PTR_ERR(st->enable_gpio),
> +				     "Failed to get enable GPIO\n");
> +
>  	indio_dev->name = spi_get_device_id(spi)->name;
>  	indio_dev->info = &ad8366_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> 

[PATCH v2 6/6] iio: amplifiers: ad8366: Update device support

[Rodrigo Alencar via B4 Relay]

From: Rodrigo Alencar <rodrigo.alencar@analog.com>

Add support for the following digital step attenuators:
- HMC271A: 1dB LSB 5-Bit Digital Attenuator SMT, 0.7 - 3.7 GHz
- ADRF5720: 0.5 dB LSB, 6-Bit, Digital Attenuator, 9 kHz to 40 GHz
- ADRF5730: 0.5 dB LSB, 6-Bit, Digital Attenuator, 100 MHz to 40 GHz
- ADRF5731: 2 dB LSB, 4-Bit, Digital Attenuator, 100 MHz to 40 GHz
- HMC1018A: 1.0 dB LSB GaAs MMIC 5-BIT DIGITAL ATTENUATOR, 0.1 - 30 GHz
- HMC1019A: 0.5 dB LSB GaAs MMIC 5-BIT DIGITAL ATTENUATOR, 0.1 - 30 GHz

Additionally, copyright notice was updated with current year.

Co-developed-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Co-developed-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
 drivers/iio/amplifiers/Kconfig  |  6 ++++
 drivers/iio/amplifiers/ad8366.c | 72 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/amplifiers/Kconfig b/drivers/iio/amplifiers/Kconfig
index a8a604863eed..39d280d4d437 100644
--- a/drivers/iio/amplifiers/Kconfig
+++ b/drivers/iio/amplifiers/Kconfig
@@ -18,7 +18,13 @@ config AD8366
 	    AD8366 Dual-Digital Variable Gain Amplifier (VGA)
 	    ADA4961 BiCMOS RF Digital Gain Amplifier (DGA)
 	    ADL5240 Digitally controlled variable gain amplifier (VGA)
+	    ADRF5720: 0.5 dB LSB, 6-Bit, Silicon Digital Attenuator
+	    ADRF5730: 0.5 dB LSB, 6-Bit, Silicon Digital Attenuator
+	    ADRF5731: 2 dB LSB, 4-Bit, Silicon Digital Attenuator
+	    HMC271A: 1dB LSB 5-Bit Digital Attenuator SMT
 	    HMC792A 0.25 dB LSB GaAs MMIC 6-Bit Digital Attenuator
+	    HMC1018A: 1.0 dB LSB GaAs MMIC 5-BIT Digital Attenuator
+	    HMC1019A: 0.5 dB LSB GaAs MMIC 5-BIT Digital Attenuator
 	    HMC1119 0.25 dB LSB, 7-Bit, Silicon Digital Attenuator
 
 	  To compile this driver as a module, choose M here: the
diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index 9ad6a628376c..14de0418a234 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -5,10 +5,16 @@
  *   AD8366 Dual-Digital Variable Gain Amplifier (VGA)
  *   ADA4961 BiCMOS RF Digital Gain Amplifier (DGA)
  *   ADL5240 Digitally controlled variable gain amplifier (VGA)
+ *   ADRF5720: 0.5 dB LSB, 6-Bit, Silicon Digital Attenuator, 9 kHz to 40 GHz
+ *   ADRF5730: 0.5 dB LSB, 6-Bit, Silicon Digital Attenuator, 100 MHz to 40 GHz
+ *   ADRF5731: 2 dB LSB, 4-Bit, Silicon Digital Attenuator, 100 MHz to 40 GHz
+ *   HMC271A: 1dB LSB 5-Bit Digital Attenuator SMT, 0.7 - 3.7 GHz
  *   HMC792A 0.25 dB LSB GaAs MMIC 6-Bit Digital Attenuator
+ *   HMC1018A: 1.0 dB LSB GaAs MMIC 5-BIT DIGITAL ATTENUATOR, 0.1 - 30 GHz
+ *   HMC1019A: 0.5 dB LSB GaAs MMIC 5-BIT DIGITAL ATTENUATOR, 0.1 - 30 GHz
  *   HMC1119 0.25 dB LSB, 7-Bit, Silicon Digital Attenuator
  *
- * Copyright 2012-2019 Analog Devices Inc.
+ * Copyright 2012-2026 Analog Devices Inc.
  */
 
 #include <linux/cleanup.h>
@@ -67,6 +73,18 @@ static size_t simple_pack_code(struct ad8366_state *st)
 	return 1;
 }
 
+static size_t adrf5731_pack_code(struct ad8366_state *st)
+{
+	st->data[0] = st->ch[0] << 2;
+	return 1;
+}
+
+static size_t hmc271_pack_code(struct ad8366_state *st)
+{
+	st->data[0] = bitrev8(st->ch[0] & 0x1F) >> 3;
+	return 1;
+}
+
 static const struct ad8366_info ad8366_chip_info = {
 	.gain_min = 4500,
 	.gain_max = 20500,
@@ -91,6 +109,30 @@ static const struct ad8366_info adl5240_chip_info = {
 	.pack_code = simple_pack_code,
 };
 
+static const struct ad8366_info adrf57x0_chip_info = {
+	.gain_min = -31500,
+	.gain_max = 0,
+	.gain_step = -500,
+	.num_channels = 1,
+	.pack_code = simple_pack_code,
+};
+
+static const struct ad8366_info adrf5731_chip_info = {
+	.gain_min = -30000,
+	.gain_max = 0,
+	.gain_step = -2000,
+	.num_channels = 1,
+	.pack_code = adrf5731_pack_code,
+};
+
+static const struct ad8366_info hmc271_chip_info = {
+	.gain_min = -31000,
+	.gain_max = 0,
+	.gain_step = 1000,
+	.num_channels = 1,
+	.pack_code = hmc271_pack_code,
+};
+
 static const struct ad8366_info hmc792_chip_info = {
 	.gain_min = -15750,
 	.gain_max = 0,
@@ -99,6 +141,22 @@ static const struct ad8366_info hmc792_chip_info = {
 	.pack_code = simple_pack_code,
 };
 
+static const struct ad8366_info hmc1018_chip_info = {
+	.gain_min = -31000,
+	.gain_max = 0,
+	.gain_step = 1000,
+	.num_channels = 1,
+	.pack_code = simple_pack_code,
+};
+
+static const struct ad8366_info hmc1019_chip_info = {
+	.gain_min = -15500,
+	.gain_max = 0,
+	.gain_step = 500,
+	.num_channels = 1,
+	.pack_code = simple_pack_code,
+};
+
 static const struct ad8366_info hmc1119_chip_info = {
 	.gain_min = -31750,
 	.gain_max = 0,
@@ -253,7 +311,13 @@ static const struct spi_device_id ad8366_id[] = {
 	{"ad8366", (kernel_ulong_t)&ad8366_chip_info},
 	{"ada4961", (kernel_ulong_t)&ada4961_chip_info},
 	{"adl5240", (kernel_ulong_t)&adl5240_chip_info},
+	{"adrf5720", (kernel_ulong_t)&adrf57x0_chip_info},
+	{"adrf5730", (kernel_ulong_t)&adrf57x0_chip_info},
+	{"adrf5731", (kernel_ulong_t)&adrf5731_chip_info},
+	{"hmc271a", (kernel_ulong_t)&hmc271_chip_info},
 	{"hmc792a", (kernel_ulong_t)&hmc792_chip_info},
+	{"hmc1018a", (kernel_ulong_t)&hmc1018_chip_info},
+	{"hmc1019a", (kernel_ulong_t)&hmc1019_chip_info},
 	{"hmc1119", (kernel_ulong_t)&hmc1119_chip_info},
 	{ }
 };
@@ -263,7 +327,13 @@ static const struct of_device_id ad8366_of_match[] = {
 	{ .compatible = "adi,ad8366", .data = &ad8366_chip_info },
 	{ .compatible = "adi,ada4961", .data = &ada4961_chip_info },
 	{ .compatible = "adi,adl5240", .data = &adl5240_chip_info },
+	{ .compatible = "adi,adrf5720", .data = &adrf57x0_chip_info },
+	{ .compatible = "adi,adrf5730", .data = &adrf57x0_chip_info },
+	{ .compatible = "adi,adrf5731", .data = &adrf5731_chip_info },
+	{ .compatible = "adi,hmc271a", .data = &hmc271_chip_info },
 	{ .compatible = "adi,hmc792a", .data = &hmc792_chip_info },
+	{ .compatible = "adi,hmc1018a", .data = &hmc1018_chip_info },
+	{ .compatible = "adi,hmc1019a", .data = &hmc1019_chip_info },
 	{ .compatible = "adi,hmc1119", .data = &hmc1119_chip_info },
 	{ }
 };

-- 
2.43.0