Extend device support for AD5686 driver

[PATCH 00/22] Extend device support for AD5686 driver

Posted by Rodrigo Alencar via B4 Relay 1 month, 3 weeks ago

This series adds support for multiple nanoDAC parts, adding triggered
buffer and gain control support to the ad5686 DAC driver family, along
with a number of driver cleanups and fixes.

Initial patches update the device-tree bindings:
- Add compatible entries for missing and new parts;
- Add GPIO properties for RESET, GAIN and LDAC pins;
- Add missing power supplies properties.

Driver cleanups and fixes:
- Refactor include headers (IWYU);
- Switch to device managed mutex initialization;
- Drop enum chip id in favor of per-device chip_info structs;
- Fix voltage reference control on single-channel devices;
- Fix powerdown control on dual-channel devices;
- Introduce bus ops struct with a sync() operation for batching
  bus transfers.

New functionality:
- Device support for: AD5316R, AD5675, AD5697R, AD5313R, AD5317R,
  AD5674, AD5679, AD5687, AD5687R, AD5689 and AD5689R;
- Consume optional reset and new power supplies;
- LDAC GPIO handling (active-low, held low when unused);
- SPI bus sync() implementation for batching multiple transfers;
- Triggered buffer support, leveraging LDAC and sync() to flush
  all channel writes atomically;
- Gain control support through the scale property.

Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Rodrigo Alencar (22):
      dt-bindings: iio: dac: ad5696: extend device support
      dt-bindings: iio: dac: ad5696: add reset/ldac/gain gpio support
      dt-bindings: iio: dac: ad5696: rework on power supplies
      dt-bindings: iio: dac: ad5686: extend device support
      dt-bindings: iio: dac: ad5686: add reset/ldac/gain gpio support
      dt-bindings: iio: dac: ad5686: rework on power supplies
      iio: dac: ad5686: refactor include headers
      iio: dac: ad5686: remove redundant register definition
      iio: dac: ad5686: drop enum id
      iio: dac: ad5686: add of_match table to the spi driver
      iio: dac: ad5686: fix ref bit initialization for single-channel parts
      iio: dac: ad5686: fix powerdown control
      iio: dac: ad5686: fix input raw value check
      iio: dac: ad5686: add support for missing power supplies
      iio: dac: ad5686: create bus ops struct
      iio: dac: ad5686: extend device support with new parts
      iio: dac: ad5686: update device list description
      iio: dac: ad5686: consume optional reset signal
      iio: dac: ad5686: add ldac gpio
      iio: dac: ad5686: implement new sync() op for the spi bus
      iio: dac: ad5686: add triggered buffer support
      iio: dac: ad5686: add gain control support

 .../devicetree/bindings/iio/dac/adi,ad5686.yaml    |  61 +-
 .../devicetree/bindings/iio/dac/adi,ad5696.yaml    |  60 +-
 drivers/iio/dac/Kconfig                            |  18 +-
 drivers/iio/dac/ad5686-spi.c                       | 200 ++++--
 drivers/iio/dac/ad5686.c                           | 775 +++++++++++++--------
 drivers/iio/dac/ad5686.h                           | 144 ++--
 drivers/iio/dac/ad5696-i2c.c                       |  94 +--
 7 files changed, 906 insertions(+), 446 deletions(-)
---
base-commit: eade2b843d9b1f668fc1775f15611bb0a1999cd9
change-id: 20260325-ad5313r-iio-support-882ad39356e1

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

Re: [PATCH 00/22] Extend device support for AD5686 driver

Posted by Krzysztof Kozlowski 1 month, 3 weeks ago

On Wed, Apr 22, 2026 at 03:45:34PM +0100, Rodrigo Alencar wrote:
> This series adds support for multiple nanoDAC parts, adding triggered
> buffer and gain control support to the ad5686 DAC driver family, along
> with a number of driver cleanups and fixes.
> 
> Initial patches update the device-tree bindings:
> - Add compatible entries for missing and new parts;
> - Add GPIO properties for RESET, GAIN and LDAC pins;
> - Add missing power supplies properties.

It's merge window and you just sent 22 big series targetting one
subsystem. Avoid doing that.

Best regards,
Krzysztof

Re: [PATCH 00/22] Extend device support for AD5686 driver

Posted by Andy Shevchenko 1 month, 3 weeks ago

On Wed, Apr 22, 2026 at 03:45:34PM +0100, Rodrigo Alencar via B4 Relay wrote:
> This series adds support for multiple nanoDAC parts, adding triggered
> buffer and gain control support to the ad5686 DAC driver family, along
> with a number of driver cleanups and fixes.
> 
> Initial patches update the device-tree bindings:
> - Add compatible entries for missing and new parts;
> - Add GPIO properties for RESET, GAIN and LDAC pins;
> - Add missing power supplies properties.

> Driver cleanups and fixes:
> - Refactor include headers (IWYU);
> - Switch to device managed mutex initialization;
> - Drop enum chip id in favor of per-device chip_info structs;
> - Fix voltage reference control on single-channel devices;
> - Fix powerdown control on dual-channel devices;
> - Introduce bus ops struct with a sync() operation for batching
>   bus transfers.
> 
> New functionality:
> - Device support for: AD5316R, AD5675, AD5697R, AD5313R, AD5317R,
>   AD5674, AD5679, AD5687, AD5687R, AD5689 and AD5689R;
> - Consume optional reset and new power supplies;
> - LDAC GPIO handling (active-low, held low when unused);
> - SPI bus sync() implementation for batching multiple transfers;
> - Triggered buffer support, leveraging LDAC and sync() to flush
>   all channel writes atomically;
> - Gain control support through the scale property.

This is rather long series. Please, start from the fixes series first that is
independent on the features.

I see here ~3 sequential series. Can we rather do them this way?

Personally I stopped reviewing on patch 12 (without even opening DT stuff)
because it's exhaustive. Documentation usually suggests the series to be
limited by ~15 patches IIRC.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/22] Extend device support for AD5686 driver

Posted by Jonathan Cameron 1 month, 3 weeks ago

On Wed, 22 Apr 2026 23:28:00 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Apr 22, 2026 at 03:45:34PM +0100, Rodrigo Alencar via B4 Relay wrote:
> > This series adds support for multiple nanoDAC parts, adding triggered
> > buffer and gain control support to the ad5686 DAC driver family, along
> > with a number of driver cleanups and fixes.
> > 
> > Initial patches update the device-tree bindings:
> > - Add compatible entries for missing and new parts;
> > - Add GPIO properties for RESET, GAIN and LDAC pins;
> > - Add missing power supplies properties.  
> 
> > Driver cleanups and fixes:
> > - Refactor include headers (IWYU);
> > - Switch to device managed mutex initialization;
> > - Drop enum chip id in favor of per-device chip_info structs;
> > - Fix voltage reference control on single-channel devices;
> > - Fix powerdown control on dual-channel devices;
> > - Introduce bus ops struct with a sync() operation for batching
> >   bus transfers.
> > 
> > New functionality:
> > - Device support for: AD5316R, AD5675, AD5697R, AD5313R, AD5317R,
> >   AD5674, AD5679, AD5687, AD5687R, AD5689 and AD5689R;
> > - Consume optional reset and new power supplies;
> > - LDAC GPIO handling (active-low, held low when unused);
> > - SPI bus sync() implementation for batching multiple transfers;
> > - Triggered buffer support, leveraging LDAC and sync() to flush
> >   all channel writes atomically;
> > - Gain control support through the scale property.  
> 
> This is rather long series. Please, start from the fixes series first that is
> independent on the features.
> 
> I see here ~3 sequential series. Can we rather do them this way?
> 
> Personally I stopped reviewing on patch 12 (without even opening DT stuff)
> because it's exhaustive. Documentation usually suggests the series to be
> limited by ~15 patches IIRC.
> 
On plus side this one was easier to review than the RFC that Rodrigo
has outstanding so I reviewed this one instead :)

Better split up though as Andy suggests. I'm less bothered than some
about merge window timing, but a set that does 3 different types of
things is never a good thing even if they are all on one driver.

Thanks,

Jonathan