Skip to contentSkip to conversation

iio: adc: mcp320x. Add support for more ADCs

PATCH Superseded Søren Andersen 2 participants 2 messages Sep 5, 2014 – Sep 9, 2014 IIO

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 2/3 iio: adc: mcp320x. Add support for more ADCs

v3:
Functions moved around.
Return values is simpler.

Signed-off-by Soeren Andersen <san at rosetechnology.dk>

---
drivers/iio/adc/mcp320x.c | 201
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------

 1 file changed, 172 insertions(+), 29 deletions(-)
drivers/iio/adc/mcp320x.c +60 -12
index 28a086e..77ee363 100644
@@ -1,9 +1,20 @@
 /*
  * Copyright (C) 2013 Oskar Andero <[email protected]>
+ * Copyright (C) 2014 Allan Bendorff Jensen <[email protected]>
  *
- * Driver for Microchip Technology's MCP3204 and MCP3208 ADC chips.
- * Datasheet can be found here:
- * http://ww1.microchip.com/downloads/en/devicedoc/21298c.pdf
+ * Driver for following ADC chips from Microchip Technology's:
+ * 10 Bit converter
+ * MCP3001
+ * MCP3002
+ * MCP3004
+ * MCP3008
+ * ------------
+ * 12 bit converter
+ * MCP3201
+ * MCP3202
+ * MCP3204
+ * MCP3208
+ * ------------
  *
  * 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
@@ -16,10 +27,16 @@
 #include <linux/iio/iio.h>
 #include <linux/regulator/consumer.h>
 
-#define MCP_SINGLE_ENDED	(1 << 3)
-#define MCP_START_BIT		(1 << 4)
+static int mcp320x_channel_to_tx_data(int device_index,
+				const unsigned int channel, bool differential);
 
 enum {
+	mcp3001,
+	mcp3002,
+	mcp3004,
+	mcp3008,
+	mcp3201,
+	mcp3202,
 	mcp3204,
 	mcp3208,
 };
@@ -36,17 +53,48 @@ struct mcp320x {
 	struct mutex lock;
 };
 
-static int mcp320x_adc_conversion(struct mcp320x *adc, u8 msg)
+static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
+					bool differential, int device_index)
 {
 	int ret;
+	__be16 buf;
 
-	adc->tx_buf = msg;
-	ret = spi_sync(adc->spi, &adc->msg);
-	if (ret < 0)
-		return ret;
+	adc->rx_buf[0] = 0;
+	adc->rx_buf[1] = 0;
+	adc->tx_buf = mcp320x_channel_to_tx_data(device_index,
+						channel, differential);
+
+	if (device_index != mcp3001 && device_index != mcp3201) {
+		ret = spi_sync(adc->spi, &adc->msg);
+		if (ret < 0)
+			return ret;
+	} else {
+		ret = spi_read(adc->spi, (u8 *)&buf, 2);
+		if (ret < 0)
+			return ret;
+	}
 
-	return ((adc->rx_buf[0] & 0x3f) << 6)  |
-		(adc->rx_buf[1] >> 2);
+	switch (device_index) {
+	case mcp3001:
+		return ((be16_to_cpu(buf) >> 3) & 0x03ff);
+	case mcp3002:
+		return (((adc->rx_buf[0] << 2) |
+				((adc->rx_buf[1] & 0xC0) >> 6)) & 0x03ff);
+	case mcp3004:
+	case mcp3008:
+		return ((((adc->rx_buf[0] & 0x7f) << 3) |
+				((adc->rx_buf[1] & 0xe0) >> 5)) & 0x03ff);
+	case mcp3201:
+		return (be16_to_cpu(buf) >> 1);
+	case mcp3202:
+		return ((adc->rx_buf[0] << 4) | ((adc->rx_buf[1] & 0xf0) >> 4));
+	case mcp3204:
+	case mcp3208:
+		return (((adc->rx_buf[0] & 0x7f) << 5) |
+				((adc->rx_buf[1] & 0xf8) >> 3));
+	default:
+		return -EINVAL;
+	}
 }
 
 static int mcp320x_read_raw(struct iio_dev *indio_dev,
@@ -55,18 +103,17 @@ static int mcp320x_read_raw(struct iio_dev

*indio_dev,
{
struct mcp320x *adc = iio_priv(indio_dev); int ret = -EINVAL;
+ int device_index = 0;

mutex_lock(&adc->lock);

+ device_index = spi_get_device_id(adc->spi)->driver_data;
+
switch (mask) {
case IIO_CHAN_INFO_RAW:
- if (channel->differential)
- ret = mcp320x_adc_conversion(adc,
- MCP_START_BIT | channel->address);
- else
- ret = mcp320x_adc_conversion(adc,
- MCP_START_BIT | MCP_SINGLE_ENDED |
- channel->address);
+ ret = mcp320x_adc_conversion(adc, channel->address,
+ channel->differential, device_index);
+
if (ret < 0)
goto out;

@@ -75,17 +122,23 @@ static int mcp320x_read_raw(struct iio_dev *indio_dev,
break;

case IIO_CHAN_INFO_SCALE:
- /* Digital output code = (4096 * Vin) / Vref */
+ /*
+ * Digital output code = (resolution * Vin) / Vref
+ * Get regulator output voltage in uV.
+ */
ret = regulator_get_voltage(adc->reg); if (ret < 0)
goto out;

+ /* convert regulator output voltage to mV */
*val = ret / 1000;
- *val2 = 12;
- ret = IIO_VAL_FRACTIONAL_LOG2;
- break;
+ if (device_index == mcp3001 || device_index == mcp3002 ||
+ device_index == mcp3004 || device_index == mcp3008)
+ *val2 = 10;
+ else
+ *val2 = 12;

- default:
+ ret = IIO_VAL_FRACTIONAL_LOG2;
break;
}

@@ -117,6 +170,16 @@ out:
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) \
}

+static const struct iio_chan_spec mcp3201_channels[] = {
+ MCP320X_VOLTAGE_CHANNEL_DIFF(0),
+};
+
+static const struct iio_chan_spec mcp3202_channels[] = {
+ MCP320X_VOLTAGE_CHANNEL(0),
+ MCP320X_VOLTAGE_CHANNEL(1),
+ MCP320X_VOLTAGE_CHANNEL_DIFF(0),
+};
+
static const struct iio_chan_spec mcp3204_channels[] = {
MCP320X_VOLTAGE_CHANNEL(0), MCP320X_VOLTAGE_CHANNEL(1),
@@ -146,12 +209,36 @@ static const struct iio_info mcp320x_info = {
.driver_module = THIS_MODULE,
};

-struct mcp3208_chip_info {
+struct mcp320x_chip_info {
const struct iio_chan_spec *channels;
unsigned int num_channels;
};

-static const struct mcp3208_chip_info mcp3208_chip_infos[] = { +static const struct mcp320x_chip_info mcp320x_chip_infos[] = {
+ [mcp3001] = {
+ .channels = mcp3201_channels,
+ .num_channels = ARRAY_SIZE(mcp3201_channels)
+ },
+ [mcp3002] = {
+ .channels = mcp3202_channels,
+ .num_channels = ARRAY_SIZE(mcp3202_channels)
+ },
+ [mcp3004] = {
+ .channels = mcp3204_channels,
+ .num_channels = ARRAY_SIZE(mcp3204_channels)
+ },
+ [mcp3008] = {
+ .channels = mcp3208_channels,
+ .num_channels = ARRAY_SIZE(mcp3208_channels)
+ },
+ [mcp3201] = {
+ .channels = mcp3201_channels,
+ .num_channels = ARRAY_SIZE(mcp3201_channels)
+ },
+ [mcp3202] = {
+ .channels = mcp3202_channels,
+ .num_channels = ARRAY_SIZE(mcp3202_channels)
+ },
[mcp3204] = {
.channels = mcp3204_channels, .num_channels = ARRAY_SIZE(mcp3204_channels)
@@ -162,11 +249,29 @@ static const struct mcp3208_chip_info mcp3208_chip_infos[] = {
},
};

+static int mcp320x_channel_to_tx_data(int device_index,
+ const unsigned int channel, bool differential)
+{
+ switch (device_index) {
+ case mcp3002:
+ case mcp3202:
+ return(0x12 | (!differential << 3) | ((channel & 0x01) << 2));
+ case mcp3004:
+ case mcp3204:
+ return(0x20 | (!differential << 4) | ((channel & 0x03) << 1));
+ case mcp3008:
+ case mcp3208:
+ return(0x20 | (!differential << 4) | ((channel & 0x07) << 1));
+ default:
+ return -EINVAL;
+ }
+}
+
static int mcp320x_probe(struct spi_device *spi) {
struct iio_dev *indio_dev;
struct mcp320x *adc;
- const struct mcp3208_chip_info *chip_info;
+ const struct mcp320x_chip_info *chip_info;
int ret;

indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
@@ -181,7 +286,7 @@ static int mcp320x_probe(struct spi_device *spi)
indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->info = &mcp320x_info;

- chip_info = &mcp3208_chip_infos[spi_get_device_id(spi)->driver_data];
+ chip_info = &mcp320x_chip_infos[spi_get_device_id(spi)->driver_data];
indio_dev->channels = chip_info->channels; indio_dev->num_channels = chip_info->num_channels;

@@ -226,7 +331,45 @@ static int mcp320x_remove(struct spi_device *spi)
return 0;
}

+#if defined(CONFIG_OF)
+static const struct of_device_id mcp320x_dt_ids[] = {
+ {
+ .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[mcp3004],
+ }, {
+ .compatible = "mcp3201",
+ .data = &mcp320x_chip_infos[mcp3201],
+ }, {
+ .compatible = "mcp3202",
+ .data = &mcp320x_chip_infos[mcp3202],
+ }, {
+ .compatible = "mcp3204",
+ .data = &mcp320x_chip_infos[mcp3204],
+ }, {
+ .compatible = "mcp3008",
+ .data = &mcp320x_chip_infos[mcp3208],
+ }, {
+ }
+};
+MODULE_DEVICE_TABLE(of, mcp320x_dt_ids);
+#endif
+
static const struct spi_device_id mcp320x_id[] = {
+ { "mcp3001", mcp3001 },
+ { "mcp3002", mcp3002 },
+ { "mcp3004", mcp3004 },
+ { "mcp3008", mcp3008 },
+ { "mcp3201", mcp3201 },
+ { "mcp3202", mcp3202 },
{ "mcp3204", mcp3204 },
{ "mcp3208", mcp3208 },
{ }
@@ -245,5 +388,5 @@ static struct spi_driver mcp320x_driver = {
module_spi_driver(mcp320x_driver);

MODULE_AUTHOR("Oskar Andero <[email protected]>");
-MODULE_DESCRIPTION("Microchip Technology MCP3204/08"); +MODULE_DESCRIPTION("Microchip Technology MCP3x01/02/04/08");
MODULE_LICENSE("GPL v2");

#1 lore raw

Søren Andersen schrieb, Am 05.09.2014 13:44:

Quoted 3 lines

v3:
Functions moved around.
Return values is simpler.

This comment should be below the ---. Yet, you lack a proper commit message.

Quoted 12 lines
Signed-off-by Soeren Andersen <san at rosetechnology.dk>

---
drivers/iio/adc/mcp320x.c | 201
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------

 1 file changed, 172 insertions(+), 29 deletions(-)
drivers/iio/adc/mcp320x.c +0 -0
index 28a086e..77ee363 100644

drivers/iio/adc/mcp320x.c line 3

Quoted 3 lines
drivers/iio/adc/mcp320x.c +1 -0
@@ -1,9 +1,20 @@
 /*
  * Copyright (C) 2013 Oskar Andero <[email protected]>
+ * Copyright (C) 2014 Allan Bendorff Jensen <[email protected]>

I'm a bit confused, as how Allan is related in the development of this driver?

drivers/iio/adc/mcp320x.c line 6

Quoted 4 lines
drivers/iio/adc/mcp320x.c +0 -3
  *
- * Driver for Microchip Technology's MCP3204 and MCP3208 ADC chips.
- * Datasheet can be found here:
- * http://ww1.microchip.com/downloads/en/devicedoc/21298c.pdf

Hmm, providing less information on where to get datasheets instead of adding the sources for the additional devices is not exactly what I like to see happening.

Quoted 17 lines
drivers/iio/adc/mcp320x.c +13 -0
+ * Driver for following ADC chips from Microchip Technology's:
+ * 10 Bit converter
+ * MCP3001
+ * MCP3002
+ * MCP3004
+ * MCP3008
+ * ------------
+ * 12 bit converter
+ * MCP3201
+ * MCP3202
+ * MCP3204
+ * MCP3208
+ * ------------
  *
  * 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

drivers/iio/adc/mcp320x.c line 31

Quoted 7 lines
drivers/iio/adc/mcp320x.c +2 -2
@@ -16,10 +27,16 @@
 #include <linux/iio/iio.h>
 #include <linux/regulator/consumer.h>
 
-#define MCP_SINGLE_ENDED	(1 << 3)
-#define MCP_START_BIT		(1 << 4)
+static int mcp320x_channel_to_tx_data(int device_index,
+				const unsigned int channel, bool differential);

What's the point in just declaring this function in this place, instead of placing it right above mcp320x_adc_conversion()?

Quoted 12 lines
drivers/iio/adc/mcp320x.c +6 -0
 enum {
+	mcp3001,
+	mcp3002,
+	mcp3004,
+	mcp3008,
+	mcp3201,
+	mcp3202,
 	mcp3204,
 	mcp3208,
 };

drivers/iio/adc/mcp320x.c struct mcp320x { · line 57

Quoted 6 lines
drivers/iio/adc/mcp320x.c +2 -1
@@ -36,17 +53,48 @@ struct mcp320x {
 	struct mutex lock;
 };
 
-static int mcp320x_adc_conversion(struct mcp320x *adc, u8 msg)
+static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
+					bool differential, int device_index)

The second line of parameters could be indented a bit more to the left to align with the first line parameters.

drivers/iio/adc/mcp320x.c struct mcp320x { · line 60

Quoted 3 lines
drivers/iio/adc/mcp320x.c +1 -0
 {
 	int ret;
+	__be16 buf;

Wouldn't it be a better idea to change the type of rx_buf in struct mcp320x to __be16 and use it for the one channel devices?

drivers/iio/adc/mcp320x.c struct mcp320x { · line 65

Quoted 9 lines
drivers/iio/adc/mcp320x.c +4 -4
-	adc->tx_buf = msg;
-	ret = spi_sync(adc->spi, &adc->msg);
-	if (ret < 0)
-		return ret;
+	adc->rx_buf[0] = 0;
+	adc->rx_buf[1] = 0;
+	adc->tx_buf = mcp320x_channel_to_tx_data(device_index,
+						channel, differential);

In your current implementation, mcp320x_channel_to_tx_data() can return -EINVAL for mcp3x01 devices, but adc->tx_buf is of type u8. It does not really do any harm, as tx_buf is not sent to these devices, but it is ugly to say the least. Not sure if it would be better to do ret = mcp320x_channel_to_tx_data(), then check ret for errors and afterwards do adc->tx_buf = ret, or just change the type of mcp320x_channel_to_tx_data() to u8. Anyway, you need to keep mcp3x01 devices in mind.

drivers/iio/adc/mcp320x.c struct mcp320x { · line 72

Quoted 7 lines
drivers/iio/adc/mcp320x.c +7 -0
+
+	if (device_index != mcp3001 && device_index != mcp3201) {
+		ret = spi_sync(adc->spi, &adc->msg);
+		if (ret < 0)
+			return ret;
+	} else {
+		ret = spi_read(adc->spi, (u8 *)&buf, 2);

Preferably this way:
ret = spi_read(adc->spi, &adc->rx_buf), sizeof(adc->rx_buf);

Quoted 14 lines
drivers/iio/adc/mcp320x.c +11 -2
+		if (ret < 0)
+			return ret;
+	}
 
-	return ((adc->rx_buf[0] & 0x3f) << 6)  |
-		(adc->rx_buf[1] >> 2);
+	switch (device_index) {
+	case mcp3001:
+		return ((be16_to_cpu(buf) >> 3) & 0x03ff);
+	case mcp3002:
+		return (((adc->rx_buf[0] << 2) |
+				((adc->rx_buf[1] & 0xC0) >> 6)) & 0x03ff);
+	case mcp3004:
+	case mcp3008:

drivers/iio/adc/mcp320x.c struct mcp320x { · line 94

Quoted 10 lines
drivers/iio/adc/mcp320x.c +10 -0
+		return ((((adc->rx_buf[0] & 0x7f) << 3) |
+				((adc->rx_buf[1] & 0xe0) >> 5)) & 0x03ff);
+	case mcp3201:
+		return (be16_to_cpu(buf) >> 1);
+	case mcp3202:
+		return ((adc->rx_buf[0] << 4) | ((adc->rx_buf[1] & 0xf0) >> 4));
+	case mcp3204:
+	case mcp3208:
+		return (((adc->rx_buf[0] & 0x7f) << 5) |
+				((adc->rx_buf[1] & 0xf8) >> 3));

As I tried to point out in my previous review, you can make the conversion process way easier by optimizing your tx-message. For all those devices with more than 1 channel, you determine with the position of the start-bit the position of the conversion data bits. Further down I will give you an example on how to do this, so that the adc will start sending its 10 or 12 bits of data starting with the MSB of rx_buf, so you will just have to shift it to the right, without need for masking out anything. This is how the above block would turn out this way:

	case mcp3001:
		return ((be16_to_cpu(buf) >> 3) & 0x03ff);
	case mcp3002:
	case mcp3004:
	case mcp3008:
		return adc->rx_buf >> 6;
	case mcp3201:
		return (be16_to_cpu(buf) >> 1);
	case mcp3202:
	case mcp3204:
	case mcp3208:
		return adc->rx_buf >> 4;
Quoted 18 lines
drivers/iio/adc/mcp320x.c +6 -0
+	default:
+		return -EINVAL;
+	}
 }
 
 static int mcp320x_read_raw(struct iio_dev *indio_dev,
@@ -55,18 +103,17 @@ static int mcp320x_read_raw(struct iio_dev
 *indio_dev,
 {
 	struct mcp320x *adc = iio_priv(indio_dev);
 	int ret = -EINVAL;
+	int device_index = 0;
 
 	mutex_lock(&adc->lock);
 
+	device_index = spi_get_device_id(adc->spi)->driver_data;
+
 	switch (mask) {

drivers/iio/adc/mcp320x.c static int mcp320x_read_raw(struct iio_dev · line 116

Quoted 10 lines
drivers/iio/adc/mcp320x.c +2 -7
 	case IIO_CHAN_INFO_RAW:
-		if (channel->differential)
-			ret = mcp320x_adc_conversion(adc,
-				MCP_START_BIT | channel->address);
-		else
-			ret = mcp320x_adc_conversion(adc,
-				MCP_START_BIT | MCP_SINGLE_ENDED |
-				channel->address);
+		ret = mcp320x_adc_conversion(adc, channel->address,
+			channel->differential, device_index);

Indentation could be better.

Quoted 5 lines
drivers/iio/adc/mcp320x.c +1 -0
+
 		if (ret < 0)
 			goto out;

drivers/iio/adc/mcp320x.c static int mcp320x_read_raw(struct iio_dev · line 129

Quoted 9 lines
drivers/iio/adc/mcp320x.c +4 -1
@@ -75,17 +122,23 @@ static int mcp320x_read_raw(struct iio_dev
 *indio_dev,
 		break;
 
 	case IIO_CHAN_INFO_SCALE:
-		/* Digital output code = (4096 * Vin) / Vref */
+		/*
+		 * Digital output code = (resolution * Vin) / Vref
+		 * Get regulator output voltage in uV.
+		 */

I don't really see the need for these comments. It's just a matter of seconds to search in LXR for the meaning of return values.

drivers/iio/adc/mcp320x.c static int mcp320x_read_raw(struct iio_dev · line 137

Quoted 11 lines
drivers/iio/adc/mcp320x.c +3 -3
 		ret = regulator_get_voltage(adc->reg);
 		if (ret < 0)
 			goto out;
 
+		/* convert regulator output voltage to mV */
 		*val = ret / 1000;
-		*val2 = 12;
-		ret = IIO_VAL_FRACTIONAL_LOG2;
-		break;
+		if (device_index == mcp3001 || device_index == mcp3002 ||
+			device_index == mcp3004 || device_index == mcp3008)

Your indentation here is wrong, as it should be aligned with the opening parenthesis. But the whole solution is quite ugly and could be improved by adding a resolution attribute to mcp320x_chip_info, which you could then access here - either by adding a reference to the entry in mcp3208_chip_infos[] to struct mcp320x, or by adding a resolution/bits variable to struct mcp320x; being set in the probe function, as soon as chip_info can be accessed. See for example in max1363, how it can be done.

drivers/iio/adc/mcp320x.c static int mcp320x_read_raw(struct iio_dev · line 141

Quoted 4 lines
drivers/iio/adc/mcp320x.c +3 -0
+			*val2 = 10;
+		else
+			*val2 = 12;
 

This empty line should also leave.

Quoted 23 lines
drivers/iio/adc/mcp320x.c +11 -1
-	default:
+		ret = IIO_VAL_FRACTIONAL_LOG2;
 		break;
 	}
@@ -117,6 +170,16 @@ out:
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) \
 	}
 
+static const struct iio_chan_spec mcp3201_channels[] = {
+	MCP320X_VOLTAGE_CHANNEL_DIFF(0),
+};
+
+static const struct iio_chan_spec mcp3202_channels[] = {
+	MCP320X_VOLTAGE_CHANNEL(0),
+	MCP320X_VOLTAGE_CHANNEL(1),
+	MCP320X_VOLTAGE_CHANNEL_DIFF(0),
+};
+
 static const struct iio_chan_spec mcp3204_channels[] = {
 	MCP320X_VOLTAGE_CHANNEL(0),
 	MCP320X_VOLTAGE_CHANNEL(1),

drivers/iio/adc/mcp320x.c static const struct iio_info mcp320x_info = { · line 214

Quoted 7 lines
drivers/iio/adc/mcp320x.c +1 -1
@@ -146,12 +209,36 @@ static const struct iio_info mcp320x_info = {
 	.driver_module = THIS_MODULE,
 };
 
-struct mcp3208_chip_info {
+struct mcp320x_chip_info {
 	const struct iio_chan_spec *channels;
 	unsigned int num_channels;

Add for example u8 resolution here.

drivers/iio/adc/mcp320x.c static const struct iio_info mcp320x_info = { · line 220

Quoted 7 lines
drivers/iio/adc/mcp320x.c +4 -1
 };
 
-static const struct mcp3208_chip_info mcp3208_chip_infos[] = {
+static const struct mcp320x_chip_info mcp320x_chip_infos[] = {
+	[mcp3001] = {
+		.channels = mcp3201_channels,
+		.num_channels = ARRAY_SIZE(mcp3201_channels)

And then set .resolution = 10 here and appropriate below.

Quoted 25 lines
drivers/iio/adc/mcp320x.c +21 -0
+	},
+	[mcp3002] = {
+		.channels = mcp3202_channels,
+		.num_channels = ARRAY_SIZE(mcp3202_channels)
+	},
+	[mcp3004] = {
+		.channels = mcp3204_channels,
+		.num_channels = ARRAY_SIZE(mcp3204_channels)
+	},
+	[mcp3008] = {
+		.channels = mcp3208_channels,
+		.num_channels = ARRAY_SIZE(mcp3208_channels)
+	},
+	[mcp3201] = {
+		.channels = mcp3201_channels,
+		.num_channels = ARRAY_SIZE(mcp3201_channels)
+	},
+	[mcp3202] = {
+		.channels = mcp3202_channels,
+		.num_channels = ARRAY_SIZE(mcp3202_channels)
+	},
 	[mcp3204] = {
 		.channels = mcp3204_channels,
 		.num_channels = ARRAY_SIZE(mcp3204_channels)

drivers/iio/adc/mcp320x.c static const struct mcp3208_chip_info · line 254

Quoted 6 lines
drivers/iio/adc/mcp320x.c +2 -0
@@ -162,11 +249,29 @@ static const struct mcp3208_chip_info
 mcp3208_chip_infos[] = {
 	},
 };
 
+static int mcp320x_channel_to_tx_data(int device_index,
+				const unsigned int channel, bool differential)

As mentioned above, the only calling instance expects u8 as return value. Up to you, what solution you will choose.

drivers/iio/adc/mcp320x.c struct mcp320x {

Quoted 5 lines
drivers/iio/adc/mcp320x.c +5 -0
+{
+	switch (device_index) {
+	case mcp3002:
+	case mcp3202:
+		return(0x12 | (!differential << 3) | ((channel & 0x01) << 2));

Missing whitespace after return.

drivers/iio/adc/mcp320x.c struct mcp320x {

Quoted 3 lines
drivers/iio/adc/mcp320x.c +3 -0
+	case mcp3004:
+	case mcp3204:
+		return(0x20 | (!differential << 4) | ((channel & 0x03) << 1));

To align the adc data like I suggested above, use this:

		return (0x40 | (!differential << 5) | ((channel & 0x03) << 2));

drivers/iio/adc/mcp320x.c struct mcp320x {

Quoted 3 lines
drivers/iio/adc/mcp320x.c +3 -0
+	case mcp3008:
+	case mcp3208:
+		return(0x20 | (!differential << 4) | ((channel & 0x07) << 1));

Same here:

		return (0x40 | (!differential << 5) | ((channel & 0x07) << 2));

drivers/iio/adc/mcp320x.c struct mcp320x { · line 96

Quoted 2 lines
drivers/iio/adc/mcp320x.c +2 -0
+	default:
+		return -EINVAL;

Cases for mcp3x01 are missing and would be caught by default.

Quoted 29 lines
drivers/iio/adc/mcp320x.c +4 -1
+	}
+}
+
 static int mcp320x_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
 	struct mcp320x *adc;
-	const struct mcp3208_chip_info *chip_info;
+	const struct mcp320x_chip_info *chip_info;
 	int ret;
 
drivers/iio/adc/mcp320x.c +5 -1
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
@@ -181,7 +286,7 @@ static int mcp320x_probe(struct spi_device *spi)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &mcp320x_info;
 
-	chip_info = &mcp3208_chip_infos[spi_get_device_id(spi)->driver_data];
+	chip_info = &mcp320x_chip_infos[spi_get_device_id(spi)->driver_data];
 	indio_dev->channels = chip_info->channels;
 	indio_dev->num_channels = chip_info->num_channels;
 
@@ -226,7 +331,45 @@ static int mcp320x_remove(struct spi_device *spi)
 	return 0;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id mcp320x_dt_ids[] = {
+	{
+		.compatible = "mcp3001",

drivers/iio/adc/mcp320x.c static int mcp320x_remove(struct spi_device *spi) · line 347

Quoted 10 lines
drivers/iio/adc/mcp320x.c +10 -0
+		.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[mcp3004],

mcp3008?

drivers/iio/adc/mcp320x.c static int mcp320x_remove(struct spi_device *spi) · line 358

Quoted 11 lines
drivers/iio/adc/mcp320x.c +11 -0
+	}, {
+		.compatible = "mcp3201",
+		.data = &mcp320x_chip_infos[mcp3201],
+	}, {
+		.compatible = "mcp3202",
+		.data = &mcp320x_chip_infos[mcp3202],
+	}, {
+		.compatible = "mcp3204",
+		.data = &mcp320x_chip_infos[mcp3204],
+	}, {
+		.compatible = "mcp3008",

mcp3208?

Quoted 33 lines
drivers/iio/adc/mcp320x.c +14 -2
+		.data = &mcp320x_chip_infos[mcp3208],
+	}, {
+	}
+};
+MODULE_DEVICE_TABLE(of, mcp320x_dt_ids);
+#endif
+
 static const struct spi_device_id mcp320x_id[] = {
+	{ "mcp3001", mcp3001 },
+	{ "mcp3002", mcp3002 },
+	{ "mcp3004", mcp3004 },
+	{ "mcp3008", mcp3008 },
+	{ "mcp3201", mcp3201 },
+	{ "mcp3202", mcp3202 },
 	{ "mcp3204", mcp3204 },
 	{ "mcp3208", mcp3208 },
 	{ }
@@ -245,5 +388,5 @@ static struct spi_driver mcp320x_driver = {
 module_spi_driver(mcp320x_driver);
 
 MODULE_AUTHOR("Oskar Andero <[email protected]>");
-MODULE_DESCRIPTION("Microchip Technology MCP3204/08");
+MODULE_DESCRIPTION("Microchip Technology MCP3x01/02/04/08");
 MODULE_LICENSE("GPL v2");
 
 
 
 
--

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