G'day Lars-Peter,
On 17/01/2018 01:01, Lars-Peter Clausen wrote:
On 01/16/2018 10:49 AM, Phil Reid wrote:
Add implementation for Analog Devices AD5272 and AD5274 digital
potentiometer devices.
Signed-off-by: Phil Reid <preid@xxxxxxxxxxxxxxxxx>
Hi,
Thanks for the patch. Looks mostly good.
diff --git a/drivers/iio/potentiometer/ad5272.c b/drivers/iio/potentiometer/ad5272.c
new file mode 100644
index 0000000..a04b018
--- /dev/null
+++ b/drivers/iio/potentiometer/ad5272.c
@@ -0,0 +1,227 @@
+/*
+ * Analog Devices AD5372 digital potentiometer driver
+ * Copyright (C) 2018 Phil Reid <preid@xxxxxxxxxxxxxxxxx>
+ *
+ * Datasheet: http://www.analog.com/media/en/technical-documentation/data-sheets/AD5272_5274.pdf
+ *
+ * DEVID #Wipers #Positions Resistor Opts (kOhm) i2c address
+ * ad5272 1 1024 20, 50, 100 01011xx
+ * ad5274 1 256 20, 100 01011xx
+ *
+ * SPDX-License-Identifier: GPL-2.0+
People will probably complain about this, there are some weired rules where
and how the SPDX identifier should be.
Jonathan suggested this in his review of v1. Forgot to mention that in the change list.
+ */
[...]
+
+static const struct ad5272_cfg ad5272_cfg[] = {
+ /* on-semiconductor parts */
Copy&paste error?
yeap.
+ [AD5272_020] = { .max_pos = 1024, .kohms = 20 },
+ [AD5272_050] = { .max_pos = 1024, .kohms = 50 },
+ [AD5272_100] = { .max_pos = 1024, .kohms = 100 },
+ [AD5274_020] = { .max_pos = 256, .kohms = 20, .shift = 2 },
+ [AD5274_100] = { .max_pos = 256, .kohms = 100, .shift = 2 },
+};
[...]
+
+static int ad5272_write(struct ad5272_data *data, int reg, int val)
+{
+ u8 buf[2] = { (reg << 2) | ((val >> 8) & 0x3), (u8)val };
+ int ret;
+
+ mutex_lock(&data->lock);
+ ret = i2c_master_send(data->client, buf, sizeof(buf));
+ mutex_unlock(&data->lock);
+ return ret < 0 ? ret : 0;
+}
+
+static int ad5272_read(struct ad5272_data *data, int reg, int *val)
+{
+ u8 buf[2] = {reg << 2, 0};
I believe the general recommendation is to not use stack based transfer
buffer to avoid potential issues in case the I2C master driver uses DMA.
There are many examples IIO example drivers that use DMA safe transfer
buffers. Look for ____cacheline_aligned.
ok.
+ int ret;
+
+ mutex_lock(&data->lock);
+ ret = i2c_master_send(data->client, buf, sizeof(buf));
+ if (ret < 0)
+ goto error;
+
+ ret = i2c_master_recv(data->client, buf, sizeof(buf));
+ if (ret < 0)
+ goto error;
+
This should be one I2C transfer, otherwise some other driver might get its
own transfer between the send and recv, which could cause issues.
Yeah I considered that.
But the datasheet for the device states that a stop must be issued after the write,
before the read.
Docs for i2c_transfer state that the stop is issued only at the end.
+ *val = ((buf[0] & 0x3) << 8) | buf[1];
+ ret = 0;
+error:
+ mutex_unlock(&data->lock);
+ return ret;
+}
--
Regards
Phil Reid
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
