Re: [PATCH v8 2/2] iio: dac: ad5706r: Add support for AD5706R DAC

[Jonathan Cameron]

On Fri, 17 Apr 2026 11:35:12 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:

> On Fri, Apr 17, 2026 at 04:27:16PM +0800, Alexis Czezar Torreno wrote:
> > Add support for the Analog Devices AD5706R, a 4-channel 16-bit
> > current output digital-to-analog converter with SPI interface.
> > 
> > Features:
> >   - 4 independent DAC channels
> >   - Hardware and software LDAC trigger
> >   - Configurable output range
> >   - PWM-based LDAC control
> >   - Dither and toggle modes
> >   - Dynamically configurable SPI speed  
> 
> ...
> 
> > +#define AD5706R_DAC_RESOLUTION		16
> > +#define AD5706R_DAC_MAX_CODE		GENMASK(15, 0)  
> 
> I know Jonathan asked for this, hence it's comment for him.
> I think that BIT() notation in a form of (BIT(16) - 1) is
> also appropriate here as it gives the relationship to the
> resolution of the given register / bitfield in HW.
> 
> GENMASK() works for me, but it might require an additional
> operation to deduce the above.
> 
> (Note, there is no request to change or resend for you, Alexis. It's just
>  a remark to make Jonathan to think about which one suits better. He might
>  change that whilst applying.)
>
I'm not against that form.  It was more being against bare BIT(16) as that was
1 greater than the maximum value it can take.
However making the relationship explicit would be even better.

#define AD5705_DAC_MAX_CODE	(BIT(AD5706R_DAC_RESOLUTION) - 1)

I might tweak it when picking this up.