Re: [PATCH v5 10/12] iio: dac: ad5686: add control_sync() for single-channel devices

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Fri, May 01, 2026 at 10:15:03AM +0100, Rodrigo Alencar via B4 Relay wrote:

> Create ad5310_control_sync() and ad5683_control_sync() functions that
> properly consume the mask definitions with FIELD_PREP(). This allows to
> reuse a function that updates the control register with cached values,
> without relying on confusing logic that depends on st->use_internal_vref,
> which is initialized earlier in ad5686_probe() because it is also
> applicable to the AD5686_REGMAP case, removing the need for the
> has_external_vref. Powerdown masks initialization is simplified as
> *_control_sync() masks outs any unused bits for the single-channel case.
> The change cleans up ad5686_write_dac_powerdown() and ad5686_probe(),
> organizing the code for feature extension, e.g. gain control support for
> single-channel devices.

...

> +	/* Initialize masks to all ones */
> +	st->pwr_down_mask = ~0U;
> +	st->pwr_down_mode = ~0U;

Thanks for this change, but be careful of what is done here. If ever the
pwr_down_* become longer than 32-bit value, this will set a wrong data.
The rule of thumb: If we want to set *all* bits to one, use ~0, the compiler
will take care of the rest.

-- 
With Best Regards,
Andy Shevchenko