All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 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.
@ 2018-10-17 20:37 Charles-Antoine Couret
  2018-10-17 20:37 ` [PATCH 2/2] Add ti,dac7311 device tree bindings in documentation Charles-Antoine Couret
  2018-10-21 15:07 ` [PATCH 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 Jonathan Cameron
  0 siblings, 2 replies; 8+ messages in thread
From: Charles-Antoine Couret @ 2018-10-17 20:37 UTC (permalink / raw)
  To: linux-iio; +Cc: Charles-Antoine Couret

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 | 341 +++++++++++++++++++++++++++++++++++
 3 files changed, 351 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..2010bf27c371
--- /dev/null
+++ b/drivers/iio/dac/ti-dac7311.c
@@ -0,0 +1,341 @@
+// 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 {
+	ID_DAC5311 = 0,
+	ID_DAC6311,
+	ID_DAC7311,
+};
+
+enum {
+	POWER_1KOHM_TO_GND = 0,
+	POWER_100KOHM_TO_GND,
+	POWER_TRI_STATE,
+};
+
+struct ti_dac_spec {
+	u8 resolution;
+};
+
+static const struct ti_dac_spec ti_dac_spec[] = {
+	[ID_DAC5311]  = { .resolution = 8  },
+	[ID_DAC6311] = { .resolution = 10 },
+	[ID_DAC7311] = { .resolution = 12 },
+};
+
+/**
+ * struct ti_dac_chip - TI DAC chip
+ * @lock: protects write sequences
+ * @vref: regulator generating Vref
+ * @spi: SPI device to send data to the device
+ * @val: cached value
+ * @powerdown: whether the chip is powered down
+ * @powerdown_mode: selected by the user
+ * @resolution: resolution of the chip
+ * @buf: buffer for transfer data
+ */
+struct ti_dac_chip {
+	struct mutex lock;
+	struct regulator *vref;
+	struct spi_device *spi;
+	u16 val;
+	bool powerdown;
+	u8 powerdown_mode;
+	u8 resolution;
+	u8 buf[2] ____cacheline_aligned;
+};
+
+static u8 ti_dac_get_power(struct ti_dac_chip *ti_dac, bool powerdown)
+{
+	if (powerdown)
+		return ti_dac->powerdown_mode + 1;
+
+	return 0;
+}
+
+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_write(ti_dac->spi, ti_dac->buf, 2);
+}
+
+static const char * const ti_dac_powerdown_modes[] = {
+	"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);
+
+	ti_dac->powerdown_mode = mode;
+	return 0;
+}
+
+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;
+	u8 power;
+	int ret;
+
+	ret = strtobool(buf, &powerdown);
+	if (ret)
+		return ret;
+
+	power = ti_dac_get_power(ti_dac, powerdown);
+
+	mutex_lock(&ti_dac->lock);
+	ret = ti_dac_cmd(ti_dac, power, 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),					\
+	.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;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regulator_get_voltage(ti_dac->vref);
+		if (ret < 0)
+			return ret;
+
+		*val = ret / 1000;
+		*val2 = ti_dac->resolution;
+		return IIO_VAL_FRACTIONAL_LOG2;
+	}
+
+	return -EINVAL;
+}
+
+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);
+	u8 power = ti_dac_get_power(ti_dac, ti_dac->powerdown);
+	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, power, 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->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 = false;
+	ti_dac->spi = spi;
+
+	spec = &ti_dac_spec[spi_get_device_id(spi)->driver_data];
+	indio_dev->num_channels = 1;
+	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", ID_DAC5311  },
+	{ "dac6311", ID_DAC6311 },
+	{ "dac7311", ID_DAC7311 },
+	{ }
+};
+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");
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] Add ti,dac7311 device tree bindings in documentation
  2018-10-17 20:37 [PATCH 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 Charles-Antoine Couret
@ 2018-10-17 20:37 ` Charles-Antoine Couret
  2018-10-21 14:58   ` Jonathan Cameron
  2018-10-21 15:07 ` [PATCH 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 Jonathan Cameron
  1 sibling, 1 reply; 8+ messages in thread
From: Charles-Antoine Couret @ 2018-10-17 20:37 UTC (permalink / raw)
  To: linux-iio; +Cc: Charles-Antoine Couret

---
 .../bindings/iio/dac/ti,dac7311.txt           | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt

diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt b/Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt
new file mode 100644
index 000000000000..771414024cd0
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt
@@ -0,0 +1,20 @@
+TI DAC7311 device tree bindings
+
+Required properties:
+
+- compatible: Must be set to "ti,dac7311", "ti,dac6311" or "ti,dac5311"
+- reg: spi chip select number for the device
+- vref-supply: The regulator supply for ADC reference voltage
+- spi-max-frequency: Max SPI frequency to use (< 50000000)
+
+Example:
+
+	spi_master {
+		dac7311: dac7311@0 {
+			compatible = "ti,dac7311";
+			reg = <0>; /* CS0 */
+			spi-max-frequency = <1000000>;
+			vref-supply = <&vdd_supply>;
+		};
+	};
+
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] Add ti,dac7311 device tree bindings in documentation
  2018-10-17 20:37 ` [PATCH 2/2] Add ti,dac7311 device tree bindings in documentation Charles-Antoine Couret
@ 2018-10-21 14:58   ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2018-10-21 14:58 UTC (permalink / raw)
  To: Charles-Antoine Couret; +Cc: linux-iio, Rob Herring, Mark Rutland, devicetree

On Wed, 17 Oct 2018 22:37:38 +0200
Charles-Antoine Couret <charles-antoine.couret@essensium.com> wrote:

Would be good to have a small amount of description here + it seems you didn't
sign off on this patch so we can't apply it to the kernel.

Please cc devicetree list and maintainers for any devicetree bindings.
+cc Rob, Mark and list.

Thanks,

Jonathan
> ---
>  .../bindings/iio/dac/ti,dac7311.txt           | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt b/Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt
> new file mode 100644
> index 000000000000..771414024cd0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ti,dac7311.txt
> @@ -0,0 +1,20 @@
> +TI DAC7311 device tree bindings
> +
> +Required properties:
> +
> +- compatible: Must be set to "ti,dac7311", "ti,dac6311" or "ti,dac5311"
> +- reg: spi chip select number for the device
> +- vref-supply: The regulator supply for ADC reference voltage
> +- spi-max-frequency: Max SPI frequency to use (< 50000000)
> +
> +Example:
> +
> +	spi_master {
> +		dac7311: dac7311@0 {
Not sure why you need the pre colon bit here.

Also I'd imagine the standard naming for a dac is probably
dac@0
(I haven't checked the devicetree spec to see if they actually
have that one in there as I'm on a train with dodgy wifi).

Jonathan
> +			compatible = "ti,dac7311";
> +			reg = <0>; /* CS0 */
> +			spi-max-frequency = <1000000>;
> +			vref-supply = <&vdd_supply>;
> +		};
> +	};
> +

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 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.
  2018-10-17 20:37 [PATCH 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 Charles-Antoine Couret
  2018-10-17 20:37 ` [PATCH 2/2] Add ti,dac7311 device tree bindings in documentation Charles-Antoine Couret
@ 2018-10-21 15:07 ` Jonathan Cameron
  1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2018-10-21 15:07 UTC (permalink / raw)
  To: Charles-Antoine Couret; +Cc: linux-iio

On Wed, 17 Oct 2018 22:37:37 +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.

Something slightly odd happened with the title of this patch.

Also, I assume it is a V2?  Then it should have 
[PATCH V2 1/2]...


> ---
Here is where we would expect to see a list of updates made from V1.


Other than that procedural stuff, looks good to me. 

I'll leave it on the list for a little longer though and I think you'll
end up doing a v3 to tidy up the DT binding example.


Thanks,

Jonathan
 
>  drivers/iio/dac/Kconfig      |   9 +
>  drivers/iio/dac/Makefile     |   1 +
>  drivers/iio/dac/ti-dac7311.c | 341 +++++++++++++++++++++++++++++++++++
>  3 files changed, 351 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..2010bf27c371
> --- /dev/null
> +++ b/drivers/iio/dac/ti-dac7311.c
> @@ -0,0 +1,341 @@
> +// 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 {
> +	ID_DAC5311 = 0,
> +	ID_DAC6311,
> +	ID_DAC7311,
> +};
> +
> +enum {
> +	POWER_1KOHM_TO_GND = 0,
> +	POWER_100KOHM_TO_GND,
> +	POWER_TRI_STATE,
> +};
> +
> +struct ti_dac_spec {
> +	u8 resolution;
> +};
> +
> +static const struct ti_dac_spec ti_dac_spec[] = {
> +	[ID_DAC5311]  = { .resolution = 8  },
> +	[ID_DAC6311] = { .resolution = 10 },
> +	[ID_DAC7311] = { .resolution = 12 },
> +};
> +
> +/**
> + * struct ti_dac_chip - TI DAC chip
> + * @lock: protects write sequences
> + * @vref: regulator generating Vref
> + * @spi: SPI device to send data to the device
> + * @val: cached value
> + * @powerdown: whether the chip is powered down
> + * @powerdown_mode: selected by the user
> + * @resolution: resolution of the chip
> + * @buf: buffer for transfer data
> + */
> +struct ti_dac_chip {
> +	struct mutex lock;
> +	struct regulator *vref;
> +	struct spi_device *spi;
> +	u16 val;
> +	bool powerdown;
> +	u8 powerdown_mode;
> +	u8 resolution;
> +	u8 buf[2] ____cacheline_aligned;
> +};
> +
> +static u8 ti_dac_get_power(struct ti_dac_chip *ti_dac, bool powerdown)
> +{
> +	if (powerdown)
> +		return ti_dac->powerdown_mode + 1;
> +
> +	return 0;
> +}
> +
> +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_write(ti_dac->spi, ti_dac->buf, 2);
> +}
> +
> +static const char * const ti_dac_powerdown_modes[] = {
> +	"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);
> +
> +	ti_dac->powerdown_mode = mode;
> +	return 0;
> +}
> +
> +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;
> +	u8 power;
> +	int ret;
> +
> +	ret = strtobool(buf, &powerdown);
> +	if (ret)
> +		return ret;
> +
> +	power = ti_dac_get_power(ti_dac, powerdown);
> +
> +	mutex_lock(&ti_dac->lock);
> +	ret = ti_dac_cmd(ti_dac, power, 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),					\
> +	.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;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regulator_get_voltage(ti_dac->vref);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = ret / 1000;
> +		*val2 = ti_dac->resolution;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +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);
> +	u8 power = ti_dac_get_power(ti_dac, ti_dac->powerdown);
> +	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, power, 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->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 = false;
> +	ti_dac->spi = spi;
> +
> +	spec = &ti_dac_spec[spi_get_device_id(spi)->driver_data];
> +	indio_dev->num_channels = 1;
> +	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", ID_DAC5311  },
> +	{ "dac6311", ID_DAC6311 },
> +	{ "dac7311", ID_DAC7311 },
> +	{ }
> +};
> +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");

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 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.
  2018-10-08 21:34   ` Couret Charles-Antoine
@ 2018-10-13 12:32     ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2018-10-13 12:32 UTC (permalink / raw)
  To: Couret Charles-Antoine; +Cc: linux-iio

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
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 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.
  2018-10-07 16:36 ` Jonathan Cameron
@ 2018-10-08 21:34   ` Couret Charles-Antoine
  2018-10-13 12:32     ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Couret Charles-Antoine @ 2018-10-08 21:34 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 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.
  2018-10-06 20:31 Charles-Antoine Couret
@ 2018-10-07 16:36 ` Jonathan Cameron
  2018-10-08 21:34   ` Couret Charles-Antoine
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2018-10-07 16:36 UTC (permalink / raw)
  To: Charles-Antoine Couret; +Cc: linux-iio

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");

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 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.
@ 2018-10-06 20:31 Charles-Antoine Couret
  2018-10-07 16:36 ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Charles-Antoine Couret @ 2018-10-06 20:31 UTC (permalink / raw)
  To: linux-iio; +Cc: Charles-Antoine Couret

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

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");
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-10-21 23:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17 20:37 [PATCH 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 Charles-Antoine Couret
2018-10-17 20:37 ` [PATCH 2/2] Add ti,dac7311 device tree bindings in documentation Charles-Antoine Couret
2018-10-21 14:58   ` Jonathan Cameron
2018-10-21 15:07 ` [PATCH 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 Jonathan Cameron
  -- strict thread matches above, loose matches on Subject: below --
2018-10-06 20:31 Charles-Antoine Couret
2018-10-07 16:36 ` Jonathan Cameron
2018-10-08 21:34   ` Couret Charles-Antoine
2018-10-13 12:32     ` 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.