On Thu, Jun 23, 2022 at 7:41 PM Marcus Folkesson <marcus.folkesson@xxxxxxxxx> 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) ... > + *val >>= channel * 3; > + *val &= 0x07; GENMASK() ? > + *val = (1 << *val); Unneeded parentheses, perhaps 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. ... > +static int mcp3911_calc_scale_table(struct mcp3911 *adc) > +{ > + u32 ref = MCP3911_INT_VREF_MV; > + u32 div; > + int ret = 0; Useless assignment. > + int tmp0, tmp1; > + s64 tmp2; > + > + if (adc->vref) { > + ret = regulator_get_voltage(adc->vref); > + if (ret < 0) { > + dev_err(&adc->spi->dev, > + "failed to get vref voltage: %d\n", > + ret); > + goto out; Return directly. > + } > + > + ref = ret / 1000; > + } > + > + /* > + * For 24bit Conversion > + * Raw = ((Voltage)/(Vref) * 2^23 * Gain * 1.5 > + * Voltage = Raw * (Vref)/(2^23 * Gain * 1.5) > + */ > + > + /* ref = Reference voltage > + * div = (2^23 * 1.5 * gain) = 12582912 * gain > + */ > + for (int i = 0; i < MCP3911_NUM_SCALES; i++) { > + div = 12582912 * BIT(i); > + tmp2 = div_s64((s64)ref * 1000000000LL, div); > + tmp1 = div; > + tmp0 = (int)div_s64_rem(tmp2, 1000000000, &tmp1); > + > + mcp3911_scale_table[i][0] = 0; > + mcp3911_scale_table[i][1] = tmp1; > + } > +out: The useless label. > + return ret; return 0; > +} -- With Best Regards, Andy Shevchenko