Add support for setting the Programmable Gain Amplifiers by adjust the
scale value.
Signed-off-by: Marcus Folkesson <[email protected]>
---
drivers/iio/adc/mcp3911.c | 135 +++++++++++++++++++++++++++++++-------
1 file changed, 110 insertions(+), 25 deletions(-)
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index 084f6d1aa6d1..631015ead8e7 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -26,6 +26,8 @@
#define MCP3911_REG_MOD 0x06
#define MCP3911_REG_PHASE 0x07
#define MCP3911_REG_GAIN 0x09
+#define MCP3911_GAIN_MASK(ch) (0x7 << 3*ch)
+#define MCP3911_GAIN_VAL(ch, val) ((val << 3*ch) & MCP3911_GAIN_MASK(ch))
#define MCP3911_REG_STATUSCOM 0x0a
#define MCP3911_STATUSCOM_DRHIZ BIT(12)
@@ -55,8 +57,10 @@
#define MCP3911_REG_WRITE(reg, id) ((((reg) << 1) | ((id) << 5) | (0 << 0)) & 0xff)
#define MCP3911_NUM_CHANNELS 2
+#define MCP3911_NUM_SCALES 6
static const int mcp3911_osr_table[] = {32, 64, 128, 256, 512, 1024, 2048, 4096};
+static u32 mcp3911_scale_table[MCP3911_NUM_SCALES][2];
struct mcp3911 {
struct spi_device *spi;
@@ -65,6 +69,7 @@ struct mcp3911 {
struct clk *clki;
u32 dev_addr;
struct iio_trigger *trig;
+ u32 gain[MCP3911_NUM_CHANNELS];
struct {
u32 channels[2];
s64 ts __aligned(8);
@@ -113,7 +118,35 @@ static int mcp3911_update(struct mcp3911 *adc, u8 reg, u32 mask,
return mcp3911_write(adc, reg, val, len);
}
+static int mcp3911_get_gain(struct mcp3911 *adc, u8 channel, u32 *val)
+{
+ int ret = mcp3911_read(adc, MCP3911_REG_GAIN, val, 1);
+
+ if (ret)
+ return ret;
+ *val >>= channel * 3;
+ *val &= 0x07;
+ *val = (1 << *val);
+
+ return 0;
+}
+
+static int mcp3911_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_PHASE:
+ return IIO_VAL_INT;
+ default:
+ return IIO_VAL_INT_PLUS_NANO;
+ }
+}
static int mcp3911_read_avail(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
const int **vals, int *type, int *length,
@@ -125,6 +158,11 @@ static int mcp3911_read_avail(struct iio_dev *indio_dev,
*vals = mcp3911_osr_table;
*length = ARRAY_SIZE(mcp3911_osr_table);
return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_SCALE:
+ *type = IIO_VAL_INT_PLUS_NANO;
+ *vals = (int *)mcp3911_scale_table;
+ *length = ARRAY_SIZE(mcp3911_scale_table) * 2;
+ return IIO_AVAIL_LIST;
default:
return -EINVAL;
}
@@ -180,29 +218,9 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev,
break;
case IIO_CHAN_INFO_SCALE:
- if (adc->vref) {
- ret = regulator_get_voltage(adc->vref);
- if (ret < 0) {
- dev_err(indio_dev->dev.parent,
- "failed to get vref voltage: %d\n",
- ret);
- goto out;
- }
-
- *val = ret / 1000;
- } else {
- *val = MCP3911_INT_VREF_MV;
- }
-
- /*
- * For 24bit Conversion
- * Raw = ((Voltage)/(Vref) * 2^23 * Gain * 1.5
- * Voltage = Raw * (Vref)/(2^23 * Gain * 1.5)
- */
-
- /* val2 = (2^23 * 1.5) */
- *val2 = 12582912;
- ret = IIO_VAL_FRACTIONAL;
+ *val = mcp3911_scale_table[ilog2(adc->gain[channel->channel])][0];
+ *val2 = mcp3911_scale_table[ilog2(adc->gain[channel->channel])][1];
+ ret = IIO_VAL_INT_PLUS_NANO;
break;
}
@@ -220,6 +238,19 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev,
mutex_lock(&adc->lock);
switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ for (int i = 0; i < MCP3911_NUM_SCALES; i++) {
+ if (val == mcp3911_scale_table[i][0] &&
+ val2 == mcp3911_scale_table[i][1]) {
+
+ adc->gain[channel->channel] = BIT(i);
+ ret = mcp3911_update(adc, MCP3911_REG_GAIN,
+ MCP3911_GAIN_MASK(channel->channel),
+ MCP3911_GAIN_VAL(channel->channel,
+ i), 1);
+ }
+ }
+ break;
case IIO_CHAN_INFO_OFFSET:
if (val2 != 0) {
ret = -EINVAL;
@@ -290,6 +321,49 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev,
return ret;
}
+static int mcp3911_calc_scale_table(struct mcp3911 *adc)
+{
+ u32 ref = MCP3911_INT_VREF_MV;
+ u32 div;
+ int ret = 0;
+ 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;
+ }
+
+ 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:
+ return ret;
+}
+
#define MCP3911_CHAN(idx) { \
.type = IIO_VOLTAGE, \
.indexed = 1, \
@@ -302,8 +376,10 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
BIT(IIO_CHAN_INFO_OFFSET) | \
BIT(IIO_CHAN_INFO_SCALE), \
- .info_mask_shared_by_type_available = \
- BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ .info_mask_shared_by_type_available = \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) \
+ .info_mask_separate_available = \
+ BIT(IIO_CHAN_INFO_SCALE), \
.scan_type = { \
.sign = 's', \
.realbits = 24, \
@@ -356,6 +432,7 @@ static const struct iio_info mcp3911_info = {
.read_raw = mcp3911_read_raw,
.write_raw = mcp3911_write_raw,
.read_avail = mcp3911_read_avail,
+ .write_raw_get_fmt = mcp3911_write_raw_get_fmt,
};
static irqreturn_t mcp3911_interrupt(int irq, void *dev_id)
@@ -482,6 +559,14 @@ static int mcp3911_probe(struct spi_device *spi)
if (ret)
goto clk_disable;
+ ret = mcp3911_calc_scale_table(adc);
+ if (ret)
+ goto clk_disable;
+
+ /* Store gain values to better calculate scale values */
+ mcp3911_get_gain(adc, 0, &adc->gain[0]);
+ mcp3911_get_gain(adc, 1, &adc->gain[1]);
+
dr_hiz = device_property_read_bool(&adc->spi->dev, "microchip,data-ready-hiz");
if (dr_hiz)
ret = mcp3911_update(adc, MCP3911_REG_STATUSCOM, MCP3911_STATUSCOM_DRHIZ,
--
2.36.1
On Thu, Jun 23, 2022 at 7:41 PM Marcus Folkesson
<[email protected]> 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
On Thu, Jun 23, 2022 at 09:15:51PM +0200, Andy Shevchenko wrote:
> On Thu, Jun 23, 2022 at 7:41 PM Marcus Folkesson
> <[email protected]> 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