Re: [PATCH v2 09/11] iio: dac: ad5686: add control_sync() for single-channel devices

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 12:30:16PM +0100, Rodrigo Alencar via B4 Relay wrote:

> Create ad5310_control_sync() and ad5683_control_sync() functions that
> properly consumes 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. 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.

...

>  #define AD5310_REF_BIT_MSK			BIT(8)
> +#define AD5310_PD_MSK				GENMASK(10, 9)
>  #define AD5683_REF_BIT_MSK			BIT(12)
> +#define AD5683_PD_MSK				GENMASK(14, 13)
>  #define AD5686_REF_BIT_MSK			BIT(0)

Since now it's a set of groups of the masks, I would add blank lines to make it
clearer (from the first glance I failed to see that, then I have noticed that
there are the same names, but prefixes, made me harder to understand).

#define AD5310_REF_BIT_MSK			BIT(8)
#define AD5310_PD_MSK				GENMASK(10, 9)

#define AD5683_REF_BIT_MSK			BIT(12)
#define AD5683_PD_MSK				GENMASK(14, 13)

#define AD5686_REF_BIT_MSK			BIT(0)

-- 
With Best Regards,
Andy Shevchenko