* [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support
@ 2018-03-22 10:23 Sean Nyekjaer
2018-03-22 10:23 ` [RFC PATCH 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Sean Nyekjaer @ 2018-03-22 10:23 UTC (permalink / raw)
To: jic23, robh+dt
Cc: Sean Nyekjaer, knaack.h, lars, pmeerw, mark.rutland, linux-iio,
devicetree
This patch adds support for the Texas Intruments DAC5755 Family.
Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ti-dac5571.c | 395 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 406 insertions(+)
create mode 100644 drivers/iio/dac/ti-dac5571.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 965d5c0d2468..c78c02f455b9 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -320,6 +320,16 @@ config TI_DAC082S085
If compiled as a module, it will be called ti-dac082s085.
+config TI_DAC5571
+ tristate "Texas Instruments 8/10/12/16-bit 1/2/4-channel DAC driver"
+ depends on I2C
+ help
+ Driver for the Texas Instruments
+ DAC5571, DAC6571, DAC7571, DAC5574, DAC6574, DAC7574, DAC5573,
+ DAC6573, DAC7573.
+
+ If compiled as a module, it will be called ti-dac5571.
+
config VF610_DAC
tristate "Vybrid vf610 DAC driver"
depends on OF
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..1ab358d522ee 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -35,4 +35,5 @@ obj-$(CONFIG_MCP4922) += mcp4922.o
obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
obj-$(CONFIG_STM32_DAC) += stm32-dac.o
obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
+obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
obj-$(CONFIG_VF610_DAC) += vf610_dac.o
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
new file mode 100644
index 000000000000..cf4ba151ed7e
--- /dev/null
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -0,0 +1,395 @@
+/*
+ * ti-dac5571.c - Texas Instruments 8/10/12-bit 1/4-channel DAC driver
+ *
+ * Copyright (C) 2018 Prevas A/S
+ *
+ * http://www.ti.com/lit/ds/symlink/dac5571.pdf
+ * http://www.ti.com/lit/ds/symlink/dac6571.pdf
+ * http://www.ti.com/lit/ds/symlink/dac7571.pdf
+ * http://www.ti.com/lit/ds/symlink/dac5574.pdf
+ * http://www.ti.com/lit/ds/symlink/dac6574.pdf
+ * http://www.ti.com/lit/ds/symlink/dac7574.pdf
+ * http://www.ti.com/lit/ds/symlink/dac5573.pdf
+ * http://www.ti.com/lit/ds/symlink/dac6573.pdf
+ * http://www.ti.com/lit/ds/symlink/dac7573.pdf
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/i2c.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+
+enum chip_id {
+ single_8bit, single_10bit, single_12bit,
+ quad_8bit, quad_10bit, quad_12bit
+};
+
+struct dac5571_spec {
+ u8 num_channels;
+ u8 resolution;
+};
+
+static const struct dac5571_spec dac5571_spec[] = {
+ [single_8bit] = {.num_channels = 1, .resolution = 8},
+ [single_10bit] = {.num_channels = 1, .resolution = 10},
+ [single_12bit] = {.num_channels = 1, .resolution = 12},
+ [quad_8bit] = {.num_channels = 4, .resolution = 8},
+ [quad_10bit] = {.num_channels = 4, .resolution = 10},
+ [quad_12bit] = {.num_channels = 4, .resolution = 12},
+};
+
+struct dac5571_data {
+ struct i2c_client *client;
+ int id;
+ struct mutex lock;
+ struct regulator *vref;
+ u16 val[4];
+ bool powerdown;
+ u8 powerdown_mode;
+ u8 resolution;
+ u8 channels;
+ u8 buf[3] ____cacheline_aligned;
+};
+
+#define POWERDOWN(mode) ((mode) + 1)
+
+static int dac5571_cmd(struct dac5571_data *data, int channel,
+ u8 pwrdwn, u16 val)
+{
+ int ret;
+ u8 shift;
+
+ switch (data->channels) {
+ case 1:
+ shift = 12 - data->resolution;
+ data->buf[0] = (val << shift);
+ data->buf[1] = pwrdwn << 4 | (val >> (8 - shift));
+ ret = i2c_master_send(data->client, data->buf, 2);
+ break;
+ case 4:
+ shift = 16 - data->resolution;
+ data->buf[0] = val << shift;
+ data->buf[1] = val >> (8 - shift) | pwrdwn << 6;
+ data->buf[2] = channel << 1 | 1 << 4;
+ if (pwrdwn)
+ data->buf[2] |= 1;
+ ret = i2c_master_send(data->client, data->buf, 3);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ };
+
+ return ret;
+}
+
+static const char *const dac5571_powerdown_modes[] = {
+ "1kohm_to_gnd", "100kohm_to_gnd", "three_state",
+};
+
+static int dac5571_get_powerdown_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+
+ return data->powerdown_mode;
+}
+
+static int dac5571_set_powerdown_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ int ret = 0;
+
+ if (data->powerdown_mode == mode)
+ return 0;
+
+ mutex_lock(&data->lock);
+ if (data->powerdown) {
+ ret = dac5571_cmd(data, chan->channel, POWERDOWN(mode), 0);
+ if (ret)
+ goto out;
+ }
+ data->powerdown_mode = mode;
+
+ out:
+ mutex_unlock(&data->lock);
+ return ret;
+}
+
+static const struct iio_enum dac5571_powerdown_mode = {
+ .items = dac5571_powerdown_modes,
+ .num_items = ARRAY_SIZE(dac5571_powerdown_modes),
+ .get = dac5571_get_powerdown_mode,
+ .set = dac5571_set_powerdown_mode,
+};
+
+static ssize_t dac5571_read_powerdown(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+
+ return sprintf(buf, "%d\n", data->powerdown);
+}
+
+static ssize_t dac5571_write_powerdown(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ bool powerdown;
+ int ret;
+
+ ret = strtobool(buf, &powerdown);
+ if (ret)
+ return ret;
+
+ if (data->powerdown == powerdown)
+ return len;
+
+ mutex_lock(&data->lock);
+ if (powerdown)
+ ret =
+ dac5571_cmd(data, chan->channel,
+ POWERDOWN(data->powerdown_mode), 0);
+ else
+ ret = dac5571_cmd(data, chan->channel, 0, data->val[0]);
+ if (!ret)
+ data->powerdown = powerdown;
+ mutex_unlock(&data->lock);
+
+ return ret ? ret : len;
+}
+
+
+static const struct iio_chan_spec_ext_info dac5571_ext_info[] = {
+ {
+ .name = "powerdown",
+ .read = dac5571_read_powerdown,
+ .write = dac5571_write_powerdown,
+ .shared = IIO_SHARED_BY_TYPE,
+ },
+ IIO_ENUM("powerdown_mode", IIO_SHARED_BY_TYPE, &dac5571_powerdown_mode),
+ IIO_ENUM_AVAILABLE("powerdown_mode", &dac5571_powerdown_mode),
+ {},
+};
+
+#define dac5571_CHANNEL(chan) { \
+ .type = IIO_VOLTAGE, \
+ .channel = (chan), \
+ .address = (chan), \
+ .indexed = true, \
+ .output = true, \
+ .datasheet_name = (const char[]){ 'A' + (chan), 0 }, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .ext_info = dac5571_ext_info, \
+}
+
+static const struct iio_chan_spec dac5571_channels[] = {
+ dac5571_CHANNEL(0),
+ dac5571_CHANNEL(1),
+ dac5571_CHANNEL(2),
+ dac5571_CHANNEL(3),
+};
+
+static int dac5571_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = data->val[chan->channel];
+ ret = IIO_VAL_INT;
+ break;
+
+ case IIO_CHAN_INFO_SCALE:
+ ret = regulator_get_voltage(data->vref);
+ if (ret < 0)
+ return ret;
+
+ *val = ret / 1000;
+ *val2 = data->resolution;
+ ret = IIO_VAL_FRACTIONAL_LOG2;
+ break;
+
+ default:
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+static int dac5571_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (data->val[chan->channel] == val)
+ return 0;
+
+ if (val >= (1 << data->resolution) || val < 0)
+ return -EINVAL;
+
+ if (data->powerdown)
+ return -EBUSY;
+
+ mutex_lock(&data->lock);
+ ret = dac5571_cmd(data, chan->channel, 0, val);
+ if (!ret)
+ data->val[chan->channel] = val;
+ mutex_unlock(&data->lock);
+ break;
+
+ default:
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+static int dac5571_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ return IIO_VAL_INT;
+}
+
+static const struct iio_info dac5571_info = {
+ .read_raw = dac5571_read_raw,
+ .write_raw = dac5571_write_raw,
+ .write_raw_get_fmt = dac5571_write_raw_get_fmt,
+};
+
+static int dac5571_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ const struct dac5571_spec *spec;
+ struct dac5571_data *data;
+ struct iio_dev *indio_dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+ if (client->dev.of_node)
+ data->id = (enum chip_id)of_device_get_match_data(&client->dev);
+ else
+ data->id = id->driver_data;
+
+ indio_dev->dev.parent = dev;
+ indio_dev->dev.of_node = client->dev.of_node;
+ indio_dev->info = &dac5571_info;
+ indio_dev->name = id->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = dac5571_channels;
+
+ spec = &dac5571_spec[id->driver_data];
+ indio_dev->num_channels = spec->num_channels;
+ data->resolution = spec->resolution;
+ data->channels = spec->num_channels;
+
+ data->vref = devm_regulator_get(dev, "vref");
+ if (IS_ERR(data->vref))
+ return PTR_ERR(data->vref);
+
+ ret = regulator_enable(data->vref);
+ if (ret < 0)
+ return ret;
+
+ mutex_init(&data->lock);
+
+ ret = dac5571_cmd(data, 0, 0, 0);
+ if (ret) {
+ dev_err(dev, "failed to initialize outputs to 0\n");
+ goto err;
+ }
+
+ ret = iio_device_register(indio_dev);
+ if (ret)
+ goto err;
+
+ return 0;
+
+ err:
+ mutex_destroy(&data->lock);
+ regulator_disable(data->vref);
+ return ret;
+}
+
+static int dac5571_remove(struct i2c_client *i2c)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(i2c);
+ struct dac5571_data *data = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ mutex_destroy(&data->lock);
+ regulator_disable(data->vref);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id dac5571_of_id[] = {
+ {.compatible = "ti,dac5571"},
+ {.compatible = "ti,dac6571"},
+ {.compatible = "ti,dac7571"},
+ {.compatible = "ti,dac5574"},
+ {.compatible = "ti,dac6574"},
+ {.compatible = "ti,dac7574"},
+ {.compatible = "ti,dac5573"},
+ {.compatible = "ti,dac6573"},
+ {.compatible = "ti,dac7573"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, dac5571_of_id);
+#endif
+
+static const struct i2c_device_id dac5571_id[] = {
+ {"dac5571", single_8bit},
+ {"dac6571", single_10bit},
+ {"dac7571", single_12bit},
+ {"dac5574", quad_8bit},
+ {"dac6574", quad_10bit},
+ {"dac7574", quad_12bit},
+ {"dac5573", quad_8bit},
+ {"dac6573", quad_10bit},
+ {"dac7573", quad_12bit},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, dac5571_id);
+
+static struct i2c_driver dac5571_driver = {
+ .driver = {
+ .name = "ti-dac5571",
+ },
+ .probe = dac5571_probe,
+ .remove = dac5571_remove,
+ .id_table = dac5571_id,
+};
+module_i2c_driver(dac5571_driver);
+
+MODULE_AUTHOR("Sean Nyekjaer <sean.nyekjaer@prevas.dk>");
+MODULE_DESCRIPTION("Texas Instruments 8/10/12-bit 1/4-channel DAC driver");
+MODULE_LICENSE("GPL v2");
--
2.16.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 2/2] iio: ti-dac5571: Add DT binding documentation
2018-03-22 10:23 [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Sean Nyekjaer
@ 2018-03-22 10:23 ` Sean Nyekjaer
2018-03-26 22:24 ` Rob Herring
2018-03-22 11:25 ` [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Peter Meerwald-Stadler
2018-04-10 8:44 ` [RFC PATCH v2 " Sean Nyekjaer
2 siblings, 1 reply; 10+ messages in thread
From: Sean Nyekjaer @ 2018-03-22 10:23 UTC (permalink / raw)
To: jic23, robh+dt
Cc: Sean Nyekjaer, knaack.h, lars, pmeerw, mark.rutland, linux-iio,
devicetree
Adding binding documentation for Texas Instruments DAC5571 Family
Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
.../devicetree/bindings/iio/dac/ti,dac5571.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt
new file mode 100644
index 000000000000..03af6b9a4d07
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt
@@ -0,0 +1,24 @@
+* Texas Instruments DAC5571 Family
+
+Required properties:
+ - compatible: Should contain
+ "ti,dac5571"
+ "ti,dac6571"
+ "ti,dac7571"
+ "ti,dac5574"
+ "ti,dac6574"
+ "ti,dac7574"
+ "ti,dac5573"
+ "ti,dac6573"
+ "ti,dac7573"
+ - reg: Should contain the DAC I2C address
+
+Optional properties:
+ - vref-supply: The regulator supply for DAC reference voltage
+
+Example:
+dac@0 {
+ compatible = "ti,dac5571";
+ reg = <0x4C>;
+ vref-supply = <&vdd_supply>;
+};
--
2.16.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support
2018-03-22 10:23 [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Sean Nyekjaer
2018-03-22 10:23 ` [RFC PATCH 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
@ 2018-03-22 11:25 ` Peter Meerwald-Stadler
2018-03-23 12:27 ` Jonathan Cameron
2018-04-10 8:44 ` [RFC PATCH v2 " Sean Nyekjaer
2 siblings, 1 reply; 10+ messages in thread
From: Peter Meerwald-Stadler @ 2018-03-22 11:25 UTC (permalink / raw)
To: Sean Nyekjaer; +Cc: jic23, knaack.h, lars, linux-iio
> This patch adds support for the Texas Intruments DAC5755 Family.
comments below
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> ---
> drivers/iio/dac/Kconfig | 10 ++
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ti-dac5571.c | 395 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 406 insertions(+)
> create mode 100644 drivers/iio/dac/ti-dac5571.c
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 965d5c0d2468..c78c02f455b9 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -320,6 +320,16 @@ config TI_DAC082S085
>
> If compiled as a module, it will be called ti-dac082s085.
>
> +config TI_DAC5571
> + tristate "Texas Instruments 8/10/12/16-bit 1/2/4-channel DAC driver"
> + depends on I2C
> + help
> + Driver for the Texas Instruments
> + DAC5571, DAC6571, DAC7571, DAC5574, DAC6574, DAC7574, DAC5573,
> + DAC6573, DAC7573.
> +
> + If compiled as a module, it will be called ti-dac5571.
> +
> config VF610_DAC
> tristate "Vybrid vf610 DAC driver"
> depends on OF
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 81e710ed7491..1ab358d522ee 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -35,4 +35,5 @@ obj-$(CONFIG_MCP4922) += mcp4922.o
> obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
> obj-$(CONFIG_STM32_DAC) += stm32-dac.o
> obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
> +obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
> obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
> new file mode 100644
> index 000000000000..cf4ba151ed7e
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac5571.c
> @@ -0,0 +1,395 @@
> +/*
> + * ti-dac5571.c - Texas Instruments 8/10/12-bit 1/4-channel DAC driver
> + *
> + * Copyright (C) 2018 Prevas A/S
> + *
> + * http://www.ti.com/lit/ds/symlink/dac5571.pdf
> + * http://www.ti.com/lit/ds/symlink/dac6571.pdf
> + * http://www.ti.com/lit/ds/symlink/dac7571.pdf
> + * http://www.ti.com/lit/ds/symlink/dac5574.pdf
> + * http://www.ti.com/lit/ds/symlink/dac6574.pdf
> + * http://www.ti.com/lit/ds/symlink/dac7574.pdf
> + * http://www.ti.com/lit/ds/symlink/dac5573.pdf
> + * http://www.ti.com/lit/ds/symlink/dac6573.pdf
> + * http://www.ti.com/lit/ds/symlink/dac7573.pdf
> + *
> + * 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
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +
> +enum chip_id {
> + single_8bit, single_10bit, single_12bit,
> + quad_8bit, quad_10bit, quad_12bit
> +};
> +
> +struct dac5571_spec {
> + u8 num_channels;
> + u8 resolution;
> +};
> +
> +static const struct dac5571_spec dac5571_spec[] = {
> + [single_8bit] = {.num_channels = 1, .resolution = 8},
> + [single_10bit] = {.num_channels = 1, .resolution = 10},
> + [single_12bit] = {.num_channels = 1, .resolution = 12},
> + [quad_8bit] = {.num_channels = 4, .resolution = 8},
> + [quad_10bit] = {.num_channels = 4, .resolution = 10},
> + [quad_12bit] = {.num_channels = 4, .resolution = 12},
> +};
> +
> +struct dac5571_data {
> + struct i2c_client *client;
> + int id;
> + struct mutex lock;
> + struct regulator *vref;
> + u16 val[4];
> + bool powerdown;
> + u8 powerdown_mode;
> + u8 resolution;
> + u8 channels;
> + u8 buf[3] ____cacheline_aligned;
> +};
> +
> +#define POWERDOWN(mode) ((mode) + 1)
DAC5571_ prefix please
> +
> +static int dac5571_cmd(struct dac5571_data *data, int channel,
> + u8 pwrdwn, u16 val)
> +{
> + int ret;
> + u8 shift;
no need for shift to be u8, use unsigned int or int
> +
> + switch (data->channels) {
> + case 1:
> + shift = 12 - data->resolution;
> + data->buf[0] = (val << shift);
parenthesis not needed here
> + data->buf[1] = pwrdwn << 4 | (val >> (8 - shift));
I suggest parenthesis around (pwrdwn << 4)
> + ret = i2c_master_send(data->client, data->buf, 2);
> + break;
> + case 4:
> + shift = 16 - data->resolution;
> + data->buf[0] = val << shift;
> + data->buf[1] = val >> (8 - shift) | pwrdwn << 6;
> + data->buf[2] = channel << 1 | 1 << 4;
I suggest parenthesis for readability
(val >> (8 - shift))
(channel << 1) | (1 << 4)
> + if (pwrdwn)
> + data->buf[2] |= 1;
> + ret = i2c_master_send(data->client, data->buf, 3);
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + };
> +
> + return ret;
> +}
> +
> +static const char *const dac5571_powerdown_modes[] = {
> + "1kohm_to_gnd", "100kohm_to_gnd", "three_state",
> +};
> +
> +static int dac5571_get_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> +
> + return data->powerdown_mode;
> +}
> +
> +static int dac5571_set_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int mode)
mode could/should be u8 as it is passed to the device, or an enum even
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + int ret = 0;
> +
> + if (data->powerdown_mode == mode)
> + return 0;
> +
> + mutex_lock(&data->lock);
> + if (data->powerdown) {
> + ret = dac5571_cmd(data, chan->channel, POWERDOWN(mode), 0);
> + if (ret)
i2c_master_send() and hence dac5571_cmd() return the number of bytes
written!? should be
if (ret <= 0)
?
> + goto out;
> + }
> + data->powerdown_mode = mode;
> +
> + out:
> + mutex_unlock(&data->lock);
> + return ret;
> +}
> +
> +static const struct iio_enum dac5571_powerdown_mode = {
> + .items = dac5571_powerdown_modes,
> + .num_items = ARRAY_SIZE(dac5571_powerdown_modes),
> + .get = dac5571_get_powerdown_mode,
> + .set = dac5571_set_powerdown_mode,
> +};
> +
> +static ssize_t dac5571_read_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> +
> + return sprintf(buf, "%d\n", data->powerdown);
> +}
> +
> +static ssize_t dac5571_write_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + bool powerdown;
> + int ret;
> +
> + ret = strtobool(buf, &powerdown);
> + if (ret)
> + return ret;
> +
> + if (data->powerdown == powerdown)
> + return len;
> +
> + mutex_lock(&data->lock);
> + if (powerdown)
> + ret =
> + dac5571_cmd(data, chan->channel,
> + POWERDOWN(data->powerdown_mode), 0);
> + else
> + ret = dac5571_cmd(data, chan->channel, 0, data->val[0]);
ret value of dac5571_cmd() is number of bytes written?
> + if (!ret)
> + data->powerdown = powerdown;
> + mutex_unlock(&data->lock);
> +
> + return ret ? ret : len;
> +}
> +
> +
> +static const struct iio_chan_spec_ext_info dac5571_ext_info[] = {
> + {
> + .name = "powerdown",
> + .read = dac5571_read_powerdown,
> + .write = dac5571_write_powerdown,
> + .shared = IIO_SHARED_BY_TYPE,
> + },
> + IIO_ENUM("powerdown_mode", IIO_SHARED_BY_TYPE, &dac5571_powerdown_mode),
> + IIO_ENUM_AVAILABLE("powerdown_mode", &dac5571_powerdown_mode),
> + {},
> +};
> +
> +#define dac5571_CHANNEL(chan) { \
> + .type = IIO_VOLTAGE, \
> + .channel = (chan), \
> + .address = (chan), \
> + .indexed = true, \
> + .output = true, \
> + .datasheet_name = (const char[]){ 'A' + (chan), 0 }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .ext_info = dac5571_ext_info, \
> +}
> +
> +static const struct iio_chan_spec dac5571_channels[] = {
> + dac5571_CHANNEL(0),
> + dac5571_CHANNEL(1),
> + dac5571_CHANNEL(2),
> + dac5571_CHANNEL(3),
> +};
> +
> +static int dac5571_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *val = data->val[chan->channel];
> + ret = IIO_VAL_INT;
return directly
> + break;
> +
> + case IIO_CHAN_INFO_SCALE:
> + ret = regulator_get_voltage(data->vref);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret / 1000;
> + *val2 = data->resolution;
> + ret = IIO_VAL_FRACTIONAL_LOG2;
return directly
> + break;
> +
> + default:
> + ret = -EINVAL;
return directly
> + }
> +
> + return ret;
> +}
> +
> +static int dac5571_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (data->val[chan->channel] == val)
> + return 0;
> +
> + if (val >= (1 << data->resolution) || val < 0)
> + return -EINVAL;
> +
> + if (data->powerdown)
> + return -EBUSY;
> +
> + mutex_lock(&data->lock);
> + ret = dac5571_cmd(data, chan->channel, 0, val);
ret value of dac5571_cmd()...
> + if (!ret)
> + data->val[chan->channel] = val;
> + mutex_unlock(&data->lock);
> + break;
return directly
> +
> + default:
> + ret = -EINVAL;
return directly
> + }
> +
> + return ret;
> +}
> +
> +static int dac5571_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + long mask)
> +{
> + return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info dac5571_info = {
> + .read_raw = dac5571_read_raw,
> + .write_raw = dac5571_write_raw,
> + .write_raw_get_fmt = dac5571_write_raw_get_fmt,
> +};
> +
> +static int dac5571_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + const struct dac5571_spec *spec;
> + struct dac5571_data *data;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> + if (client->dev.of_node)
> + data->id = (enum chip_id)of_device_get_match_data(&client->dev);
> + else
> + data->id = id->driver_data;
> +
> + indio_dev->dev.parent = dev;
> + indio_dev->dev.of_node = client->dev.of_node;
> + indio_dev->info = &dac5571_info;
> + indio_dev->name = id->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = dac5571_channels;
> +
> + spec = &dac5571_spec[id->driver_data];
> + indio_dev->num_channels = spec->num_channels;
> + data->resolution = spec->resolution;
> + data->channels = spec->num_channels;
> +
> + data->vref = devm_regulator_get(dev, "vref");
> + if (IS_ERR(data->vref))
> + return PTR_ERR(data->vref);
> +
> + ret = regulator_enable(data->vref);
> + if (ret < 0)
> + return ret;
> +
> + mutex_init(&data->lock);
> +
> + ret = dac5571_cmd(data, 0, 0, 0);
> + if (ret) {
ret value is number of bytes written?
> + dev_err(dev, "failed to initialize outputs to 0\n");
> + goto err;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err;
> +
> + return 0;
> +
> + err:
> + mutex_destroy(&data->lock);
no need to destroy mutex (at least most drivers don't)
> + regulator_disable(data->vref);
> + return ret;
> +}
> +
> +static int dac5571_remove(struct i2c_client *i2c)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(i2c);
> + struct dac5571_data *data = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + mutex_destroy(&data->lock);
no need to destroy mutex
> + regulator_disable(data->vref);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id dac5571_of_id[] = {
> + {.compatible = "ti,dac5571"},
> + {.compatible = "ti,dac6571"},
> + {.compatible = "ti,dac7571"},
> + {.compatible = "ti,dac5574"},
> + {.compatible = "ti,dac6574"},
> + {.compatible = "ti,dac7574"},
> + {.compatible = "ti,dac5573"},
> + {.compatible = "ti,dac6573"},
> + {.compatible = "ti,dac7573"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dac5571_of_id);
> +#endif
> +
> +static const struct i2c_device_id dac5571_id[] = {
> + {"dac5571", single_8bit},
> + {"dac6571", single_10bit},
> + {"dac7571", single_12bit},
> + {"dac5574", quad_8bit},
> + {"dac6574", quad_10bit},
> + {"dac7574", quad_12bit},
> + {"dac5573", quad_8bit},
> + {"dac6573", quad_10bit},
> + {"dac7573", quad_12bit},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, dac5571_id);
> +
> +static struct i2c_driver dac5571_driver = {
> + .driver = {
> + .name = "ti-dac5571",
> + },
> + .probe = dac5571_probe,
> + .remove = dac5571_remove,
> + .id_table = dac5571_id,
> +};
> +module_i2c_driver(dac5571_driver);
> +
> +MODULE_AUTHOR("Sean Nyekjaer <sean.nyekjaer@prevas.dk>");
> +MODULE_DESCRIPTION("Texas Instruments 8/10/12-bit 1/4-channel DAC driver");
> +MODULE_LICENSE("GPL v2");
>
--
Peter Meerwald-Stadler
Mobile: +43 664 24 44 418
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support
2018-03-22 11:25 ` [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Peter Meerwald-Stadler
@ 2018-03-23 12:27 ` Jonathan Cameron
2018-03-23 12:35 ` Sean Nyekjaer
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2018-03-23 12:27 UTC (permalink / raw)
To: Peter Meerwald-Stadler; +Cc: Sean Nyekjaer, jic23, knaack.h, lars, linux-iio
On Thu, 22 Mar 2018 12:25:35 +0100
Peter Meerwald-Stadler <pmeerw@pmeerw.net> wrote:
> > This patch adds support for the Texas Intruments DAC5755 Family.
>
> comments below
If posting an RFC rather than a normal patch, I really want to see an
explanation of what you would like comments on.
A few minor comments from me.
Thanks,
Jonathan
>
> > Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> > ---
> > drivers/iio/dac/Kconfig | 10 ++
> > drivers/iio/dac/Makefile | 1 +
> > drivers/iio/dac/ti-dac5571.c | 395 +++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 406 insertions(+)
> > create mode 100644 drivers/iio/dac/ti-dac5571.c
> >
> > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> > index 965d5c0d2468..c78c02f455b9 100644
> > --- a/drivers/iio/dac/Kconfig
> > +++ b/drivers/iio/dac/Kconfig
> > @@ -320,6 +320,16 @@ config TI_DAC082S085
> >
> > If compiled as a module, it will be called ti-dac082s085.
> >
> > +config TI_DAC5571
> > + tristate "Texas Instruments 8/10/12/16-bit 1/2/4-channel DAC driver"
> > + depends on I2C
> > + help
> > + Driver for the Texas Instruments
> > + DAC5571, DAC6571, DAC7571, DAC5574, DAC6574, DAC7574, DAC5573,
> > + DAC6573, DAC7573.
> > +
> > + If compiled as a module, it will be called ti-dac5571.
> > +
> > config VF610_DAC
> > tristate "Vybrid vf610 DAC driver"
> > depends on OF
> > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> > index 81e710ed7491..1ab358d522ee 100644
> > --- a/drivers/iio/dac/Makefile
> > +++ b/drivers/iio/dac/Makefile
> > @@ -35,4 +35,5 @@ obj-$(CONFIG_MCP4922) += mcp4922.o
> > obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
> > obj-$(CONFIG_STM32_DAC) += stm32-dac.o
> > obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
> > +obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
> > obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> > diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
> > new file mode 100644
> > index 000000000000..cf4ba151ed7e
> > --- /dev/null
> > +++ b/drivers/iio/dac/ti-dac5571.c
> > @@ -0,0 +1,395 @@
> > +/*
> > + * ti-dac5571.c - Texas Instruments 8/10/12-bit 1/4-channel DAC driver
> > + *
> > + * Copyright (C) 2018 Prevas A/S
> > + *
> > + * http://www.ti.com/lit/ds/symlink/dac5571.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac6571.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac7571.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac5574.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac6574.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac7574.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac5573.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac6573.pdf
> > + * http://www.ti.com/lit/ds/symlink/dac7573.pdf
> > + *
> > + * 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
> > + * published by the Free Software Foundation.
Consider SPDX. It's up to you whether you want to use it or not.
> > + */
> > +
> > +#include <linux/iio/iio.h>
> > +#include <linux/module.h>
> > +#include <linux/regulator/consumer.h>
Unless there is a reason to do otherwise, convention is alphabetical order for
includes.
> > +#include <linux/i2c.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of.h>
> > +
> > +enum chip_id {
> > + single_8bit, single_10bit, single_12bit,
> > + quad_8bit, quad_10bit, quad_12bit
> > +};
> > +
> > +struct dac5571_spec {
> > + u8 num_channels;
> > + u8 resolution;
> > +};
> > +
> > +static const struct dac5571_spec dac5571_spec[] = {
> > + [single_8bit] = {.num_channels = 1, .resolution = 8},
> > + [single_10bit] = {.num_channels = 1, .resolution = 10},
> > + [single_12bit] = {.num_channels = 1, .resolution = 12},
> > + [quad_8bit] = {.num_channels = 4, .resolution = 8},
> > + [quad_10bit] = {.num_channels = 4, .resolution = 10},
> > + [quad_12bit] = {.num_channels = 4, .resolution = 12},
> > +};
> > +
> > +struct dac5571_data {
> > + struct i2c_client *client;
> > + int id;
> > + struct mutex lock;
As ever, the lock needs documentation. What is it protecting...
> > + struct regulator *vref;
> > + u16 val[4];
> > + bool powerdown;
> > + u8 powerdown_mode;
> > + u8 resolution;
> > + u8 channels;
Rather than copying over the various elements, perhaps just
have a dac5571_spec structure pointer in here?
> > + u8 buf[3] ____cacheline_aligned;
> > +};
> > +
> > +#define POWERDOWN(mode) ((mode) + 1)
>
> DAC5571_ prefix please
>
> > +
> > +static int dac5571_cmd(struct dac5571_data *data, int channel,
> > + u8 pwrdwn, u16 val)
This seems to be combining several different things.
I would have a separate function for pwrdwn to simplify
this version which I think then becomes something that
is just used to set the value?
> > +{
> > + int ret;
> > + u8 shift;
>
> no need for shift to be u8, use unsigned int or int
>
> > +
> > + switch (data->channels) {
> > + case 1:
> > + shift = 12 - data->resolution;
> > + data->buf[0] = (val << shift);
>
> parenthesis not needed here
>
> > + data->buf[1] = pwrdwn << 4 | (val >> (8 - shift));
>
> I suggest parenthesis around (pwrdwn << 4)
>
> > + ret = i2c_master_send(data->client, data->buf, 2);
> > + break;
> > + case 4:
> > + shift = 16 - data->resolution;
> > + data->buf[0] = val << shift;
> > + data->buf[1] = val >> (8 - shift) | pwrdwn << 6;
> > + data->buf[2] = channel << 1 | 1 << 4;
>
> I suggest parenthesis for readability
> (val >> (8 - shift))
> (channel << 1) | (1 << 4)
The 1 shifted by 4 is a magic number. Use a define to give
it a meaningful name.
My temptation here would be to have two functions, one for
the single channel case and one for the 4 channel case. Then
use the device type specific structure to provide the right one
for each type of device. It moves any device type matching code
out to one place in the probe.
>
> > + if (pwrdwn)
> > + data->buf[2] |= 1;
> > + ret = i2c_master_send(data->client, data->buf, 3);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + };
> > +
> > + return ret;
> > +}
> > +
> > +static const char *const dac5571_powerdown_modes[] = {
> > + "1kohm_to_gnd", "100kohm_to_gnd", "three_state",
> > +};
> > +
> > +static int dac5571_get_powerdown_mode(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan)
> > +{
> > + struct dac5571_data *data = iio_priv(indio_dev);
> > +
> > + return data->powerdown_mode;
> > +}
> > +
> > +static int dac5571_set_powerdown_mode(struct iio_dev *indio_dev,
> > + const struct iio_chan_spec *chan,
> > + unsigned int mode)
>
> mode could/should be u8 as it is passed to the device, or an enum even
>
> > +{
> > + struct dac5571_data *data = iio_priv(indio_dev);
> > + int ret = 0;
> > +
> > + if (data->powerdown_mode == mode)
> > + return 0;
> > +
> > + mutex_lock(&data->lock);
> > + if (data->powerdown) {
> > + ret = dac5571_cmd(data, chan->channel, POWERDOWN(mode), 0);
> > + if (ret)
>
> i2c_master_send() and hence dac5571_cmd() return the number of bytes
> written!? should be
> if (ret <= 0)
> ?
>
> > + goto out;
> > + }
> > + data->powerdown_mode = mode;
> > +
> > + out:
> > + mutex_unlock(&data->lock);
> > + return ret;
> > +}
> > +
> > +static const struct iio_enum dac5571_powerdown_mode = {
> > + .items = dac5571_powerdown_modes,
> > + .num_items = ARRAY_SIZE(dac5571_powerdown_modes),
> > + .get = dac5571_get_powerdown_mode,
> > + .set = dac5571_set_powerdown_mode,
> > +};
> > +
> > +static ssize_t dac5571_read_powerdown(struct iio_dev *indio_dev,
> > + uintptr_t private,
> > + const struct iio_chan_spec *chan,
> > + char *buf)
> > +{
> > + struct dac5571_data *data = iio_priv(indio_dev);
> > +
> > + return sprintf(buf, "%d\n", data->powerdown);
> > +}
> > +
> > +static ssize_t dac5571_write_powerdown(struct iio_dev *indio_dev,
> > + uintptr_t private,
> > + const struct iio_chan_spec *chan,
> > + const char *buf, size_t len)
> > +{
> > + struct dac5571_data *data = iio_priv(indio_dev);
> > + bool powerdown;
> > + int ret;
> > +
> > + ret = strtobool(buf, &powerdown);
> > + if (ret)
> > + return ret;
> > +
> > + if (data->powerdown == powerdown)
> > + return len;
> > +
> > + mutex_lock(&data->lock);
> > + if (powerdown)
> > + ret =
> > + dac5571_cmd(data, chan->channel,
> > + POWERDOWN(data->powerdown_mode), 0);
> > + else
> > + ret = dac5571_cmd(data, chan->channel, 0, data->val[0]);
>
> ret value of dac5571_cmd() is number of bytes written?
>
> > + if (!ret)
> > + data->powerdown = powerdown;
> > + mutex_unlock(&data->lock);
> > +
> > + return ret ? ret : len;
> > +}
> > +
> > +
> > +static const struct iio_chan_spec_ext_info dac5571_ext_info[] = {
> > + {
> > + .name = "powerdown",
> > + .read = dac5571_read_powerdown,
> > + .write = dac5571_write_powerdown,
> > + .shared = IIO_SHARED_BY_TYPE,
> > + },
> > + IIO_ENUM("powerdown_mode", IIO_SHARED_BY_TYPE, &dac5571_powerdown_mode),
> > + IIO_ENUM_AVAILABLE("powerdown_mode", &dac5571_powerdown_mode),
> > + {},
> > +};
> > +
> > +#define dac5571_CHANNEL(chan) { \
> > + .type = IIO_VOLTAGE, \
> > + .channel = (chan), \
> > + .address = (chan), \
> > + .indexed = true, \
> > + .output = true, \
> > + .datasheet_name = (const char[]){ 'A' + (chan), 0 }, \
Just add a parameter to the macro and make this explicit.
It is not saving enough code to make it worthwhile to do it
in a complex fashion.
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> > + .ext_info = dac5571_ext_info, \
> > +}
> > +
> > +static const struct iio_chan_spec dac5571_channels[] = {
> > + dac5571_CHANNEL(0),
> > + dac5571_CHANNEL(1),
> > + dac5571_CHANNEL(2),
> > + dac5571_CHANNEL(3),
> > +};
> > +
> > +static int dac5571_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val, int *val2, long mask)
> > +{
> > + struct dac5571_data *data = iio_priv(indio_dev);
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + *val = data->val[chan->channel];
> > + ret = IIO_VAL_INT;
>
> return directly
>
> > + break;
> > +
> > + case IIO_CHAN_INFO_SCALE:
> > + ret = regulator_get_voltage(data->vref);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = ret / 1000;
> > + *val2 = data->resolution;
> > + ret = IIO_VAL_FRACTIONAL_LOG2;
>
> return directly
>
> > + break;
> > +
> > + default:
> > + ret = -EINVAL;
>
> return directly
>
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int dac5571_write_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int val, int val2, long mask)
> > +{
> > + struct dac5571_data *data = iio_priv(indio_dev);
> > + int ret;
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + if (data->val[chan->channel] == val)
> > + return 0;
> > +
> > + if (val >= (1 << data->resolution) || val < 0)
> > + return -EINVAL;
> > +
> > + if (data->powerdown)
> > + return -EBUSY;
> > +
> > + mutex_lock(&data->lock);
> > + ret = dac5571_cmd(data, chan->channel, 0, val);
>
> ret value of dac5571_cmd()...
>
> > + if (!ret)
> > + data->val[chan->channel] = val;
> > + mutex_unlock(&data->lock);
> > + break;
>
> return directly
>
> > +
> > + default:
> > + ret = -EINVAL;
>
> return directly
>
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int dac5571_write_raw_get_fmt(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + long mask)
> > +{
> > + return IIO_VAL_INT;
> > +}
> > +
> > +static const struct iio_info dac5571_info = {
> > + .read_raw = dac5571_read_raw,
> > + .write_raw = dac5571_write_raw,
> > + .write_raw_get_fmt = dac5571_write_raw_get_fmt,
> > +};
> > +
> > +static int dac5571_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct device *dev = &client->dev;
> > + const struct dac5571_spec *spec;
> > + struct dac5571_data *data;
> > + struct iio_dev *indio_dev;
> > + int ret;
> > +
> > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> > + if (!indio_dev)
> > + return -ENOMEM;
blank line here ever so slightly helps readability.
> > + data = iio_priv(indio_dev);
> > + i2c_set_clientdata(client, indio_dev);
> > + data->client = client;
> > + if (client->dev.of_node)
> > + data->id = (enum chip_id)of_device_get_match_data(&client->dev);
> > + else
> > + data->id = id->driver_data;
Only used locally so no need to put in the structure.
> > +
> > + indio_dev->dev.parent = dev;
> > + indio_dev->dev.of_node = client->dev.of_node;
> > + indio_dev->info = &dac5571_info;
> > + indio_dev->name = id->name;
> > + indio_dev->modes = INDIO_DIRECT_MODE;
> > + indio_dev->channels = dac5571_channels;
> > +
> > + spec = &dac5571_spec[id->driver_data];
> > + indio_dev->num_channels = spec->num_channels;
> > + data->resolution = spec->resolution;
> > + data->channels = spec->num_channels;
> > +
> > + data->vref = devm_regulator_get(dev, "vref");
> > + if (IS_ERR(data->vref))
> > + return PTR_ERR(data->vref);
> > +
> > + ret = regulator_enable(data->vref);
> > + if (ret < 0)
> > + return ret;
> > +
> > + mutex_init(&data->lock);
> > +
> > + ret = dac5571_cmd(data, 0, 0, 0);
> > + if (ret) {
>
> ret value is number of bytes written?
>
> > + dev_err(dev, "failed to initialize outputs to 0\n");
> > + goto err;
> > + }
> > +
> > + ret = iio_device_register(indio_dev);
> > + if (ret)
> > + goto err;
> > +
> > + return 0;
> > +
> > + err:
> > + mutex_destroy(&data->lock);
>
> no need to destroy mutex (at least most drivers don't)
>
> > + regulator_disable(data->vref);
> > + return ret;
> > +}
> > +
> > +static int dac5571_remove(struct i2c_client *i2c)
> > +{
> > + struct iio_dev *indio_dev = i2c_get_clientdata(i2c);
> > + struct dac5571_data *data = iio_priv(indio_dev);
> > +
> > + iio_device_unregister(indio_dev);
> > + mutex_destroy(&data->lock);
>
> no need to destroy mutex
>
> > + regulator_disable(data->vref);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id dac5571_of_id[] = {
> > + {.compatible = "ti,dac5571"},
> > + {.compatible = "ti,dac6571"},
> > + {.compatible = "ti,dac7571"},
> > + {.compatible = "ti,dac5574"},
> > + {.compatible = "ti,dac6574"},
> > + {.compatible = "ti,dac7574"},
> > + {.compatible = "ti,dac5573"},
> > + {.compatible = "ti,dac6573"},
> > + {.compatible = "ti,dac7573"},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, dac5571_of_id);
> > +#endif
> > +
> > +static const struct i2c_device_id dac5571_id[] = {
> > + {"dac5571", single_8bit},
> > + {"dac6571", single_10bit},
> > + {"dac7571", single_12bit},
> > + {"dac5574", quad_8bit},
> > + {"dac6574", quad_10bit},
> > + {"dac7574", quad_12bit},
> > + {"dac5573", quad_8bit},
> > + {"dac6573", quad_10bit},
> > + {"dac7573", quad_12bit},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, dac5571_id);
> > +
> > +static struct i2c_driver dac5571_driver = {
> > + .driver = {
> > + .name = "ti-dac5571",
> > + },
> > + .probe = dac5571_probe,
> > + .remove = dac5571_remove,
> > + .id_table = dac5571_id,
> > +};
> > +module_i2c_driver(dac5571_driver);
> > +
> > +MODULE_AUTHOR("Sean Nyekjaer <sean.nyekjaer@prevas.dk>");
> > +MODULE_DESCRIPTION("Texas Instruments 8/10/12-bit 1/4-channel DAC driver");
> > +MODULE_LICENSE("GPL v2");
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support
2018-03-23 12:27 ` Jonathan Cameron
@ 2018-03-23 12:35 ` Sean Nyekjaer
2018-03-23 12:42 ` Peter Meerwald-Stadler
0 siblings, 1 reply; 10+ messages in thread
From: Sean Nyekjaer @ 2018-03-23 12:35 UTC (permalink / raw)
To: Jonathan Cameron, Peter Meerwald-Stadler; +Cc: jic23, knaack.h, lars, linux-iio
On 23-03-2018 13:27, Jonathan Cameron wrote:
> If posting an RFC rather than a normal patch, I really want to see an
> explanation of what you would like comments on.
>
> A few minor comments from me.
>
> Thanks,
>
> Jonathan
I'll remember that :-)
But the reason for the RFC tag is that I don't have the hardware to test
this on yet...
It will arrive sometime next month
Thanks for the review
/Sean
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support
2018-03-23 12:35 ` Sean Nyekjaer
@ 2018-03-23 12:42 ` Peter Meerwald-Stadler
0 siblings, 0 replies; 10+ messages in thread
From: Peter Meerwald-Stadler @ 2018-03-23 12:42 UTC (permalink / raw)
To: Sean Nyekjaer; +Cc: Jonathan Cameron, jic23, knaack.h, lars, linux-iio
> But the reason for the RFC tag is that I don't have the hardware to test this
> on yet...
> It will arrive sometime next month
this explains the broken handling of the i2c_master_send() retval :)
p.
--
Peter Meerwald-Stadler
Mobile: +43 664 24 44 418
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 2/2] iio: ti-dac5571: Add DT binding documentation
2018-03-22 10:23 ` [RFC PATCH 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
@ 2018-03-26 22:24 ` Rob Herring
0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2018-03-26 22:24 UTC (permalink / raw)
To: Sean Nyekjaer
Cc: jic23, knaack.h, lars, pmeerw, mark.rutland, linux-iio, devicetree
On Thu, Mar 22, 2018 at 11:23:36AM +0100, Sean Nyekjaer wrote:
> Adding binding documentation for Texas Instruments DAC5571 Family
>
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> ---
> .../devicetree/bindings/iio/dac/ti,dac5571.txt | 24 ++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH v2 1/2] iio: dac: add TI DAC5755 family support
2018-03-22 10:23 [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Sean Nyekjaer
2018-03-22 10:23 ` [RFC PATCH 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
2018-03-22 11:25 ` [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Peter Meerwald-Stadler
@ 2018-04-10 8:44 ` Sean Nyekjaer
2018-04-10 8:44 ` [RFC PATCH v2 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
2018-04-15 20:01 ` [RFC PATCH v2 1/2] iio: dac: add TI DAC5755 family support Jonathan Cameron
2 siblings, 2 replies; 10+ messages in thread
From: Sean Nyekjaer @ 2018-04-10 8:44 UTC (permalink / raw)
To: jic23, robh+dt
Cc: Sean Nyekjaer, knaack.h, lars, pmeerw, mark.rutland, linux-iio,
devicetree
This patch adds support for the Texas Intruments DAC5755 Family.
Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
Still not have hardware :-( Ready next week I hope. So still RFC.
v2:
Addressed comments from Jonathan and Peter
- improved readability
- i2c return check fixed
- added structure pointer instead of copying elements
- splitted single and quad operations with function pointers
drivers/iio/dac/Kconfig | 10 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ti-dac5571.c | 430 +++++++++++++++++++++++++++++++++++
3 files changed, 441 insertions(+)
create mode 100644 drivers/iio/dac/ti-dac5571.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 965d5c0d2468..b64fabbc74f8 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -320,6 +320,16 @@ config TI_DAC082S085
If compiled as a module, it will be called ti-dac082s085.
+config TI_DAC5571
+ tristate "Texas Instruments 8/10/12/16-bit 1/2/4-channel DAC driver"
+ depends on I2C
+ help
+ Driver for the Texas Instruments
+ DAC5571, DAC6571, DAC7571, DAC5574, DAC6574, DAC7574, DAC5573,
+ DAC6573, DAC7573, DAC8571, DAC8574.
+
+ If compiled as a module, it will be called ti-dac5571.
+
config VF610_DAC
tristate "Vybrid vf610 DAC driver"
depends on OF
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..1ab358d522ee 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -35,4 +35,5 @@ obj-$(CONFIG_MCP4922) += mcp4922.o
obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
obj-$(CONFIG_STM32_DAC) += stm32-dac.o
obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
+obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
obj-$(CONFIG_VF610_DAC) += vf610_dac.o
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
new file mode 100644
index 000000000000..0ee5ca1ce6eb
--- /dev/null
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -0,0 +1,430 @@
+/*
+ * ti-dac5571.c - Texas Instruments 8/10/12-bit 1/4-channel DAC driver
+ *
+ * Copyright (C) 2018 Prevas A/S
+ *
+ * http://www.ti.com/lit/ds/symlink/dac5571.pdf
+ * http://www.ti.com/lit/ds/symlink/dac6571.pdf
+ * http://www.ti.com/lit/ds/symlink/dac7571.pdf
+ * http://www.ti.com/lit/ds/symlink/dac5574.pdf
+ * http://www.ti.com/lit/ds/symlink/dac6574.pdf
+ * http://www.ti.com/lit/ds/symlink/dac7574.pdf
+ * http://www.ti.com/lit/ds/symlink/dac5573.pdf
+ * http://www.ti.com/lit/ds/symlink/dac6573.pdf
+ * http://www.ti.com/lit/ds/symlink/dac7573.pdf
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/iio/iio.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/regulator/consumer.h>
+
+enum chip_id {
+ single_8bit, single_10bit, single_12bit,
+ quad_8bit, quad_10bit, quad_12bit
+};
+
+#define DAC5571_SINGLE_CHANNEL 1
+#define DAC5571_QUAD_CHANNEL 4
+
+struct dac5571_spec {
+ u8 num_channels;
+ u8 resolution;
+};
+
+static const struct dac5571_spec dac5571_spec[] = {
+ [single_8bit] = {.num_channels = 1, .resolution = 8},
+ [single_10bit] = {.num_channels = 1, .resolution = 10},
+ [single_12bit] = {.num_channels = 1, .resolution = 12},
+ [quad_8bit] = {.num_channels = 4, .resolution = 8},
+ [quad_10bit] = {.num_channels = 4, .resolution = 10},
+ [quad_12bit] = {.num_channels = 4, .resolution = 12},
+};
+
+struct dac5571_data {
+ struct i2c_client *client;
+ int id;
+ struct mutex lock;
+ struct regulator *vref;
+ u16 val[4];
+ bool powerdown;
+ u8 powerdown_mode;
+ struct dac5571_spec const *spec;
+ int (*dac5571_cmd) (struct dac5571_data *, int, u16);
+ int (*dac5571_pwrdwn) (struct dac5571_data *, int, u8);
+ u8 buf[3] ____cacheline_aligned;
+};
+
+#define DAC5571_POWERDOWN(mode) ((mode) + 1)
+#define DAC5571_POWERDOWN_FLAG BIT(0)
+#define DAC5571_CHANNEL_SELECT BIT(1)
+#define DAC5571_LOADMODE_DIRECT BIT(4)
+#define DAC5571_SINGLE_PWRDWN_BITS 4
+#define DAC5571_QUAD_PWRDWN_BITS 6
+
+static int dac5571_cmd_single(struct dac5571_data *data, int channel, u16 val)
+{
+ unsigned int shift;
+
+ shift = 12 - data->spec->resolution;
+ data->buf[0] = val << shift;
+ data->buf[1] = val >> (8 - shift);
+ return i2c_master_send(data->client, data->buf, 2);
+}
+
+
+static int dac5571_cmd_quad(struct dac5571_data *data, int channel, u16 val)
+{
+ unsigned int shift;
+
+ shift = 16 - data->spec->resolution;
+ data->buf[0] = val << shift;
+ data->buf[1] = (val >> (8 - shift));
+ data->buf[2] = (channel << DAC5571_CHANNEL_SELECT) |
+ DAC5571_LOADMODE_DIRECT;
+ return i2c_master_send(data->client, data->buf, 3);
+}
+
+static int dac5571_pwrdwn_single(struct dac5571_data *data, int channel, u8 pwrdwn)
+{
+ unsigned int shift;
+
+ shift = 12 - data->spec->resolution;
+ data->buf[0] = 0;
+ data->buf[1] = pwrdwn << DAC5571_SINGLE_PWRDWN_BITS;
+ return i2c_master_send(data->client, data->buf, 2);
+}
+
+static int dac5571_pwrdwn_quad(struct dac5571_data *data, int channel, u8 pwrdwn)
+{
+ unsigned int shift;
+
+ shift = 16 - data->spec->resolution;
+ data->buf[0] = 0;
+ data->buf[1] = pwrdwn << DAC5571_QUAD_PWRDWN_BITS;
+ data->buf[2] = (channel << DAC5571_CHANNEL_SELECT) |
+ DAC5571_LOADMODE_DIRECT | DAC5571_POWERDOWN_FLAG;
+ return i2c_master_send(data->client, data->buf, 3);
+}
+
+static const char *const dac5571_powerdown_modes[] = {
+ "1kohm_to_gnd", "100kohm_to_gnd", "three_state",
+};
+
+static int dac5571_get_powerdown_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+
+ return data->powerdown_mode;
+}
+
+static int dac5571_set_powerdown_mode(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ unsigned int mode)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ int ret = 0;
+
+ if (data->powerdown_mode == mode)
+ return 0;
+
+ mutex_lock(&data->lock);
+ if (data->powerdown) {
+ ret = data->dac5571_pwrdwn(data, chan->channel,
+ DAC5571_POWERDOWN(mode));
+ if (ret <= 0)
+ goto out;
+ }
+ data->powerdown_mode = mode;
+
+ out:
+ mutex_unlock(&data->lock);
+ return ret;
+}
+
+static const struct iio_enum dac5571_powerdown_mode = {
+ .items = dac5571_powerdown_modes,
+ .num_items = ARRAY_SIZE(dac5571_powerdown_modes),
+ .get = dac5571_get_powerdown_mode,
+ .set = dac5571_set_powerdown_mode,
+};
+
+static ssize_t dac5571_read_powerdown(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ char *buf)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+
+ return sprintf(buf, "%d\n", data->powerdown);
+}
+
+static ssize_t dac5571_write_powerdown(struct iio_dev *indio_dev,
+ uintptr_t private,
+ const struct iio_chan_spec *chan,
+ const char *buf, size_t len)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ bool powerdown;
+ int ret;
+
+ ret = strtobool(buf, &powerdown);
+ if (ret)
+ return ret;
+
+ if (data->powerdown == powerdown)
+ return len;
+
+ mutex_lock(&data->lock);
+ if (powerdown)
+ ret = data->dac5571_pwrdwn(data, chan->channel,
+ DAC5571_POWERDOWN(data->powerdown_mode));
+ else
+ ret = data->dac5571_cmd(data, chan->channel, data->val[0]);
+ if (ret <= 0)
+ data->powerdown = powerdown;
+ mutex_unlock(&data->lock);
+
+ return ret ? ret : len;
+}
+
+
+static const struct iio_chan_spec_ext_info dac5571_ext_info[] = {
+ {
+ .name = "powerdown",
+ .read = dac5571_read_powerdown,
+ .write = dac5571_write_powerdown,
+ .shared = IIO_SHARED_BY_TYPE,
+ },
+ IIO_ENUM("powerdown_mode", IIO_SHARED_BY_TYPE, &dac5571_powerdown_mode),
+ IIO_ENUM_AVAILABLE("powerdown_mode", &dac5571_powerdown_mode),
+ {},
+};
+
+#define dac5571_CHANNEL(chan, name) { \
+ .type = IIO_VOLTAGE, \
+ .channel = (chan), \
+ .address = (chan), \
+ .indexed = true, \
+ .output = true, \
+ .datasheet_name = name, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .ext_info = dac5571_ext_info, \
+}
+
+static const struct iio_chan_spec dac5571_channels[] = {
+ dac5571_CHANNEL(0, "A"),
+ dac5571_CHANNEL(1, "B"),
+ dac5571_CHANNEL(2, "C"),
+ dac5571_CHANNEL(3, "D"),
+};
+
+static int dac5571_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = data->val[chan->channel];
+ return IIO_VAL_INT;
+ break;
+
+ case IIO_CHAN_INFO_SCALE:
+ ret = regulator_get_voltage(data->vref);
+ if (ret < 0)
+ return ret;
+
+ *val = ret / 1000;
+ *val2 = data->spec->resolution;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ break;
+
+ default:
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+static int dac5571_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct dac5571_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (data->val[chan->channel] == val)
+ return 0;
+
+ if (val >= (1 << data->spec->resolution) || val < 0)
+ return -EINVAL;
+
+ if (data->powerdown)
+ return -EBUSY;
+
+ mutex_lock(&data->lock);
+ ret = data->dac5571_cmd(data, chan->channel, val);
+ if (ret <= 0)
+ data->val[chan->channel] = val;
+ mutex_unlock(&data->lock);
+ return ret;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static int dac5571_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ return IIO_VAL_INT;
+}
+
+static const struct iio_info dac5571_info = {
+ .read_raw = dac5571_read_raw,
+ .write_raw = dac5571_write_raw,
+ .write_raw_get_fmt = dac5571_write_raw_get_fmt,
+};
+
+static int dac5571_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ const struct dac5571_spec *spec;
+ struct dac5571_data *data;
+ struct iio_dev *indio_dev;
+ int ret, i;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+
+ indio_dev->dev.parent = dev;
+ indio_dev->dev.of_node = client->dev.of_node;
+ indio_dev->info = &dac5571_info;
+ indio_dev->name = id->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = dac5571_channels;
+
+ spec = &dac5571_spec[id->driver_data];
+ indio_dev->num_channels = spec->num_channels;
+ data->spec = spec;
+
+ data->vref = devm_regulator_get(dev, "vref");
+ if (IS_ERR(data->vref))
+ return PTR_ERR(data->vref);
+
+ ret = regulator_enable(data->vref);
+ if (ret < 0)
+ return ret;
+
+ mutex_init(&data->lock);
+
+ switch (spec->num_channels) {
+ case DAC5571_SINGLE_CHANNEL:
+ data->dac5571_cmd = dac5571_cmd_single;
+ data->dac5571_pwrdwn = dac5571_pwrdwn_single;
+ break;
+ case DAC5571_QUAD_CHANNEL:
+ data->dac5571_cmd = dac5571_cmd_quad;
+ data->dac5571_pwrdwn = dac5571_pwrdwn_quad;
+ break;
+ default:
+ goto err;
+ break;
+ }
+
+ for (i = 0; i < spec->num_channels; i++) {
+ ret = data->dac5571_cmd(data, i, 0);
+ if (ret <= 0) {
+ dev_err(dev, "failed to initialize channel %d to 0\n", i);
+ goto err;
+ }
+ }
+
+ ret = iio_device_register(indio_dev);
+ if (ret)
+ goto err;
+
+ return 0;
+
+ err:
+ regulator_disable(data->vref);
+ return ret;
+}
+
+static int dac5571_remove(struct i2c_client *i2c)
+{
+ struct iio_dev *indio_dev = i2c_get_clientdata(i2c);
+ struct dac5571_data *data = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ regulator_disable(data->vref);
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id dac5571_of_id[] = {
+ {.compatible = "ti,dac5571"},
+ {.compatible = "ti,dac6571"},
+ {.compatible = "ti,dac7571"},
+ {.compatible = "ti,dac5574"},
+ {.compatible = "ti,dac6574"},
+ {.compatible = "ti,dac7574"},
+ {.compatible = "ti,dac5573"},
+ {.compatible = "ti,dac6573"},
+ {.compatible = "ti,dac7573"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, dac5571_of_id);
+#endif
+
+static const struct i2c_device_id dac5571_id[] = {
+ {"dac5571", single_8bit},
+ {"dac6571", single_10bit},
+ {"dac7571", single_12bit},
+ {"dac5574", quad_8bit},
+ {"dac6574", quad_10bit},
+ {"dac7574", quad_12bit},
+ {"dac5573", quad_8bit},
+ {"dac6573", quad_10bit},
+ {"dac7573", quad_12bit},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, dac5571_id);
+
+static struct i2c_driver dac5571_driver = {
+ .driver = {
+ .name = "ti-dac5571",
+ },
+ .probe = dac5571_probe,
+ .remove = dac5571_remove,
+ .id_table = dac5571_id,
+};
+module_i2c_driver(dac5571_driver);
+
+MODULE_AUTHOR("Sean Nyekjaer <sean.nyekjaer@prevas.dk>");
+MODULE_DESCRIPTION("Texas Instruments 8/10/12-bit 1/4-channel DAC driver");
+MODULE_LICENSE("GPL v2");
--
2.17.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v2 2/2] iio: ti-dac5571: Add DT binding documentation
2018-04-10 8:44 ` [RFC PATCH v2 " Sean Nyekjaer
@ 2018-04-10 8:44 ` Sean Nyekjaer
2018-04-15 20:01 ` [RFC PATCH v2 1/2] iio: dac: add TI DAC5755 family support Jonathan Cameron
1 sibling, 0 replies; 10+ messages in thread
From: Sean Nyekjaer @ 2018-04-10 8:44 UTC (permalink / raw)
To: jic23, robh+dt
Cc: Sean Nyekjaer, knaack.h, lars, pmeerw, mark.rutland, linux-iio,
devicetree
Adding binding documentation for Texas Instruments DAC5571 Family
Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/iio/dac/ti,dac5571.txt | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt
new file mode 100644
index 000000000000..03af6b9a4d07
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ti,dac5571.txt
@@ -0,0 +1,24 @@
+* Texas Instruments DAC5571 Family
+
+Required properties:
+ - compatible: Should contain
+ "ti,dac5571"
+ "ti,dac6571"
+ "ti,dac7571"
+ "ti,dac5574"
+ "ti,dac6574"
+ "ti,dac7574"
+ "ti,dac5573"
+ "ti,dac6573"
+ "ti,dac7573"
+ - reg: Should contain the DAC I2C address
+
+Optional properties:
+ - vref-supply: The regulator supply for DAC reference voltage
+
+Example:
+dac@0 {
+ compatible = "ti,dac5571";
+ reg = <0x4C>;
+ vref-supply = <&vdd_supply>;
+};
--
2.17.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v2 1/2] iio: dac: add TI DAC5755 family support
2018-04-10 8:44 ` [RFC PATCH v2 " Sean Nyekjaer
2018-04-10 8:44 ` [RFC PATCH v2 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
@ 2018-04-15 20:01 ` Jonathan Cameron
1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2018-04-15 20:01 UTC (permalink / raw)
To: Sean Nyekjaer
Cc: robh+dt, knaack.h, lars, pmeerw, mark.rutland, linux-iio, devicetree
On Tue, 10 Apr 2018 10:44:52 +0200
Sean Nyekjaer <sean.nyekjaer@prevas.dk> wrote:
> This patch adds support for the Texas Intruments DAC5755 Family.
>
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
First comment, please don't be tempted to reply to an earlier
version of a patch with a new version. It just makes for confusing
reading (particularly once we reach version 15 *evil laugh*)
I nearly missed this entirely as it was many pages earlier in
my email.
A few oddities inline in handling of pwrdwn in particular.
Thanks,
Jonathan
> ---
>
> Still not have hardware :-( Ready next week I hope. So still RFC.
>
> v2:
> Addressed comments from Jonathan and Peter
> - improved readability
> - i2c return check fixed
> - added structure pointer instead of copying elements
> - splitted single and quad operations with function pointers
>
> drivers/iio/dac/Kconfig | 10 +
> drivers/iio/dac/Makefile | 1 +
> drivers/iio/dac/ti-dac5571.c | 430 +++++++++++++++++++++++++++++++++++
> 3 files changed, 441 insertions(+)
> create mode 100644 drivers/iio/dac/ti-dac5571.c
>
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 965d5c0d2468..b64fabbc74f8 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -320,6 +320,16 @@ config TI_DAC082S085
>
> If compiled as a module, it will be called ti-dac082s085.
>
> +config TI_DAC5571
> + tristate "Texas Instruments 8/10/12/16-bit 1/2/4-channel DAC driver"
> + depends on I2C
> + help
> + Driver for the Texas Instruments
> + DAC5571, DAC6571, DAC7571, DAC5574, DAC6574, DAC7574, DAC5573,
> + DAC6573, DAC7573, DAC8571, DAC8574.
> +
> + If compiled as a module, it will be called ti-dac5571.
> +
> config VF610_DAC
> tristate "Vybrid vf610 DAC driver"
> depends on OF
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index 81e710ed7491..1ab358d522ee 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -35,4 +35,5 @@ obj-$(CONFIG_MCP4922) += mcp4922.o
> obj-$(CONFIG_STM32_DAC_CORE) += stm32-dac-core.o
> obj-$(CONFIG_STM32_DAC) += stm32-dac.o
> obj-$(CONFIG_TI_DAC082S085) += ti-dac082s085.o
> +obj-$(CONFIG_TI_DAC5571) += ti-dac5571.o
> obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
> new file mode 100644
> index 000000000000..0ee5ca1ce6eb
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac5571.c
> @@ -0,0 +1,430 @@
> +/*
> + * ti-dac5571.c - Texas Instruments 8/10/12-bit 1/4-channel DAC driver
> + *
> + * Copyright (C) 2018 Prevas A/S
> + *
> + * http://www.ti.com/lit/ds/symlink/dac5571.pdf
> + * http://www.ti.com/lit/ds/symlink/dac6571.pdf
> + * http://www.ti.com/lit/ds/symlink/dac7571.pdf
> + * http://www.ti.com/lit/ds/symlink/dac5574.pdf
> + * http://www.ti.com/lit/ds/symlink/dac6574.pdf
> + * http://www.ti.com/lit/ds/symlink/dac7574.pdf
> + * http://www.ti.com/lit/ds/symlink/dac5573.pdf
> + * http://www.ti.com/lit/ds/symlink/dac6573.pdf
> + * http://www.ti.com/lit/ds/symlink/dac7573.pdf
> + *
> + * 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
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/iio/iio.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/regulator/consumer.h>
> +
> +enum chip_id {
> + single_8bit, single_10bit, single_12bit,
> + quad_8bit, quad_10bit, quad_12bit
> +};
> +
> +#define DAC5571_SINGLE_CHANNEL 1
> +#define DAC5571_QUAD_CHANNEL 4
> +
> +struct dac5571_spec {
> + u8 num_channels;
> + u8 resolution;
> +};
> +
> +static const struct dac5571_spec dac5571_spec[] = {
> + [single_8bit] = {.num_channels = 1, .resolution = 8},
> + [single_10bit] = {.num_channels = 1, .resolution = 10},
> + [single_12bit] = {.num_channels = 1, .resolution = 12},
> + [quad_8bit] = {.num_channels = 4, .resolution = 8},
> + [quad_10bit] = {.num_channels = 4, .resolution = 10},
> + [quad_12bit] = {.num_channels = 4, .resolution = 12},
> +};
> +
> +struct dac5571_data {
> + struct i2c_client *client;
> + int id;
> + struct mutex lock;
> + struct regulator *vref;
> + u16 val[4];
> + bool powerdown;
> + u8 powerdown_mode;
> + struct dac5571_spec const *spec;
> + int (*dac5571_cmd) (struct dac5571_data *, int, u16);
> + int (*dac5571_pwrdwn) (struct dac5571_data *, int, u8);
> + u8 buf[3] ____cacheline_aligned;
> +};
> +
> +#define DAC5571_POWERDOWN(mode) ((mode) + 1)
> +#define DAC5571_POWERDOWN_FLAG BIT(0)
> +#define DAC5571_CHANNEL_SELECT BIT(1)
> +#define DAC5571_LOADMODE_DIRECT BIT(4)
> +#define DAC5571_SINGLE_PWRDWN_BITS 4
> +#define DAC5571_QUAD_PWRDWN_BITS 6
> +
> +static int dac5571_cmd_single(struct dac5571_data *data, int channel, u16 val)
> +{
> + unsigned int shift;
> +
> + shift = 12 - data->spec->resolution;
> + data->buf[0] = val << shift;
> + data->buf[1] = val >> (8 - shift);
> + return i2c_master_send(data->client, data->buf, 2);
> +}
> +
> +
> +static int dac5571_cmd_quad(struct dac5571_data *data, int channel, u16 val)
> +{
> + unsigned int shift;
> +
> + shift = 16 - data->spec->resolution;
> + data->buf[0] = val << shift;
> + data->buf[1] = (val >> (8 - shift));
> + data->buf[2] = (channel << DAC5571_CHANNEL_SELECT) |
> + DAC5571_LOADMODE_DIRECT;
> + return i2c_master_send(data->client, data->buf, 3);
> +}
> +
> +static int dac5571_pwrdwn_single(struct dac5571_data *data, int channel, u8 pwrdwn)
> +{
> + unsigned int shift;
> +
> + shift = 12 - data->spec->resolution;
> + data->buf[0] = 0;
> + data->buf[1] = pwrdwn << DAC5571_SINGLE_PWRDWN_BITS;
> + return i2c_master_send(data->client, data->buf, 2);
> +}
> +
> +static int dac5571_pwrdwn_quad(struct dac5571_data *data, int channel, u8 pwrdwn)
> +{
> + unsigned int shift;
> +
> + shift = 16 - data->spec->resolution;
> + data->buf[0] = 0;
> + data->buf[1] = pwrdwn << DAC5571_QUAD_PWRDWN_BITS;
> + data->buf[2] = (channel << DAC5571_CHANNEL_SELECT) |
> + DAC5571_LOADMODE_DIRECT | DAC5571_POWERDOWN_FLAG;
> + return i2c_master_send(data->client, data->buf, 3);
I would check the return == 3 and return an error if not.
> +}
> +
> +static const char *const dac5571_powerdown_modes[] = {
> + "1kohm_to_gnd", "100kohm_to_gnd", "three_state",
> +};
> +
> +static int dac5571_get_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> +
> + return data->powerdown_mode;
> +}
> +
> +static int dac5571_set_powerdown_mode(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int mode)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + int ret = 0;
> +
> + if (data->powerdown_mode == mode)
> + return 0;
> +
> + mutex_lock(&data->lock);
> + if (data->powerdown) {
> + ret = data->dac5571_pwrdwn(data, chan->channel,
> + DAC5571_POWERDOWN(mode));
> + if (ret <= 0)
> + goto out;
This is unusual. I would handle the 0 case inside the pwrdwn functions
so we can assume 0 is good.
> + }
> + data->powerdown_mode = mode;
> +
> + out:
> + mutex_unlock(&data->lock);
blank line here would help readability a little.
> + return ret;
> +}
> +
> +static const struct iio_enum dac5571_powerdown_mode = {
> + .items = dac5571_powerdown_modes,
> + .num_items = ARRAY_SIZE(dac5571_powerdown_modes),
> + .get = dac5571_get_powerdown_mode,
> + .set = dac5571_set_powerdown_mode,
> +};
> +
> +static ssize_t dac5571_read_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + char *buf)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> +
> + return sprintf(buf, "%d\n", data->powerdown);
> +}
> +
> +static ssize_t dac5571_write_powerdown(struct iio_dev *indio_dev,
> + uintptr_t private,
> + const struct iio_chan_spec *chan,
> + const char *buf, size_t len)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + bool powerdown;
> + int ret;
> +
> + ret = strtobool(buf, &powerdown);
> + if (ret)
> + return ret;
> +
> + if (data->powerdown == powerdown)
> + return len;
> +
> + mutex_lock(&data->lock);
> + if (powerdown)
> + ret = data->dac5571_pwrdwn(data, chan->channel,
> + DAC5571_POWERDOWN(data->powerdown_mode));
> + else
> + ret = data->dac5571_cmd(data, chan->channel, data->val[0]);
> + if (ret <= 0)
> + data->powerdown = powerdown;
Not sure I follow the logic here. I think we failed to power down
but you are storing it anyway?
I would indent the bad path and use a goto to jump statements in the good path.
> + mutex_unlock(&data->lock);
> +
> + return ret ? ret : len;
> +}
> +
> +
> +static const struct iio_chan_spec_ext_info dac5571_ext_info[] = {
> + {
> + .name = "powerdown",
> + .read = dac5571_read_powerdown,
> + .write = dac5571_write_powerdown,
> + .shared = IIO_SHARED_BY_TYPE,
> + },
> + IIO_ENUM("powerdown_mode", IIO_SHARED_BY_TYPE, &dac5571_powerdown_mode),
> + IIO_ENUM_AVAILABLE("powerdown_mode", &dac5571_powerdown_mode),
> + {},
> +};
> +
> +#define dac5571_CHANNEL(chan, name) { \
> + .type = IIO_VOLTAGE, \
> + .channel = (chan), \
> + .address = (chan), \
> + .indexed = true, \
> + .output = true, \
> + .datasheet_name = name, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .ext_info = dac5571_ext_info, \
> +}
> +
> +static const struct iio_chan_spec dac5571_channels[] = {
> + dac5571_CHANNEL(0, "A"),
> + dac5571_CHANNEL(1, "B"),
> + dac5571_CHANNEL(2, "C"),
> + dac5571_CHANNEL(3, "D"),
> +};
> +
> +static int dac5571_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *val = data->val[chan->channel];
> + return IIO_VAL_INT;
break after a return is pointless, get rid of them.
> + break;
> +
> + case IIO_CHAN_INFO_SCALE:
> + ret = regulator_get_voltage(data->vref);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret / 1000;
> + *val2 = data->spec->resolution;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + break;
> +
> + default:
> + ret = -EINVAL;
return -EINVAL;
> + }
> +
> + return ret;
Not needed if we return from the default: case.
> +}
> +
> +static int dac5571_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct dac5571_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (data->val[chan->channel] == val)
> + return 0;
> +
> + if (val >= (1 << data->spec->resolution) || val < 0)
> + return -EINVAL;
> +
> + if (data->powerdown)
> + return -EBUSY;
> +
> + mutex_lock(&data->lock);
> + ret = data->dac5571_cmd(data, chan->channel, val);
> + if (ret <= 0)
> + data->val[chan->channel] = val;
> + mutex_unlock(&data->lock);
> + return ret;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int dac5571_write_raw_get_fmt(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + long mask)
> +{
> + return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info dac5571_info = {
> + .read_raw = dac5571_read_raw,
> + .write_raw = dac5571_write_raw,
> + .write_raw_get_fmt = dac5571_write_raw_get_fmt,
> +};
> +
> +static int dac5571_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + const struct dac5571_spec *spec;
> + struct dac5571_data *data;
> + struct iio_dev *indio_dev;
> + int ret, i;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> +
> + indio_dev->dev.parent = dev;
> + indio_dev->dev.of_node = client->dev.of_node;
> + indio_dev->info = &dac5571_info;
> + indio_dev->name = id->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = dac5571_channels;
> +
> + spec = &dac5571_spec[id->driver_data];
> + indio_dev->num_channels = spec->num_channels;
> + data->spec = spec;
> +
> + data->vref = devm_regulator_get(dev, "vref");
> + if (IS_ERR(data->vref))
> + return PTR_ERR(data->vref);
> +
> + ret = regulator_enable(data->vref);
> + if (ret < 0)
> + return ret;
> +
> + mutex_init(&data->lock);
> +
> + switch (spec->num_channels) {
> + case DAC5571_SINGLE_CHANNEL:
> + data->dac5571_cmd = dac5571_cmd_single;
> + data->dac5571_pwrdwn = dac5571_pwrdwn_single;
> + break;
> + case DAC5571_QUAD_CHANNEL:
> + data->dac5571_cmd = dac5571_cmd_quad;
> + data->dac5571_pwrdwn = dac5571_pwrdwn_quad;
> + break;
> + default:
> + goto err;
> + break;
> + }
> +
> + for (i = 0; i < spec->num_channels; i++) {
> + ret = data->dac5571_cmd(data, i, 0);
> + if (ret <= 0) {
> + dev_err(dev, "failed to initialize channel %d to 0\n", i);
> + goto err;
> + }
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err;
> +
> + return 0;
> +
> + err:
> + regulator_disable(data->vref);
> + return ret;
> +}
> +
> +static int dac5571_remove(struct i2c_client *i2c)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(i2c);
> + struct dac5571_data *data = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + regulator_disable(data->vref);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id dac5571_of_id[] = {
> + {.compatible = "ti,dac5571"},
> + {.compatible = "ti,dac6571"},
> + {.compatible = "ti,dac7571"},
> + {.compatible = "ti,dac5574"},
> + {.compatible = "ti,dac6574"},
> + {.compatible = "ti,dac7574"},
> + {.compatible = "ti,dac5573"},
> + {.compatible = "ti,dac6573"},
> + {.compatible = "ti,dac7573"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dac5571_of_id);
> +#endif
> +
> +static const struct i2c_device_id dac5571_id[] = {
> + {"dac5571", single_8bit},
> + {"dac6571", single_10bit},
> + {"dac7571", single_12bit},
> + {"dac5574", quad_8bit},
> + {"dac6574", quad_10bit},
> + {"dac7574", quad_12bit},
> + {"dac5573", quad_8bit},
> + {"dac6573", quad_10bit},
> + {"dac7573", quad_12bit},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, dac5571_id);
> +
> +static struct i2c_driver dac5571_driver = {
> + .driver = {
> + .name = "ti-dac5571",
> + },
> + .probe = dac5571_probe,
> + .remove = dac5571_remove,
> + .id_table = dac5571_id,
> +};
> +module_i2c_driver(dac5571_driver);
> +
> +MODULE_AUTHOR("Sean Nyekjaer <sean.nyekjaer@prevas.dk>");
> +MODULE_DESCRIPTION("Texas Instruments 8/10/12-bit 1/4-channel DAC driver");
> +MODULE_LICENSE("GPL v2");
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-04-15 20:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 10:23 [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Sean Nyekjaer
2018-03-22 10:23 ` [RFC PATCH 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
2018-03-26 22:24 ` Rob Herring
2018-03-22 11:25 ` [RFC PATCH 1/2] iio: dac: add TI DAC5755 family support Peter Meerwald-Stadler
2018-03-23 12:27 ` Jonathan Cameron
2018-03-23 12:35 ` Sean Nyekjaer
2018-03-23 12:42 ` Peter Meerwald-Stadler
2018-04-10 8:44 ` [RFC PATCH v2 " Sean Nyekjaer
2018-04-10 8:44 ` [RFC PATCH v2 2/2] iio: ti-dac5571: Add DT binding documentation Sean Nyekjaer
2018-04-15 20:01 ` [RFC PATCH v2 1/2] iio: dac: add TI DAC5755 family support Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.