diff mbox series

[1/2] Add driver for Texas Instrument DAC7311 It is a driver for Texas Instruments 8/10/12-bit 1-channel compatible with DAC6311 and DAC5311 chips.

Message ID 20181006203108.20439-1-charles-antoine.couret@essensium.com (mailing list archive)
State New, archived
Headers show
Series [1/2] Add driver for Texas Instrument DAC7311 It is a driver for Texas Instruments 8/10/12-bit 1-channel compatible with DAC6311 and DAC5311 chips. | expand

Commit Message

Couret Charles-Antoine Oct. 6, 2018, 8:31 p.m. UTC 
Datasheet of this chip:
http://www.ti.com/lit/ds/symlink/dac7311.pdf

Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@essensium.com>
---
 drivers/iio/dac/Kconfig      |   9 +
 drivers/iio/dac/Makefile     |   1 +
 drivers/iio/dac/ti-dac7311.c | 361 +++++++++++++++++++++++++++++++++++
 3 files changed, 371 insertions(+)
 create mode 100644 drivers/iio/dac/ti-dac7311.c

Comments

Jonathan Cameron Oct. 7, 2018, 4:36 p.m. UTC | #1 
On Sat,  6 Oct 2018 22:31:07 +0200
Charles-Antoine Couret <charles-antoine.couret@essensium.com> wrote:

> Datasheet of this chip:
> http://www.ti.com/lit/ds/symlink/dac7311.pdf
> 
> Signed-off-by: Charles-Antoine Couret <charles-antoine.couret@essensium.com>
Hi Charles-Antoine,

A few questions and comments inline.

Thanks,

Jonathan

> ---
>  drivers/iio/dac/Kconfig      |   9 +
>  drivers/iio/dac/Makefile     |   1 +
>  drivers/iio/dac/ti-dac7311.c | 361 +++++++++++++++++++++++++++++++++++
>  3 files changed, 371 insertions(+)
>  create mode 100644 drivers/iio/dac/ti-dac7311.c
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index 80beb64e9e0c..ac205c8f5de0 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -356,6 +356,15 @@ config TI_DAC5571
>  
>  	  If compiled as a module, it will be called ti-dac5571.
>  
> +config TI_DAC7311
> +	tristate "Texas Instruments 8/10/12-bit 1-channel DAC driver"
> +	depends on SPI
> +	help
> +	  Driver for the Texas Instruments
> +	  DAC7311, DAC6311, DAC5311.
> +
> +	  If compiled as a module, it will be called ti-dac7311.
> +
>  config VF610_DAC
>  	tristate "Vybrid vf610 DAC driver"
>  	depends on OF
> diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> index a1b37cf99441..4b02021cc30f 100644
> --- a/drivers/iio/dac/Makefile
> +++ b/drivers/iio/dac/Makefile
> @@ -39,4 +39,5 @@ 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_TI_DAC7311) += ti-dac7311.o
>  obj-$(CONFIG_VF610_DAC) += vf610_dac.o
> diff --git a/drivers/iio/dac/ti-dac7311.c b/drivers/iio/dac/ti-dac7311.c
> new file mode 100644
> index 000000000000..e7e61a7ce619
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac7311.c
> @@ -0,0 +1,361 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* ti-dac7311.c - Texas Instruments 8/10/12-bit 1-channel DAC driver
> + *
> + * Copyright (C) 2018 CMC NV
> + *
> + * http://www.ti.com/lit/ds/symlink/dac7311.pdf
> + */
> +
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/spi/spi.h>
> +
> +enum {
> +	SINGLE_8BITS = 0,
> +	SINGLE_10BITS,
> +	SINGLE_12BITS,
> +};
> +
> +enum {
> +	POWER_RUNNING = 0,
> +	POWER_1KOHM_TO_GND,
> +	POWER_100KOHM_TO_GND,
> +	POWER_TRI_STATE,
> +};
> +
> +struct ti_dac_spec {
> +	u8 num_channels;
> +	u8 resolution;
> +};
> +
> +static const struct ti_dac_spec ti_dac_spec[] = {
> +	[SINGLE_8BITS]  = { .num_channels = 1, .resolution = 8  },
I think I'd prefer to see the enum as part names.  It is easy
to see the number of channels and resolution here anyway.

Not to mention there is little point in having num_channels in here
at all with them all being the same.

> +	[SINGLE_10BITS] = { .num_channels = 1, .resolution = 10 },
> +	[SINGLE_12BITS] = { .num_channels = 1, .resolution = 12 },
> +};
> +
> +/**
> + * struct ti_dac_chip - TI DAC chip
> + * @lock: protects write sequences
> + * @vref: regulator generating Vref
> + * @mesg: SPI message to perform a write
> + * @xfer: SPI transfer used by @mesg
> + * @val: cached value
> + * @powerdown: whether the chip is powered down
> + * @powerdown_mode: selected by the user
> + * @resolution: resolution of the chip
> + * @buf: buffer for @xfer
> + */
> +struct ti_dac_chip {
> +	struct mutex lock;
> +	struct regulator *vref;
> +	struct spi_message mesg;
> +	struct spi_transfer xfer;
> +	u16 val;
> +	bool powerdown;
> +	u8 powerdown_mode;
> +	u8 resolution;
> +	u8 buf[2] ____cacheline_aligned;
> +};
> +
> +static int ti_dac_cmd(struct ti_dac_chip *ti_dac, u8 power, u16 val)
> +{
> +	u8 shift = 14 - ti_dac->resolution;
> +
> +	ti_dac->buf[0] = (val << shift) & 0xFF;
> +	ti_dac->buf[1] = (power << 6) | (val >> (8 - shift));
> +	return spi_sync(ti_dac->mesg.spi, &ti_dac->mesg);
As I mention below, perhaps just use spi_write?
> +}
> +
> +static const char * const ti_dac_powerdown_modes[] = {
> +	"running",

What is running in a power down mode?

> +	"1kohm_to_gnd",
> +	"100kohm_to_gnd",
> +	"three_state",
> +};
> +
> +static int ti_dac_get_powerdown_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan)
> +{
> +	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
> +
> +	return ti_dac->powerdown_mode;
> +}
> +
> +static int ti_dac_set_powerdown_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     unsigned int mode)
> +{
> +	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
> +	int ret = 0;
> +
> +	if (ti_dac->powerdown_mode == mode)
> +		return ret;
> +
> +	mutex_lock(&ti_dac->lock);
> +	if (ti_dac->powerdown) {
> +		ret = ti_dac_cmd(ti_dac, ti_dac->powerdown_mode, ti_dac->val);
> +		if (ret)
> +			goto out;
> +	}
> +	ti_dac->powerdown_mode = mode;
> +
> +out:
> +	mutex_unlock(&ti_dac->lock);
> +	return ret;
> +}
> +
> +static const struct iio_enum ti_dac_powerdown_mode = {
> +	.items = ti_dac_powerdown_modes,
> +	.num_items = ARRAY_SIZE(ti_dac_powerdown_modes),
> +	.get = ti_dac_get_powerdown_mode,
> +	.set = ti_dac_set_powerdown_mode,
> +};
> +
> +static ssize_t ti_dac_read_powerdown(struct iio_dev *indio_dev,
> +				     uintptr_t private,
> +				     const struct iio_chan_spec *chan,
> +				     char *buf)
> +{
> +	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
> +
> +	return sprintf(buf, "%d\n", ti_dac->powerdown);
> +}
> +
> +static ssize_t ti_dac_write_powerdown(struct iio_dev *indio_dev,
> +				      uintptr_t private,
> +				      const struct iio_chan_spec *chan,
> +				      const char *buf, size_t len)
> +{
> +	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
> +	bool powerdown;
> +	int ret;
> +
> +	ret = strtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	if (ti_dac->powerdown == powerdown)
> +		return len;
> +
> +	mutex_lock(&ti_dac->lock);
> +		ret = ti_dac_cmd(ti_dac, ti_dac->powerdown_mode, 0);
Strange indenting here.

> +	if (!ret)
> +		ti_dac->powerdown = powerdown;
> +	mutex_unlock(&ti_dac->lock);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct iio_chan_spec_ext_info ti_dac_ext_info[] = {
> +	{
> +		.name	   = "powerdown",
> +		.read	   = ti_dac_read_powerdown,
> +		.write	   = ti_dac_write_powerdown,
> +		.shared	   = IIO_SHARED_BY_TYPE,
> +	},
> +	IIO_ENUM("powerdown_mode", IIO_SHARED_BY_TYPE, &ti_dac_powerdown_mode),
> +	IIO_ENUM_AVAILABLE("powerdown_mode", &ti_dac_powerdown_mode),
> +	{ },
> +};
> +
> +#define TI_DAC_CHANNEL(chan) {					\
> +	.type = IIO_VOLTAGE,					\
> +	.channel = (chan),					\
> +	.address = (chan),					\
Little purpose in setting address if it is always the same as channel.
Might as well use channel instead..

> +	.output = true,						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +	.ext_info = ti_dac_ext_info,				\
> +}
> +
> +static const struct iio_chan_spec ti_dac_channels[] = {
> +	TI_DAC_CHANNEL(0),
> +};
> +
> +static int ti_dac_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*val = ti_dac->val;
> +		ret = IIO_VAL_INT;
return IIO_VAL_INT;

same throughout please.
> +		break;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regulator_get_voltage(ti_dac->vref);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = ret / 1000;
> +		*val2 = ti_dac->resolution;
> +		ret = IIO_VAL_FRACTIONAL_LOG2;
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int ti_dac_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (ti_dac->val == val)
> +			return 0;
> +
> +		if (val >= (1 << ti_dac->resolution) || val < 0)
> +			return -EINVAL;
> +
> +		if (ti_dac->powerdown)
> +			return -EBUSY;
> +
> +		mutex_lock(&ti_dac->lock);
> +		ret = ti_dac_cmd(ti_dac, ti_dac->powerdown_mode, val);
> +		if (!ret)
> +			ti_dac->val = val;
> +		mutex_unlock(&ti_dac->lock);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int ti_dac_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 ti_dac_info = {
> +	.read_raw	   = ti_dac_read_raw,
> +	.write_raw	   = ti_dac_write_raw,
> +	.write_raw_get_fmt = ti_dac_write_raw_get_fmt,
> +};
> +
> +static int ti_dac_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	const struct ti_dac_spec *spec;
> +	struct ti_dac_chip *ti_dac;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*ti_dac));
> +	if (!indio_dev) {
> +		dev_err(dev, "can not allocate iio device\n");
> +		return -ENOMEM;
> +	}
> +
> +	spi->max_speed_hz = 50000000;
> +	spi->mode = SPI_MODE_1;
> +	spi->irq = -1;
I don't know of a reason you'd want to set this to anything in particular.
Please explain if I'm missing something.

> +	spi->bits_per_word = 16;
> +	spi_setup(spi);
> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->dev.of_node = spi->dev.of_node;
> +	indio_dev->info = &ti_dac_info;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ti_dac_channels;
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	ti_dac = iio_priv(indio_dev);
> +	ti_dac->powerdown_mode = POWER_RUNNING;
> +	ti_dac->val = 0;

The private structure is initialized with kzalloc so
there is no need to set defaults to 0.

> +	ti_dac->xfer.tx_buf = &ti_dac->buf;
> +	ti_dac->xfer.len = sizeof(ti_dac->buf);
> +	spi_message_init_with_transfers(&ti_dac->mesg, &ti_dac->xfer, 1);
> +	ti_dac->mesg.spi = spi;
Is this always just a write?  Why not just use spi_write instead of
the more complex spi_sync with it's need for manual building of
the message etc?

> +
> +	spec = &ti_dac_spec[spi_get_device_id(spi)->driver_data];
> +	indio_dev->num_channels = spec->num_channels;
> +	ti_dac->resolution = spec->resolution;
> +
> +	ti_dac->vref = devm_regulator_get(dev, "vref");
> +	if (IS_ERR(ti_dac->vref)) {
> +		dev_err(dev, "error to get regulator\n");
> +		return PTR_ERR(ti_dac->vref);
> +	}
> +
> +	ret = regulator_enable(ti_dac->vref);
> +	if (ret < 0) {
> +		dev_err(dev, "can not enable regulator\n");
> +		return ret;
> +	}
> +
> +	mutex_init(&ti_dac->lock);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "fail to register iio device: %d\n", ret);
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	mutex_destroy(&ti_dac->lock);
> +	regulator_disable(ti_dac->vref);
> +	return ret;
> +}
> +
> +static int ti_dac_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	mutex_destroy(&ti_dac->lock);
> +	regulator_disable(ti_dac->vref);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id ti_dac_of_id[] = {
> +	{ .compatible = "ti,dac5311" },
> +	{ .compatible = "ti,dac6311" },
> +	{ .compatible = "ti,dac7311" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ti_dac_of_id);
> +#endif
> +
> +static const struct spi_device_id ti_dac_spi_id[] = {
> +	{ "dac5311", SINGLE_8BITS  },
> +	{ "dac6311", SINGLE_10BITS },
> +	{ "dac7311", SINGLE_12BITS },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, ti_dac_spi_id);
> +
> +static struct spi_driver ti_dac_driver = {
> +	.driver = {
> +		.name		= "ti-dac7311",
> +		.of_match_table	= of_match_ptr(ti_dac_of_id),
> +	},
> +	.probe	  = ti_dac_probe,
> +	.remove   = ti_dac_remove,
> +	.id_table = ti_dac_spi_id,
> +};
> +module_spi_driver(ti_dac_driver);
> +
> +MODULE_AUTHOR("Charles-Antoine Couret <charles-antoine.couret@essensium.com>");
> +MODULE_DESCRIPTION("Texas Instruments 8/10/12-bit 1-channel DAC driver");
> +MODULE_LICENSE("GPL v2");
Couret Charles-Antoine Oct. 8, 2018, 9:34 p.m. UTC | #2 
Hi,
Like the previous driver, thank you for the code review, I have some 
questions before making V2.

Le 07/10/2018 à 18:36, Jonathan Cameron a écrit :
>> +enum {
>> +	SINGLE_8BITS = 0,
>> +	SINGLE_10BITS,
>> +	SINGLE_12BITS,
>> +};
>> +
>> +enum {
>> +	POWER_RUNNING = 0,
>> +	POWER_1KOHM_TO_GND,
>> +	POWER_100KOHM_TO_GND,
>> +	POWER_TRI_STATE,
>> +};
>> +
>> +struct ti_dac_spec {
>> +	u8 num_channels;
>> +	u8 resolution;
>> +};
>> +
>> +static const struct ti_dac_spec ti_dac_spec[] = {
>> +	[SINGLE_8BITS]  = { .num_channels = 1, .resolution = 8  },
> I think I'd prefer to see the enum as part names.  It is easy
> to see the number of channels and resolution here anyway.
I still don't understand the "part names" thing here.
>> +}
>> +
>> +static const char * const ti_dac_powerdown_modes[] = {
>> +	"running",
> What is running in a power down mode?

For me it was the list of power status of the device. And using 
"running" allow us to set to running state with the right command.

But I probably misunderstand its purpose. Why we have to put only power 
down in that structure?


Thank you.

Regards,

Charles-Antoine Couret
Jonathan Cameron Oct. 13, 2018, 12:32 p.m. UTC | #3 
On Mon, 8 Oct 2018 23:34:27 +0200
Couret Charles-Antoine <charles-antoine.couret@essensium.com> wrote:

> Hi,
> Like the previous driver, thank you for the code review, I have some 
> questions before making V2.

Hi Charles-Antoine,

Sorry for the slow responses, it's been a rather busy week!

Anyhow, answers inline.
> 
> Le 07/10/2018 à 18:36, Jonathan Cameron a écrit :
> >> +enum {
> >> +	SINGLE_8BITS = 0,
> >> +	SINGLE_10BITS,
> >> +	SINGLE_12BITS,
> >> +};
> >> +
> >> +enum {
> >> +	POWER_RUNNING = 0,
> >> +	POWER_1KOHM_TO_GND,
> >> +	POWER_100KOHM_TO_GND,
> >> +	POWER_TRI_STATE,
> >> +};
> >> +
> >> +struct ti_dac_spec {
> >> +	u8 num_channels;
> >> +	u8 resolution;
> >> +};
> >> +
> >> +static const struct ti_dac_spec ti_dac_spec[] = {
> >> +	[SINGLE_8BITS]  = { .num_channels = 1, .resolution = 8  },  
> > I think I'd prefer to see the enum as part names.  It is easy
> > to see the number of channels and resolution here anyway.  
> I still don't understand the "part names" thing here.
This got clarified I think in the other thread but in brief

[DAC7311] = { .num_channels = 1, .resolution = 8 },
[DAC6411] - {...}

It's a convention that makes it easier to extend the driver
when we get something that needs another little change...

[SINGLE_8BITS_WITH_POWERDOWN_MAGIC] etc doesn't scale.
Sometimes it's just better to use the number of the part from
the datasheet to index these things.

Note that if you have two compatible parts with different numbers
then you can use the first part that was supported as the enum
name and just make the connection in the device_tables that are
used to get this enum value in the first place.

> >> +}
> >> +
> >> +static const char * const ti_dac_powerdown_modes[] = {
> >> +	"running",  
> > What is running in a power down mode?  
> 
> For me it was the list of power status of the device. And using 
> "running" allow us to set to running state with the right command.
> 
> But I probably misunderstand its purpose. Why we have to put only power 
> down in that structure?

There is a separation in the interface between what we want to do
when we power down and the power down control itself.   Arguably they
could have been combined, but IIRC (and I may have this wrong) the earliest
devices could only power down in one way so we had a boolean flag to do
it.  Later we had devices with lots of options and hence needed to describe
them.  So it's basically a legacy thing.  We could have done it the way
you describe, but we didn't. Now for userspace to know what is happening
we need to stick to it.


Thanks,

Jonathan
> 
> 
> Thank you.
> 
> Regards,
> 
> Charles-Antoine Couret
>
diff mbox series

Patch

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 80beb64e9e0c..ac205c8f5de0 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -356,6 +356,15 @@  config TI_DAC5571
 
 	  If compiled as a module, it will be called ti-dac5571.
 
+config TI_DAC7311
+	tristate "Texas Instruments 8/10/12-bit 1-channel DAC driver"
+	depends on SPI
+	help
+	  Driver for the Texas Instruments
+	  DAC7311, DAC6311, DAC5311.
+
+	  If compiled as a module, it will be called ti-dac7311.
+
 config VF610_DAC
 	tristate "Vybrid vf610 DAC driver"
 	depends on OF
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index a1b37cf99441..4b02021cc30f 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -39,4 +39,5 @@  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_TI_DAC7311) += ti-dac7311.o
 obj-$(CONFIG_VF610_DAC) += vf610_dac.o
diff --git a/drivers/iio/dac/ti-dac7311.c b/drivers/iio/dac/ti-dac7311.c
new file mode 100644
index 000000000000..e7e61a7ce619
--- /dev/null
+++ b/drivers/iio/dac/ti-dac7311.c
@@ -0,0 +1,361 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* ti-dac7311.c - Texas Instruments 8/10/12-bit 1-channel DAC driver
+ *
+ * Copyright (C) 2018 CMC NV
+ *
+ * http://www.ti.com/lit/ds/symlink/dac7311.pdf
+ */
+
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+enum {
+	SINGLE_8BITS = 0,
+	SINGLE_10BITS,
+	SINGLE_12BITS,
+};
+
+enum {
+	POWER_RUNNING = 0,
+	POWER_1KOHM_TO_GND,
+	POWER_100KOHM_TO_GND,
+	POWER_TRI_STATE,
+};
+
+struct ti_dac_spec {
+	u8 num_channels;
+	u8 resolution;
+};
+
+static const struct ti_dac_spec ti_dac_spec[] = {
+	[SINGLE_8BITS]  = { .num_channels = 1, .resolution = 8  },
+	[SINGLE_10BITS] = { .num_channels = 1, .resolution = 10 },
+	[SINGLE_12BITS] = { .num_channels = 1, .resolution = 12 },
+};
+
+/**
+ * struct ti_dac_chip - TI DAC chip
+ * @lock: protects write sequences
+ * @vref: regulator generating Vref
+ * @mesg: SPI message to perform a write
+ * @xfer: SPI transfer used by @mesg
+ * @val: cached value
+ * @powerdown: whether the chip is powered down
+ * @powerdown_mode: selected by the user
+ * @resolution: resolution of the chip
+ * @buf: buffer for @xfer
+ */
+struct ti_dac_chip {
+	struct mutex lock;
+	struct regulator *vref;
+	struct spi_message mesg;
+	struct spi_transfer xfer;
+	u16 val;
+	bool powerdown;
+	u8 powerdown_mode;
+	u8 resolution;
+	u8 buf[2] ____cacheline_aligned;
+};
+
+static int ti_dac_cmd(struct ti_dac_chip *ti_dac, u8 power, u16 val)
+{
+	u8 shift = 14 - ti_dac->resolution;
+
+	ti_dac->buf[0] = (val << shift) & 0xFF;
+	ti_dac->buf[1] = (power << 6) | (val >> (8 - shift));
+	return spi_sync(ti_dac->mesg.spi, &ti_dac->mesg);
+}
+
+static const char * const ti_dac_powerdown_modes[] = {
+	"running",
+	"1kohm_to_gnd",
+	"100kohm_to_gnd",
+	"three_state",
+};
+
+static int ti_dac_get_powerdown_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
+
+	return ti_dac->powerdown_mode;
+}
+
+static int ti_dac_set_powerdown_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int mode)
+{
+	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
+	int ret = 0;
+
+	if (ti_dac->powerdown_mode == mode)
+		return ret;
+
+	mutex_lock(&ti_dac->lock);
+	if (ti_dac->powerdown) {
+		ret = ti_dac_cmd(ti_dac, ti_dac->powerdown_mode, ti_dac->val);
+		if (ret)
+			goto out;
+	}
+	ti_dac->powerdown_mode = mode;
+
+out:
+	mutex_unlock(&ti_dac->lock);
+	return ret;
+}
+
+static const struct iio_enum ti_dac_powerdown_mode = {
+	.items = ti_dac_powerdown_modes,
+	.num_items = ARRAY_SIZE(ti_dac_powerdown_modes),
+	.get = ti_dac_get_powerdown_mode,
+	.set = ti_dac_set_powerdown_mode,
+};
+
+static ssize_t ti_dac_read_powerdown(struct iio_dev *indio_dev,
+				     uintptr_t private,
+				     const struct iio_chan_spec *chan,
+				     char *buf)
+{
+	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
+
+	return sprintf(buf, "%d\n", ti_dac->powerdown);
+}
+
+static ssize_t ti_dac_write_powerdown(struct iio_dev *indio_dev,
+				      uintptr_t private,
+				      const struct iio_chan_spec *chan,
+				      const char *buf, size_t len)
+{
+	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
+	bool powerdown;
+	int ret;
+
+	ret = strtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	if (ti_dac->powerdown == powerdown)
+		return len;
+
+	mutex_lock(&ti_dac->lock);
+		ret = ti_dac_cmd(ti_dac, ti_dac->powerdown_mode, 0);
+	if (!ret)
+		ti_dac->powerdown = powerdown;
+	mutex_unlock(&ti_dac->lock);
+
+	return ret ? ret : len;
+}
+
+static const struct iio_chan_spec_ext_info ti_dac_ext_info[] = {
+	{
+		.name	   = "powerdown",
+		.read	   = ti_dac_read_powerdown,
+		.write	   = ti_dac_write_powerdown,
+		.shared	   = IIO_SHARED_BY_TYPE,
+	},
+	IIO_ENUM("powerdown_mode", IIO_SHARED_BY_TYPE, &ti_dac_powerdown_mode),
+	IIO_ENUM_AVAILABLE("powerdown_mode", &ti_dac_powerdown_mode),
+	{ },
+};
+
+#define TI_DAC_CHANNEL(chan) {					\
+	.type = IIO_VOLTAGE,					\
+	.channel = (chan),					\
+	.address = (chan),					\
+	.output = true,						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+	.ext_info = ti_dac_ext_info,				\
+}
+
+static const struct iio_chan_spec ti_dac_channels[] = {
+	TI_DAC_CHANNEL(0),
+};
+
+static int ti_dac_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*val = ti_dac->val;
+		ret = IIO_VAL_INT;
+		break;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regulator_get_voltage(ti_dac->vref);
+		if (ret < 0)
+			return ret;
+
+		*val = ret / 1000;
+		*val2 = ti_dac->resolution;
+		ret = IIO_VAL_FRACTIONAL_LOG2;
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int ti_dac_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (ti_dac->val == val)
+			return 0;
+
+		if (val >= (1 << ti_dac->resolution) || val < 0)
+			return -EINVAL;
+
+		if (ti_dac->powerdown)
+			return -EBUSY;
+
+		mutex_lock(&ti_dac->lock);
+		ret = ti_dac_cmd(ti_dac, ti_dac->powerdown_mode, val);
+		if (!ret)
+			ti_dac->val = val;
+		mutex_unlock(&ti_dac->lock);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int ti_dac_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 ti_dac_info = {
+	.read_raw	   = ti_dac_read_raw,
+	.write_raw	   = ti_dac_write_raw,
+	.write_raw_get_fmt = ti_dac_write_raw_get_fmt,
+};
+
+static int ti_dac_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	const struct ti_dac_spec *spec;
+	struct ti_dac_chip *ti_dac;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*ti_dac));
+	if (!indio_dev) {
+		dev_err(dev, "can not allocate iio device\n");
+		return -ENOMEM;
+	}
+
+	spi->max_speed_hz = 50000000;
+	spi->mode = SPI_MODE_1;
+	spi->irq = -1;
+	spi->bits_per_word = 16;
+	spi_setup(spi);
+
+	indio_dev->dev.parent = dev;
+	indio_dev->dev.of_node = spi->dev.of_node;
+	indio_dev->info = &ti_dac_info;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ti_dac_channels;
+	spi_set_drvdata(spi, indio_dev);
+
+	ti_dac = iio_priv(indio_dev);
+	ti_dac->powerdown_mode = POWER_RUNNING;
+	ti_dac->val = 0;
+	ti_dac->xfer.tx_buf = &ti_dac->buf;
+	ti_dac->xfer.len = sizeof(ti_dac->buf);
+	spi_message_init_with_transfers(&ti_dac->mesg, &ti_dac->xfer, 1);
+	ti_dac->mesg.spi = spi;
+
+	spec = &ti_dac_spec[spi_get_device_id(spi)->driver_data];
+	indio_dev->num_channels = spec->num_channels;
+	ti_dac->resolution = spec->resolution;
+
+	ti_dac->vref = devm_regulator_get(dev, "vref");
+	if (IS_ERR(ti_dac->vref)) {
+		dev_err(dev, "error to get regulator\n");
+		return PTR_ERR(ti_dac->vref);
+	}
+
+	ret = regulator_enable(ti_dac->vref);
+	if (ret < 0) {
+		dev_err(dev, "can not enable regulator\n");
+		return ret;
+	}
+
+	mutex_init(&ti_dac->lock);
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "fail to register iio device: %d\n", ret);
+		goto err;
+	}
+
+	return 0;
+
+err:
+	mutex_destroy(&ti_dac->lock);
+	regulator_disable(ti_dac->vref);
+	return ret;
+}
+
+static int ti_dac_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct ti_dac_chip *ti_dac = iio_priv(indio_dev);
+
+	iio_device_unregister(indio_dev);
+	mutex_destroy(&ti_dac->lock);
+	regulator_disable(ti_dac->vref);
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id ti_dac_of_id[] = {
+	{ .compatible = "ti,dac5311" },
+	{ .compatible = "ti,dac6311" },
+	{ .compatible = "ti,dac7311" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_dac_of_id);
+#endif
+
+static const struct spi_device_id ti_dac_spi_id[] = {
+	{ "dac5311", SINGLE_8BITS  },
+	{ "dac6311", SINGLE_10BITS },
+	{ "dac7311", SINGLE_12BITS },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ti_dac_spi_id);
+
+static struct spi_driver ti_dac_driver = {
+	.driver = {
+		.name		= "ti-dac7311",
+		.of_match_table	= of_match_ptr(ti_dac_of_id),
+	},
+	.probe	  = ti_dac_probe,
+	.remove   = ti_dac_remove,
+	.id_table = ti_dac_spi_id,
+};
+module_spi_driver(ti_dac_driver);
+
+MODULE_AUTHOR("Charles-Antoine Couret <charles-antoine.couret@essensium.com>");
+MODULE_DESCRIPTION("Texas Instruments 8/10/12-bit 1-channel DAC driver");
+MODULE_LICENSE("GPL v2");