[PATCH v6 0/9] Improve MCP3911 driver

[PATCH v6 0/9] Improve MCP3911 driver

[Marcus Folkesson]

This patch series intend to fix bugs and improve functionality of 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.

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

v4:
- Split up devm-cleanup functions 
- Cosmetic cleanups
- Add
	select IIO_BUFFER
	select IIO_TRIGGERED_BUFFER
    To Kconfig
- Add .endianness = IIO_BE

v5:
- Drop remove function
- Split tx&rx transfers in mcp3911_trigger_handler()
- Moved Kconfig changes to right patch

v6:
- Go for devm_clk_get_enabled()
- Cosmetic cleanups
- Clarify the description of microchip,data-ready-hiz


Best regards,
Marcus Folkesson

[PATCH v6 1/9] iio: adc: mcp3911: make use of the sign bit

[Marcus Folkesson]

The device supports negative values as well.

Fixes: 3a89b289df5d ("iio: adc: add support for mcp3911")
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/mcp3911.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index 1cb4590fe412..f581cefb6719 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -113,6 +113,8 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			goto out;
 
+		*val = sign_extend32(*val, 23);
+
 		ret = IIO_VAL_INT;
 		break;
 
-- 
2.37.1

[PATCH v6 2/9] iio: adc: mcp3911: correct "microchip,device-addr" property

[Marcus Folkesson]

Go for the right property name that is documented in the bindings.

Fixes: 3a89b289df5d ("iio: adc: add support for mcp3911")
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/mcp3911.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index f581cefb6719..f8875076ae80 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -210,7 +210,14 @@ static int mcp3911_config(struct mcp3911 *adc)
 	u32 configreg;
 	int ret;
 
-	device_property_read_u32(dev, "device-addr", &adc->dev_addr);
+	ret = device_property_read_u32(dev, "microchip,device-addr", &adc->dev_addr);
+
+	/*
+	 * Fallback to "device-addr" due to historical mismatch between
+	 * dt-bindings and implementation
+	 */
+	if (ret)
+		device_property_read_u32(dev, "device-addr", &adc->dev_addr);
 	if (adc->dev_addr > 3) {
 		dev_err(&adc->spi->dev,
 			"invalid device address (%i). Must be in range 0-3.\n",
-- 
2.37.1

[PATCH v6 3/9] iio: adc: mcp3911: use correct formula for AD conversion

[Marcus Folkesson]

The ADC conversion is actually not rail-to-rail but with a factor 1.5.
Make use of this factor when calculating actual voltage.

Fixes: 3a89b289df5d ("iio: adc: add support for mcp3911")
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/mcp3911.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index f8875076ae80..890af7dca62d 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -40,8 +40,8 @@
 #define MCP3911_CHANNEL(x)		(MCP3911_REG_CHANNEL0 + x * 3)
 #define MCP3911_OFFCAL(x)		(MCP3911_REG_OFFCAL_CH0 + x * 6)
 
-/* Internal voltage reference in uV */
-#define MCP3911_INT_VREF_UV		1200000
+/* Internal voltage reference in mV */
+#define MCP3911_INT_VREF_MV		1200
 
 #define MCP3911_REG_READ(reg, id)	((((reg) << 1) | ((id) << 5) | (1 << 0)) & 0xff)
 #define MCP3911_REG_WRITE(reg, id)	((((reg) << 1) | ((id) << 5) | (0 << 0)) & 0xff)
@@ -139,11 +139,18 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev,
 
 			*val = ret / 1000;
 		} else {
-			*val = MCP3911_INT_VREF_UV;
+			*val = MCP3911_INT_VREF_MV;
 		}
 
-		*val2 = 24;
-		ret = IIO_VAL_FRACTIONAL_LOG2;
+		/*
+		 * 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;
 		break;
 	}
 
-- 
2.37.1

[PATCH v6 4/9] iio: adc: mcp3911: use resource-managed version of iio_device_register

[Marcus Folkesson]

Keep using managed resources as much as possible.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/mcp3911.c | 53 ++++++++++++---------------------------
 1 file changed, 16 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index 890af7dca62d..7e2efe702e57 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -258,6 +258,13 @@ static int mcp3911_config(struct mcp3911 *adc)
 	return  mcp3911_write(adc, MCP3911_REG_CONFIG, configreg, 2);
 }
 
+static void mcp3911_cleanup_regulator(void *_adc)
+{
+	struct mcp3911 *adc = _adc;
+
+	regulator_disable(adc->vref);
+}
+
 static int mcp3911_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
@@ -286,9 +293,14 @@ static int mcp3911_probe(struct spi_device *spi)
 		ret = regulator_enable(adc->vref);
 		if (ret)
 			return ret;
+
+		ret = devm_add_action_or_reset(&spi->dev,
+				mcp3911_cleanup_regulator, adc);
+		if (ret)
+			return ret;
 	}
 
-	adc->clki = devm_clk_get(&adc->spi->dev, NULL);
+	adc->clki = devm_clk_get_enabled(&adc->spi->dev, NULL);
 	if (IS_ERR(adc->clki)) {
 		if (PTR_ERR(adc->clki) == -ENOENT) {
 			adc->clki = NULL;
@@ -296,21 +308,13 @@ static int mcp3911_probe(struct spi_device *spi)
 			dev_err(&adc->spi->dev,
 				"failed to get adc clk (%ld)\n",
 				PTR_ERR(adc->clki));
-			ret = PTR_ERR(adc->clki);
-			goto reg_disable;
-		}
-	} else {
-		ret = clk_prepare_enable(adc->clki);
-		if (ret < 0) {
-			dev_err(&adc->spi->dev,
-				"Failed to enable clki: %d\n", ret);
-			goto reg_disable;
+			return PTR_ERR(adc->clki);
 		}
 	}
 
 	ret = mcp3911_config(adc);
 	if (ret)
-		goto clk_disable;
+		return ret;
 
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
@@ -322,31 +326,7 @@ static int mcp3911_probe(struct spi_device *spi)
 
 	mutex_init(&adc->lock);
 
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto clk_disable;
-
-	return ret;
-
-clk_disable:
-	clk_disable_unprepare(adc->clki);
-reg_disable:
-	if (adc->vref)
-		regulator_disable(adc->vref);
-
-	return ret;
-}
-
-static void mcp3911_remove(struct spi_device *spi)
-{
-	struct iio_dev *indio_dev = spi_get_drvdata(spi);
-	struct mcp3911 *adc = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-
-	clk_disable_unprepare(adc->clki);
-	if (adc->vref)
-		regulator_disable(adc->vref);
+	return devm_iio_device_register(&adc->spi->dev, indio_dev);
 }
 
 static const struct of_device_id mcp3911_dt_ids[] = {
@@ -367,7 +347,6 @@ static struct spi_driver mcp3911_driver = {
 		.of_match_table = mcp3911_dt_ids,
 	},
 	.probe = mcp3911_probe,
-	.remove = mcp3911_remove,
 	.id_table = mcp3911_id,
 };
 module_spi_driver(mcp3911_driver);
-- 
2.37.1

[PATCH v6 5/9] iio: adc: mcp3911: add support for buffers

[Marcus Folkesson]

Add support for buffers to make the driver fit for more use cases.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/Kconfig   |   2 +
 drivers/iio/adc/mcp3911.c | 103 +++++++++++++++++++++++++++++++++++---
 2 files changed, 97 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7fe5930891e0..3b5db59f3931 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -718,6 +718,8 @@ config MCP3422
 config MCP3911
 	tristate "Microchip Technology MCP3911 driver"
 	depends on SPI
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for Microchip Technology's MCP3911
 	  analog to digital converter.
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index 7e2efe702e57..4452b01c76f3 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -5,16 +5,25 @@
  * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@gmail.com>
  * Copyright (C) 2018 Kent Gustavsson <kent@minoris.se>
  */
+#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/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.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 <asm/unaligned.h>
+
 #define MCP3911_REG_CHANNEL0		0x00
 #define MCP3911_REG_CHANNEL1		0x03
 #define MCP3911_REG_MOD			0x06
@@ -22,6 +31,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 +64,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 __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 +213,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,			\
+			.endianness = IIO_BE,			\
+		},						\
 }
 
 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,
+			.len = 1,
+		}, {
+			.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 = MCP3911_REG_READ(MCP3911_CHANNEL(0), adc->dev_addr);
+	ret = spi_sync_transfer(adc->spi, xfer, ARRAY_SIZE(xfer));
+	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] = get_unaligned_be24(&adc->rx_buf[scan_chan->channel * 3]);
+		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 +281,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 +300,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_regulator(void *_adc)
@@ -326,6 +407,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.37.1

[PATCH v6 6/9] iio: adc: mcp3911: add support for interrupts

[Marcus Folkesson]

Make it possible to read values upon interrupts.
Configure Data Ready Signal Output Pin to either HiZ or push-pull and
use it as interrupt source.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/mcp3911.c | 53 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index 4452b01c76f3..b76d825c984d 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -31,6 +31,7 @@
 #define MCP3911_REG_GAIN		0x09
 
 #define MCP3911_REG_STATUSCOM		0x0a
+#define MCP3911_STATUSCOM_DRHIZ         BIT(12)
 #define MCP3911_STATUSCOM_READ		GENMASK(7, 6)
 #define MCP3911_STATUSCOM_CH1_24WIDTH	BIT(4)
 #define MCP3911_STATUSCOM_CH0_24WIDTH	BIT(3)
@@ -64,6 +65,7 @@ struct mcp3911 {
 	struct regulator *vref;
 	struct clk *clki;
 	u32 dev_addr;
+	struct iio_trigger *trig;
 	struct {
 		u32 channels[MCP3911_NUM_CHANNELS];
 		s64 ts __aligned(8);
@@ -346,6 +348,23 @@ static void mcp3911_cleanup_regulator(void *_adc)
 	regulator_disable(adc->vref);
 }
 
+static int mcp3911_set_trigger_state(struct iio_trigger *trig, bool enable)
+{
+	struct mcp3911 *adc = iio_trigger_get_drvdata(trig);
+
+	if (enable)
+		enable_irq(adc->spi->irq);
+	else
+		disable_irq(adc->spi->irq);
+
+	return 0;
+}
+
+static const struct iio_trigger_ops mcp3911_trigger_ops = {
+	.validate_device = iio_trigger_validate_own_device,
+	.set_trigger_state = mcp3911_set_trigger_state,
+};
+
 static int mcp3911_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
@@ -397,6 +416,15 @@ static int mcp3911_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	if (device_property_read_bool(&adc->spi->dev, "microchip,data-ready-hiz"))
+		ret = mcp3911_update(adc, MCP3911_REG_STATUSCOM, MCP3911_STATUSCOM_DRHIZ,
+				0, 2);
+	else
+		ret = mcp3911_update(adc, MCP3911_REG_STATUSCOM, MCP3911_STATUSCOM_DRHIZ,
+				MCP3911_STATUSCOM_DRHIZ, 2);
+	if (ret)
+		return ret;
+
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &mcp3911_info;
@@ -407,6 +435,31 @@ static int mcp3911_probe(struct spi_device *spi)
 
 	mutex_init(&adc->lock);
 
+	if (spi->irq > 0) {
+		adc->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
+				indio_dev->name,
+				iio_device_id(indio_dev));
+		if (!adc->trig)
+			return PTR_ERR(adc->trig);
+
+		adc->trig->ops = &mcp3911_trigger_ops;
+		iio_trigger_set_drvdata(adc->trig, adc);
+		ret = devm_iio_trigger_register(&spi->dev, adc->trig);
+		if (ret)
+			return ret;
+
+		/*
+		 * The device generates interrupts as long as it is powered up.
+		 * Some platforms might not allow the option to power it down so
+		 * don't enable the interrupt to avoid extra load on the system.
+		 */
+		ret = devm_request_irq(&spi->dev, spi->irq,
+				&iio_trigger_generic_data_rdy_poll, IRQF_NO_AUTOEN | IRQF_ONESHOT,
+				indio_dev->name, adc->trig);
+		if (ret)
+			return ret;
+	}
+
 	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
 			NULL,
 			mcp3911_trigger_handler, NULL);
-- 
2.37.1

[PATCH v6 7/9] dt-bindings: iio: adc: mcp3911: add microchip,data-ready-hiz entry

[Marcus Folkesson]

The Data Ready Output Pin is either hard wired to work as high
impedance or push-pull. Make it configurable.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/iio/adc/microchip,mcp3911.yaml     | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml
index 95ab285f4eba..6bee332ed58f 100644
--- a/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml
@@ -36,6 +36,13 @@ properties:
     description: IRQ line of the ADC
     maxItems: 1
 
+  microchip,data-ready-hiz:
+    description:
+      Data Ready Pin Inactive State Control
+      true = The DR pin state is high-impedance
+      false = The DR pin state is logic high
+    type: boolean
+
   microchip,device-addr:
     description: Device address when multiple MCP3911 chips are present on the same SPI bus.
     $ref: /schemas/types.yaml#/definitions/uint32
-- 
2.37.1

[PATCH v6 8/9] iio: adc: mcp3911: add support for oversampling ratio

[Marcus Folkesson]

The chip supports oversampling ratio, so expose it to userspace.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/mcp3911.c | 58 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index b76d825c984d..0151258b456c 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -41,6 +41,7 @@
 #define MCP3911_REG_CONFIG		0x0c
 #define MCP3911_CONFIG_CLKEXT		BIT(1)
 #define MCP3911_CONFIG_VREFEXT		BIT(2)
+#define MCP3911_CONFIG_OSR		GENMASK(13, 11)
 
 #define MCP3911_REG_OFFCAL_CH0		0x0e
 #define MCP3911_REG_GAINCAL_CH0		0x11
@@ -59,6 +60,8 @@
 
 #define MCP3911_NUM_CHANNELS		2
 
+static const int mcp3911_osr_table[] = { 32, 64, 128, 256, 512, 1024, 2048, 4096 };
+
 struct mcp3911 {
 	struct spi_device *spi;
 	struct mutex lock;
@@ -117,6 +120,36 @@ static int mcp3911_update(struct mcp3911 *adc, u8 reg, u32 mask,
 	return mcp3911_write(adc, reg, val, len);
 }
 
+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;
+	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,
+			     long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*type = IIO_VAL_INT;
+		*vals = mcp3911_osr_table;
+		*length = ARRAY_SIZE(mcp3911_osr_table);
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int mcp3911_read_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *channel, int *val,
 			    int *val2, long mask)
@@ -145,6 +178,15 @@ static int mcp3911_read_raw(struct iio_dev *indio_dev,
 
 		ret = IIO_VAL_INT;
 		break;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		ret = mcp3911_read(adc, MCP3911_REG_CONFIG, val, 2);
+		if (ret)
+			goto out;
+
+		*val = FIELD_GET(MCP3911_CONFIG_OSR, *val);
+		*val = 32 << *val;
+		ret = IIO_VAL_INT;
+		break;
 
 	case IIO_CHAN_INFO_SCALE:
 		if (adc->vref) {
@@ -204,6 +246,17 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev,
 				MCP3911_STATUSCOM_EN_OFFCAL,
 				MCP3911_STATUSCOM_EN_OFFCAL, 2);
 		break;
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		for (int i = 0; i < sizeof(mcp3911_osr_table); i++) {
+			if (val == mcp3911_osr_table[i]) {
+				val = FIELD_PREP(MCP3911_CONFIG_OSR, i);
+				ret = mcp3911_update(adc, MCP3911_REG_CONFIG, MCP3911_CONFIG_OSR,
+						val, 2);
+				break;
+			}
+		}
+		break;
 	}
 
 out:
@@ -216,9 +269,12 @@ static int mcp3911_write_raw(struct iio_dev *indio_dev,
 		.indexed = 1,					\
 		.channel = idx,					\
 		.scan_index = idx,				\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
 		.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),	\
 		.scan_type = {					\
 			.sign = 's',				\
 			.realbits = 24,				\
@@ -278,6 +334,8 @@ static irqreturn_t mcp3911_trigger_handler(int irq, void *p)
 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 int mcp3911_config(struct mcp3911 *adc)
-- 
2.37.1

[PATCH v6 9/9] iio: adc: mcp3911: add support to set PGA

[Marcus Folkesson]

Add support for setting the Programmable Gain Amplifiers by adjust the
scale value.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/mcp3911.c | 107 +++++++++++++++++++++++++++++---------
 1 file changed, 83 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
index 0151258b456c..de85a87b9287 100644
--- a/drivers/iio/adc/mcp3911.c
+++ b/drivers/iio/adc/mcp3911.c
@@ -29,6 +29,8 @@
 #define MCP3911_REG_MOD			0x06
 #define MCP3911_REG_PHASE		0x07
 #define MCP3911_REG_GAIN		0x09
+#define MCP3911_GAIN_MASK(ch)		(GENMASK(2, 0) << 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)
@@ -59,8 +61,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;
@@ -69,6 +73,7 @@ struct mcp3911 {
 	struct clk *clki;
 	u32 dev_addr;
 	struct iio_trigger *trig;
+	u32 gain[MCP3911_NUM_CHANNELS];
 	struct {
 		u32 channels[MCP3911_NUM_CHANNELS];
 		s64 ts __aligned(8);
@@ -145,6 +150,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;
 	}
@@ -189,29 +199,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;
 	}
 
@@ -229,6 +219,18 @@ 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;
@@ -264,6 +266,47 @@ 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;
+	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);
+			return ret;
+		}
+
+		ref = ret / 1000;
+	}
+
+	/*
+	 * For 24-bit 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;
+	}
+
+	return 0;
+}
+
 #define MCP3911_CHAN(idx) {					\
 		.type = IIO_VOLTAGE,				\
 		.indexed = 1,					\
@@ -273,8 +316,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 =		\
+		.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,				\
@@ -483,6 +528,20 @@ static int mcp3911_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
+	ret = mcp3911_calc_scale_table(adc);
+	if (ret)
+		return ret;
+
+       /* Set gain to 1 for all channels */
+	for (int i = 0; i < MCP3911_NUM_CHANNELS; i++) {
+		adc->gain[i] = 1;
+		ret = mcp3911_update(adc, MCP3911_REG_GAIN,
+				MCP3911_GAIN_MASK(i),
+				MCP3911_GAIN_VAL(i, 0), 1);
+		if (ret)
+			return ret;
+	}
+
 	indio_dev->name = spi_get_device_id(spi)->name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &mcp3911_info;
-- 
2.37.1

Re: [PATCH v6 0/9] Improve MCP3911 driver

[Krzysztof Kozlowski]

On 15/08/2022 09:16, Marcus Folkesson wrote:
> v5:
> - Drop remove function
> - Split tx&rx transfers in mcp3911_trigger_handler()
> - Moved Kconfig changes to right patch
> 
> v6:
> - Go for devm_clk_get_enabled()
> - Cosmetic cleanups
> - Clarify the description of microchip,data-ready-hiz
> 

Please include the diffstat (format-patch does it for you, but if you
prefer to use other tools - it's your job).

Best regards,
Krzysztof

Re: [PATCH v6 0/9] Improve MCP3911 driver

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]

On Tue, Aug 16, 2022 at 01:22:31PM +0300, Krzysztof Kozlowski wrote:
> On 15/08/2022 09:16, Marcus Folkesson wrote:
> > v5:
> > - Drop remove function
> > - Split tx&rx transfers in mcp3911_trigger_handler()
> > - Moved Kconfig changes to right patch
> > 
> > v6:
> > - Go for devm_clk_get_enabled()
> > - Cosmetic cleanups
> > - Clarify the description of microchip,data-ready-hiz
> > 
> 
> Please include the diffstat (format-patch does it for you, but if you
> prefer to use other tools - it's your job).

I'm sorry, I do not get you.
The diffstat is included in the patches, should it be included here as
well?

/Marcus

> 
> Best regards,
> Krzysztof

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 0/9] Improve MCP3911 driver

[Krzysztof Kozlowski]

On 16/08/2022 17:57, Marcus Folkesson wrote:
> On Tue, Aug 16, 2022 at 01:22:31PM +0300, Krzysztof Kozlowski wrote:
>> On 15/08/2022 09:16, Marcus Folkesson wrote:
>>> v5:
>>> - Drop remove function
>>> - Split tx&rx transfers in mcp3911_trigger_handler()
>>> - Moved Kconfig changes to right patch
>>>
>>> v6:
>>> - Go for devm_clk_get_enabled()
>>> - Cosmetic cleanups
>>> - Clarify the description of microchip,data-ready-hiz
>>>
>>
>> Please include the diffstat (format-patch does it for you, but if you
>> prefer to use other tools - it's your job).
> 
> I'm sorry, I do not get you.
> The diffstat is included in the patches, should it be included here as
> well?

Yes, in the cover letter, so when someone partially interested (e.g. me)
opens the cover letter, it's easy to notice what files are affected.

Again, this is exactly what Git is doing, so you have to on purpose use
less-standard process to achieve cover letter without diffstat.

Best regards,
Krzysztof

Re: [PATCH v6 0/9] Improve MCP3911 driver

[Marcus Folkesson]

[-- Attachment #1: Type: text/plain, Size: 2073 bytes --]

On Mon, Aug 15, 2022 at 08:16:16AM +0200, Marcus Folkesson wrote:
> This patch series intend to fix bugs and improve functionality of 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.
> 
> 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
> 
> v4:
> - Split up devm-cleanup functions 
> - Cosmetic cleanups
> - Add
> 	select IIO_BUFFER
> 	select IIO_TRIGGERED_BUFFER
>     To Kconfig
> - Add .endianness = IIO_BE
> 
> v5:
> - Drop remove function
> - Split tx&rx transfers in mcp3911_trigger_handler()
> - Moved Kconfig changes to right patch
> 
> v6:
> - Go for devm_clk_get_enabled()
> - Cosmetic cleanups
> - Clarify the description of microchip,data-ready-hiz
> 

Marcus Folkesson (9):
  iio: adc: mcp3911: make use of the sign bit
  iio: adc: mcp3911: correct "microchip,device-addr" property
  iio: adc: mcp3911: use correct formula for AD conversion
  iio: adc: mcp3911: use resource-managed version of iio_device_register
  iio: adc: mcp3911: add support for buffers
  iio: adc: mcp3911: add support for interrupts
  dt-bindings: iio: adc: mcp3911: add microchip,data-ready-hiz entry
  iio: adc: mcp3911: add support for oversampling ratio
  iio: adc: mcp3911: add support to set PGA

 .../bindings/iio/adc/microchip,mcp3911.yaml   |   7 +
 drivers/iio/adc/Kconfig                       |   2 +
 drivers/iio/adc/mcp3911.c                     | 372 +++++++++++++++---
 3 files changed, 321 insertions(+), 60 deletions(-)

> 
> Best regards,
> Marcus Folkesson
> 
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 4/9] iio: adc: mcp3911: use resource-managed version of iio_device_register

[Jonathan Cameron]

On Mon, 15 Aug 2022 08:16:20 +0200
Marcus Folkesson <marcus.folkesson@gmail.com> wrote:

> Keep using managed resources as much as possible.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  drivers/iio/adc/mcp3911.c | 53 ++++++++++++---------------------------
>  1 file changed, 16 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
> index 890af7dca62d..7e2efe702e57 100644
> --- a/drivers/iio/adc/mcp3911.c
> +++ b/drivers/iio/adc/mcp3911.c
> @@ -258,6 +258,13 @@ static int mcp3911_config(struct mcp3911 *adc)
>  	return  mcp3911_write(adc, MCP3911_REG_CONFIG, configreg, 2);
>  }
>  
> +static void mcp3911_cleanup_regulator(void *_adc)

Missed this on previous versions, but why not pass
the regulator pointer in as the parameter for the callback?

static void mcp391_cleanup_regulator(void *reg)
{
	regulator_disable(adc->vref);
}

Note this can't use the new devm_regulator_get_enable()
because we need access to the regulator within the driver.

I can tidy this up whilst applying (or given it's really minor I might
not bother :)

Note we are stalled at the moment with this series on getting the
fixes upstream.  I'll probably send that pull request shortly.

> +{
> +	struct mcp3911 *adc = _adc;
> +
> +	regulator_disable(adc->vref);
> +}
> +
>  static int mcp3911_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
> @@ -286,9 +293,14 @@ static int mcp3911_probe(struct spi_device *spi)
>  		ret = regulator_enable(adc->vref);
>  		if (ret)
>  			return ret;
> +
> +		ret = devm_add_action_or_reset(&spi->dev,
> +				mcp3911_cleanup_regulator, adc);
> +		if (ret)
> +			return ret;
>  	}
>  
> -	adc->clki = devm_clk_get(&adc->spi->dev, NULL);
> +	adc->clki = devm_clk_get_enabled(&adc->spi->dev, NULL);
>  	if (IS_ERR(adc->clki)) {
>  		if (PTR_ERR(adc->clki) == -ENOENT) {
>  			adc->clki = NULL;
> @@ -296,21 +308,13 @@ static int mcp3911_probe(struct spi_device *spi)
>  			dev_err(&adc->spi->dev,
>  				"failed to get adc clk (%ld)\n",
>  				PTR_ERR(adc->clki));
> -			ret = PTR_ERR(adc->clki);
> -			goto reg_disable;
> -		}
> -	} else {
> -		ret = clk_prepare_enable(adc->clki);
> -		if (ret < 0) {
> -			dev_err(&adc->spi->dev,
> -				"Failed to enable clki: %d\n", ret);
> -			goto reg_disable;
> +			return PTR_ERR(adc->clki);
>  		}
>  	}
>  
>  	ret = mcp3911_config(adc);
>  	if (ret)
> -		goto clk_disable;
> +		return ret;
>  
>  	indio_dev->name = spi_get_device_id(spi)->name;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -322,31 +326,7 @@ static int mcp3911_probe(struct spi_device *spi)
>  
>  	mutex_init(&adc->lock);
>  
> -	ret = iio_device_register(indio_dev);
> -	if (ret)
> -		goto clk_disable;
> -
> -	return ret;
> -
> -clk_disable:
> -	clk_disable_unprepare(adc->clki);
> -reg_disable:
> -	if (adc->vref)
> -		regulator_disable(adc->vref);
> -
> -	return ret;
> -}
> -
> -static void mcp3911_remove(struct spi_device *spi)
> -{
> -	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> -	struct mcp3911 *adc = iio_priv(indio_dev);
> -
> -	iio_device_unregister(indio_dev);
> -
> -	clk_disable_unprepare(adc->clki);
> -	if (adc->vref)
> -		regulator_disable(adc->vref);
> +	return devm_iio_device_register(&adc->spi->dev, indio_dev);
>  }
>  
>  static const struct of_device_id mcp3911_dt_ids[] = {
> @@ -367,7 +347,6 @@ static struct spi_driver mcp3911_driver = {
>  		.of_match_table = mcp3911_dt_ids,
>  	},
>  	.probe = mcp3911_probe,
> -	.remove = mcp3911_remove,
>  	.id_table = mcp3911_id,
>  };
>  module_spi_driver(mcp3911_driver);

Re: [PATCH v6 4/9] iio: adc: mcp3911: use resource-managed version of iio_device_register

[Marcus Folkesson]

Hi,

On Sat, Aug 20, 2022 at 01:41:50PM +0100, Jonathan Cameron wrote:
> On Mon, 15 Aug 2022 08:16:20 +0200
> Marcus Folkesson <marcus.folkesson@gmail.com> wrote:
> 
> > Keep using managed resources as much as possible.
> > 
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > ---
> >  drivers/iio/adc/mcp3911.c | 53 ++++++++++++---------------------------
> >  1 file changed, 16 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
> > index 890af7dca62d..7e2efe702e57 100644
> > --- a/drivers/iio/adc/mcp3911.c
> > +++ b/drivers/iio/adc/mcp3911.c
> > @@ -258,6 +258,13 @@ static int mcp3911_config(struct mcp3911 *adc)
> >  	return  mcp3911_write(adc, MCP3911_REG_CONFIG, configreg, 2);
> >  }
> >  
> > +static void mcp3911_cleanup_regulator(void *_adc)
> 
> Missed this on previous versions, but why not pass
> the regulator pointer in as the parameter for the callback?
> 
> static void mcp391_cleanup_regulator(void *reg)
> {
> 	regulator_disable(adc->vref);
> }
> 
> Note this can't use the new devm_regulator_get_enable()
> because we need access to the regulator within the driver.
> 
> I can tidy this up whilst applying (or given it's really minor I might
> not bother :)
> 
> Note we are stalled at the moment with this series on getting the
> fixes upstream.  I'll probably send that pull request shortly.

Just a friendly reminder to not forget to pick up this series.

Thanks,

/Marcus

Re: [PATCH v6 4/9] iio: adc: mcp3911: use resource-managed version of iio_device_register

[Jonathan Cameron]

On Mon, 12 Sep 2022 13:02:19 +0200
Marcus Folkesson <marcus.folkesson@gmail.com> wrote:

> Hi,
> 
> On Sat, Aug 20, 2022 at 01:41:50PM +0100, Jonathan Cameron wrote:
> > On Mon, 15 Aug 2022 08:16:20 +0200
> > Marcus Folkesson <marcus.folkesson@gmail.com> wrote:
> >   
> > > Keep using managed resources as much as possible.
> > > 
> > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > ---
> > >  drivers/iio/adc/mcp3911.c | 53 ++++++++++++---------------------------
> > >  1 file changed, 16 insertions(+), 37 deletions(-)
> > > 
> > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
> > > index 890af7dca62d..7e2efe702e57 100644
> > > --- a/drivers/iio/adc/mcp3911.c
> > > +++ b/drivers/iio/adc/mcp3911.c
> > > @@ -258,6 +258,13 @@ static int mcp3911_config(struct mcp3911 *adc)
> > >  	return  mcp3911_write(adc, MCP3911_REG_CONFIG, configreg, 2);
> > >  }
> > >  
> > > +static void mcp3911_cleanup_regulator(void *_adc)  
> > 
> > Missed this on previous versions, but why not pass
> > the regulator pointer in as the parameter for the callback?
> > 
> > static void mcp391_cleanup_regulator(void *reg)
> > {
> > 	regulator_disable(adc->vref);
> > }
> > 
> > Note this can't use the new devm_regulator_get_enable()
> > because we need access to the regulator within the driver.
> > 
> > I can tidy this up whilst applying (or given it's really minor I might
> > not bother :)
> > 
> > Note we are stalled at the moment with this series on getting the
> > fixes upstream.  I'll probably send that pull request shortly.  
> 
> Just a friendly reminder to not forget to pick up this series.

If things go according to plan, Greg will take the pull request
I sent the other day and I can fast forward the tree such that
the first 3 patches are in my upstream, then do a second pull
request with these applied. They aim here being to keep the
history reasonably clean rather than spaghetti merges.
Fingers crossed on timing working out. This all got delayed
because I sat on the pull request for a week due to travel and
dodgy airport wifi etc.

Jonathan

> 
> Thanks,
> 
> /Marcus

Re: [PATCH v6 4/9] iio: adc: mcp3911: use resource-managed version of iio_device_register

[Jonathan Cameron]

On Mon, 19 Sep 2022 16:36:00 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon, 12 Sep 2022 13:02:19 +0200
> Marcus Folkesson <marcus.folkesson@gmail.com> wrote:
> 
> > Hi,
> > 
> > On Sat, Aug 20, 2022 at 01:41:50PM +0100, Jonathan Cameron wrote:  
> > > On Mon, 15 Aug 2022 08:16:20 +0200
> > > Marcus Folkesson <marcus.folkesson@gmail.com> wrote:
> > >     
> > > > Keep using managed resources as much as possible.
> > > > 
> > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > ---
> > > >  drivers/iio/adc/mcp3911.c | 53 ++++++++++++---------------------------
> > > >  1 file changed, 16 insertions(+), 37 deletions(-)
> > > > 
> > > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
> > > > index 890af7dca62d..7e2efe702e57 100644
> > > > --- a/drivers/iio/adc/mcp3911.c
> > > > +++ b/drivers/iio/adc/mcp3911.c
> > > > @@ -258,6 +258,13 @@ static int mcp3911_config(struct mcp3911 *adc)
> > > >  	return  mcp3911_write(adc, MCP3911_REG_CONFIG, configreg, 2);
> > > >  }
> > > >  
> > > > +static void mcp3911_cleanup_regulator(void *_adc)    
> > > 
> > > Missed this on previous versions, but why not pass
> > > the regulator pointer in as the parameter for the callback?
> > > 
> > > static void mcp391_cleanup_regulator(void *reg)
> > > {
> > > 	regulator_disable(adc->vref);
> > > }
> > > 
> > > Note this can't use the new devm_regulator_get_enable()
> > > because we need access to the regulator within the driver.
> > > 
> > > I can tidy this up whilst applying (or given it's really minor I might
> > > not bother :)
> > > 
> > > Note we are stalled at the moment with this series on getting the
> > > fixes upstream.  I'll probably send that pull request shortly.    
> > 
> > Just a friendly reminder to not forget to pick up this series.  
> 
> If things go according to plan, Greg will take the pull request
> I sent the other day and I can fast forward the tree such that
> the first 3 patches are in my upstream, then do a second pull
> request with these applied. They aim here being to keep the
> history reasonably clean rather than spaghetti merges.
> Fingers crossed on timing working out. This all got delayed
> because I sat on the pull request for a week due to travel and
> dodgy airport wifi etc.
> 

Now applied to the togreg branch of iio.git.
Pushed out as testing for some brief testing by 0day etc.

Jonathan

> Jonathan
> 
> > 
> > Thanks,
> > 
> > /Marcus  
>