Re: [PATCH v2 4/4] iio: dac: ad3530r: Add support for AD3532R/AD3532
From: Andy Shevchenko
Date: Fri Jun 26 2026 - 06:11:41 EST
On Thu, Jun 25, 2026 at 10:06:26AM +0000, Paller, Kim Seer wrote:
> > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> > 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