Re: [PATCH v2 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532

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

On Thu, Jun 25, 2026 at 10:06:26AM +0000, Paller, Kim Seer wrote:
> > From: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Sent: Monday, June 15, 2026 6:06 PM
> > On Mon, Jun 15, 2026 at 02:20:18PM +0800, Kim Seer Paller wrote:

...

> > > +	local_ch = chan->channel % AD3530R_CH_PER_BANK;
> > 
> > > +	reg = (chan->channel < AD3530R_CH_PER_BANK ?
> > AD3532R_OUTPUT_OPERATING_MODE_0 :
> > > +	       AD3532R_OUTPUT_OPERATING_MODE_2) +
> > > +	       local_ch / AD3530R_CH_PER_REG;
> > 
> > This is unreadable. Can you refactor it?
> 
> Would this be clearer?
> 
> unsigned int bank_base;
> 
> local_ch = chan->channel % AD3530R_CH_PER_BANK;
> bank_base = chan->channel < AD3530R_CH_PER_BANK ?
> 	    AD3532R_OUTPUT_OPERATING_MODE_0 : AD3532R_OUTPUT_OPERATING_MODE_2;
> reg = bank_base + local_ch / AD3530R_CH_PER_REG;

No. Too much voodoo arithmetics and comparisons.

> > > +	mask = AD3530R_OP_MODE_CHAN_MSK(local_ch %
> > AD3530R_CH_PER_REG);

Including this one.
I would expect to see maybe two more variables to hold
local_ch % BANK and local_ch / BANK.

-- 
With Best Regards,
Andy Shevchenko