[PATCH v2 0/3] iio: adc: add support for ltc2496

[PATCH v2 0/3] iio: adc: add support for ltc2496

[Uwe Kleine-König]

Hello,

this is v2 of a series sent earlier starting with Message-id:
20191111214025.18310-1-u.kleine-koenig@pengutronix.de and addresses the
comments by Alexandru Ardelean. I'm a bit naughty here because I didn't
carry out all suggestions. So I didn't use ...-spi.c and -i2c.c as file
names, because this isn't about a single chip supporting both, but two
different ones supporting only one bus each. So I thought ltc2496.c,
ltc2497.c and ltc249x.c was fine.

Also I didn't merge the binding docs of ltc2496 and ltc2497 for the same
reason. I'm still not entirely happy with the (now) yaml document. I'd
like to have a statement that says: This is an spi device, so it might
have all the properties described in
Documentation/devicetree/bindings/spi/spi-controller.yaml for an SPI
device.

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        |  30 +++
 MAINTAINERS                                   |   2 +-
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   3 +-
 drivers/iio/adc/ltc2496.c                     | 100 +++++++
 drivers/iio/adc/ltc2497.c                     | 237 ++---------------
 drivers/iio/adc/ltc249x.c                     | 244 ++++++++++++++++++
 drivers/iio/adc/ltc249x.h                     |  18 ++
 8 files changed, 433 insertions(+), 211 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/ltc249x.c
 create mode 100644 drivers/iio/adc/ltc249x.h


base-commit: 31f4f5b495a62c9a8b15b1c3581acd5efeb9af8c
-- 
2.24.0

[RFC PATCH v2 1/3] iio: adc: ltc2496: provide device tree binding document

[Uwe Kleine-König]

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>
---
Hello,

I didn't say anything about stuff like spi-max-frequency, spi-cpha etc
and even reg because I wonder if there is something I can write into the
document to just reference the generic spi device binding. So I marked
as RFC.

Best regards
Uwe
 .../bindings/iio/adc/lltc,ltc2496.yaml        | 30 +++++++++++++++++++
 1 file changed, 30 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..e8acab5f04ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/lltc,ltc2496.yaml
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: GPL-2.0
+%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
+
+properties:
+  compatible:
+    enum:
+      - lltc,ltc2496
+
+  vref-supply:
+    description: phandle to an external regulator providing the reference voltage
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+  - compatible
+  - vref-supply
+
+examples:
+  - |
+	adc@0 {
+		compatible = "lltc,ltc2496";
+		reg = <0>;
+		vref-supply = <&ltc249x_reg>;
+		spi-max-frequency = <2000000>;
+	};
-- 
2.24.0

[PATCH v2 2/3] iio: adc: ltc2497: split protocol independent part in a separate module

[Uwe Kleine-König]

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.

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

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 MAINTAINERS               |   2 +-
 drivers/iio/adc/Makefile  |   2 +-
 drivers/iio/adc/ltc2497.c | 237 +++++-------------------------------
 drivers/iio/adc/ltc249x.c | 244 ++++++++++++++++++++++++++++++++++++++
 drivers/iio/adc/ltc249x.h |  18 +++
 5 files changed, 292 insertions(+), 211 deletions(-)
 create mode 100644 drivers/iio/adc/ltc249x.c
 create mode 100644 drivers/iio/adc/ltc249x.h

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/Makefile b/drivers/iio/adc/Makefile
index ef9cc485fb67..660242c2cca7 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 ltc249x.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1118) += max1118.o
diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
index 470406032720..640c512c5090 100644
--- a/drivers/iio/adc/ltc2497.c
+++ b/drivers/iio/adc/ltc2497.c
@@ -7,260 +7,79 @@
  * 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 "ltc249x.h"
 
 struct ltc2497_st {
+	/* this must be the first member */
+	struct ltc249x_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.
-	 */
-	__be32 buf ____cacheline_aligned;
 };
 
-static int ltc2497_wait_conv(struct ltc2497_st *st)
-{
-	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)
+static int ltc2497_result_and_measure(struct ltc249x_driverdata *ddata,
+				      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();
+	struct ltc2497_st *st =
+		container_of(ddata, struct ltc2497_st, common_ddata);
 
-	/* convert and shift the result,
-	 * and finally convert from offset binary to signed integer
+	/*
+	 * DMA (thus cache coherency maintenance) requires the
+	 * transfer buffers to live in their own cache lines.
 	 */
-	*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);
+	__be32 buf ____cacheline_aligned;
 	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)
+	if (val) {
+		ret = i2c_master_recv(st->client, (char *)&buf, 3);
+		if (ret < 0) {
+			dev_err(&st->client->dev, "i2c_master_recv failed\n");
 			return ret;
+		}
 
-		return IIO_VAL_INT;
-
-	case IIO_CHAN_INFO_SCALE:
-		ret = regulator_get_voltage(st->ref);
-		if (ret < 0)
-			return ret;
-
-		*val = ret / 1000;
-		*val2 = 17;
-
-		return IIO_VAL_FRACTIONAL_LOG2;
-
-	default:
-		return -EINVAL;
+		*val = (be32_to_cpu(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,
+				   LTC249X_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 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 ltc249x_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);
+	ltc249x_remove(indio_dev);
 
 	return 0;
 }
diff --git a/drivers/iio/adc/ltc249x.c b/drivers/iio/adc/ltc249x.c
new file mode 100644
index 000000000000..7af1086236c0
--- /dev/null
+++ b/drivers/iio/adc/ltc249x.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ltc249x.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 "ltc249x.h"
+
+#define LTC249X_SGL			BIT(4)
+#define LTC249X_DIFF			0
+#define LTC249X_SIGN			BIT(3)
+
+static int ltc249x_wait_conv(struct ltc249x_driverdata *ddata)
+{
+	s64 time_elapsed;
+
+	time_elapsed = ktime_ms_delta(ktime_get(), ddata->time_prev);
+
+	if (time_elapsed < LTC249X_CONVERSION_TIME_MS) {
+		/* delay if conversion time not passed
+		 * since last read or write
+		 */
+		if (msleep_interruptible(
+		    LTC249X_CONVERSION_TIME_MS - time_elapsed))
+			return -ERESTARTSYS;
+
+		return 0;
+	}
+
+	if (time_elapsed - LTC249X_CONVERSION_TIME_MS <= 0) {
+		/* We're in automatic mode -
+		 * so the last reading is still not outdated
+		 */
+		return 0;
+	}
+
+	return 1;
+}
+
+static int ltc249x_read(struct ltc249x_driverdata *ddata, u8 address, int *val)
+{
+	int ret;
+
+	ret = ltc249x_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(LTC249X_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 ltc249x_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&indio_dev->mlock);
+		ret = ltc249x_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 LTC249X_CHAN(_chan, _addr, _ds_name) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.channel = (_chan), \
+	.address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC249X_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 LTC249X_CHAN_DIFF(_chan, _addr) { \
+	.type = IIO_VOLTAGE, \
+	.indexed = 1, \
+	.channel = (_chan) * 2 + ((_addr) & LTC249X_SIGN ? 1 : 0), \
+	.channel2 = (_chan) * 2 + ((_addr) & LTC249X_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 ltc249x_channel[] = {
+	LTC249X_CHAN(0, LTC249X_SGL, "CH0"),
+	LTC249X_CHAN(1, LTC249X_SGL, "CH1"),
+	LTC249X_CHAN(2, LTC249X_SGL, "CH2"),
+	LTC249X_CHAN(3, LTC249X_SGL, "CH3"),
+	LTC249X_CHAN(4, LTC249X_SGL, "CH4"),
+	LTC249X_CHAN(5, LTC249X_SGL, "CH5"),
+	LTC249X_CHAN(6, LTC249X_SGL, "CH6"),
+	LTC249X_CHAN(7, LTC249X_SGL, "CH7"),
+	LTC249X_CHAN(8, LTC249X_SGL, "CH8"),
+	LTC249X_CHAN(9, LTC249X_SGL, "CH9"),
+	LTC249X_CHAN(10, LTC249X_SGL, "CH10"),
+	LTC249X_CHAN(11, LTC249X_SGL, "CH11"),
+	LTC249X_CHAN(12, LTC249X_SGL, "CH12"),
+	LTC249X_CHAN(13, LTC249X_SGL, "CH13"),
+	LTC249X_CHAN(14, LTC249X_SGL, "CH14"),
+	LTC249X_CHAN(15, LTC249X_SGL, "CH15"),
+	LTC249X_CHAN_DIFF(0, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(1, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(2, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(3, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(4, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(5, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(6, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(7, LTC249X_DIFF),
+	LTC249X_CHAN_DIFF(0, LTC249X_DIFF | LTC249X_SIGN),
+	LTC249X_CHAN_DIFF(1, LTC249X_DIFF | LTC249X_SIGN),
+	LTC249X_CHAN_DIFF(2, LTC249X_DIFF | LTC249X_SIGN),
+	LTC249X_CHAN_DIFF(3, LTC249X_DIFF | LTC249X_SIGN),
+	LTC249X_CHAN_DIFF(4, LTC249X_DIFF | LTC249X_SIGN),
+	LTC249X_CHAN_DIFF(5, LTC249X_DIFF | LTC249X_SIGN),
+	LTC249X_CHAN_DIFF(6, LTC249X_DIFF | LTC249X_SIGN),
+	LTC249X_CHAN_DIFF(7, LTC249X_DIFF | LTC249X_SIGN),
+};
+
+static const struct iio_info ltc249x_info = {
+	.read_raw = ltc249x_read_raw,
+};
+
+int ltc249x_probe(struct device *dev, struct iio_dev *indio_dev)
+{
+	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
+	int ret;
+
+	indio_dev->dev.parent = dev;
+	indio_dev->name = dev_name(dev);
+	indio_dev->info = &ltc249x_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ltc249x_channel;
+	indio_dev->num_channels = ARRAY_SIZE(ltc249x_channel);
+
+	ret = ddata->result_and_measure(ddata, LTC249X_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 = LTC249X_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(ltc249x_probe, LTC249X);
+
+void ltc249x_remove(struct iio_dev *indio_dev)
+{
+	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+
+	iio_map_array_unregister(indio_dev);
+
+	regulator_disable(ddata->ref);
+}
+EXPORT_SYMBOL_NS(ltc249x_remove, LTC249X);
+
+MODULE_DESCRIPTION("common code for LTC2496/LTC2497 drivers");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/adc/ltc249x.h b/drivers/iio/adc/ltc249x.h
new file mode 100644
index 000000000000..b272a3f96d03
--- /dev/null
+++ b/drivers/iio/adc/ltc249x.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#define LTC249X_ENABLE			0xA0
+#define LTC249X_CONFIG_DEFAULT		LTC249X_ENABLE
+#define LTC249X_CONVERSION_TIME_MS	150ULL
+
+struct ltc249x_driverdata {
+	struct regulator *ref;
+	ktime_t	time_prev;
+	u8 addr_prev;
+	int (*result_and_measure)(struct ltc249x_driverdata *ddata,
+				  u8 address, int *val);
+};
+
+int ltc249x_probe(struct device *dev, struct iio_dev *indio_dev);
+void ltc249x_remove(struct iio_dev *indio_dev);
+
+MODULE_IMPORT_NS(LTC249X);
-- 
2.24.0

Re: [PATCH v2 2/3] iio: adc: ltc2497: split protocol independent part in a separate module

[Jonathan Cameron]

On Thu, 14 Nov 2019 11:51:58 +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.
> 
> Also generalize the entry in MAINTAINER to cover the newly introduced
> files.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
I'm fairly sure there is no way of easily enforcing data being it's own
cacheline whilst on the stack.  I would just put that back in the generic
structure and add a note saying it's only used for the SPI users of
the core structure.

Jonathan

> ---
>  MAINTAINERS               |   2 +-
>  drivers/iio/adc/Makefile  |   2 +-
>  drivers/iio/adc/ltc2497.c | 237 +++++-------------------------------
>  drivers/iio/adc/ltc249x.c | 244 ++++++++++++++++++++++++++++++++++++++
>  drivers/iio/adc/ltc249x.h |  18 +++
>  5 files changed, 292 insertions(+), 211 deletions(-)
>  create mode 100644 drivers/iio/adc/ltc249x.c
>  create mode 100644 drivers/iio/adc/ltc249x.h
> 
> 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/Makefile b/drivers/iio/adc/Makefile
> index ef9cc485fb67..660242c2cca7 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 ltc249x.o
>  obj-$(CONFIG_MAX1027) += max1027.o
>  obj-$(CONFIG_MAX11100) += max11100.o
>  obj-$(CONFIG_MAX1118) += max1118.o
> diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> index 470406032720..640c512c5090 100644
> --- a/drivers/iio/adc/ltc2497.c
> +++ b/drivers/iio/adc/ltc2497.c
> @@ -7,260 +7,79 @@
>   * 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 "ltc249x.h"
>  
>  struct ltc2497_st {
> +	/* this must be the first member */
> +	struct ltc249x_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.
> -	 */
> -	__be32 buf ____cacheline_aligned;
>  };
>  
> -static int ltc2497_wait_conv(struct ltc2497_st *st)
> -{
> -	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)
> +static int ltc2497_result_and_measure(struct ltc249x_driverdata *ddata,
> +				      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();
> +	struct ltc2497_st *st =
> +		container_of(ddata, struct ltc2497_st, common_ddata);
>  
> -	/* convert and shift the result,
> -	 * and finally convert from offset binary to signed integer
> +	/*
> +	 * DMA (thus cache coherency maintenance) requires the
> +	 * transfer buffers to live in their own cache lines.
>  	 */
> -	*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);
> +	__be32 buf ____cacheline_aligned;

From what I recall that has no useful affect on the stack.
Can only be used to enforce padding within a structure on the
heap.  Directly allocating from the heap here would also
provide relevant guarantees.

>  	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)
> +	if (val) {
> +		ret = i2c_master_recv(st->client, (char *)&buf, 3);
> +		if (ret < 0) {
> +			dev_err(&st->client->dev, "i2c_master_recv failed\n");
>  			return ret;
> +		}
>  
> -		return IIO_VAL_INT;
> -
> -	case IIO_CHAN_INFO_SCALE:
> -		ret = regulator_get_voltage(st->ref);
> -		if (ret < 0)
> -			return ret;
> -
> -		*val = ret / 1000;
> -		*val2 = 17;
> -
> -		return IIO_VAL_FRACTIONAL_LOG2;
> -
> -	default:
> -		return -EINVAL;
> +		*val = (be32_to_cpu(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,
> +				   LTC249X_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 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 ltc249x_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);
> +	ltc249x_remove(indio_dev);
>  
>  	return 0;
>  }
> diff --git a/drivers/iio/adc/ltc249x.c b/drivers/iio/adc/ltc249x.c
> new file mode 100644
> index 000000000000..7af1086236c0
> --- /dev/null
> +++ b/drivers/iio/adc/ltc249x.c
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ltc249x.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 "ltc249x.h"
> +
> +#define LTC249X_SGL			BIT(4)
> +#define LTC249X_DIFF			0
> +#define LTC249X_SIGN			BIT(3)
> +
> +static int ltc249x_wait_conv(struct ltc249x_driverdata *ddata)
> +{
> +	s64 time_elapsed;
> +
> +	time_elapsed = ktime_ms_delta(ktime_get(), ddata->time_prev);
> +
> +	if (time_elapsed < LTC249X_CONVERSION_TIME_MS) {
> +		/* delay if conversion time not passed
> +		 * since last read or write
> +		 */
> +		if (msleep_interruptible(
> +		    LTC249X_CONVERSION_TIME_MS - time_elapsed))
> +			return -ERESTARTSYS;
> +
> +		return 0;
> +	}
> +
> +	if (time_elapsed - LTC249X_CONVERSION_TIME_MS <= 0) {
> +		/* We're in automatic mode -
> +		 * so the last reading is still not outdated
> +		 */
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static int ltc249x_read(struct ltc249x_driverdata *ddata, u8 address, int *val)
> +{
> +	int ret;
> +
> +	ret = ltc249x_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(LTC249X_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 ltc249x_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&indio_dev->mlock);
> +		ret = ltc249x_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 LTC249X_CHAN(_chan, _addr, _ds_name) { \
> +	.type = IIO_VOLTAGE, \
> +	.indexed = 1, \
> +	.channel = (_chan), \
> +	.address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC249X_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 LTC249X_CHAN_DIFF(_chan, _addr) { \
> +	.type = IIO_VOLTAGE, \
> +	.indexed = 1, \
> +	.channel = (_chan) * 2 + ((_addr) & LTC249X_SIGN ? 1 : 0), \
> +	.channel2 = (_chan) * 2 + ((_addr) & LTC249X_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 ltc249x_channel[] = {
> +	LTC249X_CHAN(0, LTC249X_SGL, "CH0"),
> +	LTC249X_CHAN(1, LTC249X_SGL, "CH1"),
> +	LTC249X_CHAN(2, LTC249X_SGL, "CH2"),
> +	LTC249X_CHAN(3, LTC249X_SGL, "CH3"),
> +	LTC249X_CHAN(4, LTC249X_SGL, "CH4"),
> +	LTC249X_CHAN(5, LTC249X_SGL, "CH5"),
> +	LTC249X_CHAN(6, LTC249X_SGL, "CH6"),
> +	LTC249X_CHAN(7, LTC249X_SGL, "CH7"),
> +	LTC249X_CHAN(8, LTC249X_SGL, "CH8"),
> +	LTC249X_CHAN(9, LTC249X_SGL, "CH9"),
> +	LTC249X_CHAN(10, LTC249X_SGL, "CH10"),
> +	LTC249X_CHAN(11, LTC249X_SGL, "CH11"),
> +	LTC249X_CHAN(12, LTC249X_SGL, "CH12"),
> +	LTC249X_CHAN(13, LTC249X_SGL, "CH13"),
> +	LTC249X_CHAN(14, LTC249X_SGL, "CH14"),
> +	LTC249X_CHAN(15, LTC249X_SGL, "CH15"),
> +	LTC249X_CHAN_DIFF(0, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(1, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(2, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(3, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(4, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(5, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(6, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(7, LTC249X_DIFF),
> +	LTC249X_CHAN_DIFF(0, LTC249X_DIFF | LTC249X_SIGN),
> +	LTC249X_CHAN_DIFF(1, LTC249X_DIFF | LTC249X_SIGN),
> +	LTC249X_CHAN_DIFF(2, LTC249X_DIFF | LTC249X_SIGN),
> +	LTC249X_CHAN_DIFF(3, LTC249X_DIFF | LTC249X_SIGN),
> +	LTC249X_CHAN_DIFF(4, LTC249X_DIFF | LTC249X_SIGN),
> +	LTC249X_CHAN_DIFF(5, LTC249X_DIFF | LTC249X_SIGN),
> +	LTC249X_CHAN_DIFF(6, LTC249X_DIFF | LTC249X_SIGN),
> +	LTC249X_CHAN_DIFF(7, LTC249X_DIFF | LTC249X_SIGN),
> +};
> +
> +static const struct iio_info ltc249x_info = {
> +	.read_raw = ltc249x_read_raw,
> +};
> +
> +int ltc249x_probe(struct device *dev, struct iio_dev *indio_dev)
> +{
> +	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
> +	int ret;
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->info = &ltc249x_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ltc249x_channel;
> +	indio_dev->num_channels = ARRAY_SIZE(ltc249x_channel);
> +
> +	ret = ddata->result_and_measure(ddata, LTC249X_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 = LTC249X_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(ltc249x_probe, LTC249X);
> +
> +void ltc249x_remove(struct iio_dev *indio_dev)
> +{
> +	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	iio_map_array_unregister(indio_dev);
> +
> +	regulator_disable(ddata->ref);
> +}
> +EXPORT_SYMBOL_NS(ltc249x_remove, LTC249X);
> +
> +MODULE_DESCRIPTION("common code for LTC2496/LTC2497 drivers");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/adc/ltc249x.h b/drivers/iio/adc/ltc249x.h
> new file mode 100644
> index 000000000000..b272a3f96d03
> --- /dev/null
> +++ b/drivers/iio/adc/ltc249x.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#define LTC249X_ENABLE			0xA0
> +#define LTC249X_CONFIG_DEFAULT		LTC249X_ENABLE
> +#define LTC249X_CONVERSION_TIME_MS	150ULL
> +
> +struct ltc249x_driverdata {
> +	struct regulator *ref;
> +	ktime_t	time_prev;
> +	u8 addr_prev;
> +	int (*result_and_measure)(struct ltc249x_driverdata *ddata,
> +				  u8 address, int *val);
> +};
> +
> +int ltc249x_probe(struct device *dev, struct iio_dev *indio_dev);
> +void ltc249x_remove(struct iio_dev *indio_dev);
> +
> +MODULE_IMPORT_NS(LTC249X);

Re: [PATCH v2 2/3] iio: adc: ltc2497: split protocol independent part in a separate module

[Jonathan Cameron]

On Sat, 16 Nov 2019 15:58:08 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Thu, 14 Nov 2019 11:51:58 +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.
> > 
> > Also generalize the entry in MAINTAINER to cover the newly introduced
> > files.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>  
> I'm fairly sure there is no way of easily enforcing data being it's own
> cacheline whilst on the stack.  I would just put that back in the generic
> structure and add a note saying it's only used for the SPI users of
> the core structure.
Which is odd given this is an i2c driver (so far).  Whilst wolfram
has added the option to do direct DMA on i2c devices, I don't think this
driver is using it anyway.  Hence that ___cacheline_aligned magic
probably wasn't needed here in the first place.

The SPI driver however definitely needs those guarantees. Comments in patch 3.

Jonathan

> 
> Jonathan
> 
> > ---
> >  MAINTAINERS               |   2 +-
> >  drivers/iio/adc/Makefile  |   2 +-
> >  drivers/iio/adc/ltc2497.c | 237 +++++-------------------------------
> >  drivers/iio/adc/ltc249x.c | 244 ++++++++++++++++++++++++++++++++++++++
> >  drivers/iio/adc/ltc249x.h |  18 +++
> >  5 files changed, 292 insertions(+), 211 deletions(-)
> >  create mode 100644 drivers/iio/adc/ltc249x.c
> >  create mode 100644 drivers/iio/adc/ltc249x.h
> > 
> > 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/Makefile b/drivers/iio/adc/Makefile
> > index ef9cc485fb67..660242c2cca7 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 ltc249x.o
> >  obj-$(CONFIG_MAX1027) += max1027.o
> >  obj-$(CONFIG_MAX11100) += max11100.o
> >  obj-$(CONFIG_MAX1118) += max1118.o
> > diff --git a/drivers/iio/adc/ltc2497.c b/drivers/iio/adc/ltc2497.c
> > index 470406032720..640c512c5090 100644
> > --- a/drivers/iio/adc/ltc2497.c
> > +++ b/drivers/iio/adc/ltc2497.c
> > @@ -7,260 +7,79 @@
> >   * 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 "ltc249x.h"
> >  
> >  struct ltc2497_st {
> > +	/* this must be the first member */
> > +	struct ltc249x_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.
> > -	 */
> > -	__be32 buf ____cacheline_aligned;
> >  };
> >  
> > -static int ltc2497_wait_conv(struct ltc2497_st *st)
> > -{
> > -	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)
> > +static int ltc2497_result_and_measure(struct ltc249x_driverdata *ddata,
> > +				      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();
> > +	struct ltc2497_st *st =
> > +		container_of(ddata, struct ltc2497_st, common_ddata);
> >  
> > -	/* convert and shift the result,
> > -	 * and finally convert from offset binary to signed integer
> > +	/*
> > +	 * DMA (thus cache coherency maintenance) requires the
> > +	 * transfer buffers to live in their own cache lines.
> >  	 */
> > -	*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);
> > +	__be32 buf ____cacheline_aligned;  
> 
> From what I recall that has no useful affect on the stack.
> Can only be used to enforce padding within a structure on the
> heap.  Directly allocating from the heap here would also
> provide relevant guarantees.
> 
> >  	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)
> > +	if (val) {
> > +		ret = i2c_master_recv(st->client, (char *)&buf, 3);
> > +		if (ret < 0) {
> > +			dev_err(&st->client->dev, "i2c_master_recv failed\n");
> >  			return ret;
> > +		}
> >  
> > -		return IIO_VAL_INT;
> > -
> > -	case IIO_CHAN_INFO_SCALE:
> > -		ret = regulator_get_voltage(st->ref);
> > -		if (ret < 0)
> > -			return ret;
> > -
> > -		*val = ret / 1000;
> > -		*val2 = 17;
> > -
> > -		return IIO_VAL_FRACTIONAL_LOG2;
> > -
> > -	default:
> > -		return -EINVAL;
> > +		*val = (be32_to_cpu(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,
> > +				   LTC249X_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 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 ltc249x_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);
> > +	ltc249x_remove(indio_dev);
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/iio/adc/ltc249x.c b/drivers/iio/adc/ltc249x.c
> > new file mode 100644
> > index 000000000000..7af1086236c0
> > --- /dev/null
> > +++ b/drivers/iio/adc/ltc249x.c
> > @@ -0,0 +1,244 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * ltc249x.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 "ltc249x.h"
> > +
> > +#define LTC249X_SGL			BIT(4)
> > +#define LTC249X_DIFF			0
> > +#define LTC249X_SIGN			BIT(3)
> > +
> > +static int ltc249x_wait_conv(struct ltc249x_driverdata *ddata)
> > +{
> > +	s64 time_elapsed;
> > +
> > +	time_elapsed = ktime_ms_delta(ktime_get(), ddata->time_prev);
> > +
> > +	if (time_elapsed < LTC249X_CONVERSION_TIME_MS) {
> > +		/* delay if conversion time not passed
> > +		 * since last read or write
> > +		 */
> > +		if (msleep_interruptible(
> > +		    LTC249X_CONVERSION_TIME_MS - time_elapsed))
> > +			return -ERESTARTSYS;
> > +
> > +		return 0;
> > +	}
> > +
> > +	if (time_elapsed - LTC249X_CONVERSION_TIME_MS <= 0) {
> > +		/* We're in automatic mode -
> > +		 * so the last reading is still not outdated
> > +		 */
> > +		return 0;
> > +	}
> > +
> > +	return 1;
> > +}
> > +
> > +static int ltc249x_read(struct ltc249x_driverdata *ddata, u8 address, int *val)
> > +{
> > +	int ret;
> > +
> > +	ret = ltc249x_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(LTC249X_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 ltc249x_read_raw(struct iio_dev *indio_dev,
> > +			    struct iio_chan_spec const *chan,
> > +			    int *val, int *val2, long mask)
> > +{
> > +	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		mutex_lock(&indio_dev->mlock);
> > +		ret = ltc249x_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 LTC249X_CHAN(_chan, _addr, _ds_name) { \
> > +	.type = IIO_VOLTAGE, \
> > +	.indexed = 1, \
> > +	.channel = (_chan), \
> > +	.address = (_addr | (_chan / 2) | ((_chan & 1) ? LTC249X_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 LTC249X_CHAN_DIFF(_chan, _addr) { \
> > +	.type = IIO_VOLTAGE, \
> > +	.indexed = 1, \
> > +	.channel = (_chan) * 2 + ((_addr) & LTC249X_SIGN ? 1 : 0), \
> > +	.channel2 = (_chan) * 2 + ((_addr) & LTC249X_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 ltc249x_channel[] = {
> > +	LTC249X_CHAN(0, LTC249X_SGL, "CH0"),
> > +	LTC249X_CHAN(1, LTC249X_SGL, "CH1"),
> > +	LTC249X_CHAN(2, LTC249X_SGL, "CH2"),
> > +	LTC249X_CHAN(3, LTC249X_SGL, "CH3"),
> > +	LTC249X_CHAN(4, LTC249X_SGL, "CH4"),
> > +	LTC249X_CHAN(5, LTC249X_SGL, "CH5"),
> > +	LTC249X_CHAN(6, LTC249X_SGL, "CH6"),
> > +	LTC249X_CHAN(7, LTC249X_SGL, "CH7"),
> > +	LTC249X_CHAN(8, LTC249X_SGL, "CH8"),
> > +	LTC249X_CHAN(9, LTC249X_SGL, "CH9"),
> > +	LTC249X_CHAN(10, LTC249X_SGL, "CH10"),
> > +	LTC249X_CHAN(11, LTC249X_SGL, "CH11"),
> > +	LTC249X_CHAN(12, LTC249X_SGL, "CH12"),
> > +	LTC249X_CHAN(13, LTC249X_SGL, "CH13"),
> > +	LTC249X_CHAN(14, LTC249X_SGL, "CH14"),
> > +	LTC249X_CHAN(15, LTC249X_SGL, "CH15"),
> > +	LTC249X_CHAN_DIFF(0, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(1, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(2, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(3, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(4, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(5, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(6, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(7, LTC249X_DIFF),
> > +	LTC249X_CHAN_DIFF(0, LTC249X_DIFF | LTC249X_SIGN),
> > +	LTC249X_CHAN_DIFF(1, LTC249X_DIFF | LTC249X_SIGN),
> > +	LTC249X_CHAN_DIFF(2, LTC249X_DIFF | LTC249X_SIGN),
> > +	LTC249X_CHAN_DIFF(3, LTC249X_DIFF | LTC249X_SIGN),
> > +	LTC249X_CHAN_DIFF(4, LTC249X_DIFF | LTC249X_SIGN),
> > +	LTC249X_CHAN_DIFF(5, LTC249X_DIFF | LTC249X_SIGN),
> > +	LTC249X_CHAN_DIFF(6, LTC249X_DIFF | LTC249X_SIGN),
> > +	LTC249X_CHAN_DIFF(7, LTC249X_DIFF | LTC249X_SIGN),
> > +};
> > +
> > +static const struct iio_info ltc249x_info = {
> > +	.read_raw = ltc249x_read_raw,
> > +};
> > +
> > +int ltc249x_probe(struct device *dev, struct iio_dev *indio_dev)
> > +{
> > +	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	indio_dev->dev.parent = dev;
> > +	indio_dev->name = dev_name(dev);
> > +	indio_dev->info = &ltc249x_info;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->channels = ltc249x_channel;
> > +	indio_dev->num_channels = ARRAY_SIZE(ltc249x_channel);
> > +
> > +	ret = ddata->result_and_measure(ddata, LTC249X_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 = LTC249X_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(ltc249x_probe, LTC249X);
> > +
> > +void ltc249x_remove(struct iio_dev *indio_dev)
> > +{
> > +	struct ltc249x_driverdata *ddata = iio_priv(indio_dev);
> > +
> > +	iio_device_unregister(indio_dev);
> > +
> > +	iio_map_array_unregister(indio_dev);
> > +
> > +	regulator_disable(ddata->ref);
> > +}
> > +EXPORT_SYMBOL_NS(ltc249x_remove, LTC249X);
> > +
> > +MODULE_DESCRIPTION("common code for LTC2496/LTC2497 drivers");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/iio/adc/ltc249x.h b/drivers/iio/adc/ltc249x.h
> > new file mode 100644
> > index 000000000000..b272a3f96d03
> > --- /dev/null
> > +++ b/drivers/iio/adc/ltc249x.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#define LTC249X_ENABLE			0xA0
> > +#define LTC249X_CONFIG_DEFAULT		LTC249X_ENABLE
> > +#define LTC249X_CONVERSION_TIME_MS	150ULL
> > +
> > +struct ltc249x_driverdata {
> > +	struct regulator *ref;
> > +	ktime_t	time_prev;
> > +	u8 addr_prev;
> > +	int (*result_and_measure)(struct ltc249x_driverdata *ddata,
> > +				  u8 address, int *val);
> > +};
> > +
> > +int ltc249x_probe(struct device *dev, struct iio_dev *indio_dev);
> > +void ltc249x_remove(struct iio_dev *indio_dev);
> > +
> > +MODULE_IMPORT_NS(LTC249X);  
> 

[PATCH v2 3/3] iio: adc: new driver to support Linear technology's ltc2496

[Uwe Kleine-König]

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.

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

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 660242c2cca7..afe2b6db4a5e 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 ltc249x.o
 obj-$(CONFIG_LTC2497) += ltc2497.o ltc249x.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..870526c6df1b
--- /dev/null
+++ b/drivers/iio/adc/ltc2496.c
@@ -0,0 +1,100 @@
+// 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 "ltc249x.h"
+
+struct ltc2496_st {
+	/* this must be the first member */
+	struct ltc249x_driverdata common_ddata;
+	struct spi_device *spi;
+};
+
+static int ltc2496_result_and_measure(struct ltc249x_driverdata *ddata,
+				      u8 address, int *val)
+{
+	struct ltc2496_st *st =
+		container_of(ddata, struct ltc2496_st, common_ddata);
+	unsigned char txbuf[3] = { LTC249X_ENABLE | address, };
+	unsigned char rxbuf[3];
+	struct spi_transfer t = {
+		.tx_buf = txbuf,
+		.rx_buf = rxbuf,
+		.len = sizeof(txbuf),
+	};
+	int ret;
+
+	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 = ((rxbuf[0] & 0x3f) << 12 | rxbuf[1] << 4 | rxbuf[2] >> 4)
+			- (1 << 17);
+
+	return 0;
+}
+
+static int ltc2496_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct ltc2496_st *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 ltc249x_probe(dev, indio_dev);
+}
+
+static int ltc2496_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	ltc249x_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

Re: [PATCH v2 3/3] iio: adc: new driver to support Linear technology's ltc2496

[Jonathan Cameron]

On Thu, 14 Nov 2019 11:51:59 +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.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
As mentioned earlier, there are restrictions on buffers being dma
safe when passed to spi calls.   This more or less requires that they
never share a cacheline with other data and the only easy way to
do that is to allocate them from the heap.  It's better to do this
once rather than every time, hence the standard trick with ___cacheline_aligned
in the iio_priv structure.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/Kconfig   |  10 ++++
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/ltc2496.c | 100 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 111 insertions(+)
>  create mode 100644 drivers/iio/adc/ltc2496.c
> 
> 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 660242c2cca7..afe2b6db4a5e 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 ltc249x.o
>  obj-$(CONFIG_LTC2497) += ltc2497.o ltc249x.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..870526c6df1b
> --- /dev/null
> +++ b/drivers/iio/adc/ltc2496.c
> @@ -0,0 +1,100 @@
> +// 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 "ltc249x.h"
> +
> +struct ltc2496_st {
> +	/* this must be the first member */
> +	struct ltc249x_driverdata common_ddata;
> +	struct spi_device *spi;
> +};
> +
> +static int ltc2496_result_and_measure(struct ltc249x_driverdata *ddata,
> +				      u8 address, int *val)
> +{
> +	struct ltc2496_st *st =
> +		container_of(ddata, struct ltc2496_st, common_ddata);
> +	unsigned char txbuf[3] = { LTC249X_ENABLE | address, };
> +	unsigned char rxbuf[3];

You must make sure that buffers passed directly to the SPI subsystem
are not sharing a cacheline with other elements.  Hence these need
to be on the heap.. Easiest way of doing that is to use the
___cacheline_aligned tricks at the end of a iio_priv structure
(which is appropriately placed and aligned to allow this to work).

> +	struct spi_transfer t = {
> +		.tx_buf = txbuf,
> +		.rx_buf = rxbuf,
> +		.len = sizeof(txbuf),
> +	};
> +	int ret;
> +
> +	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 = ((rxbuf[0] & 0x3f) << 12 | rxbuf[1] << 4 | rxbuf[2] >> 4)
> +			- (1 << 17);
> +
> +	return 0;
> +}
> +
> +static int ltc2496_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct ltc2496_st *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 ltc249x_probe(dev, indio_dev);
> +}
> +
> +static int ltc2496_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	ltc249x_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");

Re: [PATCH v2 0/3] iio: adc: add support for ltc2496

[Jonathan Cameron]

On Thu, 14 Nov 2019 11:51:56 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> Hello,
> 
> this is v2 of a series sent earlier starting with Message-id:
> 20191111214025.18310-1-u.kleine-koenig@pengutronix.de and addresses the
> comments by Alexandru Ardelean. I'm a bit naughty here because I didn't
> carry out all suggestions. So I didn't use ...-spi.c and -i2c.c as file
> names, because this isn't about a single chip supporting both, but two
> different ones supporting only one bus each. So I thought ltc2496.c,
> ltc2497.c and ltc249x.c was fine.
Not using the particular naming Alex suggested is fine, but wild cards
in names is not.  It's gone wrong far too often in the past.
Much better to just pick a supported part and stick to that for naming.
Obviously that's tricky here as you want to reuse the same name hence..

Clunky though it may seem ltc2497-core.c ltc2497 and ltc2496 would be my
suggestion..

> 
> Also I didn't merge the binding docs of ltc2496 and ltc2497 for the same
> reason. I'm still not entirely happy with the (now) yaml document. I'd
> like to have a statement that says: This is an spi device, so it might
> have all the properties described in
> Documentation/devicetree/bindings/spi/spi-controller.yaml for an SPI
> device.

I don't think the is any way of doing this.  For standard elements you
normally just list them with no help etc.

> 
> 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        |  30 +++
>  MAINTAINERS                                   |   2 +-
>  drivers/iio/adc/Kconfig                       |  10 +
>  drivers/iio/adc/Makefile                      |   3 +-
>  drivers/iio/adc/ltc2496.c                     | 100 +++++++
>  drivers/iio/adc/ltc2497.c                     | 237 ++---------------
>  drivers/iio/adc/ltc249x.c                     | 244 ++++++++++++++++++
>  drivers/iio/adc/ltc249x.h                     |  18 ++
>  8 files changed, 433 insertions(+), 211 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/ltc249x.c
>  create mode 100644 drivers/iio/adc/ltc249x.h
> 
> 
> base-commit: 31f4f5b495a62c9a8b15b1c3581acd5efeb9af8c