All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] iio: adc: add support for ltc2496
@ 2019-12-09 20:32 Uwe Kleine-König
  2019-12-09 20:32 ` [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Uwe Kleine-König @ 2019-12-09 20:32 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	Michael Hennerich, Stefan Popa, Alexandru Ardelean
  Cc: linux-iio, kernel

Hello,

changes since v3 (Message-Id:
<20191121210007.25646-1-u.kleine-koenig@pengutronix.de>) are:

 - dropped one ____cacheline_aligned from the spi driver struct
 - fixed the dt binding to make dt_binding_check happy.

I'm not sure if including patches 2 and 3 again is the right thing as
Jonathan already talked about applying v3, so I hope I don't confuse his
book keeping.

Best regards
Uwe

Uwe Kleine-König (3):
  iio: adc: ltc2496: provide device tree binding document
  iio: adc: ltc2497: split protocol independent part in a separate
    module
  iio: adc: new driver to support Linear technology's ltc2496

 .../bindings/iio/adc/lltc,ltc2496.yaml        |  47 ++++
 MAINTAINERS                                   |   2 +-
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   3 +-
 drivers/iio/adc/ltc2496.c                     | 108 ++++++++
 drivers/iio/adc/ltc2497-core.c                | 243 ++++++++++++++++++
 drivers/iio/adc/ltc2497.c                     | 234 ++---------------
 drivers/iio/adc/ltc2497.h                     |  18 ++
 8 files changed, 455 insertions(+), 210 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
 create mode 100644 drivers/iio/adc/ltc2496.c
 create mode 100644 drivers/iio/adc/ltc2497-core.c
 create mode 100644 drivers/iio/adc/ltc2497.h

-- 
2.24.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document
  2019-12-09 20:32 [PATCH v4 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König
@ 2019-12-09 20:32 ` Uwe Kleine-König
  2019-12-15 11:49   ` Jonathan Cameron
  2019-12-09 20:32 ` [PATCH v4 2/3] iio: adc: ltc2497: split protocol independent part in a separate module Uwe Kleine-König
  2019-12-09 20:32 ` [PATCH v4 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König
  2 siblings, 1 reply; 13+ messages in thread
From: Uwe Kleine-König @ 2019-12-09 20:32 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	Michael Hennerich, Stefan Popa, Alexandru Ardelean
  Cc: linux-iio, kernel

The ADC only requires the standard stuff for spi devices and a reference
voltage.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../bindings/iio/adc/lltc,ltc2496.yaml        | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
new file mode 100644
index 000000000000..195c8c8f2f4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/lltc,ltc2496.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linear Technology / Analog Devices LTC2496 ADC
+
+maintainers:
+ - Lars-Peter Clausen <lars@metafoo.de>
+ - Michael Hennerich <Michael.Hennerich@analog.com>
+ - Stefan Popa <stefan.popa@analog.com>
+
+properties:
+  compatible:
+    enum:
+      - lltc,ltc2496
+
+  vref-supply:
+    description: phandle to an external regulator providing the reference voltage
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle
+
+  reg:
+    description: spi chipselect number according to the usual spi bindings
+
+  spi-max-frequency:
+    description: maximal spi bus frequency supported by the chip
+
+required:
+  - compatible
+  - vref-supply
+  - reg
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+        	compatible = "lltc,ltc2496";
+        	reg = <0>;
+        	vref-supply = <&ltc2496_reg>;
+        	spi-max-frequency = <2000000>;
+        };
+    };
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 2/3] iio: adc: ltc2497: split protocol independent part in a separate module
  2019-12-09 20:32 [PATCH v4 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König
  2019-12-09 20:32 ` [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König
@ 2019-12-09 20:32 ` Uwe Kleine-König
  2019-12-15 11:55   ` Jonathan Cameron
  2019-12-09 20:32 ` [PATCH v4 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König
  2 siblings, 1 reply; 13+ messages in thread
From: Uwe Kleine-König @ 2019-12-09 20:32 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	Michael Hennerich, Stefan Popa, Alexandru Ardelean
  Cc: linux-iio, kernel

This allows to share most of this driver for the ltc2496 driver added in
the next commit that is an SPI variant of the ltc2497. Initially I named
the generic part ltc249x, but wild card names are frowned upon, so the
generic part is called ltc2497-core even though it's not obvious that
this is then to be reused for the ltc2496 driver.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/iio/adc/Makefile       |   2 +-
 drivers/iio/adc/ltc2497-core.c | 243 +++++++++++++++++++++++++++++++++
 drivers/iio/adc/ltc2497.c      | 234 ++++---------------------------
 drivers/iio/adc/ltc2497.h      |  18 +++
 4 files changed, 288 insertions(+), 209 deletions(-)
 create mode 100644 drivers/iio/adc/ltc2497-core.c
 create mode 100644 drivers/iio/adc/ltc2497.h

diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ef9cc485fb67..ee0c8dcfb501 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
 obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
 obj-$(CONFIG_LTC2471) += ltc2471.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
-obj-$(CONFIG_LTC2497) += ltc2497.o
+obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1118) += max1118.o
diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
new file mode 100644
index 000000000000..f5f7039caacc
--- /dev/null
+++ b/drivers/iio/adc/ltc2497-core.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ltc2497-core.c - Common code for Analog Devices/Linear Technology
+ * LTC2496 and LTC2497 ADCs
+ *
+ * Copyright (C) 2017 Analog Devices Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+
+#include "ltc2497.h"
+
+#define LTC2497_SGL			BIT(4)
+#define LTC2497_DIFF			0
+#define LTC2497_SIGN			BIT(3)
+
+static int ltc2497core_wait_conv(struct ltc2497core_driverdata *ddata)
+{
+	s64 time_elapsed;
+
+	time_elapsed = ktime_ms_delta(ktime_get(), ddata->time_prev);
+
+	if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
+		/* delay if conversion time not passed
+		 * since last read or write
+		 */
+		if (msleep_interruptible(
+		    LTC2497_CONVERSION_TIME_MS - time_elapsed))
+			return -ERESTARTSYS;
+
+		return 0;
+	}
+
+	if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
+		/* We're in automatic mode -
+		 * so the last reading is still not outdated
+		 */
+		return 0;
+	}
+
+	return 1;
+}
+
+static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address, int *val)
+{
+	int ret;
+
+	ret = ltc2497core_wait_conv(ddata);
+	if (ret < 0)
+		return ret;
+
+	if (ret || ddata->addr_prev != address) {
+		ret = ddata->result_and_measure(ddata, address, NULL);
+		if (ret < 0)
+			return ret;
+		ddata->addr_prev = address;
+
+		if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS))
+			return -ERESTARTSYS;
+	}
+
+	ret = ddata->result_and_measure(ddata, address, val);
+	if (ret < 0)
+		return ret;
+
+	ddata->time_prev = ktime_get();
+
+	return ret;
+}
+
+static int ltc2497core_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&indio_dev->mlock);
+		ret = ltc2497core_read(ddata, chan->address, val);
+		mutex_unlock(&indio_dev->mlock);
+		if (ret < 0)
+			return ret;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regulator_get_voltage(ddata->ref);
+		if (ret < 0)
+			return ret;
+
+		*val = ret / 1000;
+		*val2 = 17;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+#define LTC2497_CHAN(_chan, _addr, _ds_name) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.channel = (_chan), \
+	.address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC2497_SIGN : 0)), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+	.datasheet_name = (_ds_name), \
+}
+
+#define LTC2497_CHAN_DIFF(_chan, _addr) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.channel = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 1 : 0), \
+	.channel2 = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 0 : 1),\
+	.address = (_addr | _chan), \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+	.differential = 1, \
+}
+
+static const struct iio_chan_spec ltc2497core_channel[] = {
+	LTC2497_CHAN(0, LTC2497_SGL, "CH0"),
+	LTC2497_CHAN(1, LTC2497_SGL, "CH1"),
+	LTC2497_CHAN(2, LTC2497_SGL, "CH2"),
+	LTC2497_CHAN(3, LTC2497_SGL, "CH3"),
+	LTC2497_CHAN(4, LTC2497_SGL, "CH4"),
+	LTC2497_CHAN(5, LTC2497_SGL, "CH5"),
+	LTC2497_CHAN(6, LTC2497_SGL, "CH6"),
+	LTC2497_CHAN(7, LTC2497_SGL, "CH7"),
+	LTC2497_CHAN(8, LTC2497_SGL, "CH8"),
+	LTC2497_CHAN(9, LTC2497_SGL, "CH9"),
+	LTC2497_CHAN(10, LTC2497_SGL, "CH10"),
+	LTC2497_CHAN(11, LTC2497_SGL, "CH11"),
+	LTC2497_CHAN(12, LTC2497_SGL, "CH12"),
+	LTC2497_CHAN(13, LTC2497_SGL, "CH13"),
+	LTC2497_CHAN(14, LTC2497_SGL, "CH14"),
+	LTC2497_CHAN(15, LTC2497_SGL, "CH15"),
+	LTC2497_CHAN_DIFF(0, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(1, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(2, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(3, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(4, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(5, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(6, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(7, LTC2497_DIFF),
+	LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN),
+	LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN),
+	LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN),
+	LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN),
+	LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN),
+	LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN),
+	LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN),
+	LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN),
+};
+
+static const struct iio_info ltc2497core_info = {
+	.read_raw = ltc2497core_read_raw,
+};
+
+int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev)
+{
+	struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
+	int ret;
+
+	indio_dev->dev.parent = dev;
+	indio_dev->name = dev_name(dev);
+	indio_dev->info = &ltc2497core_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ltc2497core_channel;
+	indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel);
+
+	ret = ddata->result_and_measure(ddata, LTC2497_CONFIG_DEFAULT, NULL);
+	if (ret < 0)
+		return ret;
+
+	ddata->ref = devm_regulator_get(dev, "vref");
+	if (IS_ERR(ddata->ref)) {
+		if (PTR_ERR(ddata->ref) != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get vref regulator: %pe\n",
+				ddata->ref);
+
+		return PTR_ERR(ddata->ref);
+	}
+
+	ret = regulator_enable(ddata->ref);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable vref regulator: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	if (dev->platform_data) {
+		struct iio_map *plat_data;
+
+		plat_data = (struct iio_map *)dev->platform_data;
+
+		ret = iio_map_array_register(indio_dev, plat_data);
+		if (ret) {
+			dev_err(&indio_dev->dev, "iio map err: %d\n", ret);
+			goto err_regulator_disable;
+		}
+	}
+
+	ddata->addr_prev = LTC2497_CONFIG_DEFAULT;
+	ddata->time_prev = ktime_get();
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto err_array_unregister;
+
+	return 0;
+
+err_array_unregister:
+	iio_map_array_unregister(indio_dev);
+
+err_regulator_disable:
+	regulator_disable(ddata->ref);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS(ltc2497core_probe, LTC2497);
+
+void ltc2497core_remove(struct iio_dev *indio_dev)
+{
+	struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	iio_map_array_unregister(indio_dev);
+
+	regulator_disable(ddata->ref);
+}
+EXPORT_SYMBOL_NS(ltc2497core_remove, LTC2497);
+
+MODULE_DESCRIPTION("common code for LTC2496/LTC2497 drivers");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
index 470406032720..5db63d7c6bc5 100644
--- a/drivers/iio/adc/ltc2497.c
+++ b/drivers/iio/adc/ltc2497.c
@@ -7,27 +7,18 @@
  * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf
  */
 
-#include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/driver.h>
-#include <linux/iio/sysfs.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/regulator/consumer.h>
 
-#define LTC2497_ENABLE			0xA0
-#define LTC2497_SGL			BIT(4)
-#define LTC2497_DIFF			0
-#define LTC2497_SIGN			BIT(3)
-#define LTC2497_CONFIG_DEFAULT		LTC2497_ENABLE
-#define LTC2497_CONVERSION_TIME_MS	150ULL
+#include "ltc2497.h"
 
-struct ltc2497_st {
+struct ltc2497_driverdata {
+	/* this must be the first member */
+	struct ltc2497core_driverdata common_ddata;
 	struct i2c_client *client;
-	struct regulator *ref;
-	ktime_t	time_prev;
-	u8 addr_prev;
 	/*
 	 * DMA (thus cache coherency maintenance) requires the
 	 * transfer buffers to live in their own cache lines.
@@ -35,232 +26,59 @@ struct ltc2497_st {
 	__be32 buf ____cacheline_aligned;
 };
 
-static int ltc2497_wait_conv(struct ltc2497_st *st)
+static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata,
+				      u8 address, int *val)
 {
-	s64 time_elapsed;
-
-	time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
-
-	if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
-		/* delay if conversion time not passed
-		 * since last read or write
-		 */
-		if (msleep_interruptible(
-		    LTC2497_CONVERSION_TIME_MS - time_elapsed))
-			return -ERESTARTSYS;
-
-		return 0;
-	}
-
-	if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
-		/* We're in automatic mode -
-		 * so the last reading is stil not outdated
-		 */
-		return 0;
-	}
-
-	return 1;
-}
-
-static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val)
-{
-	struct i2c_client *client = st->client;
-	int ret;
-
-	ret = ltc2497_wait_conv(st);
-	if (ret < 0)
-		return ret;
-
-	if (ret || st->addr_prev != address) {
-		ret = i2c_smbus_write_byte(st->client,
-					   LTC2497_ENABLE | address);
-		if (ret < 0)
-			return ret;
-		st->addr_prev = address;
-		if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS))
-			return -ERESTARTSYS;
-	}
-	ret = i2c_master_recv(client, (char *)&st->buf, 3);
-	if (ret < 0)  {
-		dev_err(&client->dev, "i2c_master_recv failed\n");
-		return ret;
-	}
-	st->time_prev = ktime_get();
-
-	/* convert and shift the result,
-	 * and finally convert from offset binary to signed integer
-	 */
-	*val = (be32_to_cpu(st->buf) >> 14) - (1 << 17);
-
-	return ret;
-}
-
-static int ltc2497_read_raw(struct iio_dev *indio_dev,
-			    struct iio_chan_spec const *chan,
-			    int *val, int *val2, long mask)
-{
-	struct ltc2497_st *st = iio_priv(indio_dev);
+	struct ltc2497_driverdata *st =
+		container_of(ddata, struct ltc2497_driverdata, common_ddata);
 	int ret;
 
-	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-		mutex_lock(&indio_dev->mlock);
-		ret = ltc2497_read(st, chan->address, val);
-		mutex_unlock(&indio_dev->mlock);
-		if (ret < 0)
-			return ret;
-
-		return IIO_VAL_INT;
-
-	case IIO_CHAN_INFO_SCALE:
-		ret = regulator_get_voltage(st->ref);
-		if (ret < 0)
+	if (val) {
+		ret = i2c_master_recv(st->client, (char *)&st->buf, 3);
+		if (ret < 0) {
+			dev_err(&st->client->dev, "i2c_master_recv failed\n");
 			return ret;
+		}
 
-		*val = ret / 1000;
-		*val2 = 17;
-
-		return IIO_VAL_FRACTIONAL_LOG2;
-
-	default:
-		return -EINVAL;
+		*val = (be32_to_cpu(st->buf) >> 14) - (1 << 17);
 	}
-}
 
-#define LTC2497_CHAN(_chan, _addr, _ds_name) { \
-	.type = IIO_VOLTAGE, \
-	.indexed = 1, \
-	.channel = (_chan), \
-	.address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC2497_SIGN : 0)), \
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
-	.datasheet_name = (_ds_name), \
-}
-
-#define LTC2497_CHAN_DIFF(_chan, _addr) { \
-	.type = IIO_VOLTAGE, \
-	.indexed = 1, \
-	.channel = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 1 : 0), \
-	.channel2 = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 0 : 1),\
-	.address = (_addr | _chan), \
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
-	.differential = 1, \
+	ret = i2c_smbus_write_byte(st->client,
+				   LTC2497_ENABLE | address);
+	if (ret)
+		dev_err(&st->client->dev, "i2c transfer failed: %pe\n",
+			ERR_PTR(ret));
+	return ret;
 }
 
-static const struct iio_chan_spec ltc2497_channel[] = {
-	LTC2497_CHAN(0, LTC2497_SGL, "CH0"),
-	LTC2497_CHAN(1, LTC2497_SGL, "CH1"),
-	LTC2497_CHAN(2, LTC2497_SGL, "CH2"),
-	LTC2497_CHAN(3, LTC2497_SGL, "CH3"),
-	LTC2497_CHAN(4, LTC2497_SGL, "CH4"),
-	LTC2497_CHAN(5, LTC2497_SGL, "CH5"),
-	LTC2497_CHAN(6, LTC2497_SGL, "CH6"),
-	LTC2497_CHAN(7, LTC2497_SGL, "CH7"),
-	LTC2497_CHAN(8, LTC2497_SGL, "CH8"),
-	LTC2497_CHAN(9, LTC2497_SGL, "CH9"),
-	LTC2497_CHAN(10, LTC2497_SGL, "CH10"),
-	LTC2497_CHAN(11, LTC2497_SGL, "CH11"),
-	LTC2497_CHAN(12, LTC2497_SGL, "CH12"),
-	LTC2497_CHAN(13, LTC2497_SGL, "CH13"),
-	LTC2497_CHAN(14, LTC2497_SGL, "CH14"),
-	LTC2497_CHAN(15, LTC2497_SGL, "CH15"),
-	LTC2497_CHAN_DIFF(0, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(1, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(2, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(3, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(4, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(5, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(6, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(7, LTC2497_DIFF),
-	LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN),
-	LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN),
-	LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN),
-	LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN),
-	LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN),
-	LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN),
-	LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN),
-	LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN),
-};
-
-static const struct iio_info ltc2497_info = {
-	.read_raw = ltc2497_read_raw,
-};
-
 static int ltc2497_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct iio_dev *indio_dev;
-	struct ltc2497_st *st;
-	struct iio_map *plat_data;
-	int ret;
+	struct ltc2497_driverdata *st;
+	struct device *dev = &client->dev;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
 				     I2C_FUNC_SMBUS_WRITE_BYTE))
 		return -EOPNOTSUPP;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
 	i2c_set_clientdata(client, indio_dev);
 	st->client = client;
+	st->common_ddata.result_and_measure = ltc2497_result_and_measure;
 
-	indio_dev->dev.parent = &client->dev;
-	indio_dev->name = id->name;
-	indio_dev->info = &ltc2497_info;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = ltc2497_channel;
-	indio_dev->num_channels = ARRAY_SIZE(ltc2497_channel);
-
-	st->ref = devm_regulator_get(&client->dev, "vref");
-	if (IS_ERR(st->ref))
-		return PTR_ERR(st->ref);
-
-	ret = regulator_enable(st->ref);
-	if (ret < 0)
-		return ret;
-
-	if (client->dev.platform_data) {
-		plat_data = ((struct iio_map *)client->dev.platform_data);
-		ret = iio_map_array_register(indio_dev, plat_data);
-		if (ret) {
-			dev_err(&indio_dev->dev, "iio map err: %d\n", ret);
-			goto err_regulator_disable;
-		}
-	}
-
-	ret = i2c_smbus_write_byte(st->client, LTC2497_CONFIG_DEFAULT);
-	if (ret < 0)
-		goto err_array_unregister;
-
-	st->addr_prev = LTC2497_CONFIG_DEFAULT;
-	st->time_prev = ktime_get();
-
-	ret = iio_device_register(indio_dev);
-	if (ret < 0)
-		goto err_array_unregister;
-
-	return 0;
-
-err_array_unregister:
-	iio_map_array_unregister(indio_dev);
-
-err_regulator_disable:
-	regulator_disable(st->ref);
-
-	return ret;
+	return ltc2497core_probe(dev, indio_dev);
 }
 
 static int ltc2497_remove(struct i2c_client *client)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
-	struct ltc2497_st *st = iio_priv(indio_dev);
 
-	iio_map_array_unregister(indio_dev);
-	iio_device_unregister(indio_dev);
-	regulator_disable(st->ref);
+	ltc2497core_remove(indio_dev);
 
 	return 0;
 }
diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
new file mode 100644
index 000000000000..d0b42dd6b8ad
--- /dev/null
+++ b/drivers/iio/adc/ltc2497.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#define LTC2497_ENABLE			0xA0
+#define LTC2497_CONFIG_DEFAULT		LTC2497_ENABLE
+#define LTC2497_CONVERSION_TIME_MS	150ULL
+
+struct ltc2497core_driverdata {
+	struct regulator *ref;
+	ktime_t	time_prev;
+	u8 addr_prev;
+	int (*result_and_measure)(struct ltc2497core_driverdata *ddata,
+				  u8 address, int *val);
+};
+
+int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev);
+void ltc2497core_remove(struct iio_dev *indio_dev);
+
+MODULE_IMPORT_NS(LTC2497);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v4 3/3] iio: adc: new driver to support Linear technology's ltc2496
  2019-12-09 20:32 [PATCH v4 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König
  2019-12-09 20:32 ` [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König
  2019-12-09 20:32 ` [PATCH v4 2/3] iio: adc: ltc2497: split protocol independent part in a separate module Uwe Kleine-König
@ 2019-12-09 20:32 ` Uwe Kleine-König
  2019-12-15 12:16   ` Jonathan Cameron
  2 siblings, 1 reply; 13+ messages in thread
From: Uwe Kleine-König @ 2019-12-09 20:32 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	Michael Hennerich, Stefan Popa, Alexandru Ardelean
  Cc: linux-iio, kernel

This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C
and so has a slightly different protocol. Only the actual hardware
access is different. The spi protocol is different enough to not be able
to map the differences via a regmap.

Also generalize the entry in MAINTAINER to cover the newly introduced
file.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 MAINTAINERS               |   2 +-
 drivers/iio/adc/Kconfig   |  10 ++++
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/ltc2496.c | 108 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/adc/ltc2496.c

diff --git a/MAINTAINERS b/MAINTAINERS
index eb19fad370d7..8b1211038617 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1029,7 +1029,7 @@ S:	Supported
 F:	Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9523
 F:	Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4350
 F:	drivers/iio/*/ad*
-F:	drivers/iio/adc/ltc2497*
+F:	drivers/iio/adc/ltc249*
 X:	drivers/iio/*/adjd*
 F:	drivers/staging/iio/*/ad*
 
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f0af3a42f53c..deb86f6039b3 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -492,6 +492,16 @@ config LTC2485
 	  To compile this driver as a module, choose M here: the module will be
 	  called ltc2485.
 
+config LTC2496
+	tristate "Linear Technology LTC2496 ADC driver"
+	depends on SPI
+	help
+	  Say yes here to build support for Linear Technology LTC2496
+	  16-Bit 8-/16-Channel Delta Sigma ADC.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ltc2496.
+
 config LTC2497
 	tristate "Linear Technology LTC2497 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ee0c8dcfb501..c3dc2a12766a 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
 obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
 obj-$(CONFIG_LTC2471) += ltc2471.o
 obj-$(CONFIG_LTC2485) += ltc2485.o
+obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o
 obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
diff --git a/drivers/iio/adc/ltc2496.c b/drivers/iio/adc/ltc2496.c
new file mode 100644
index 000000000000..88a30156a849
--- /dev/null
+++ b/drivers/iio/adc/ltc2496.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ltc2496.c - Driver for Analog Devices/Linear Technology LTC2496 ADC
+ *
+ * Based on ltc2497.c which has
+ * Copyright (C) 2017 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2.
+ *
+ * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2496fc.pdf
+ */
+
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/driver.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include "ltc2497.h"
+
+struct ltc2496_driverdata {
+	/* this must be the first member */
+	struct ltc2497core_driverdata common_ddata;
+	struct spi_device *spi;
+
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
+	 */
+	unsigned char rxbuf[3] ____cacheline_aligned;
+	unsigned char txbuf[3];
+};
+
+static int ltc2496_result_and_measure(struct ltc2497core_driverdata *ddata,
+				      u8 address, int *val)
+{
+	struct ltc2496_driverdata *st =
+		container_of(ddata, struct ltc2496_driverdata, common_ddata);
+	struct spi_transfer t = {
+		.tx_buf = st->txbuf,
+		.rx_buf = st->rxbuf,
+		.len = sizeof(st->txbuf),
+	};
+	int ret;
+
+	st->txbuf[0] = LTC2497_ENABLE | address;
+
+	ret = spi_sync_transfer(st->spi, &t, 1);
+	if (ret < 0)  {
+		dev_err(&st->spi->dev, "spi_sync_transfer failed: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+
+	if (val)
+		*val = ((st->rxbuf[0] & 0x3f) << 12 |
+			st->rxbuf[1] << 4 | st->rxbuf[2] >> 4) -
+			(1 << 17);
+
+	return 0;
+}
+
+static int ltc2496_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ltc2496_driverdata *st;
+	struct device *dev = &spi->dev;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	spi_set_drvdata(spi, indio_dev);
+	st->spi = spi;
+	st->common_ddata.result_and_measure = ltc2496_result_and_measure;
+
+	return ltc2497core_probe(dev, indio_dev);
+}
+
+static int ltc2496_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	ltc2497core_remove(indio_dev);
+
+	return 0;
+}
+
+static const struct of_device_id ltc2496_of_match[] = {
+	{ .compatible = "lltc,ltc2496", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ltc2496_of_match);
+
+static struct spi_driver ltc2496_driver = {
+	.driver = {
+		.name = "ltc2496",
+		.of_match_table = of_match_ptr(ltc2496_of_match),
+	},
+	.probe = ltc2496_probe,
+	.remove = ltc2496_remove,
+};
+module_spi_driver(ltc2496_driver);
+
+MODULE_AUTHOR("Uwe Kleine-König <u.kleine-könig@pengutronix.de>");
+MODULE_DESCRIPTION("Linear Technology LTC2496 ADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document
  2019-12-09 20:32 ` [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König
@ 2019-12-15 11:49   ` Jonathan Cameron
  2019-12-16  8:37     ` Uwe Kleine-König
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2019-12-15 11:49 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa,
	Alexandru Ardelean, linux-iio, kernel

On Mon,  9 Dec 2019 21:32:46 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> The ADC only requires the standard stuff for spi devices and a reference
> voltage.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks for figuring out what was wrong and fixing it up.

I've made a couple of small tweaks whilst applying.  See below.

Applied to the togreg branch of iio.git and pushed out as testing.
I'm not planning to do a pull request until at least next weekend
so plenty of time if a DT maintainer wants to take another look.

Thanks,

Jonathan

> ---
>  .../bindings/iio/adc/lltc,ltc2496.yaml        | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
> new file mode 100644
> index 000000000000..195c8c8f2f4c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/lltc,ltc2496.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Linear Technology / Analog Devices LTC2496 ADC
> +
> +maintainers:
> + - Lars-Peter Clausen <lars@metafoo.de>
> + - Michael Hennerich <Michael.Hennerich@analog.com>
> + - Stefan Popa <stefan.popa@analog.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - lltc,ltc2496
> +
> +  vref-supply:
> +    description: phandle to an external regulator providing the reference voltage
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/phandle
> +
> +  reg:
> +    description: spi chipselect number according to the usual spi bindings
> +
> +  spi-max-frequency:
> +    description: maximal spi bus frequency supported by the chip

dropped the "by the chip" as this is also about the wiring on the board.  If it
were just the chip, it could be put in the driver.  The unknown bit is
if there is some other reason why it might need to be set lower than the maximum.

> +
> +required:
> +  - compatible
> +  - vref-supply
> +  - reg
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +        	compatible = "lltc,ltc2496";
You can't easily see it here, but this is a mixture of spaces
and tabs.  Should be all spaces.  I've tidied that up.
> +        	reg = <0>;
> +        	vref-supply = <&ltc2496_reg>;
> +        	spi-max-frequency = <2000000>;
> +        };
> +    };


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/3] iio: adc: ltc2497: split protocol independent part in a separate module
  2019-12-09 20:32 ` [PATCH v4 2/3] iio: adc: ltc2497: split protocol independent part in a separate module Uwe Kleine-König
@ 2019-12-15 11:55   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-12-15 11:55 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa,
	Alexandru Ardelean, linux-iio, kernel

On Mon,  9 Dec 2019 21:32:47 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> This allows to share most of this driver for the ltc2496 driver added in
> the next commit that is an SPI variant of the ltc2497. Initially I named
> the generic part ltc249x, but wild card names are frowned upon, so the
> generic part is called ltc2497-core even though it's not obvious that
> this is then to be reused for the ltc2496 driver.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/Makefile       |   2 +-
>  drivers/iio/adc/ltc2497-core.c | 243 +++++++++++++++++++++++++++++++++
>  drivers/iio/adc/ltc2497.c      | 234 ++++---------------------------
>  drivers/iio/adc/ltc2497.h      |  18 +++
>  4 files changed, 288 insertions(+), 209 deletions(-)
>  create mode 100644 drivers/iio/adc/ltc2497-core.c
>  create mode 100644 drivers/iio/adc/ltc2497.h
> 
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ef9cc485fb67..ee0c8dcfb501 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -47,7 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
>  obj-$(CONFIG_LTC2471) += ltc2471.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
> -obj-$(CONFIG_LTC2497) += ltc2497.o
> +obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1118) += max1118.o
> diff --git a/drivers/iio/adc/ltc2497-core.c b/drivers/iio/adc/ltc2497-core.c
> new file mode 100644
> index 000000000000..f5f7039caacc
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2497-core.c
> @@ -0,0 +1,243 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ltc2497-core.c - Common code for Analog Devices/Linear Technology
> + * LTC2496 and LTC2497 ADCs
> + *
> + * Copyright (C) 2017 Analog Devices Inc.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/driver.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include "ltc2497.h"
> +
> +#define LTC2497_SGL			BIT(4)
> +#define LTC2497_DIFF			0
> +#define LTC2497_SIGN			BIT(3)
> +
> +static int ltc2497core_wait_conv(struct ltc2497core_driverdata *ddata)
> +{
> +	s64 time_elapsed;
> +
> +	time_elapsed = ktime_ms_delta(ktime_get(), ddata->time_prev);
> +
> +	if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
> +		/* delay if conversion time not passed
> +		 * since last read or write
> +		 */
> +		if (msleep_interruptible(
> +		    LTC2497_CONVERSION_TIME_MS - time_elapsed))
> +			return -ERESTARTSYS;
> +
> +		return 0;
> +	}
> +
> +	if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
> +		/* We're in automatic mode -
> +		 * so the last reading is still not outdated
> +		 */
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static int ltc2497core_read(struct ltc2497core_driverdata *ddata, u8 address, int *val)
> +{
> +	int ret;
> +
> +	ret = ltc2497core_wait_conv(ddata);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret || ddata->addr_prev != address) {
> +		ret = ddata->result_and_measure(ddata, address, NULL);
> +		if (ret < 0)
> +			return ret;
> +		ddata->addr_prev = address;
> +
> +		if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS))
> +			return -ERESTARTSYS;
> +	}
> +
> +	ret = ddata->result_and_measure(ddata, address, val);
> +	if (ret < 0)
> +		return ret;
> +
> +	ddata->time_prev = ktime_get();
> +
> +	return ret;
> +}
> +
> +static int ltc2497core_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&indio_dev->mlock);
> +		ret = ltc2497core_read(ddata, chan->address, val);
> +		mutex_unlock(&indio_dev->mlock);
> +		if (ret < 0)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regulator_get_voltage(ddata->ref);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = ret / 1000;
> +		*val2 = 17;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +#define LTC2497_CHAN(_chan, _addr, _ds_name) { \
> +	.type = IIO_VOLTAGE, \
> +	.indexed = 1, \
> +	.channel = (_chan), \
> +	.address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC2497_SIGN : 0)), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +	.datasheet_name = (_ds_name), \
> +}
> +
> +#define LTC2497_CHAN_DIFF(_chan, _addr) { \
> +	.type = IIO_VOLTAGE, \
> +	.indexed = 1, \
> +	.channel = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 1 : 0), \
> +	.channel2 = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 0 : 1),\
> +	.address = (_addr | _chan), \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +	.differential = 1, \
> +}
> +
> +static const struct iio_chan_spec ltc2497core_channel[] = {
> +	LTC2497_CHAN(0, LTC2497_SGL, "CH0"),
> +	LTC2497_CHAN(1, LTC2497_SGL, "CH1"),
> +	LTC2497_CHAN(2, LTC2497_SGL, "CH2"),
> +	LTC2497_CHAN(3, LTC2497_SGL, "CH3"),
> +	LTC2497_CHAN(4, LTC2497_SGL, "CH4"),
> +	LTC2497_CHAN(5, LTC2497_SGL, "CH5"),
> +	LTC2497_CHAN(6, LTC2497_SGL, "CH6"),
> +	LTC2497_CHAN(7, LTC2497_SGL, "CH7"),
> +	LTC2497_CHAN(8, LTC2497_SGL, "CH8"),
> +	LTC2497_CHAN(9, LTC2497_SGL, "CH9"),
> +	LTC2497_CHAN(10, LTC2497_SGL, "CH10"),
> +	LTC2497_CHAN(11, LTC2497_SGL, "CH11"),
> +	LTC2497_CHAN(12, LTC2497_SGL, "CH12"),
> +	LTC2497_CHAN(13, LTC2497_SGL, "CH13"),
> +	LTC2497_CHAN(14, LTC2497_SGL, "CH14"),
> +	LTC2497_CHAN(15, LTC2497_SGL, "CH15"),
> +	LTC2497_CHAN_DIFF(0, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(1, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(2, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(3, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(4, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(5, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(6, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(7, LTC2497_DIFF),
> +	LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN),
> +	LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN),
> +	LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN),
> +	LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN),
> +	LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN),
> +	LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN),
> +	LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN),
> +	LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN),
> +};
> +
> +static const struct iio_info ltc2497core_info = {
> +	.read_raw = ltc2497core_read_raw,
> +};
> +
> +int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev)
> +{
> +	struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
> +	int ret;
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->info = &ltc2497core_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ltc2497core_channel;
> +	indio_dev->num_channels = ARRAY_SIZE(ltc2497core_channel);
> +
> +	ret = ddata->result_and_measure(ddata, LTC2497_CONFIG_DEFAULT, NULL);
> +	if (ret < 0)
> +		return ret;
> +
> +	ddata->ref = devm_regulator_get(dev, "vref");
> +	if (IS_ERR(ddata->ref)) {
> +		if (PTR_ERR(ddata->ref) != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to get vref regulator: %pe\n",
> +				ddata->ref);
> +
> +		return PTR_ERR(ddata->ref);
> +	}
> +
> +	ret = regulator_enable(ddata->ref);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable vref regulator: %pe\n",
> +			ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	if (dev->platform_data) {
> +		struct iio_map *plat_data;
> +
> +		plat_data = (struct iio_map *)dev->platform_data;
> +
> +		ret = iio_map_array_register(indio_dev, plat_data);
> +		if (ret) {
> +			dev_err(&indio_dev->dev, "iio map err: %d\n", ret);
> +			goto err_regulator_disable;
> +		}
> +	}
> +
> +	ddata->addr_prev = LTC2497_CONFIG_DEFAULT;
> +	ddata->time_prev = ktime_get();
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0)
> +		goto err_array_unregister;
> +
> +	return 0;
> +
> +err_array_unregister:
> +	iio_map_array_unregister(indio_dev);
> +
> +err_regulator_disable:
> +	regulator_disable(ddata->ref);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS(ltc2497core_probe, LTC2497);
> +
> +void ltc2497core_remove(struct iio_dev *indio_dev)
> +{
> +	struct ltc2497core_driverdata *ddata = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	iio_map_array_unregister(indio_dev);
> +
> +	regulator_disable(ddata->ref);
> +}
> +EXPORT_SYMBOL_NS(ltc2497core_remove, LTC2497);
> +
> +MODULE_DESCRIPTION("common code for LTC2496/LTC2497 drivers");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> index 470406032720..5db63d7c6bc5 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -7,27 +7,18 @@
>   * Datasheet: http://cds.linear.com/docs/en/datasheet/2497fd.pdf
>   */
>  
> -#include <linux/delay.h>
>  #include <linux/i2c.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/driver.h>
> -#include <linux/iio/sysfs.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> -#include <linux/regulator/consumer.h>
>  
> -#define LTC2497_ENABLE			0xA0
> -#define LTC2497_SGL			BIT(4)
> -#define LTC2497_DIFF			0
> -#define LTC2497_SIGN			BIT(3)
> -#define LTC2497_CONFIG_DEFAULT		LTC2497_ENABLE
> -#define LTC2497_CONVERSION_TIME_MS	150ULL
> +#include "ltc2497.h"
>  
> -struct ltc2497_st {
> +struct ltc2497_driverdata {
> +	/* this must be the first member */
> +	struct ltc2497core_driverdata common_ddata;
>  	struct i2c_client *client;
> -	struct regulator *ref;
> -	ktime_t	time_prev;
> -	u8 addr_prev;
>  	/*
>  	 * DMA (thus cache coherency maintenance) requires the
>  	 * transfer buffers to live in their own cache lines.
> @@ -35,232 +26,59 @@ struct ltc2497_st {
>  	__be32 buf ____cacheline_aligned;
>  };
>  
> -static int ltc2497_wait_conv(struct ltc2497_st *st)
> +static int ltc2497_result_and_measure(struct ltc2497core_driverdata *ddata,
> +				      u8 address, int *val)
>  {
> -	s64 time_elapsed;
> -
> -	time_elapsed = ktime_ms_delta(ktime_get(), st->time_prev);
> -
> -	if (time_elapsed < LTC2497_CONVERSION_TIME_MS) {
> -		/* delay if conversion time not passed
> -		 * since last read or write
> -		 */
> -		if (msleep_interruptible(
> -		    LTC2497_CONVERSION_TIME_MS - time_elapsed))
> -			return -ERESTARTSYS;
> -
> -		return 0;
> -	}
> -
> -	if (time_elapsed - LTC2497_CONVERSION_TIME_MS <= 0) {
> -		/* We're in automatic mode -
> -		 * so the last reading is stil not outdated
> -		 */
> -		return 0;
> -	}
> -
> -	return 1;
> -}
> -
> -static int ltc2497_read(struct ltc2497_st *st, u8 address, int *val)
> -{
> -	struct i2c_client *client = st->client;
> -	int ret;
> -
> -	ret = ltc2497_wait_conv(st);
> -	if (ret < 0)
> -		return ret;
> -
> -	if (ret || st->addr_prev != address) {
> -		ret = i2c_smbus_write_byte(st->client,
> -					   LTC2497_ENABLE | address);
> -		if (ret < 0)
> -			return ret;
> -		st->addr_prev = address;
> -		if (msleep_interruptible(LTC2497_CONVERSION_TIME_MS))
> -			return -ERESTARTSYS;
> -	}
> -	ret = i2c_master_recv(client, (char *)&st->buf, 3);
> -	if (ret < 0)  {
> -		dev_err(&client->dev, "i2c_master_recv failed\n");
> -		return ret;
> -	}
> -	st->time_prev = ktime_get();
> -
> -	/* convert and shift the result,
> -	 * and finally convert from offset binary to signed integer
> -	 */
> -	*val = (be32_to_cpu(st->buf) >> 14) - (1 << 17);
> -
> -	return ret;
> -}
> -
> -static int ltc2497_read_raw(struct iio_dev *indio_dev,
> -			    struct iio_chan_spec const *chan,
> -			    int *val, int *val2, long mask)
> -{
> -	struct ltc2497_st *st = iio_priv(indio_dev);
> +	struct ltc2497_driverdata *st =
> +		container_of(ddata, struct ltc2497_driverdata, common_ddata);
>  	int ret;
>  
> -	switch (mask) {
> -	case IIO_CHAN_INFO_RAW:
> -		mutex_lock(&indio_dev->mlock);
> -		ret = ltc2497_read(st, chan->address, val);
> -		mutex_unlock(&indio_dev->mlock);
> -		if (ret < 0)
> -			return ret;
> -
> -		return IIO_VAL_INT;
> -
> -	case IIO_CHAN_INFO_SCALE:
> -		ret = regulator_get_voltage(st->ref);
> -		if (ret < 0)
> +	if (val) {
> +		ret = i2c_master_recv(st->client, (char *)&st->buf, 3);
> +		if (ret < 0) {
> +			dev_err(&st->client->dev, "i2c_master_recv failed\n");
>  			return ret;
> +		}
>  
> -		*val = ret / 1000;
> -		*val2 = 17;
> -
> -		return IIO_VAL_FRACTIONAL_LOG2;
> -
> -	default:
> -		return -EINVAL;
> +		*val = (be32_to_cpu(st->buf) >> 14) - (1 << 17);
>  	}
> -}
>  
> -#define LTC2497_CHAN(_chan, _addr, _ds_name) { \
> -	.type = IIO_VOLTAGE, \
> -	.indexed = 1, \
> -	.channel = (_chan), \
> -	.address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC2497_SIGN : 0)), \
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> -	.datasheet_name = (_ds_name), \
> -}
> -
> -#define LTC2497_CHAN_DIFF(_chan, _addr) { \
> -	.type = IIO_VOLTAGE, \
> -	.indexed = 1, \
> -	.channel = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 1 : 0), \
> -	.channel2 = (_chan) * 2 + ((_addr) & LTC2497_SIGN ? 0 : 1),\
> -	.address = (_addr | _chan), \
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> -	.differential = 1, \
> +	ret = i2c_smbus_write_byte(st->client,
> +				   LTC2497_ENABLE | address);
> +	if (ret)
> +		dev_err(&st->client->dev, "i2c transfer failed: %pe\n",
> +			ERR_PTR(ret));
> +	return ret;
>  }
>  
> -static const struct iio_chan_spec ltc2497_channel[] = {
> -	LTC2497_CHAN(0, LTC2497_SGL, "CH0"),
> -	LTC2497_CHAN(1, LTC2497_SGL, "CH1"),
> -	LTC2497_CHAN(2, LTC2497_SGL, "CH2"),
> -	LTC2497_CHAN(3, LTC2497_SGL, "CH3"),
> -	LTC2497_CHAN(4, LTC2497_SGL, "CH4"),
> -	LTC2497_CHAN(5, LTC2497_SGL, "CH5"),
> -	LTC2497_CHAN(6, LTC2497_SGL, "CH6"),
> -	LTC2497_CHAN(7, LTC2497_SGL, "CH7"),
> -	LTC2497_CHAN(8, LTC2497_SGL, "CH8"),
> -	LTC2497_CHAN(9, LTC2497_SGL, "CH9"),
> -	LTC2497_CHAN(10, LTC2497_SGL, "CH10"),
> -	LTC2497_CHAN(11, LTC2497_SGL, "CH11"),
> -	LTC2497_CHAN(12, LTC2497_SGL, "CH12"),
> -	LTC2497_CHAN(13, LTC2497_SGL, "CH13"),
> -	LTC2497_CHAN(14, LTC2497_SGL, "CH14"),
> -	LTC2497_CHAN(15, LTC2497_SGL, "CH15"),
> -	LTC2497_CHAN_DIFF(0, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(1, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(2, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(3, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(4, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(5, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(6, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(7, LTC2497_DIFF),
> -	LTC2497_CHAN_DIFF(0, LTC2497_DIFF | LTC2497_SIGN),
> -	LTC2497_CHAN_DIFF(1, LTC2497_DIFF | LTC2497_SIGN),
> -	LTC2497_CHAN_DIFF(2, LTC2497_DIFF | LTC2497_SIGN),
> -	LTC2497_CHAN_DIFF(3, LTC2497_DIFF | LTC2497_SIGN),
> -	LTC2497_CHAN_DIFF(4, LTC2497_DIFF | LTC2497_SIGN),
> -	LTC2497_CHAN_DIFF(5, LTC2497_DIFF | LTC2497_SIGN),
> -	LTC2497_CHAN_DIFF(6, LTC2497_DIFF | LTC2497_SIGN),
> -	LTC2497_CHAN_DIFF(7, LTC2497_DIFF | LTC2497_SIGN),
> -};
> -
> -static const struct iio_info ltc2497_info = {
> -	.read_raw = ltc2497_read_raw,
> -};
> -
>  static int ltc2497_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct iio_dev *indio_dev;
> -	struct ltc2497_st *st;
> -	struct iio_map *plat_data;
> -	int ret;
> +	struct ltc2497_driverdata *st;
> +	struct device *dev = &client->dev;
>  
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
>  				     I2C_FUNC_SMBUS_WRITE_BYTE))
>  		return -EOPNOTSUPP;
>  
> -	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
>  	i2c_set_clientdata(client, indio_dev);
>  	st->client = client;
> +	st->common_ddata.result_and_measure = ltc2497_result_and_measure;
>  
> -	indio_dev->dev.parent = &client->dev;
> -	indio_dev->name = id->name;
> -	indio_dev->info = &ltc2497_info;
> -	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = ltc2497_channel;
> -	indio_dev->num_channels = ARRAY_SIZE(ltc2497_channel);
> -
> -	st->ref = devm_regulator_get(&client->dev, "vref");
> -	if (IS_ERR(st->ref))
> -		return PTR_ERR(st->ref);
> -
> -	ret = regulator_enable(st->ref);
> -	if (ret < 0)
> -		return ret;
> -
> -	if (client->dev.platform_data) {
> -		plat_data = ((struct iio_map *)client->dev.platform_data);
> -		ret = iio_map_array_register(indio_dev, plat_data);
> -		if (ret) {
> -			dev_err(&indio_dev->dev, "iio map err: %d\n", ret);
> -			goto err_regulator_disable;
> -		}
> -	}
> -
> -	ret = i2c_smbus_write_byte(st->client, LTC2497_CONFIG_DEFAULT);
> -	if (ret < 0)
> -		goto err_array_unregister;
> -
> -	st->addr_prev = LTC2497_CONFIG_DEFAULT;
> -	st->time_prev = ktime_get();
> -
> -	ret = iio_device_register(indio_dev);
> -	if (ret < 0)
> -		goto err_array_unregister;
> -
> -	return 0;
> -
> -err_array_unregister:
> -	iio_map_array_unregister(indio_dev);
> -
> -err_regulator_disable:
> -	regulator_disable(st->ref);
> -
> -	return ret;
> +	return ltc2497core_probe(dev, indio_dev);
>  }
>  
>  static int ltc2497_remove(struct i2c_client *client)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> -	struct ltc2497_st *st = iio_priv(indio_dev);
>  
> -	iio_map_array_unregister(indio_dev);
> -	iio_device_unregister(indio_dev);
> -	regulator_disable(st->ref);
> +	ltc2497core_remove(indio_dev);
>  
>  	return 0;
>  }
> diff --git a/drivers/iio/adc/ltc2497.h b/drivers/iio/adc/ltc2497.h
> new file mode 100644
> index 000000000000..d0b42dd6b8ad
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2497.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#define LTC2497_ENABLE			0xA0
> +#define LTC2497_CONFIG_DEFAULT		LTC2497_ENABLE
> +#define LTC2497_CONVERSION_TIME_MS	150ULL
> +
> +struct ltc2497core_driverdata {
> +	struct regulator *ref;
> +	ktime_t	time_prev;
> +	u8 addr_prev;
> +	int (*result_and_measure)(struct ltc2497core_driverdata *ddata,
> +				  u8 address, int *val);
> +};
> +
> +int ltc2497core_probe(struct device *dev, struct iio_dev *indio_dev);
> +void ltc2497core_remove(struct iio_dev *indio_dev);
> +
> +MODULE_IMPORT_NS(LTC2497);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 3/3] iio: adc: new driver to support Linear technology's ltc2496
  2019-12-09 20:32 ` [PATCH v4 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König
@ 2019-12-15 12:16   ` Jonathan Cameron
  2019-12-16  8:40     ` Uwe Kleine-König
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2019-12-15 12:16 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Mark Rutland, Michael Hennerich, Stefan Popa,
	Alexandru Ardelean, linux-iio, kernel

On Mon,  9 Dec 2019 21:32:48 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C
> and so has a slightly different protocol. Only the actual hardware
> access is different. The spi protocol is different enough to not be able
> to map the differences via a regmap.
> 
> Also generalize the entry in MAINTAINER to cover the newly introduced
> file.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Hmm. I'm a little surprised this doesn't run into the issues we used
to have with single files being linked twice and the interactions
between built in drivers and modules.

However I've just tried every combination on x86_64 and it seems to
be fine.  Perhaps something got made more flexible in the build system
recently that I missed, or there is a subtle detail that is different
about this particular case.

With that in mind, applied to the togreg branch of iio.git and pushed
out as testing to let the autobuilders play with it.

Thanks,

Jonathan

> ---
>  MAINTAINERS               |   2 +-
>  drivers/iio/adc/Kconfig   |  10 ++++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/ltc2496.c | 108 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 120 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/iio/adc/ltc2496.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eb19fad370d7..8b1211038617 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1029,7 +1029,7 @@ S:	Supported
>  F:	Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9523
>  F:	Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4350
>  F:	drivers/iio/*/ad*
> -F:	drivers/iio/adc/ltc2497*
> +F:	drivers/iio/adc/ltc249*
>  X:	drivers/iio/*/adjd*
>  F:	drivers/staging/iio/*/ad*
>  
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f0af3a42f53c..deb86f6039b3 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -492,6 +492,16 @@ config LTC2485
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called ltc2485.
>  
> +config LTC2496
> +	tristate "Linear Technology LTC2496 ADC driver"
> +	depends on SPI
> +	help
> +	  Say yes here to build support for Linear Technology LTC2496
> +	  16-Bit 8-/16-Channel Delta Sigma ADC.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called ltc2496.
> +
>  config LTC2497
>  	tristate "Linear Technology LTC2497 ADC driver"
>  	depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ee0c8dcfb501..c3dc2a12766a 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -47,6 +47,7 @@ obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
>  obj-$(CONFIG_LPC32XX_ADC) += lpc32xx_adc.o
>  obj-$(CONFIG_LTC2471) += ltc2471.o
>  obj-$(CONFIG_LTC2485) += ltc2485.o
> +obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o
>  obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX11100) += max11100.o
> diff --git a/drivers/iio/adc/ltc2496.c b/drivers/iio/adc/ltc2496.c
> new file mode 100644
> index 000000000000..88a30156a849
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2496.c
> @@ -0,0 +1,108 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ltc2496.c - Driver for Analog Devices/Linear Technology LTC2496 ADC
> + *
> + * Based on ltc2497.c which has
> + * Copyright (C) 2017 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2.
> + *
> + * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/2496fc.pdf
> + */
> +
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/driver.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "ltc2497.h"
> +
> +struct ltc2496_driverdata {
> +	/* this must be the first member */
> +	struct ltc2497core_driverdata common_ddata;
> +	struct spi_device *spi;
> +
> +	/*
> +	 * DMA (thus cache coherency maintenance) requires the
> +	 * transfer buffers to live in their own cache lines.
> +	 */
> +	unsigned char rxbuf[3] ____cacheline_aligned;
> +	unsigned char txbuf[3];
> +};
> +
> +static int ltc2496_result_and_measure(struct ltc2497core_driverdata *ddata,
> +				      u8 address, int *val)
> +{
> +	struct ltc2496_driverdata *st =
> +		container_of(ddata, struct ltc2496_driverdata, common_ddata);
> +	struct spi_transfer t = {
> +		.tx_buf = st->txbuf,
> +		.rx_buf = st->rxbuf,
> +		.len = sizeof(st->txbuf),
> +	};
> +	int ret;
> +
> +	st->txbuf[0] = LTC2497_ENABLE | address;
> +
> +	ret = spi_sync_transfer(st->spi, &t, 1);
> +	if (ret < 0)  {
> +		dev_err(&st->spi->dev, "spi_sync_transfer failed: %pe\n",
> +			ERR_PTR(ret));
> +		return ret;
> +	}
> +
> +	if (val)
> +		*val = ((st->rxbuf[0] & 0x3f) << 12 |
> +			st->rxbuf[1] << 4 | st->rxbuf[2] >> 4) -
> +			(1 << 17);
> +
> +	return 0;
> +}
> +
> +static int ltc2496_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ltc2496_driverdata *st;
> +	struct device *dev = &spi->dev;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	spi_set_drvdata(spi, indio_dev);
> +	st->spi = spi;
> +	st->common_ddata.result_and_measure = ltc2496_result_and_measure;
> +
> +	return ltc2497core_probe(dev, indio_dev);
> +}
> +
> +static int ltc2496_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	ltc2497core_remove(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ltc2496_of_match[] = {
> +	{ .compatible = "lltc,ltc2496", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ltc2496_of_match);
> +
> +static struct spi_driver ltc2496_driver = {
> +	.driver = {
> +		.name = "ltc2496",
> +		.of_match_table = of_match_ptr(ltc2496_of_match),
> +	},
> +	.probe = ltc2496_probe,
> +	.remove = ltc2496_remove,
> +};
> +module_spi_driver(ltc2496_driver);
> +
> +MODULE_AUTHOR("Uwe Kleine-König <u.kleine-könig@pengutronix.de>");
> +MODULE_DESCRIPTION("Linear Technology LTC2496 ADC driver");
> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document
  2019-12-15 11:49   ` Jonathan Cameron
@ 2019-12-16  8:37     ` Uwe Kleine-König
  2019-12-23 17:58       ` Jonathan Cameron
  0 siblings, 1 reply; 13+ messages in thread
From: Uwe Kleine-König @ 2019-12-16  8:37 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Mark Rutland, Lars-Peter Clausen, Stefan Popa, Michael Hennerich,
	linux-iio, Rob Herring, Peter Meerwald-Stadler, Hartmut Knaack,
	Alexandru Ardelean, kernel

Hello Jonathan,

On Sun, Dec 15, 2019 at 11:49:58AM +0000, Jonathan Cameron wrote:
> On Mon,  9 Dec 2019 21:32:46 +0100
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > The ADC only requires the standard stuff for spi devices and a reference
> > voltage.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Thanks for figuring out what was wrong and fixing it up.

It wasn't done primarily to do you a favour :-)

> > +  spi-max-frequency:
> > +    description: maximal spi bus frequency supported by the chip
> 
> dropped the "by the chip" as this is also about the wiring on the board.  If it
> were just the chip, it could be put in the driver.  The unknown bit is
> if there is some other reason why it might need to be set lower than the maximum.

fine for me.

> > +required:
> > +  - compatible
> > +  - vref-supply
> > +  - reg
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        adc@0 {
> > +        	compatible = "lltc,ltc2496";
> You can't easily see it here, but this is a mixture of spaces
> and tabs.  Should be all spaces.  I've tidied that up.

I did that on purpose, spaces to have the needed indention for the yaml
syntax and then tabs for dts indention (as done in the dts itself, too).
I thought this to be the right mix, but this was my first yaml binding,
so you're probably right.

Best regards and thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 3/3] iio: adc: new driver to support Linear technology's ltc2496
  2019-12-15 12:16   ` Jonathan Cameron
@ 2019-12-16  8:40     ` Uwe Kleine-König
  0 siblings, 0 replies; 13+ messages in thread
From: Uwe Kleine-König @ 2019-12-16  8:40 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Mark Rutland, Lars-Peter Clausen, Stefan Popa, Michael Hennerich,
	linux-iio, Rob Herring, Peter Meerwald-Stadler, Hartmut Knaack,
	Alexandru Ardelean, kernel

Hello Jonathan,

On Sun, Dec 15, 2019 at 12:16:12PM +0000, Jonathan Cameron wrote:
> On Mon,  9 Dec 2019 21:32:48 +0100
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > This chip is similar to the LTC2497 ADC, it just uses SPI instead of I2C
> > and so has a slightly different protocol. Only the actual hardware
> > access is different. The spi protocol is different enough to not be able
> > to map the differences via a regmap.
> > 
> > Also generalize the entry in MAINTAINER to cover the newly introduced
> > file.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Hmm. I'm a little surprised this doesn't run into the issues we used
> to have with single files being linked twice and the interactions
> between built in drivers and modules.

I had in mind that is didn't work in the past, too. But like you, I also
didn't find a broken config and so kept it as is.

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document
  2019-12-16  8:37     ` Uwe Kleine-König
@ 2019-12-23 17:58       ` Jonathan Cameron
  2020-01-09 20:59         ` Uwe Kleine-König
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2019-12-23 17:58 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Mark Rutland, Lars-Peter Clausen, Stefan Popa, Michael Hennerich,
	linux-iio, Rob Herring, Peter Meerwald-Stadler, Hartmut Knaack,
	Alexandru Ardelean, kernel

On Mon, 16 Dec 2019 09:37:36 +0100
Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:

> Hello Jonathan,
> 
> On Sun, Dec 15, 2019 at 11:49:58AM +0000, Jonathan Cameron wrote:
> > On Mon,  9 Dec 2019 21:32:46 +0100
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> >   
> > > The ADC only requires the standard stuff for spi devices and a reference
> > > voltage.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>  
> > Thanks for figuring out what was wrong and fixing it up.  
> 
> It wasn't done primarily to do you a favour :-)
> 
> > > +  spi-max-frequency:
> > > +    description: maximal spi bus frequency supported by the chip  
> > 
> > dropped the "by the chip" as this is also about the wiring on the board.  If it
> > were just the chip, it could be put in the driver.  The unknown bit is
> > if there is some other reason why it might need to be set lower than the maximum.  
> 
> fine for me.
> 
> > > +required:
> > > +  - compatible
> > > +  - vref-supply
> > > +  - reg
> > > +
> > > +examples:
> > > +  - |
> > > +    spi {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        adc@0 {
> > > +        	compatible = "lltc,ltc2496";  
> > You can't easily see it here, but this is a mixture of spaces
> > and tabs.  Should be all spaces.  I've tidied that up.  
> 
> I did that on purpose, spaces to have the needed indention for the yaml
> syntax and then tabs for dts indention (as done in the dts itself, too).
> I thought this to be the right mix, but this was my first yaml binding,
> so you're probably right.

I'll be honest, I'm far from sure on this.

I usually rely on Rob moaning about it if it's wrong :)

Oh well. What's there builds. If we get a 'fix' later then so be it.

Thanks,

Jonathan

> 
> Best regards and thanks
> Uwe
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document
  2019-12-23 17:58       ` Jonathan Cameron
@ 2020-01-09 20:59         ` Uwe Kleine-König
  2020-01-10 10:01           ` Jonathan Cameron
  0 siblings, 1 reply; 13+ messages in thread
From: Uwe Kleine-König @ 2020-01-09 20:59 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Mark Rutland, Lars-Peter Clausen, Stefan Popa, Michael Hennerich,
	linux-iio, Rob Herring, Peter Meerwald-Stadler, Hartmut Knaack,
	Alexandru Ardelean, kernel

Hello Jonathan,

On Mon, Dec 23, 2019 at 05:58:54PM +0000, Jonathan Cameron wrote:
> On Mon, 16 Dec 2019 09:37:36 +0100
> Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:
> 
> > Hello Jonathan,
> > 
> > On Sun, Dec 15, 2019 at 11:49:58AM +0000, Jonathan Cameron wrote:
> > > On Mon,  9 Dec 2019 21:32:46 +0100
> > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > >   
> > > > The ADC only requires the standard stuff for spi devices and a reference
> > > > voltage.
> > > > 
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>  
> > > Thanks for figuring out what was wrong and fixing it up.  
> > 
> > It wasn't done primarily to do you a favour :-)
> > 
> > > > +  spi-max-frequency:
> > > > +    description: maximal spi bus frequency supported by the chip  
> > > 
> > > dropped the "by the chip" as this is also about the wiring on the board.  If it
> > > were just the chip, it could be put in the driver.  The unknown bit is
> > > if there is some other reason why it might need to be set lower than the maximum.  
> > 
> > fine for me.
> > 
> > > > +required:
> > > > +  - compatible
> > > > +  - vref-supply
> > > > +  - reg
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    spi {
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +
> > > > +        adc@0 {
> > > > +        	compatible = "lltc,ltc2496";  
> > > You can't easily see it here, but this is a mixture of spaces
> > > and tabs.  Should be all spaces.  I've tidied that up.  
> > 
> > I did that on purpose, spaces to have the needed indention for the yaml
> > syntax and then tabs for dts indention (as done in the dts itself, too).
> > I thought this to be the right mix, but this was my first yaml binding,
> > so you're probably right.
> 
> I'll be honest, I'm far from sure on this.
> 
> I usually rely on Rob moaning about it if it's wrong :)
> 
> Oh well. What's there builds. If we get a 'fix' later then so be it.

I found my commits in your tree (at
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
and wonder why they don't appear in next. IMHO it would be good to have
some exposure there to give more people the opportunity to moan. Is this
a mistake that your tree is not included in next?

I assume being in your testing branch means I can reasonably expect they
will go into 5.6-rc1?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document
  2020-01-09 20:59         ` Uwe Kleine-König
@ 2020-01-10 10:01           ` Jonathan Cameron
  2020-01-10 10:24             ` Uwe Kleine-König
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2020-01-10 10:01 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jonathan Cameron, Mark Rutland, Lars-Peter Clausen, Stefan Popa,
	Michael Hennerich, linux-iio, Rob Herring,
	Peter Meerwald-Stadler, Hartmut Knaack, Alexandru Ardelean,
	kernel

On Thu, 9 Jan 2020 21:59:54 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> Hello Jonathan,
> 
> On Mon, Dec 23, 2019 at 05:58:54PM +0000, Jonathan Cameron wrote:
> > On Mon, 16 Dec 2019 09:37:36 +0100
> > Uwe Kleine-König         <u.kleine-koenig@pengutronix.de> wrote:
> >   
> > > Hello Jonathan,
> > > 
> > > On Sun, Dec 15, 2019 at 11:49:58AM +0000, Jonathan Cameron wrote:  
> > > > On Mon,  9 Dec 2019 21:32:46 +0100
> > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > > >     
> > > > > The ADC only requires the standard stuff for spi devices and a reference
> > > > > voltage.
> > > > > 
> > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>    
> > > > Thanks for figuring out what was wrong and fixing it up.    
> > > 
> > > It wasn't done primarily to do you a favour :-)
> > >   
> > > > > +  spi-max-frequency:
> > > > > +    description: maximal spi bus frequency supported by the chip    
> > > > 
> > > > dropped the "by the chip" as this is also about the wiring on the board.  If it
> > > > were just the chip, it could be put in the driver.  The unknown bit is
> > > > if there is some other reason why it might need to be set lower than the maximum.    
> > > 
> > > fine for me.
> > >   
> > > > > +required:
> > > > > +  - compatible
> > > > > +  - vref-supply
> > > > > +  - reg
> > > > > +
> > > > > +examples:
> > > > > +  - |
> > > > > +    spi {
> > > > > +        #address-cells = <1>;
> > > > > +        #size-cells = <0>;
> > > > > +
> > > > > +        adc@0 {
> > > > > +        	compatible = "lltc,ltc2496";    
> > > > You can't easily see it here, but this is a mixture of spaces
> > > > and tabs.  Should be all spaces.  I've tidied that up.    
> > > 
> > > I did that on purpose, spaces to have the needed indention for the yaml
> > > syntax and then tabs for dts indention (as done in the dts itself, too).
> > > I thought this to be the right mix, but this was my first yaml binding,
> > > so you're probably right.  
> > 
> > I'll be honest, I'm far from sure on this.
> > 
> > I usually rely on Rob moaning about it if it's wrong :)
> > 
> > Oh well. What's there builds. If we get a 'fix' later then so be it.  
> 
> I found my commits in your tree (at
> https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
> and wonder why they don't appear in next. IMHO it would be good to have
> some exposure there to give more people the opportunity to moan. Is this
> a mistake that your tree is not included in next?
> 
> I assume being in your testing branch means I can reasonably expect they
> will go into 5.6-rc1?

Given timing yes. I tend to do last pull request 1-2 weeks before the merge window
opens.  Greg KH takes IIO through his staging tree.  One day that might change
but for now that's how we do it.  Note that I'll carry on taking things into
the togreg branch (which first becomes visible as testing) after that, but they'll
not go anywhere until after the merge window.

There was a slight delay this time around, but Greg took the pull request this
morning so they should be in Linux-next on Monday.

They are indeed heading for 5.6-rc1.

Thanks,

Jonathan

> 
> Best regards
> Uwe
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document
  2020-01-10 10:01           ` Jonathan Cameron
@ 2020-01-10 10:24             ` Uwe Kleine-König
  0 siblings, 0 replies; 13+ messages in thread
From: Uwe Kleine-König @ 2020-01-10 10:24 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Mark Rutland, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Michael Hennerich, linux-iio, Rob Herring, Stefan Popa,
	Hartmut Knaack, Alexandru Ardelean, kernel, Jonathan Cameron

Hello Jonathan,

On Fri, Jan 10, 2020 at 10:01:48AM +0000, Jonathan Cameron wrote:
> On Thu, 9 Jan 2020 21:59:54 +0100
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> > I found my commits in your tree (at
> > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing)
> > and wonder why they don't appear in next. IMHO it would be good to have
> > some exposure there to give more people the opportunity to moan. Is this
> > a mistake that your tree is not included in next?
> > 
> > I assume being in your testing branch means I can reasonably expect they
> > will go into 5.6-rc1?
> 
> Given timing yes. I tend to do last pull request 1-2 weeks before the merge window
> opens.  Greg KH takes IIO through his staging tree.  One day that might change
> but for now that's how we do it.  Note that I'll carry on taking things into
> the togreg branch (which first becomes visible as testing) after that, but they'll
> not go anywhere until after the merge window.
> 
> There was a slight delay this time around, but Greg took the pull request this
> morning so they should be in Linux-next on Monday.
> 
> They are indeed heading for 5.6-rc1.

Thanks for your explanation and taking the patches.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-01-10 10:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 20:32 [PATCH v4 0/3] iio: adc: add support for ltc2496 Uwe Kleine-König
2019-12-09 20:32 ` [PATCH v4 1/3] iio: adc: ltc2496: provide device tree binding document Uwe Kleine-König
2019-12-15 11:49   ` Jonathan Cameron
2019-12-16  8:37     ` Uwe Kleine-König
2019-12-23 17:58       ` Jonathan Cameron
2020-01-09 20:59         ` Uwe Kleine-König
2020-01-10 10:01           ` Jonathan Cameron
2020-01-10 10:24             ` Uwe Kleine-König
2019-12-09 20:32 ` [PATCH v4 2/3] iio: adc: ltc2497: split protocol independent part in a separate module Uwe Kleine-König
2019-12-15 11:55   ` Jonathan Cameron
2019-12-09 20:32 ` [PATCH v4 3/3] iio: adc: new driver to support Linear technology's ltc2496 Uwe Kleine-König
2019-12-15 12:16   ` Jonathan Cameron
2019-12-16  8:40     ` Uwe Kleine-König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.