2022-07-04 17:36:09

by Marcus Folkesson

[permalink] [raw]
Subject: [PATCH v3 0/9] Improve MCP3911 driver

Hi,

This patch series intend to fix bugs and add functionality to the
MCP3911 driver.
The main features added are
- Support for buffers
- Interrupt driven readings
- Support for oversampling ratio
- Support for set scale values (Gain)

Among the bug fixes, there are changes in the formula for calculate raw
value and a fix for mismatch in the devicetree property.

Another general improvement for the driver is to use managed resources
for all allocated resources.

See patch notes for more specific changes between versions.

General changes for the series:

v3:
- Drop Phase patch
- Add Fixes tags for those patches that are fixes
- Move Fixes patches to the beginning of the patchset


Best regards,
Marcus Folkesson



2022-07-04 17:36:35

by Marcus Folkesson

[permalink] [raw]
Subject: [PATCH v3 5/9] iio: adc: mcp3911: add support for buffers

Add support for buffers to make the driver fit for more usecases.

Signed-off-by: Marcus Folkesson <[email protected]>
---
drivers/iio/adc/mcp3911.c | 99 ++++++++++++++++++++++++++++++++++++---
1 file changed, 92 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index f5fd9da6ab55..cf4dacc7de06 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -5,10 +5,16 @@
* Copyright (C) 2018 Marcus Folkesson <[email protected]>
* Copyright (C) 2018 Kent Gustavsson <[email protected]>
*/
+#include <linux/bitfield.h>
+#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/trigger.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/property.h>
@@ -22,6 +28,7 @@
#define MCP3911_REG_GAIN 0x09

#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_READ GENMASK(7, 6)
#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
#define MCP3911_STATUSCOM_EN_OFFCAL BIT(2)
@@ -54,6 +61,13 @@ struct mcp3911 {
struct regulator *vref;
struct clk *clki;
u32 dev_addr;
+ struct {
+ u32 channels[MCP3911_NUM_CHANNELS];
+ s64 ts __aligned(8);
+ } scan;
+
+ u8 tx_buf[MCP3911_NUM_CHANNELS * 3] __aligned(IIO_DMA_MINALIGN);
+ u8 rx_buf[MCP3911_NUM_CHANNELS * 3];
};

static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
@@ -196,16 +210,66 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev,
.type = IIO_VOLTAGE, \
.indexed = 1, \
.channel = idx, \
+ .scan_index = idx, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
BIT(IIO_CHAN_INFO_OFFSET) | \
BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = 24, \
+ .storagebits = 32, \
+ }, \
}

static const struct iio_chan_spec mcp3911_channels[] = {
MCP3911_CHAN(0),
MCP3911_CHAN(1),
+ IIO_CHAN_SOFT_TIMESTAMP(2),
};

+static irqreturn_t mcp3911_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct mcp3911 *adc = iio_priv(indio_dev);
+ struct spi_transfer xfer = {
+ .tx_buf = adc->tx_buf,
+ .rx_buf = adc->rx_buf,
+ .len = sizeof(adc->rx_buf),
+ };
+ int scan_index;
+ int i = 0;
+ int ret;
+
+ mutex_lock(&adc->lock);
+ adc->tx_buf[0] = MCP3911_REG_READ(MCP3911_CHANNEL(0), adc->dev_addr);
+ ret = spi_sync_transfer(adc->spi, &xfer, 1);
+ if (ret < 0) {
+ dev_warn(&adc->spi->dev,
+ "failed to get conversion data\n");
+ goto out;
+ }
+
+ for_each_set_bit(scan_index, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+ const struct iio_chan_spec *scan_chan =
+ &indio_dev->channels[scan_index];
+
+ adc->scan.channels[i] = adc->rx_buf[scan_chan->channel * 3 + 0] << 16 |
+ adc->rx_buf[scan_chan->channel * 3 + 1] << 8 |
+ adc->rx_buf[scan_chan->channel * 3 + 2] << 0;
+
+ i++;
+ }
+ iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan,
+ iio_get_time_ns(indio_dev));
+out:
+ mutex_unlock(&adc->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
static const struct iio_info mcp3911_info = {
.read_raw = mcp3911_read_raw,
.write_raw = mcp3911_write_raw,
@@ -214,7 +278,7 @@ static const struct iio_info mcp3911_info = {
static int mcp3911_config(struct mcp3911 *adc)
{
struct device *dev = &adc->spi->dev;
- u32 configreg;
+ u32 regval;
int ret;

ret = device_property_read_u32(dev, "microchip,device-addr", &adc->dev_addr);
@@ -233,29 +297,43 @@ static int mcp3911_config(struct mcp3911 *adc)
}
dev_dbg(&adc->spi->dev, "use device address %i\n", adc->dev_addr);

- ret = mcp3911_read(adc, MCP3911_REG_CONFIG, &configreg, 2);
+ ret = mcp3911_read(adc, MCP3911_REG_CONFIG, &regval, 2);
if (ret)
return ret;

+ regval &= ~MCP3911_CONFIG_VREFEXT;
if (adc->vref) {
dev_dbg(&adc->spi->dev, "use external voltage reference\n");
- configreg |= MCP3911_CONFIG_VREFEXT;
+ regval |= FIELD_PREP(MCP3911_CONFIG_VREFEXT, 1);
} else {
dev_dbg(&adc->spi->dev,
"use internal voltage reference (1.2V)\n");
- configreg &= ~MCP3911_CONFIG_VREFEXT;
+ regval |= FIELD_PREP(MCP3911_CONFIG_VREFEXT, 0);
}

+ regval &= ~MCP3911_CONFIG_CLKEXT;
if (adc->clki) {
dev_dbg(&adc->spi->dev, "use external clock as clocksource\n");
- configreg |= MCP3911_CONFIG_CLKEXT;
+ regval |= FIELD_PREP(MCP3911_CONFIG_CLKEXT, 1);
} else {
dev_dbg(&adc->spi->dev,
"use crystal oscillator as clocksource\n");
- configreg &= ~MCP3911_CONFIG_CLKEXT;
+ regval |= FIELD_PREP(MCP3911_CONFIG_CLKEXT, 0);
}

- return mcp3911_write(adc, MCP3911_REG_CONFIG, configreg, 2);
+ ret = mcp3911_write(adc, MCP3911_REG_CONFIG, regval, 2);
+ if (ret)
+ return ret;
+
+ ret = mcp3911_read(adc, MCP3911_REG_STATUSCOM, &regval, 2);
+ if (ret)
+ return ret;
+
+ /* Address counter incremented, cycle through register types */
+ regval &= ~MCP3911_STATUSCOM_READ;
+ regval |= FIELD_PREP(MCP3911_STATUSCOM_READ, 0x02);
+
+ return mcp3911_write(adc, MCP3911_REG_STATUSCOM, regval, 2);
}

static void mcp3911_cleanup(void *_adc)
@@ -325,6 +403,7 @@ static int mcp3911_probe(struct spi_device *spi)
if (ret)
return ret;

+
indio_dev->name = spi_get_device_id(spi)->name;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &mcp3911_info;
@@ -335,6 +414,12 @@ static int mcp3911_probe(struct spi_device *spi)

mutex_init(&adc->lock);

+ ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+ NULL,
+ mcp3911_trigger_handler, NULL);
+ if (ret)
+ return ret;
+
return devm_iio_device_register(&adc->spi->dev, indio_dev);
}

--
2.36.1

2022-07-04 22:05:58

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 5/9] iio: adc: mcp3911: add support for buffers

On Mon, Jul 4, 2022 at 7:20 PM Marcus Folkesson
<[email protected]> wrote:
>
> Add support for buffers to make the driver fit for more usecases.

use cases

...

> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + const struct iio_chan_spec *scan_chan =
> + &indio_dev->channels[scan_index];
> +
> + adc->scan.channels[i] = adc->rx_buf[scan_chan->channel * 3 + 0] << 16 |
> + adc->rx_buf[scan_chan->channel * 3 + 1] << 8 |
> + adc->rx_buf[scan_chan->channel * 3 + 2] << 0;

get_unaligned_be24()

> + i++;
> + }

...

> @@ -325,6 +403,7 @@ static int mcp3911_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> +
> indio_dev->name = spi_get_device_id(spi)->name;
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->info = &mcp3911_info;

Stray change.

--
With Best Regards,
Andy Shevchenko

2022-07-04 22:53:25

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] Improve MCP3911 driver

On Mon, Jul 4, 2022 at 7:20 PM Marcus Folkesson
<[email protected]> wrote:
>
> Hi,
>
> This patch series intend to fix bugs and add functionality to the
> MCP3911 driver.
> The main features added are
> - Support for buffers
> - Interrupt driven readings
> - Support for oversampling ratio
> - Support for set scale values (Gain)
>
> Among the bug fixes, there are changes in the formula for calculate raw
> value and a fix for mismatch in the devicetree property.
>
> Another general improvement for the driver is to use managed resources
> for all allocated resources.
>
> See patch notes for more specific changes between versions.
>
> General changes for the series:

For non-commented patches (excluding patch 7!)
Reviewed-by: Andy Shevchenko <[email protected]>

For commented ones, you may take the tag if you are going to address
them as suggested.

> v3:
> - Drop Phase patch
> - Add Fixes tags for those patches that are fixes
> - Move Fixes patches to the beginning of the patchset

--
With Best Regards,
Andy Shevchenko