Messages in this thread | | | Date | Fri, 24 Jun 2022 14:05:08 +0200 | From | Marcus Folkesson <> | Subject | Re: [PATCH 10/10] iio: adc: mcp3911: add support to set PGA |
| |
On Thu, Jun 23, 2022 at 09:15:51PM +0200, Andy Shevchenko wrote: > On Thu, Jun 23, 2022 at 7:41 PM Marcus Folkesson > <marcus.folkesson@gmail.com> wrote: > > > > Add support for setting the Programmable Gain Amplifiers by adjust the > > scale value. > > ... > > > + int ret = mcp3911_read(adc, MCP3911_REG_GAIN, val, 1); > > + > > + if (ret) > > + return ret; > > Please split the assignment. > > int ret; > > ret = ... > if (ret)
OK
> > > ... > > > + *val >>= channel * 3; > > + *val &= 0x07; > > GENMASK() ?
I will use GENMASK > > > + *val = (1 << *val); > > Unneeded parentheses, perhaps BIT()?
I will switch to BIT() > > ... > > > + ret = mcp3911_update(adc, MCP3911_REG_GAIN, > > + MCP3911_GAIN_MASK(channel->channel), > > + MCP3911_GAIN_VAL(channel->channel, > > + i), 1); > > This is not good indentation, at least i), should be on the previous line.
Agree
> > ... > > > +static int mcp3911_calc_scale_table(struct mcp3911 *adc) > > +{ > > + u32 ref = MCP3911_INT_VREF_MV; > > + u32 div; > > > + int ret = 0; > > Useless assignment.
Agree > > > Return directly.
OK > > > The useless label.
Agree > > > + return ret; >
Thanks, Marcus Folkesson [unhandled content-type:application/pgp-signature] | |