Re: [PATCH 10/10] iio: adc: mcp3911: add support to set PGA

From: Andy Shevchenko
Date: Thu Jun 23 2022 - 15:35:10 EST


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