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

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

On Fri, Apr 17, 2026 at 01:56:24PM +0100, Jonathan Cameron wrote:
> On Fri, 17 Apr 2026 11:35:12 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Fri, Apr 17, 2026 at 04:27:16PM +0800, Alexis Czezar Torreno wrote:

...

> > > +#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)

While that's technically correct, I would still prefer an explicit number

#define AD5705_DAC_MAX_CODE	(BIT(16) - 1)

Note, GENMASK() is also fine with me, this is really a minor difference.

> I might tweak it when picking this up.

-- 
With Best Regards,
Andy Shevchenko