[PATCH v2 0/6] IIO driver for MCP3550/1/3

[PATCH v2 0/6] IIO driver for MCP3550/1/3

[Lukas Wunner]

One speedup, one cleanup, one kerneldoc patch for mcp320x.c
plus support for the MCP3550/1/3 used by the "Revolution Pi"
family of open source PLCs (https://revolution.kunbus.com/).

Changes since v1:
- Drop two patches which are now queued. (Jonathan)

- Move kerneldoc to separate patch. (Jonathan)

- Move support for continuous conversion mode to separate patch
  which is marked informational / not for merging.  Rename device
  property to "continuous" for consistency with the existing
  similar property of drivers/iio/light/us5182d.c. (Rob, Jonathan)

- Rework calculation of raw value in patch [5/6]:  Instead of
  byte-wise mangling, convert the big endian value clocked out
  by the chip to host byte order and mangle the resulting 32-bit
  value.  Reduces the amount of code and improves readability as
  the bit numbers referenced in the code comment and datasheet
  are used verbatim in the code.

- Use switch/case-statement instead of if-clause when applying
  chip-specific quirks in mcp320x_probe(). (Jonathan)

- Expand code comment explaining the two consecutive conversions
  in mcp320x_probe(). (Jonathan)

Thanks,

Lukas


Lukas Wunner (6):
  iio: adc: mcp320x: Speed up readout of single-channel ADCs
  iio: adc: mcp320x: Drop unnecessary of_device_id attributes
  iio: adc: mcp320x: Document struct mcp320x
  dt-bindings: iio: adc: mcp320x: Update for mcp3550/1/3
  iio: adc: mcp320x: Add support for mcp3550/1/3
  iio: adc: mcp320x: Support continuous conversion mode

 .../devicetree/bindings/iio/adc/mcp320x.txt        |  23 ++
 drivers/iio/adc/Kconfig                            |   5 +-
 drivers/iio/adc/mcp320x.c                          | 240 ++++++++++++++-------
 3 files changed, 188 insertions(+), 80 deletions(-)

-- 
2.11.0

[PATCH v2 1/6] iio: adc: mcp320x: Speed up readout of single-channel ADCs

[Lukas Wunner]

Single-channel converters such as mcp3001, mcp3201, mcp3301 and the
upcoming mcp3550/1/3 lack a MOSI pin, so there's no need to call
mcp320x_channel_to_tx_data() for them.

Moreover, instead of calling spi_read() for these converters, which
generates an spi_message and spi_transfer on the stack on every readout,
it's more efficient to use the spi_message and spi_transfer[] included
in struct mcp320x (as we do for multi-channel ADCs), but initialize the
spi_message only with the receive transfer.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/iio/adc/mcp320x.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index 49e80b959089..15b5fa7ad427 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -78,10 +78,6 @@ static int mcp320x_channel_to_tx_data(int device_index,
 	int start_bit = 1;
 
 	switch (device_index) {
-	case mcp3001:
-	case mcp3201:
-	case mcp3301:
-		return 0;
 	case mcp3002:
 	case mcp3202:
 		return ((start_bit << 4) | (!differential << 3) |
@@ -102,20 +98,14 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
 {
 	int ret;
 
-	adc->rx_buf[0] = 0;
-	adc->rx_buf[1] = 0;
-	adc->tx_buf = mcp320x_channel_to_tx_data(device_index,
-						channel, differential);
+	memset(&adc->rx_buf, 0, sizeof(adc->rx_buf));
+	if (adc->chip_info->num_channels > 1)
+		adc->tx_buf = mcp320x_channel_to_tx_data(device_index, channel,
+							 differential);
 
-	if (device_index != mcp3001 && device_index != mcp3201 && device_index != mcp3301) {
-		ret = spi_sync(adc->spi, &adc->msg);
-		if (ret < 0)
-			return ret;
-	} else {
-		ret = spi_read(adc->spi, &adc->rx_buf, sizeof(adc->rx_buf));
-		if (ret < 0)
-			return ret;
-	}
+	ret = spi_sync(adc->spi, &adc->msg);
+	if (ret < 0)
+		return ret;
 
 	switch (device_index) {
 	case mcp3001:
@@ -329,9 +319,13 @@ static int mcp320x_probe(struct spi_device *spi)
 	adc->transfer[0].len = sizeof(adc->tx_buf);
 	adc->transfer[1].rx_buf = adc->rx_buf;
 	adc->transfer[1].len = sizeof(adc->rx_buf);
-
-	spi_message_init_with_transfers(&adc->msg, adc->transfer,
-					ARRAY_SIZE(adc->transfer));
+	if (chip_info->num_channels == 1)
+		/* single-channel converters are rx only (no MOSI pin) */
+		spi_message_init_with_transfers(&adc->msg,
+						&adc->transfer[1], 1);
+	else
+		spi_message_init_with_transfers(&adc->msg, adc->transfer,
+						ARRAY_SIZE(adc->transfer));
 
 	adc->reg = devm_regulator_get(&spi->dev, "vref");
 	if (IS_ERR(adc->reg))
-- 
2.11.0

[PATCH v2 2/6] iio: adc: mcp320x: Drop unnecessary of_device_id attributes

[Lukas Wunner]

The driver sets a .data pointer for each .compatible string but never
calls of_device_get_match_data().  Instead, ADC properties are looked up
with spi_get_device_id().  The .data pointer is therefore unnecessary,
so drop it.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/iio/adc/mcp320x.c | 75 ++++++++++++-----------------------------------
 1 file changed, 19 insertions(+), 56 deletions(-)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index 15b5fa7ad427..bf4794009413 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -363,62 +363,25 @@ static int mcp320x_remove(struct spi_device *spi)
 #if defined(CONFIG_OF)
 static const struct of_device_id mcp320x_dt_ids[] = {
 	/* NOTE: The use of compatibles with no vendor prefix is deprecated. */
-	{
-		.compatible = "mcp3001",
-		.data = &mcp320x_chip_infos[mcp3001],
-	}, {
-		.compatible = "mcp3002",
-		.data = &mcp320x_chip_infos[mcp3002],
-	}, {
-		.compatible = "mcp3004",
-		.data = &mcp320x_chip_infos[mcp3004],
-	}, {
-		.compatible = "mcp3008",
-		.data = &mcp320x_chip_infos[mcp3008],
-	}, {
-		.compatible = "mcp3201",
-		.data = &mcp320x_chip_infos[mcp3201],
-	}, {
-		.compatible = "mcp3202",
-		.data = &mcp320x_chip_infos[mcp3202],
-	}, {
-		.compatible = "mcp3204",
-		.data = &mcp320x_chip_infos[mcp3204],
-	}, {
-		.compatible = "mcp3208",
-		.data = &mcp320x_chip_infos[mcp3208],
-	}, {
-		.compatible = "mcp3301",
-		.data = &mcp320x_chip_infos[mcp3301],
-	}, {
-		.compatible = "microchip,mcp3001",
-		.data = &mcp320x_chip_infos[mcp3001],
-	}, {
-		.compatible = "microchip,mcp3002",
-		.data = &mcp320x_chip_infos[mcp3002],
-	}, {
-		.compatible = "microchip,mcp3004",
-		.data = &mcp320x_chip_infos[mcp3004],
-	}, {
-		.compatible = "microchip,mcp3008",
-		.data = &mcp320x_chip_infos[mcp3008],
-	}, {
-		.compatible = "microchip,mcp3201",
-		.data = &mcp320x_chip_infos[mcp3201],
-	}, {
-		.compatible = "microchip,mcp3202",
-		.data = &mcp320x_chip_infos[mcp3202],
-	}, {
-		.compatible = "microchip,mcp3204",
-		.data = &mcp320x_chip_infos[mcp3204],
-	}, {
-		.compatible = "microchip,mcp3208",
-		.data = &mcp320x_chip_infos[mcp3208],
-	}, {
-		.compatible = "microchip,mcp3301",
-		.data = &mcp320x_chip_infos[mcp3301],
-	}, {
-	}
+	{ .compatible = "mcp3001" },
+	{ .compatible = "mcp3002" },
+	{ .compatible = "mcp3004" },
+	{ .compatible = "mcp3008" },
+	{ .compatible = "mcp3201" },
+	{ .compatible = "mcp3202" },
+	{ .compatible = "mcp3204" },
+	{ .compatible = "mcp3208" },
+	{ .compatible = "mcp3301" },
+	{ .compatible = "microchip,mcp3001" },
+	{ .compatible = "microchip,mcp3002" },
+	{ .compatible = "microchip,mcp3004" },
+	{ .compatible = "microchip,mcp3008" },
+	{ .compatible = "microchip,mcp3201" },
+	{ .compatible = "microchip,mcp3202" },
+	{ .compatible = "microchip,mcp3204" },
+	{ .compatible = "microchip,mcp3208" },
+	{ .compatible = "microchip,mcp3301" },
+	{ }
 };
 MODULE_DEVICE_TABLE(of, mcp320x_dt_ids);
 #endif
-- 
2.11.0

[PATCH v2 3/6] iio: adc: mcp320x: Document struct mcp320x

[Lukas Wunner]

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Changes since v1:
- Move kerneldoc to separate patch. (Jonathan)

 drivers/iio/adc/mcp320x.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index bf4794009413..32859188d653 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -59,6 +59,17 @@ struct mcp320x_chip_info {
 	unsigned int resolution;
 };
 
+/**
+ * struct mcp320x - Microchip SPI ADC instance
+ * @spi: SPI slave (parent of the IIO device)
+ * @msg: SPI message to select a channel and receive a value from the ADC
+ * @transfer: SPI transfers used by @msg
+ * @reg: regulator generating Vref
+ * @lock: protects read sequences
+ * @chip_info: ADC properties
+ * @tx_buf: buffer for @transfer[0] (not used on single-channel converters)
+ * @rx_buf: buffer for @transfer[1]
+ */
 struct mcp320x {
 	struct spi_device *spi;
 	struct spi_message msg;
-- 
2.11.0

[PATCH v2 4/6] dt-bindings: iio: adc: mcp320x: Update for mcp3550/1/3

[Lukas Wunner]

All chips supported by this driver clock data out on the falling edge
and latch data in on the rising edge, hence SPI mode (0,0) or (1,1)
must be used.

Furthermore, none of the chips has an internal reference voltage
regulator, so an external supply is always required and needs to be
specified in the device tree lest the IIO "scale" in sysfs cannot be
calculated.

Document these requirements in the device tree binding, add compatible
strings for the newly supported mcp3550/1/3 and explain that SPI mode
(0,0) should be preferred for these chips.

Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Changes since v1:
- Move support for continuous conversion mode to separate patch
  which is marked informational / not for merging. (Rob, Jonathan)

 Documentation/devicetree/bindings/iio/adc/mcp320x.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
index bcd3ac8e6e0c..7d64753df949 100644
--- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
+++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
@@ -29,15 +29,29 @@ Required properties:
 				"microchip,mcp3204"
 				"microchip,mcp3208"
 				"microchip,mcp3301"
+				"microchip,mcp3550-50"
+				"microchip,mcp3550-60"
+				"microchip,mcp3551"
+				"microchip,mcp3553"
 
 			NOTE: The use of the compatibles with no vendor prefix
 			is deprecated and only listed because old DT use them.
 
+	- spi-cpha, spi-cpol (boolean):
+			Either SPI mode (0,0) or (1,1) must be used, so specify
+			none or both of spi-cpha, spi-cpol.  The MCP3550/1/3
+			is more efficient in mode (1,1) as only 3 instead of
+			4 bytes need to be read from the ADC, but not all SPI
+			masters support it.
+
+	- vref-supply:	Phandle to the external reference voltage supply.
+
 Examples:
 spi_controller {
 	mcp3x0x@0 {
 		compatible = "mcp3002";
 		reg = <0>;
 		spi-max-frequency = <1000000>;
+		vref-supply = <&vref_reg>;
 	};
 };
-- 
2.11.0

[PATCH v2 5/6] iio: adc: mcp320x: Add support for mcp3550/1/3

[Lukas Wunner]

These ADCs are marketed as single-channel 22 bit delta-sigma ADCs, but
in reality their resolution is 21 bit with an overrange or underrange
of 12% beyond Vref.  In other words, "full scale" means +/- 2^20.

This driver does not explicitly signal back to the user when an
overrange or underrange occurs, but the user can detect it by comparing
the raw value to +/- 2^20 (or the scaled value to Vref).

The chips feature an extended temperature range and high accuracy,
low noise characteristics, but their conversion times are slow with
up to 80 ms +/- 2% (on the MCP3550-50).

Hence, unlike the other ADCs supported by the driver, conversion does
not take place in realtime upon lowering CS.  Instead, CS is asserted
for 8 usec to start the conversion.  After waiting for the duration of
the conversion, the result can be fetched.  While waiting, control of
the bus is ceased so it may be used by a different device.

After the result has been fetched and 10 us have passed, the chip goes
into shutdown and an additional power-up delay of 144 clock periods is
then required to wake the analog circuitry upon the next conversion
(footnote below table 4-1, page 16 in the spec).

Optionally, the chips can be used in so-called "continuous conversion
mode":  Conversions then take place continuously and the last result may
be fetched at any time without observing a delay.  The mode is enabled
by permanently driving CS low, e.g. by wiring it to ground.  The driver
only supports "single conversion mode" for now but should be adaptable
to "continuous conversion mode" with moderate effort.

The chips clock out a 3 byte word, unlike the other ADCs supported by
the driver which all have a lower resolution than 16 bit and thus make
do with 2 bytes.  Calculate the word length on probe by rounding up the
resolution to full bytes.  Crucially, if the clock idles low, the
transfer is preceded by a useless Data Ready bit which increases its
length from 24 bit to 25 bit = 4 bytes (section 5.5 in the spec).
Autosense this based on the SPI slave's configuration.

Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Changes since v1:
- Move kerneldoc to separate patch. (Jonathan)

- Move support for continuous conversion mode to separate patch
  which is marked informational / not for merging. (Rob, Jonathan)

- Rework calculation of raw value in patch [5/6]:  Instead of
  byte-wise mangling, convert the big endian value clocked out
  by the chip to host byte order and mangle the resulting 32-bit
  value.  Reduces the amount of code and improves readability as
  the bit numbers referenced in the code comment and datasheet
  are used verbatim in the code.

- Use switch/case-statement instead of if-clause when applying
  chip-specific quirks in mcp320x_probe(). (Jonathan)

- Expand code comment explaining the two consecutive conversions
  in mcp320x_probe(). (Jonathan)

 drivers/iio/adc/Kconfig   |   5 +-
 drivers/iio/adc/mcp320x.c | 120 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 118 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 57625653fcb6..84dd04621650 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -475,12 +475,13 @@ config	MAX9611
 	  called max9611.
 
 config MCP320X
-	tristate "Microchip Technology MCP3x01/02/04/08"
+	tristate "Microchip Technology MCP3x01/02/04/08 and MCP3550/1/3"
 	depends on SPI
 	help
 	  Say yes here to build support for Microchip Technology's
 	  MCP3001, MCP3002, MCP3004, MCP3008, MCP3201, MCP3202, MCP3204,
-	  MCP3208 or MCP3301 analog to digital converter.
+	  MCP3208, MCP3301, MCP3550, MCP3551 and MCP3553 analog to digital
+	  converters.
 
 	  This driver can also be built as a module. If so, the module will be
 	  called mcp320x.
diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index 32859188d653..a04856d8afdb 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -19,6 +19,11 @@
  * ------------
  * 13 bit converter
  * MCP3301
+ * ------------
+ * 22 bit converter
+ * MCP3550
+ * MCP3551
+ * MCP3553
  *
  * Datasheet can be found here:
  * http://ww1.microchip.com/downloads/en/DeviceDoc/21293C.pdf  mcp3001
@@ -28,6 +33,7 @@
  * http://ww1.microchip.com/downloads/en/DeviceDoc/21034D.pdf  mcp3202
  * http://ww1.microchip.com/downloads/en/DeviceDoc/21298c.pdf  mcp3204/08
  * http://ww1.microchip.com/downloads/en/DeviceDoc/21700E.pdf  mcp3301
+ * http://ww1.microchip.com/downloads/en/DeviceDoc/21950D.pdf  mcp3550/1/3
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -51,12 +57,17 @@ enum {
 	mcp3204,
 	mcp3208,
 	mcp3301,
+	mcp3550_50,
+	mcp3550_60,
+	mcp3551,
+	mcp3553,
 };
 
 struct mcp320x_chip_info {
 	const struct iio_chan_spec *channels;
 	unsigned int num_channels;
 	unsigned int resolution;
+	unsigned int conv_time; /* usec */
 };
 
 /**
@@ -64,6 +75,8 @@ struct mcp320x_chip_info {
  * @spi: SPI slave (parent of the IIO device)
  * @msg: SPI message to select a channel and receive a value from the ADC
  * @transfer: SPI transfers used by @msg
+ * @start_conv_msg: SPI message to start a conversion by briefly asserting CS
+ * @start_conv_transfer: SPI transfer used by @start_conv_msg
  * @reg: regulator generating Vref
  * @lock: protects read sequences
  * @chip_info: ADC properties
@@ -74,13 +87,15 @@ struct mcp320x {
 	struct spi_device *spi;
 	struct spi_message msg;
 	struct spi_transfer transfer[2];
+	struct spi_message start_conv_msg;
+	struct spi_transfer start_conv_transfer;
 
 	struct regulator *reg;
 	struct mutex lock;
 	const struct mcp320x_chip_info *chip_info;
 
 	u8 tx_buf ____cacheline_aligned;
-	u8 rx_buf[2];
+	u8 rx_buf[4];
 };
 
 static int mcp320x_channel_to_tx_data(int device_index,
@@ -109,6 +124,15 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
 {
 	int ret;
 
+	if (adc->chip_info->conv_time) {
+		ret = spi_sync(adc->spi, &adc->start_conv_msg);
+		if (ret < 0)
+			return ret;
+
+		usleep_range(adc->chip_info->conv_time,
+			     adc->chip_info->conv_time + 100);
+	}
+
 	memset(&adc->rx_buf, 0, sizeof(adc->rx_buf));
 	if (adc->chip_info->num_channels > 1)
 		adc->tx_buf = mcp320x_channel_to_tx_data(device_index, channel,
@@ -139,6 +163,31 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
 		*val = sign_extend32((adc->rx_buf[0] & 0x1f) << 8
 				    | adc->rx_buf[1], 12);
 		return 0;
+	case mcp3550_50:
+	case mcp3550_60:
+	case mcp3551:
+	case mcp3553: {
+		u32 raw = be32_to_cpup((u32 *)adc->rx_buf);
+
+		if (!(adc->spi->mode & SPI_CPOL))
+			raw <<= 1; /* strip Data Ready bit in SPI mode 0,0 */
+
+		/*
+		 * If the input is within -vref and vref, bit 21 is the sign.
+		 * Up to 12% overrange or underrange are allowed, in which case
+		 * bit 23 is the sign and bit 0 to 21 is the value.
+		 */
+		raw >>= 8;
+		if (raw & BIT(22) && raw & BIT(23))
+			return -EIO; /* cannot have overrange AND underrange */
+		else if (raw & BIT(22))
+			raw &= ~BIT(22); /* overrange */
+		else if (raw & BIT(23) || raw & BIT(21))
+			raw |= GENMASK(31, 22); /* underrange or negative */
+
+		*val = (s32)raw;
+		return 0;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -297,6 +346,31 @@ static const struct mcp320x_chip_info mcp320x_chip_infos[] = {
 		.num_channels = ARRAY_SIZE(mcp3201_channels),
 		.resolution = 13
 	},
+	[mcp3550_50] = {
+		.channels = mcp3201_channels,
+		.num_channels = ARRAY_SIZE(mcp3201_channels),
+		.resolution = 21,
+		/* 2% max deviation + 144 clock periods to exit shutdown */
+		.conv_time = 80000 * 1.02 + 144000 / 102.4,
+	},
+	[mcp3550_60] = {
+		.channels = mcp3201_channels,
+		.num_channels = ARRAY_SIZE(mcp3201_channels),
+		.resolution = 21,
+		.conv_time = 66670 * 1.02 + 144000 / 122.88,
+	},
+	[mcp3551] = {
+		.channels = mcp3201_channels,
+		.num_channels = ARRAY_SIZE(mcp3201_channels),
+		.resolution = 21,
+		.conv_time = 73100 * 1.02 + 144000 / 112.64,
+	},
+	[mcp3553] = {
+		.channels = mcp3201_channels,
+		.num_channels = ARRAY_SIZE(mcp3201_channels),
+		.resolution = 21,
+		.conv_time = 16670 * 1.02 + 144000 / 122.88,
+	},
 };
 
 static int mcp320x_probe(struct spi_device *spi)
@@ -304,7 +378,7 @@ static int mcp320x_probe(struct spi_device *spi)
 	struct iio_dev *indio_dev;
 	struct mcp320x *adc;
 	const struct mcp320x_chip_info *chip_info;
-	int ret;
+	int ret, device_index;
 
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
 	if (!indio_dev)
@@ -320,7 +394,8 @@ static int mcp320x_probe(struct spi_device *spi)
 	indio_dev->info = &mcp320x_info;
 	spi_set_drvdata(spi, indio_dev);
 
-	chip_info = &mcp320x_chip_infos[spi_get_device_id(spi)->driver_data];
+	device_index = spi_get_device_id(spi)->driver_data;
+	chip_info = &mcp320x_chip_infos[device_index];
 	indio_dev->channels = chip_info->channels;
 	indio_dev->num_channels = chip_info->num_channels;
 
@@ -329,7 +404,8 @@ static int mcp320x_probe(struct spi_device *spi)
 	adc->transfer[0].tx_buf = &adc->tx_buf;
 	adc->transfer[0].len = sizeof(adc->tx_buf);
 	adc->transfer[1].rx_buf = adc->rx_buf;
-	adc->transfer[1].len = sizeof(adc->rx_buf);
+	adc->transfer[1].len = DIV_ROUND_UP(chip_info->resolution, 8);
+
 	if (chip_info->num_channels == 1)
 		/* single-channel converters are rx only (no MOSI pin) */
 		spi_message_init_with_transfers(&adc->msg,
@@ -338,6 +414,32 @@ static int mcp320x_probe(struct spi_device *spi)
 		spi_message_init_with_transfers(&adc->msg, adc->transfer,
 						ARRAY_SIZE(adc->transfer));
 
+	switch (device_index) {
+	case mcp3550_50:
+	case mcp3550_60:
+	case mcp3551:
+	case mcp3553:
+		/* rx len increases from 24 to 25 bit in SPI mode 0,0 */
+		if (!(spi->mode & SPI_CPOL))
+			adc->transfer[1].len++;
+
+		/* conversions are started by asserting CS pin for 8 usec */
+		adc->start_conv_transfer.delay_usecs = 8;
+		spi_message_init_with_transfers(&adc->start_conv_msg,
+						&adc->start_conv_transfer, 1);
+
+		/*
+		 * If CS was previously kept low (continuous conversion mode)
+		 * and then changed to high, the chip is in shutdown.
+		 * Sometimes it fails to wake from shutdown and clocks out
+		 * only 0xffffff.  The magic sequence of performing two
+		 * conversions without delay between them resets the chip
+		 * and ensures all subsequent conversions succeed.
+		 */
+		mcp320x_adc_conversion(adc, 0, 1, device_index, &ret);
+		mcp320x_adc_conversion(adc, 0, 1, device_index, &ret);
+	}
+
 	adc->reg = devm_regulator_get(&spi->dev, "vref");
 	if (IS_ERR(adc->reg))
 		return PTR_ERR(adc->reg);
@@ -392,6 +494,10 @@ static const struct of_device_id mcp320x_dt_ids[] = {
 	{ .compatible = "microchip,mcp3204" },
 	{ .compatible = "microchip,mcp3208" },
 	{ .compatible = "microchip,mcp3301" },
+	{ .compatible = "microchip,mcp3550-50" },
+	{ .compatible = "microchip,mcp3550-60" },
+	{ .compatible = "microchip,mcp3551" },
+	{ .compatible = "microchip,mcp3553" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mcp320x_dt_ids);
@@ -407,6 +513,10 @@ static const struct spi_device_id mcp320x_id[] = {
 	{ "mcp3204", mcp3204 },
 	{ "mcp3208", mcp3208 },
 	{ "mcp3301", mcp3301 },
+	{ "mcp3550-50", mcp3550_50 },
+	{ "mcp3550-60", mcp3550_60 },
+	{ "mcp3551", mcp3551 },
+	{ "mcp3553", mcp3553 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, mcp320x_id);
@@ -423,5 +533,5 @@ static struct spi_driver mcp320x_driver = {
 module_spi_driver(mcp320x_driver);
 
 MODULE_AUTHOR("Oskar Andero <oskar.andero@gmail.com>");
-MODULE_DESCRIPTION("Microchip Technology MCP3x01/02/04/08");
+MODULE_DESCRIPTION("Microchip Technology MCP3x01/02/04/08 and MCP3550/1/3");
 MODULE_LICENSE("GPL v2");
-- 
2.11.0

[PATCH v2 6/6 INFORMATIONAL/RFT] iio: adc: mcp320x: Support continuous conversion mode

[Lukas Wunner]

The MCP3550/1/3 can be run in "continuous conversion mode", allowing
conversions to be fetched at any time without observing a delay,
at the expense of higher energy consumption.

The mode is enabled by permanently driving CS low, e.g. by wiring it to
ground.  This requires either exclusive bus usage by the chip or that
CLK and MISO are AND-gated with the CS signal coming from the master.

I only have boards available which use the chip in single conversion
mode, so cannot test its behaviour in continuous conversion mode.
I am putting this commit out there as a basis for others to work on.
It introduces a device property to indicate that continuous conversion
mode is enabled permanently.

In theory it may also be possible to enter the mode temporarily, e.g. as
long as the chardev is opened:  Setting the cs_change bit of transfer[1]
in struct mcp320x should enter the mode, clearing the bit should exit it.
However as can be seen in mcp320x_probe(), a magic sequence of two
consecutive conversions is sometimes necessary to switch from continuous
to single conversion mode lest the chip remains stuck in shutdown.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
Changes since v1:
- Move support for continuous conversion mode to separate patch
  which is marked informational / not for merging.  Rename device
  property to "continuous" for consistency with the existing
  similar property of drivers/iio/light/us5182d.c. (Rob, Jonathan)

 Documentation/devicetree/bindings/iio/adc/mcp320x.txt | 9 +++++++++
 drivers/iio/adc/mcp320x.c                             | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
index 7d64753df949..cf28af9ec0ac 100644
--- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
+++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
@@ -46,6 +46,15 @@ Required properties:
 
 	- vref-supply:	Phandle to the external reference voltage supply.
 
+Optional properties:
+	- microchip,continuous (boolean):
+			Only applicable to MCP3550/1/3:  These ADCs have long
+			conversion times and therefore support "continuous
+			conversion mode" to allow retrieval of conversions
+			at any time without observing a delay.  The mode is
+			enabled by permanently driving CS low, e.g. by wiring
+			it to ground.
+
 Examples:
 spi_controller {
 	mcp3x0x@0 {
diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index a04856d8afdb..446700ea6110 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -77,6 +77,9 @@ struct mcp320x_chip_info {
  * @transfer: SPI transfers used by @msg
  * @start_conv_msg: SPI message to start a conversion by briefly asserting CS
  * @start_conv_transfer: SPI transfer used by @start_conv_msg
+ * @continuous_conv: whether CS is permanently driven low such that conversions
+ *	take place continuously, obviating the need to explicitly start them
+ *	and wait for them to finish
  * @reg: regulator generating Vref
  * @lock: protects read sequences
  * @chip_info: ADC properties
@@ -89,6 +92,7 @@ struct mcp320x {
 	struct spi_transfer transfer[2];
 	struct spi_message start_conv_msg;
 	struct spi_transfer start_conv_transfer;
+	bool continuous_conv;
 
 	struct regulator *reg;
 	struct mutex lock;
@@ -124,7 +128,7 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
 {
 	int ret;
 
-	if (adc->chip_info->conv_time) {
+	if (adc->chip_info->conv_time && !adc->continuous_conv) {
 		ret = spi_sync(adc->spi, &adc->start_conv_msg);
 		if (ret < 0)
 			return ret;
@@ -427,6 +431,8 @@ static int mcp320x_probe(struct spi_device *spi)
 		adc->start_conv_transfer.delay_usecs = 8;
 		spi_message_init_with_transfers(&adc->start_conv_msg,
 						&adc->start_conv_transfer, 1);
+		adc->continuous_conv = device_property_read_bool(&spi->dev,
+						      "microchip,continuous");
 
 		/*
 		 * If CS was previously kept low (continuous conversion mode)
-- 
2.11.0

Re: [PATCH v2 1/6] iio: adc: mcp320x: Speed up readout of single-channel ADCs

[Jonathan Cameron]

On Sat, 9 Sep 2017 20:32:41 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> Single-channel converters such as mcp3001, mcp3201, mcp3301 and the
> upcoming mcp3550/1/3 lack a MOSI pin, so there's no need to call
> mcp320x_channel_to_tx_data() for them.
> 
> Moreover, instead of calling spi_read() for these converters, which
> generates an spi_message and spi_transfer on the stack on every readout,
> it's more efficient to use the spi_message and spi_transfer[] included
> in struct mcp320x (as we do for multi-channel ADCs), but initialize the
> spi_message only with the receive transfer.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Looks good to me.

Note if anyone wants to add anything it'll only be exposed in the testing
branch of iio.git for a week or so - hence not in a state where I'm
unwilling to rebased.  All tested-by's etc particularly welcome!

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/mcp320x.c | 34 ++++++++++++++--------------------
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index 49e80b959089..15b5fa7ad427 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -78,10 +78,6 @@ static int mcp320x_channel_to_tx_data(int device_index,
>  	int start_bit = 1;
>  
>  	switch (device_index) {
> -	case mcp3001:
> -	case mcp3201:
> -	case mcp3301:
> -		return 0;
>  	case mcp3002:
>  	case mcp3202:
>  		return ((start_bit << 4) | (!differential << 3) |
> @@ -102,20 +98,14 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
>  {
>  	int ret;
>  
> -	adc->rx_buf[0] = 0;
> -	adc->rx_buf[1] = 0;
> -	adc->tx_buf = mcp320x_channel_to_tx_data(device_index,
> -						channel, differential);
> +	memset(&adc->rx_buf, 0, sizeof(adc->rx_buf));
> +	if (adc->chip_info->num_channels > 1)
> +		adc->tx_buf = mcp320x_channel_to_tx_data(device_index, channel,
> +							 differential);
>  
> -	if (device_index != mcp3001 && device_index != mcp3201 && device_index != mcp3301) {
> -		ret = spi_sync(adc->spi, &adc->msg);
> -		if (ret < 0)
> -			return ret;
> -	} else {
> -		ret = spi_read(adc->spi, &adc->rx_buf, sizeof(adc->rx_buf));
> -		if (ret < 0)
> -			return ret;
> -	}
> +	ret = spi_sync(adc->spi, &adc->msg);
> +	if (ret < 0)
> +		return ret;
>  
>  	switch (device_index) {
>  	case mcp3001:
> @@ -329,9 +319,13 @@ static int mcp320x_probe(struct spi_device *spi)
>  	adc->transfer[0].len = sizeof(adc->tx_buf);
>  	adc->transfer[1].rx_buf = adc->rx_buf;
>  	adc->transfer[1].len = sizeof(adc->rx_buf);
> -
> -	spi_message_init_with_transfers(&adc->msg, adc->transfer,
> -					ARRAY_SIZE(adc->transfer));
> +	if (chip_info->num_channels == 1)
> +		/* single-channel converters are rx only (no MOSI pin) */
> +		spi_message_init_with_transfers(&adc->msg,
> +						&adc->transfer[1], 1);
> +	else
> +		spi_message_init_with_transfers(&adc->msg, adc->transfer,
> +						ARRAY_SIZE(adc->transfer));
>  
>  	adc->reg = devm_regulator_get(&spi->dev, "vref");
>  	if (IS_ERR(adc->reg))

Re: [PATCH v2 2/6] iio: adc: mcp320x: Drop unnecessary of_device_id attributes

[Jonathan Cameron]

On Sat, 9 Sep 2017 20:32:41 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> The driver sets a .data pointer for each .compatible string but never
> calls of_device_get_match_data().  Instead, ADC properties are looked up
> with spi_get_device_id().  The .data pointer is therefore unnecessary,
> so drop it.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.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/mcp320x.c | 75 ++++++++++++-----------------------------------
>  1 file changed, 19 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index 15b5fa7ad427..bf4794009413 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -363,62 +363,25 @@ static int mcp320x_remove(struct spi_device *spi)
>  #if defined(CONFIG_OF)
>  static const struct of_device_id mcp320x_dt_ids[] = {
>  	/* NOTE: The use of compatibles with no vendor prefix is deprecated. */
> -	{
> -		.compatible = "mcp3001",
> -		.data = &mcp320x_chip_infos[mcp3001],
> -	}, {
> -		.compatible = "mcp3002",
> -		.data = &mcp320x_chip_infos[mcp3002],
> -	}, {
> -		.compatible = "mcp3004",
> -		.data = &mcp320x_chip_infos[mcp3004],
> -	}, {
> -		.compatible = "mcp3008",
> -		.data = &mcp320x_chip_infos[mcp3008],
> -	}, {
> -		.compatible = "mcp3201",
> -		.data = &mcp320x_chip_infos[mcp3201],
> -	}, {
> -		.compatible = "mcp3202",
> -		.data = &mcp320x_chip_infos[mcp3202],
> -	}, {
> -		.compatible = "mcp3204",
> -		.data = &mcp320x_chip_infos[mcp3204],
> -	}, {
> -		.compatible = "mcp3208",
> -		.data = &mcp320x_chip_infos[mcp3208],
> -	}, {
> -		.compatible = "mcp3301",
> -		.data = &mcp320x_chip_infos[mcp3301],
> -	}, {
> -		.compatible = "microchip,mcp3001",
> -		.data = &mcp320x_chip_infos[mcp3001],
> -	}, {
> -		.compatible = "microchip,mcp3002",
> -		.data = &mcp320x_chip_infos[mcp3002],
> -	}, {
> -		.compatible = "microchip,mcp3004",
> -		.data = &mcp320x_chip_infos[mcp3004],
> -	}, {
> -		.compatible = "microchip,mcp3008",
> -		.data = &mcp320x_chip_infos[mcp3008],
> -	}, {
> -		.compatible = "microchip,mcp3201",
> -		.data = &mcp320x_chip_infos[mcp3201],
> -	}, {
> -		.compatible = "microchip,mcp3202",
> -		.data = &mcp320x_chip_infos[mcp3202],
> -	}, {
> -		.compatible = "microchip,mcp3204",
> -		.data = &mcp320x_chip_infos[mcp3204],
> -	}, {
> -		.compatible = "microchip,mcp3208",
> -		.data = &mcp320x_chip_infos[mcp3208],
> -	}, {
> -		.compatible = "microchip,mcp3301",
> -		.data = &mcp320x_chip_infos[mcp3301],
> -	}, {
> -	}
> +	{ .compatible = "mcp3001" },
> +	{ .compatible = "mcp3002" },
> +	{ .compatible = "mcp3004" },
> +	{ .compatible = "mcp3008" },
> +	{ .compatible = "mcp3201" },
> +	{ .compatible = "mcp3202" },
> +	{ .compatible = "mcp3204" },
> +	{ .compatible = "mcp3208" },
> +	{ .compatible = "mcp3301" },
> +	{ .compatible = "microchip,mcp3001" },
> +	{ .compatible = "microchip,mcp3002" },
> +	{ .compatible = "microchip,mcp3004" },
> +	{ .compatible = "microchip,mcp3008" },
> +	{ .compatible = "microchip,mcp3201" },
> +	{ .compatible = "microchip,mcp3202" },
> +	{ .compatible = "microchip,mcp3204" },
> +	{ .compatible = "microchip,mcp3208" },
> +	{ .compatible = "microchip,mcp3301" },
> +	{ }
>  };
>  MODULE_DEVICE_TABLE(of, mcp320x_dt_ids);
>  #endif

Re: [PATCH v2 3/6] iio: adc: mcp320x: Document struct mcp320x

[Jonathan Cameron]

On Sat, 9 Sep 2017 20:32:41 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Applied.
> ---
> Changes since v1:
> - Move kerneldoc to separate patch. (Jonathan)
> 
>  drivers/iio/adc/mcp320x.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index bf4794009413..32859188d653 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -59,6 +59,17 @@ struct mcp320x_chip_info {
>  	unsigned int resolution;
>  };
>  
> +/**
> + * struct mcp320x - Microchip SPI ADC instance
> + * @spi: SPI slave (parent of the IIO device)
> + * @msg: SPI message to select a channel and receive a value from the ADC
> + * @transfer: SPI transfers used by @msg
> + * @reg: regulator generating Vref
> + * @lock: protects read sequences
> + * @chip_info: ADC properties
> + * @tx_buf: buffer for @transfer[0] (not used on single-channel converters)
> + * @rx_buf: buffer for @transfer[1]
> + */
>  struct mcp320x {
>  	struct spi_device *spi;
>  	struct spi_message msg;

Re: [PATCH v2 5/6] iio: adc: mcp320x: Add support for mcp3550/1/3

[Jonathan Cameron]

On Sat, 9 Sep 2017 20:32:41 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> These ADCs are marketed as single-channel 22 bit delta-sigma ADCs, but
> in reality their resolution is 21 bit with an overrange or underrange
> of 12% beyond Vref.  In other words, "full scale" means +/- 2^20.
> 
> This driver does not explicitly signal back to the user when an
> overrange or underrange occurs, but the user can detect it by comparing
> the raw value to +/- 2^20 (or the scaled value to Vref).
> 
> The chips feature an extended temperature range and high accuracy,
> low noise characteristics, but their conversion times are slow with
> up to 80 ms +/- 2% (on the MCP3550-50).
> 
> Hence, unlike the other ADCs supported by the driver, conversion does
> not take place in realtime upon lowering CS.  Instead, CS is asserted
> for 8 usec to start the conversion.  After waiting for the duration of
> the conversion, the result can be fetched.  While waiting, control of
> the bus is ceased so it may be used by a different device.
> 
> After the result has been fetched and 10 us have passed, the chip goes
> into shutdown and an additional power-up delay of 144 clock periods is
> then required to wake the analog circuitry upon the next conversion
> (footnote below table 4-1, page 16 in the spec).
> 
> Optionally, the chips can be used in so-called "continuous conversion
> mode":  Conversions then take place continuously and the last result may
> be fetched at any time without observing a delay.  The mode is enabled
> by permanently driving CS low, e.g. by wiring it to ground.  The driver
> only supports "single conversion mode" for now but should be adaptable
> to "continuous conversion mode" with moderate effort.
> 
> The chips clock out a 3 byte word, unlike the other ADCs supported by
> the driver which all have a lower resolution than 16 bit and thus make
> do with 2 bytes.  Calculate the word length on probe by rounding up the
> resolution to full bytes.  Crucially, if the clock idles low, the
> transfer is preceded by a useless Data Ready bit which increases its
> length from 24 bit to 25 bit = 4 bytes (section 5.5 in the spec).
> Autosense this based on the SPI slave's configuration.
> 
> Cc: Mathias Duckeck <m.duckeck@kunbus.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Excellent.  Applied to the togreg branch of iio.git and pushed out as
testing.

Thanks,

Jonathan

> ---
> Changes since v1:
> - Move kerneldoc to separate patch. (Jonathan)
> 
> - Move support for continuous conversion mode to separate patch
>   which is marked informational / not for merging. (Rob, Jonathan)
> 
> - Rework calculation of raw value in patch [5/6]:  Instead of
>   byte-wise mangling, convert the big endian value clocked out
>   by the chip to host byte order and mangle the resulting 32-bit
>   value.  Reduces the amount of code and improves readability as
>   the bit numbers referenced in the code comment and datasheet
>   are used verbatim in the code.
> 
> - Use switch/case-statement instead of if-clause when applying
>   chip-specific quirks in mcp320x_probe(). (Jonathan)
> 
> - Expand code comment explaining the two consecutive conversions
>   in mcp320x_probe(). (Jonathan)
> 
>  drivers/iio/adc/Kconfig   |   5 +-
>  drivers/iio/adc/mcp320x.c | 120 ++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 118 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 57625653fcb6..84dd04621650 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -475,12 +475,13 @@ config	MAX9611
>  	  called max9611.
>  
>  config MCP320X
> -	tristate "Microchip Technology MCP3x01/02/04/08"
> +	tristate "Microchip Technology MCP3x01/02/04/08 and MCP3550/1/3"
>  	depends on SPI
>  	help
>  	  Say yes here to build support for Microchip Technology's
>  	  MCP3001, MCP3002, MCP3004, MCP3008, MCP3201, MCP3202, MCP3204,
> -	  MCP3208 or MCP3301 analog to digital converter.
> +	  MCP3208, MCP3301, MCP3550, MCP3551 and MCP3553 analog to digital
> +	  converters.
>  
>  	  This driver can also be built as a module. If so, the module will be
>  	  called mcp320x.
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index 32859188d653..a04856d8afdb 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -19,6 +19,11 @@
>   * ------------
>   * 13 bit converter
>   * MCP3301
> + * ------------
> + * 22 bit converter
> + * MCP3550
> + * MCP3551
> + * MCP3553
>   *
>   * Datasheet can be found here:
>   * http://ww1.microchip.com/downloads/en/DeviceDoc/21293C.pdf  mcp3001
> @@ -28,6 +33,7 @@
>   * http://ww1.microchip.com/downloads/en/DeviceDoc/21034D.pdf  mcp3202
>   * http://ww1.microchip.com/downloads/en/DeviceDoc/21298c.pdf  mcp3204/08
>   * http://ww1.microchip.com/downloads/en/DeviceDoc/21700E.pdf  mcp3301
> + * http://ww1.microchip.com/downloads/en/DeviceDoc/21950D.pdf  mcp3550/1/3
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -51,12 +57,17 @@ enum {
>  	mcp3204,
>  	mcp3208,
>  	mcp3301,
> +	mcp3550_50,
> +	mcp3550_60,
> +	mcp3551,
> +	mcp3553,
>  };
>  
>  struct mcp320x_chip_info {
>  	const struct iio_chan_spec *channels;
>  	unsigned int num_channels;
>  	unsigned int resolution;
> +	unsigned int conv_time; /* usec */
>  };
>  
>  /**
> @@ -64,6 +75,8 @@ struct mcp320x_chip_info {
>   * @spi: SPI slave (parent of the IIO device)
>   * @msg: SPI message to select a channel and receive a value from the ADC
>   * @transfer: SPI transfers used by @msg
> + * @start_conv_msg: SPI message to start a conversion by briefly asserting CS
> + * @start_conv_transfer: SPI transfer used by @start_conv_msg
>   * @reg: regulator generating Vref
>   * @lock: protects read sequences
>   * @chip_info: ADC properties
> @@ -74,13 +87,15 @@ struct mcp320x {
>  	struct spi_device *spi;
>  	struct spi_message msg;
>  	struct spi_transfer transfer[2];
> +	struct spi_message start_conv_msg;
> +	struct spi_transfer start_conv_transfer;
>  
>  	struct regulator *reg;
>  	struct mutex lock;
>  	const struct mcp320x_chip_info *chip_info;
>  
>  	u8 tx_buf ____cacheline_aligned;
> -	u8 rx_buf[2];
> +	u8 rx_buf[4];
>  };
>  
>  static int mcp320x_channel_to_tx_data(int device_index,
> @@ -109,6 +124,15 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
>  {
>  	int ret;
>  
> +	if (adc->chip_info->conv_time) {
> +		ret = spi_sync(adc->spi, &adc->start_conv_msg);
> +		if (ret < 0)
> +			return ret;
> +
> +		usleep_range(adc->chip_info->conv_time,
> +			     adc->chip_info->conv_time + 100);
> +	}
> +
>  	memset(&adc->rx_buf, 0, sizeof(adc->rx_buf));
>  	if (adc->chip_info->num_channels > 1)
>  		adc->tx_buf = mcp320x_channel_to_tx_data(device_index, channel,
> @@ -139,6 +163,31 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
>  		*val = sign_extend32((adc->rx_buf[0] & 0x1f) << 8
>  				    | adc->rx_buf[1], 12);
>  		return 0;
> +	case mcp3550_50:
> +	case mcp3550_60:
> +	case mcp3551:
> +	case mcp3553: {
> +		u32 raw = be32_to_cpup((u32 *)adc->rx_buf);
> +
> +		if (!(adc->spi->mode & SPI_CPOL))
> +			raw <<= 1; /* strip Data Ready bit in SPI mode 0,0 */
> +
> +		/*
> +		 * If the input is within -vref and vref, bit 21 is the sign.
> +		 * Up to 12% overrange or underrange are allowed, in which case
> +		 * bit 23 is the sign and bit 0 to 21 is the value.
> +		 */
> +		raw >>= 8;
> +		if (raw & BIT(22) && raw & BIT(23))
> +			return -EIO; /* cannot have overrange AND underrange */
> +		else if (raw & BIT(22))
> +			raw &= ~BIT(22); /* overrange */
> +		else if (raw & BIT(23) || raw & BIT(21))
> +			raw |= GENMASK(31, 22); /* underrange or negative */
> +
> +		*val = (s32)raw;
> +		return 0;
> +		}
>  	default:
>  		return -EINVAL;
>  	}
> @@ -297,6 +346,31 @@ static const struct mcp320x_chip_info mcp320x_chip_infos[] = {
>  		.num_channels = ARRAY_SIZE(mcp3201_channels),
>  		.resolution = 13
>  	},
> +	[mcp3550_50] = {
> +		.channels = mcp3201_channels,
> +		.num_channels = ARRAY_SIZE(mcp3201_channels),
> +		.resolution = 21,
> +		/* 2% max deviation + 144 clock periods to exit shutdown */
> +		.conv_time = 80000 * 1.02 + 144000 / 102.4,
> +	},
> +	[mcp3550_60] = {
> +		.channels = mcp3201_channels,
> +		.num_channels = ARRAY_SIZE(mcp3201_channels),
> +		.resolution = 21,
> +		.conv_time = 66670 * 1.02 + 144000 / 122.88,
> +	},
> +	[mcp3551] = {
> +		.channels = mcp3201_channels,
> +		.num_channels = ARRAY_SIZE(mcp3201_channels),
> +		.resolution = 21,
> +		.conv_time = 73100 * 1.02 + 144000 / 112.64,
> +	},
> +	[mcp3553] = {
> +		.channels = mcp3201_channels,
> +		.num_channels = ARRAY_SIZE(mcp3201_channels),
> +		.resolution = 21,
> +		.conv_time = 16670 * 1.02 + 144000 / 122.88,
> +	},
>  };
>  
>  static int mcp320x_probe(struct spi_device *spi)
> @@ -304,7 +378,7 @@ static int mcp320x_probe(struct spi_device *spi)
>  	struct iio_dev *indio_dev;
>  	struct mcp320x *adc;
>  	const struct mcp320x_chip_info *chip_info;
> -	int ret;
> +	int ret, device_index;
>  
>  	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
>  	if (!indio_dev)
> @@ -320,7 +394,8 @@ static int mcp320x_probe(struct spi_device *spi)
>  	indio_dev->info = &mcp320x_info;
>  	spi_set_drvdata(spi, indio_dev);
>  
> -	chip_info = &mcp320x_chip_infos[spi_get_device_id(spi)->driver_data];
> +	device_index = spi_get_device_id(spi)->driver_data;
> +	chip_info = &mcp320x_chip_infos[device_index];
>  	indio_dev->channels = chip_info->channels;
>  	indio_dev->num_channels = chip_info->num_channels;
>  
> @@ -329,7 +404,8 @@ static int mcp320x_probe(struct spi_device *spi)
>  	adc->transfer[0].tx_buf = &adc->tx_buf;
>  	adc->transfer[0].len = sizeof(adc->tx_buf);
>  	adc->transfer[1].rx_buf = adc->rx_buf;
> -	adc->transfer[1].len = sizeof(adc->rx_buf);
> +	adc->transfer[1].len = DIV_ROUND_UP(chip_info->resolution, 8);
> +
>  	if (chip_info->num_channels == 1)
>  		/* single-channel converters are rx only (no MOSI pin) */
>  		spi_message_init_with_transfers(&adc->msg,
> @@ -338,6 +414,32 @@ static int mcp320x_probe(struct spi_device *spi)
>  		spi_message_init_with_transfers(&adc->msg, adc->transfer,
>  						ARRAY_SIZE(adc->transfer));
>  
> +	switch (device_index) {
> +	case mcp3550_50:
> +	case mcp3550_60:
> +	case mcp3551:
> +	case mcp3553:
> +		/* rx len increases from 24 to 25 bit in SPI mode 0,0 */
> +		if (!(spi->mode & SPI_CPOL))
> +			adc->transfer[1].len++;
> +
> +		/* conversions are started by asserting CS pin for 8 usec */
> +		adc->start_conv_transfer.delay_usecs = 8;
> +		spi_message_init_with_transfers(&adc->start_conv_msg,
> +						&adc->start_conv_transfer, 1);
> +
> +		/*
> +		 * If CS was previously kept low (continuous conversion mode)
> +		 * and then changed to high, the chip is in shutdown.
> +		 * Sometimes it fails to wake from shutdown and clocks out
> +		 * only 0xffffff.  The magic sequence of performing two
> +		 * conversions without delay between them resets the chip
> +		 * and ensures all subsequent conversions succeed.
> +		 */
> +		mcp320x_adc_conversion(adc, 0, 1, device_index, &ret);
> +		mcp320x_adc_conversion(adc, 0, 1, device_index, &ret);
> +	}
> +
>  	adc->reg = devm_regulator_get(&spi->dev, "vref");
>  	if (IS_ERR(adc->reg))
>  		return PTR_ERR(adc->reg);
> @@ -392,6 +494,10 @@ static const struct of_device_id mcp320x_dt_ids[] = {
>  	{ .compatible = "microchip,mcp3204" },
>  	{ .compatible = "microchip,mcp3208" },
>  	{ .compatible = "microchip,mcp3301" },
> +	{ .compatible = "microchip,mcp3550-50" },
> +	{ .compatible = "microchip,mcp3550-60" },
> +	{ .compatible = "microchip,mcp3551" },
> +	{ .compatible = "microchip,mcp3553" },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, mcp320x_dt_ids);
> @@ -407,6 +513,10 @@ static const struct spi_device_id mcp320x_id[] = {
>  	{ "mcp3204", mcp3204 },
>  	{ "mcp3208", mcp3208 },
>  	{ "mcp3301", mcp3301 },
> +	{ "mcp3550-50", mcp3550_50 },
> +	{ "mcp3550-60", mcp3550_60 },
> +	{ "mcp3551", mcp3551 },
> +	{ "mcp3553", mcp3553 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(spi, mcp320x_id);
> @@ -423,5 +533,5 @@ static struct spi_driver mcp320x_driver = {
>  module_spi_driver(mcp320x_driver);
>  
>  MODULE_AUTHOR("Oskar Andero <oskar.andero@gmail.com>");
> -MODULE_DESCRIPTION("Microchip Technology MCP3x01/02/04/08");
> +MODULE_DESCRIPTION("Microchip Technology MCP3x01/02/04/08 and MCP3550/1/3");
>  MODULE_LICENSE("GPL v2");

Re: [PATCH v2 4/6] dt-bindings: iio: adc: mcp320x: Update for mcp3550/1/3

[Jonathan Cameron]

On Sat, 9 Sep 2017 20:32:41 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> All chips supported by this driver clock data out on the falling edge
> and latch data in on the rising edge, hence SPI mode (0,0) or (1,1)
> must be used.
> 
> Furthermore, none of the chips has an internal reference voltage
> regulator, so an external supply is always required and needs to be
> specified in the device tree lest the IIO "scale" in sysfs cannot be
> calculated.
> 
> Document these requirements in the device tree binding, add compatible
> strings for the newly supported mcp3550/1/3 and explain that SPI mode
> (0,0) should be preferred for these chips.
> 
> Cc: Mathias Duckeck <m.duckeck@kunbus.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

I'm happy with this, but just want to give Rob / Mark more time to take
a look at it.

Give me a poke if I seem to have forgotten it in a week or so.

Thanks,

Jonathan
> ---
> Changes since v1:
> - Move support for continuous conversion mode to separate patch
>   which is marked informational / not for merging. (Rob, Jonathan)
> 
>  Documentation/devicetree/bindings/iio/adc/mcp320x.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
> index bcd3ac8e6e0c..7d64753df949 100644
> --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
> @@ -29,15 +29,29 @@ Required properties:
>  				"microchip,mcp3204"
>  				"microchip,mcp3208"
>  				"microchip,mcp3301"
> +				"microchip,mcp3550-50"
> +				"microchip,mcp3550-60"
> +				"microchip,mcp3551"
> +				"microchip,mcp3553"
>  
>  			NOTE: The use of the compatibles with no vendor prefix
>  			is deprecated and only listed because old DT use them.
>  
> +	- spi-cpha, spi-cpol (boolean):
> +			Either SPI mode (0,0) or (1,1) must be used, so specify
> +			none or both of spi-cpha, spi-cpol.  The MCP3550/1/3
> +			is more efficient in mode (1,1) as only 3 instead of
> +			4 bytes need to be read from the ADC, but not all SPI
> +			masters support it.
> +
> +	- vref-supply:	Phandle to the external reference voltage supply.
> +
>  Examples:
>  spi_controller {
>  	mcp3x0x@0 {
>  		compatible = "mcp3002";
>  		reg = <0>;
>  		spi-max-frequency = <1000000>;
> +		vref-supply = <&vref_reg>;
>  	};
>  };

Re: [PATCH v2 4/6] dt-bindings: iio: adc: mcp320x: Update for mcp3550/1/3

[Rob Herring]

On Sat, Sep 09, 2017 at 08:32:41PM +0200, Lukas Wunner wrote:
> All chips supported by this driver clock data out on the falling edge
> and latch data in on the rising edge, hence SPI mode (0,0) or (1,1)
> must be used.
> 
> Furthermore, none of the chips has an internal reference voltage
> regulator, so an external supply is always required and needs to be
> specified in the device tree lest the IIO "scale" in sysfs cannot be
> calculated.
> 
> Document these requirements in the device tree binding, add compatible
> strings for the newly supported mcp3550/1/3 and explain that SPI mode
> (0,0) should be preferred for these chips.
> 
> Cc: Mathias Duckeck <m.duckeck@kunbus.de>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> Changes since v1:
> - Move support for continuous conversion mode to separate patch
>   which is marked informational / not for merging. (Rob, Jonathan)
> 
>  Documentation/devicetree/bindings/iio/adc/mcp320x.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 5/6] iio: adc: mcp320x: Add support for mcp3550/1/3

[Lukas Wunner]

On Sun, Sep 10, 2017 at 05:15:14PM +0100, Jonathan Cameron wrote:
> On Sat, 9 Sep 2017 20:32:41 +0200 Lukas Wunner <lukas@wunner.de> wrote:
> > These ADCs are marketed as single-channel 22 bit delta-sigma ADCs, but
> > in reality their resolution is 21 bit with an overrange or underrange
> > of 12% beyond Vref.  In other words, "full scale" means +/- 2^20.
[snip]
> 
> Excellent.  Applied to the togreg branch of iio.git and pushed out as
> testing.

Just a gentle reminder, this patch isn't on your kernel.org branches yet.
I assumed the reason was a dependency / merge conflict with my two patches
on the fixes-togreg branch and you were waiting for them to be in upstream.
They have landed in Linus' tree yesterday, I'm not sure if anything else is
necessary (backmerge or some such).

Thanks,

Lukas

Re: [PATCH v2 5/6] iio: adc: mcp320x: Add support for mcp3550/1/3

[Jonathan Cameron]

On 4 October 2017 20:50:37 BST, Lukas Wunner <lukas@wunner=2Ede> wrote:
>On Sun, Sep 10, 2017 at 05:15:14PM +0100, Jonathan Cameron wrote:
>> On Sat, 9 Sep 2017 20:32:41 +0200 Lukas Wunner <lukas@wunner=2Ede>
>wrote:
>> > These ADCs are marketed as single-channel 22 bit delta-sigma ADCs,
>but
>> > in reality their resolution is 21 bit with an overrange or
>underrange
>> > of 12% beyond Vref=2E  In other words, "full scale" means +/- 2^20=2E
>[snip]
>>=20
>> Excellent=2E  Applied to the togreg branch of iio=2Egit and pushed out =
as
>> testing=2E
>
>Just a gentle reminder, this patch isn't on your kernel=2Eorg branches
>yet=2E
>I assumed the reason was a dependency / merge conflict with my two
>patches
>on the fixes-togreg branch and you were waiting for them to be in
>upstream=2E
>They have landed in Linus' tree yesterday, I'm not sure if anything
>else is
>necessary (backmerge or some such)=2E
>
>Thanks,
>
>Lukas

Will investigate=2E  Odd that I didn't mention waiting for dependencies=2E=
=2E=2E  If necessary it may
 be another week or so for it to propagate back to the IIO tree=2E

Jonathan

>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger=2Ekernel=2Eorg
>More majordomo info at  http://vger=2Ekernel=2Eorg/majordomo-info=2Ehtml

--=20
Sent from my Android device with K-9 Mail=2E Please excuse my brevity=2E

Re: [PATCH v2 5/6] iio: adc: mcp320x: Add support for mcp3550/1/3

[Jonathan Cameron]

On Wed, 04 Oct 2017 21:53:07 +0100
Jonathan Cameron <jic23@jic23.retrosnub.co.uk> wrote:

> On 4 October 2017 20:50:37 BST, Lukas Wunner <lukas@wunner.de> wrote:
> >On Sun, Sep 10, 2017 at 05:15:14PM +0100, Jonathan Cameron wrote:  
> >> On Sat, 9 Sep 2017 20:32:41 +0200 Lukas Wunner <lukas@wunner.de>  
> >wrote:  
> >> > These ADCs are marketed as single-channel 22 bit delta-sigma ADCs,  
> >but  
> >> > in reality their resolution is 21 bit with an overrange or  
> >underrange  
> >> > of 12% beyond Vref.  In other words, "full scale" means +/- 2^20.  
> >[snip]  
> >> 
> >> Excellent.  Applied to the togreg branch of iio.git and pushed out as
> >> testing.  
> >
> >Just a gentle reminder, this patch isn't on your kernel.org branches
> >yet.
> >I assumed the reason was a dependency / merge conflict with my two
> >patches
> >on the fixes-togreg branch and you were waiting for them to be in
> >upstream.
> >They have landed in Linus' tree yesterday, I'm not sure if anything
> >else is
> >necessary (backmerge or some such).
> >
> >Thanks,
> >
> >Lukas  
> 
> Will investigate.  Odd that I didn't mention waiting for dependencies...  If necessary it may
>  be another week or so for it to propagate back to the IIO tree.
> 
OK.  Looks like you were correct - this is waiting on the fixes to work
their way back into my tree.  After the next rc is out (first one with these
in as they aren't in rc3) I'll ask Greg if he is willing to do a merge
with mainline as I have a load of ST patches queued up waiting for
some fixes as well.  Right now I'm deliberately not pushing out a non
rebasing branch like I normally would - so as to be able to rebase on
these fixes when they become available in Greg's tree.

Thanks and sorry about the completely incorrect email I sent previously
saying I'd applied these.

Jonathan

> Jonathan
> 
> >--
> >To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html  
> 

Re: [PATCH v2 5/6] iio: adc: mcp320x: Add support for mcp3550/1/3

[Lukas Wunner]

On Sun, Oct 08, 2017 at 11:30:13AM +0100, Jonathan Cameron wrote:
> On Wed, 04 Oct 2017 21:53:07 +0100 Jonathan Cameron <jic23@jic23.retrosnub.co.uk> wrote:
> > On 4 October 2017 20:50:37 BST, Lukas Wunner <lukas@wunner.de> wrote:
> > > Just a gentle reminder, this patch isn't on your kernel.org branches
> > > yet. I assumed the reason was a dependency / merge conflict with my
> > > two patches on the fixes-togreg branch and you were waiting for them
> > > to be in upstream.
> > > They have landed in Linus' tree yesterday, I'm not sure if anything
> > > else is necessary (backmerge or some such).
>
> OK.  Looks like you were correct - this is waiting on the fixes to work
> their way back into my tree.  After the next rc is out (first one with these
> in as they aren't in rc3) I'll ask Greg if he is willing to do a merge
> with mainline as I have a load of ST patches queued up waiting for
> some fixes as well.  Right now I'm deliberately not pushing out a non
> rebasing branch like I normally would - so as to be able to rebase on
> these fixes when they become available in Greg's tree.

Greg merged 4.14-rc4 into staging-next 48 minutes ago, so ready to go now.

Thanks,

Lukas

Re: [PATCH v2 5/6] iio: adc: mcp320x: Add support for mcp3550/1/3

[Jonathan Cameron]

On Mon, 9 Oct 2017 09:54:16 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> On Sun, Oct 08, 2017 at 11:30:13AM +0100, Jonathan Cameron wrote:
> > On Wed, 04 Oct 2017 21:53:07 +0100 Jonathan Cameron <jic23@jic23.retrosnub.co.uk> wrote:  
> > > On 4 October 2017 20:50:37 BST, Lukas Wunner <lukas@wunner.de> wrote:  
> > > > Just a gentle reminder, this patch isn't on your kernel.org branches
> > > > yet. I assumed the reason was a dependency / merge conflict with my
> > > > two patches on the fixes-togreg branch and you were waiting for them
> > > > to be in upstream.
> > > > They have landed in Linus' tree yesterday, I'm not sure if anything
> > > > else is necessary (backmerge or some such).  
> >
> > OK.  Looks like you were correct - this is waiting on the fixes to work
> > their way back into my tree.  After the next rc is out (first one with these
> > in as they aren't in rc3) I'll ask Greg if he is willing to do a merge
> > with mainline as I have a load of ST patches queued up waiting for
> > some fixes as well.  Right now I'm deliberately not pushing out a non
> > rebasing branch like I normally would - so as to be able to rebase on
> > these fixes when they become available in Greg's tree.  
> 
> Greg merged 4.14-rc4 into staging-next 48 minutes ago, so ready to go now.
> 
Thanks.  I've rebased my (not yet public) togreg branch and pushed out
as testing with this patch applied.

Thanks,

Jonathan

> Thanks,
> 
> Lukas