From: Michael Hennerich <[email protected]>
Changes since RFC/v1:
IIO: Apply list review feedback:
Apply list review feedback:
Restructure documentation according to list feedback.
Rename attributes to fit IIO convention used in other drivers.
Fix typos.
Provide ddsX_out_enable as opposed to ddsX_out_disable
Signed-off-by: Michael Hennerich <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
---
.../staging/iio/Documentation/sysfs-bus-iio-dds | 94 ++++++++++++++++++++
1 files changed, 94 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-dds
diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-dds b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
new file mode 100644
index 0000000..6791174
--- /dev/null
+++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
@@ -0,0 +1,94 @@
+
+What: /sys/bus/iio/devices/device[n]/ddsX_freqY
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Stores frequency into tuning word register Y.
+ There can be more than one ddsX_freqY file, which allows for
+ pin controlled FSK Frequency Shift Keying
+ (ddsX_pincontrol_freq_en is active) or the user can control
+ the desired active tuning word by writing Y to the
+ ddsX_freqsymbol file.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_freqY_scale
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Scale to be applied to ddsX_freqY in order to obtain the
+ desired value in Hz. If shared across all frequency registers
+ Y is not present. It is also possible X is not present if
+ shared across all channels.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_freqsymbol
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Specifies the active output frequency tuning word. The value
+ corresponds to the Y in ddsX_freqY. To exit this mode the user
+ can write ddsX_pincontrol_freq_en or ddsX_out_disable file.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_phaseY
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Stores phase into phase register Y.
+ There can be more than one ddsX_phaseY file, which allows for
+ pin controlled PSK Phase Shift Keying
+ (ddsX_pincontrol_phase_en is active) or the user can
+ control the desired phase Y which is added to the phase
+ accumulator output by writing Y to the en_phase file.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_phaseY_scale
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Scale to be applied to ddsX_phaseY in order to obtain the
+ desired value in rad. If shared across all phase registers
+ Y is not present. It is also possible X is not present if
+ shared across all channels.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_phasesymbol
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Specifies the active phase Y which is added to the phase
+ accumulator output. The value corresponds to the Y in
+ ddsX_phaseY. To exit this mode the user can write
+ ddsX_pincontrol_phase_en or disable file.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_en
+What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_freq_en
+What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_phase_en
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ ddsX_pincontrol_en: Both, the active frequency and phase is
+ controlled by the respective phase and frequency control inputs.
+ In case the device in question allows to independent controls,
+ then there are dedicated files (ddsX_pincontrol_freq_en,
+ ddsX_pincontrol_phase_en).
+
+What: /sys/bus/iio/devices/device[n]/ddsX_out_enable
+What: /sys/bus/iio/devices/device[n]/ddsX_outY_enable
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ ddsX_out_enable: Enables signal generation on all outputs
+ of channel X.
+ ddsX_outY_enable: Enables signal generation on output Y,
+ of channel X.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Specifies the output waveform.
+ (sine, triangle, ramp, square, ...)
+ For a list of available output waveform options read
+ available_output_modes.
+
+What: /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype_available
+KernelVersion: 2.6.37
+Contact: [email protected]
+Description:
+ Lists all available output waveform options.
--
1.6.0.2
From: Michael Hennerich <[email protected]>
Changes since RFC/v1:
IIO: Apply list review feedback
Apply list review feedback:
Rename attributes to fit IIO convention used in other drivers.
Provide ddsX_out_enable as opposed to ddsX_out_disable.
Fix typos.
Signed-off-by: Michael Hennerich <[email protected]>
---
drivers/staging/iio/dds/dds.h | 152 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 152 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/iio/dds/dds.h
diff --git a/drivers/staging/iio/dds/dds.h b/drivers/staging/iio/dds/dds.h
new file mode 100644
index 0000000..a8dd7be
--- /dev/null
+++ b/drivers/staging/iio/dds/dds.h
@@ -0,0 +1,152 @@
+/*
+ * dds.h - sysfs attributes associated with DDS devices
+ *
+ * Copyright (c) 2010 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_freqY
+ * Description:
+ * Stores frequency into tuning word register Y.
+ * There can be more than one ddsX_freqY file, which allows for pin controlled
+ * FSK Frequency Shift Keying (ddsX_pincontrol_freq_en is active) or the user
+ * can control the desired active tuning word by writing Y to the
+ * ddsX_freqsymbol file.
+ */
+
+#define IIO_DEV_ATTR_FREQ(_device, _num, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_freq##_num, S_IWUSR, NULL, _store, _addr)
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_freqY_scale
+ * Description:
+ * Scale to be applied to ddsX_freqY in order to obtain the
+ * desired value in Hz. If shared across all frequency registers
+ * Y is not present.
+ */
+
+#define IIO_CONST_ATTR_FREQ_SCALE(_device, _string) \
+ IIO_CONST_ATTR(dds##_device##_freq_scale, _string)
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_freqsymbol
+ * Description:
+ * Specifies the active output frequency tuning word. The value corresponds
+ * to the Y in ddsX_freqY. To exit this mode the user can write
+ * ddsX_pincontrol_freq_en or ddsX_out_disable file.
+ */
+
+#define IIO_DEV_ATTR_FREQSYMBOL(_device, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_freqsymbol, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_phaseY
+ * Description:
+ * Stores phase into phase register Y.
+ * There can be more than one ddsX_phaseY file, which allows for pin controlled
+ * PSK Phase Shift Keying (ddsX_pincontrol_phase_en is active) or the user can
+ * control the desired phase Y which is added to the phase accumulator output
+ * by writing Y to the en_phase file.
+ */
+
+#define IIO_DEV_ATTR_PHASE(_device, _num, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_phase##_num, \
+ S_IWUSR, NULL, _store, _addr)
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_phaseY_scale
+ * Description:
+ * Scale to be applied to ddsX_phaseY in order to obtain the
+ * desired value in rad. If shared across all phase registers
+ * Y is not present.
+ */
+
+#define IIO_CONST_ATTR_PHASE_SCALE(_device, _string) \
+ IIO_CONST_ATTR(dds##_device##_phase_scale, _string)
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_phasesymbol
+ * Description:
+ * Specifies the active phase Y which is added to the phase accumulator output.
+ * The value corresponds to the Y in ddsX_phaseY. To exit this mode the user
+ * can write ddsX_pincontrol_phase_en or disable file.
+ */
+
+#define IIO_DEV_ATTR_PHASESYMBOL(_device, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_phasesymbol, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_en
+ * Description:
+ * Both, the active frequency and phase is controlled by the respective
+ * phase and frequency control inputs.
+ */
+
+#define IIO_DEV_ATTR_PINCONTROL_EN(_device, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_pincontrol_en, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_freq_en
+ * Description:
+ * The active frequency is controlled by the respective
+ * frequency control/select inputs.
+ */
+
+#define IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_device, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_pincontrol_freq_en, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_phase_en
+ * Description:
+ * The active phase is controlled by the respective
+ * phase control/select inputs.
+ */
+
+#define IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_device, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_pincontrol_phase_en, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_out_enable
+ * Description:
+ * Disables any signal generation on all outputs.
+ */
+
+#define IIO_DEV_ATTR_OUT_ENABLE(_device, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_out_enable, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_outY_enable
+ * Description:
+ * Disables any signal generation on output Y.
+ */
+
+#define IIO_DEV_ATTR_OUTY_ENABLE(_device, _output, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_out##_output##_enable, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype
+ * Description:
+ * Specifies the output waveform. (sine, triangle, ramp, square, ...)
+ * For a list of available output waveform options read available_output_modes.
+ */
+#define IIO_DEV_ATTR_OUT_WAVETYPE(_device, _output, _store, _addr) \
+ IIO_DEVICE_ATTR(dds##_device##_out##_output##_wavetype, \
+ S_IWUSR, NULL, _store, _addr);
+
+/**
+ * /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype_available
+ * Description:
+ * Lists all available output waveform options.
+ */
+#define IIO_CONST_ATTR_OUT_WAVETYPES_AVAILABLE(_device, _output, _modes)\
+ IIO_CONST_ATTR(dds##_device##_out##_output##_wavetype_available,\
+ _modes);
--
1.6.0.2
From: Michael Hennerich <[email protected]>
Changes since RFC/v1:
IIO: Apply list review feedback
Apply list review feedback:
Rename attributes to fit IIO convention used in other drivers.
Fix typos.
Provide ddsX_out_enable as opposed to ddsX_out_disable.
Use proper __devexit marking.
Use strict_strtoul() to avoid negatives.
Signed-off-by: Michael Hennerich <[email protected]>
Reviewed-by: Datta Shubhrajyoti <[email protected]>
---
drivers/staging/iio/dds/Kconfig | 7 +
drivers/staging/iio/dds/Makefile | 1 +
drivers/staging/iio/dds/ad9834.c | 482 ++++++++++++++++++++++++++++++++++++++
drivers/staging/iio/dds/ad9834.h | 112 +++++++++
4 files changed, 602 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/iio/dds/ad9834.c
create mode 100644 drivers/staging/iio/dds/ad9834.h
diff --git a/drivers/staging/iio/dds/Kconfig b/drivers/staging/iio/dds/Kconfig
index 7969be2..4c9cce3 100644
--- a/drivers/staging/iio/dds/Kconfig
+++ b/drivers/staging/iio/dds/Kconfig
@@ -17,6 +17,13 @@ config AD9832
Say yes here to build support for Analog Devices DDS chip
ad9832 and ad9835, provides direct access via sysfs.
+config AD9834
+ tristate "Analog Devices ad9833/4/ driver"
+ depends on SPI
+ help
+ Say yes here to build support for Analog Devices DDS chip
+ AD9833 and AD9834, provides direct access via sysfs.
+
config AD9850
tristate "Analog Devices ad9850/1 driver"
depends on SPI
diff --git a/drivers/staging/iio/dds/Makefile b/drivers/staging/iio/dds/Makefile
index 6f274ac..1477461 100644
--- a/drivers/staging/iio/dds/Makefile
+++ b/drivers/staging/iio/dds/Makefile
@@ -4,6 +4,7 @@
obj-$(CONFIG_AD5930) += ad5930.o
obj-$(CONFIG_AD9832) += ad9832.o
+obj-$(CONFIG_AD9834) += ad9834.o
obj-$(CONFIG_AD9850) += ad9850.o
obj-$(CONFIG_AD9852) += ad9852.o
obj-$(CONFIG_AD9910) += ad9910.o
diff --git a/drivers/staging/iio/dds/ad9834.c b/drivers/staging/iio/dds/ad9834.c
new file mode 100644
index 0000000..df3d68c
--- /dev/null
+++ b/drivers/staging/iio/dds/ad9834.c
@@ -0,0 +1,482 @@
+/*
+ * AD9834 SPI DAC driver
+ *
+ * Copyright 2010 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/list.h>
+#include <linux/spi/spi.h>
+#include <linux/regulator/consumer.h>
+#include <linux/err.h>
+#include <asm/div64.h>
+
+#include "../iio.h"
+#include "../sysfs.h"
+#include "dds.h"
+
+#include "ad9834.h"
+
+static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
+{
+ unsigned long long freqreg = (u64) fout * (u64) (1 << AD9834_FREQ_BITS);
+ do_div(freqreg, mclk);
+ return freqreg;
+}
+
+static int ad9834_write_frequency(struct ad9834_state *st,
+ unsigned long addr, unsigned long fout)
+{
+ unsigned long regval;
+
+ if (fout > (st->mclk / 2))
+ return -EINVAL;
+
+ regval = ad9834_calc_freqreg(st->mclk, fout);
+
+ st->freq_data[0] = cpu_to_be16(addr | (regval &
+ RES_MASK(AD9834_FREQ_BITS / 2)));
+ st->freq_data[1] = cpu_to_be16(addr | ((regval >>
+ (AD9834_FREQ_BITS / 2)) &
+ RES_MASK(AD9834_FREQ_BITS / 2)));
+
+ return spi_sync(st->spi, &st->freq_msg);;
+}
+
+static int ad9834_write_phase(struct ad9834_state *st,
+ unsigned long addr, unsigned long phase)
+{
+ if (phase > (1 << AD9834_PHASE_BITS))
+ return -EINVAL;
+ st->data = cpu_to_be16(addr | phase);
+
+ return spi_sync(st->spi, &st->msg);
+}
+
+static ssize_t ad9834_write(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t len)
+{
+ struct iio_dev *dev_info = dev_get_drvdata(dev);
+ struct ad9834_state *st = dev_info->dev_data;
+ struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+ int ret;
+ long val;
+
+ ret = strict_strtoul(buf, 10, &val);
+ if (ret)
+ goto error_ret;
+
+ mutex_lock(&dev_info->mlock);
+ switch (this_attr->address) {
+ case AD9834_REG_FREQ0:
+ case AD9834_REG_FREQ1:
+ ret = ad9834_write_frequency(st, this_attr->address, val);
+ break;
+ case AD9834_REG_PHASE0:
+ case AD9834_REG_PHASE1:
+ ret = ad9834_write_phase(st, this_attr->address, val);
+ break;
+ case AD9834_OPBITEN:
+ if (st->control & AD9834_MODE) {
+ ret = -EINVAL; /* AD9843 reserved mode */
+ break;
+ }
+
+ if (val)
+ st->control |= AD9834_OPBITEN;
+ else
+ st->control &= ~AD9834_OPBITEN;
+
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ break;
+ case AD9834_PIN_SW:
+ if (val)
+ st->control |= AD9834_PIN_SW;
+ else
+ st->control &= ~AD9834_PIN_SW;
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ break;
+ case AD9834_FSEL:
+ case AD9834_PSEL:
+ if (val == 0)
+ st->control &= ~(this_attr->address | AD9834_PIN_SW);
+ else if (val == 1) {
+ st->control |= this_attr->address;
+ st->control &= ~AD9834_PIN_SW;
+ } else {
+ ret = -EINVAL;
+ break;
+ }
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ break;
+ case AD9834_RESET:
+ if (val)
+ st->control &= ~AD9834_RESET;
+ else
+ st->control |= AD9834_RESET;
+
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ break;
+ default:
+ ret = -ENODEV;
+ }
+ mutex_unlock(&dev_info->mlock);
+
+error_ret:
+ return ret ? ret : len;
+}
+
+static ssize_t ad9834_store_wavetype(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf,
+ size_t len)
+{
+ struct iio_dev *dev_info = dev_get_drvdata(dev);
+ struct ad9834_state *st = dev_info->dev_data;
+ struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+ int ret = 0;
+ bool is_ad9833 = st->devid == ID_AD9833;
+
+ mutex_lock(&dev_info->mlock);
+
+ switch (this_attr->address) {
+ case 0:
+ if (sysfs_streq(buf, "sine")) {
+ st->control &= ~AD9834_MODE;
+ if (is_ad9833)
+ st->control &= ~AD9834_OPBITEN;
+ } else if (sysfs_streq(buf, "triangle")) {
+ if (is_ad9833) {
+ st->control &= ~AD9834_OPBITEN;
+ st->control |= AD9834_MODE;
+ } else if (st->control & AD9834_OPBITEN) {
+ ret = -EINVAL; /* AD9843 reserved mode */
+ } else {
+ st->control |= AD9834_MODE;
+ }
+ } else if (is_ad9833 && sysfs_streq(buf, "square")) {
+ st->control &= ~AD9834_MODE;
+ st->control |= AD9834_OPBITEN;
+ } else {
+ ret = -EINVAL;
+ }
+
+ break;
+ case 1:
+ if (sysfs_streq(buf, "square") &&
+ !(st->control & AD9834_MODE)) {
+ st->control &= ~AD9834_MODE;
+ st->control |= AD9834_OPBITEN;
+ } else {
+ ret = -EINVAL;
+ }
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (!ret) {
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ }
+ mutex_unlock(&dev_info->mlock);
+
+ return ret ? ret : len;
+}
+
+static ssize_t ad9834_show_name(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *dev_info = dev_get_drvdata(dev);
+ struct ad9834_state *st = iio_dev_get_devdata(dev_info);
+
+ return sprintf(buf, "%s\n", spi_get_device_id(st->spi)->name);
+}
+static IIO_DEVICE_ATTR(name, S_IRUGO, ad9834_show_name, NULL, 0);
+
+static ssize_t ad9834_show_out0_wavetype_available(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *dev_info = dev_get_drvdata(dev);
+ struct ad9834_state *st = iio_dev_get_devdata(dev_info);
+ char *str;
+
+ if (st->devid == ID_AD9833)
+ str = "sine triangle square";
+ else if (st->control & AD9834_OPBITEN)
+ str = "sine";
+ else
+ str = "sine triangle";
+
+ return sprintf(buf, "%s\n", str);
+}
+
+
+static IIO_DEVICE_ATTR(dds0_out0_wavetype_available, S_IRUGO,
+ ad9834_show_out0_wavetype_available, NULL, 0);
+
+static ssize_t ad9834_show_out1_wavetype_available(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *dev_info = dev_get_drvdata(dev);
+ struct ad9834_state *st = iio_dev_get_devdata(dev_info);
+ char *str;
+
+ if (st->control & AD9834_MODE)
+ str = "";
+ else
+ str = "square";
+
+ return sprintf(buf, "%s\n", str);
+}
+
+static IIO_DEVICE_ATTR(dds0_out1_wavetype_available, S_IRUGO,
+ ad9834_show_out1_wavetype_available, NULL, 0);
+
+/**
+ * see dds.h for further information
+ */
+
+static IIO_DEV_ATTR_FREQ(0, 0, ad9834_write, AD9834_REG_FREQ0);
+static IIO_DEV_ATTR_FREQ(0, 1, ad9834_write, AD9834_REG_FREQ1);
+static IIO_DEV_ATTR_FREQSYMBOL(0, ad9834_write, AD9834_FSEL);
+static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
+
+static IIO_DEV_ATTR_PHASE(0, 0, ad9834_write, AD9834_REG_PHASE0);
+static IIO_DEV_ATTR_PHASE(0, 1, ad9834_write, AD9834_REG_PHASE1);
+static IIO_DEV_ATTR_PHASESYMBOL(0, ad9834_write, AD9834_PSEL);
+static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12 rad*/
+
+static IIO_DEV_ATTR_PINCONTROL_EN(0, ad9834_write, AD9834_PIN_SW);
+static IIO_DEV_ATTR_OUT_ENABLE(0, ad9834_write, AD9834_RESET);
+static IIO_DEV_ATTR_OUTY_ENABLE(0, 1, ad9834_write, AD9834_OPBITEN);
+static IIO_DEV_ATTR_OUT_WAVETYPE(0, 0, ad9834_store_wavetype, 0);
+static IIO_DEV_ATTR_OUT_WAVETYPE(0, 1, ad9834_store_wavetype, 1);
+
+static struct attribute *ad9834_attributes[] = {
+ &iio_dev_attr_dds0_freq0.dev_attr.attr,
+ &iio_dev_attr_dds0_freq1.dev_attr.attr,
+ &iio_const_attr_dds0_freq_scale.dev_attr.attr,
+ &iio_dev_attr_dds0_phase0.dev_attr.attr,
+ &iio_dev_attr_dds0_phase1.dev_attr.attr,
+ &iio_const_attr_dds0_phase_scale.dev_attr.attr,
+ &iio_dev_attr_dds0_pincontrol_en.dev_attr.attr,
+ &iio_dev_attr_dds0_freqsymbol.dev_attr.attr,
+ &iio_dev_attr_dds0_phasesymbol.dev_attr.attr,
+ &iio_dev_attr_dds0_out_enable.dev_attr.attr,
+ &iio_dev_attr_dds0_out1_enable.dev_attr.attr,
+ &iio_dev_attr_dds0_out0_wavetype.dev_attr.attr,
+ &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr,
+ &iio_dev_attr_dds0_out0_wavetype_available.dev_attr.attr,
+ &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr,
+ &iio_dev_attr_name.dev_attr.attr,
+ NULL,
+};
+
+static mode_t ad9834_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct iio_dev *dev_info = dev_get_drvdata(dev);
+ struct ad9834_state *st = iio_dev_get_devdata(dev_info);
+
+ mode_t mode = attr->mode;
+
+ if (st->devid == ID_AD9834)
+ return mode;
+
+ if ((attr == &iio_dev_attr_dds0_out1_enable.dev_attr.attr) ||
+ (attr == &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr) ||
+ (attr ==
+ &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr))
+ mode = 0;
+
+ return mode;
+}
+
+static const struct attribute_group ad9834_attribute_group = {
+ .attrs = ad9834_attributes,
+ .is_visible = ad9834_attr_is_visible,
+};
+
+static int __devinit ad9834_probe(struct spi_device *spi)
+{
+ struct ad9834_platform_data *pdata = spi->dev.platform_data;
+ struct ad9834_state *st;
+ int ret;
+
+ if (!pdata) {
+ dev_dbg(&spi->dev, "no platform data?\n");
+ return -ENODEV;
+ }
+
+ st = kzalloc(sizeof(*st), GFP_KERNEL);
+ if (st == NULL) {
+ ret = -ENOMEM;
+ goto error_ret;
+ }
+
+ st->reg = regulator_get(&spi->dev, "vcc");
+ if (!IS_ERR(st->reg)) {
+ ret = regulator_enable(st->reg);
+ if (ret)
+ goto error_put_reg;
+ }
+
+ st->mclk = pdata->mclk;
+
+ spi_set_drvdata(spi, st);
+
+ st->spi = spi;
+ st->devid = spi_get_device_id(spi)->driver_data;
+
+ st->indio_dev = iio_allocate_device();
+ if (st->indio_dev == NULL) {
+ ret = -ENOMEM;
+ goto error_disable_reg;
+ }
+
+ st->indio_dev->dev.parent = &spi->dev;
+ st->indio_dev->attrs = &ad9834_attribute_group;
+ st->indio_dev->dev_data = (void *)(st);
+ st->indio_dev->driver_module = THIS_MODULE;
+ st->indio_dev->modes = INDIO_DIRECT_MODE;
+
+ /* Setup default messages */
+
+ st->xfer.tx_buf = &st->data;
+ st->xfer.len = 2;
+
+ spi_message_init(&st->msg);
+ spi_message_add_tail(&st->xfer, &st->msg);
+
+ st->freq_xfer[0].tx_buf = &st->freq_data[0];
+ st->freq_xfer[0].len = 2;
+ st->freq_xfer[0].cs_change = 1;
+ st->freq_xfer[1].tx_buf = &st->freq_data[1];
+ st->freq_xfer[1].len = 2;
+
+ spi_message_init(&st->freq_msg);
+ spi_message_add_tail(&st->freq_xfer[0], &st->freq_msg);
+ spi_message_add_tail(&st->freq_xfer[1], &st->freq_msg);
+
+ st->control = AD9834_B28 | AD9834_RESET;
+
+ if (!pdata->en_div2)
+ st->control |= AD9834_DIV2;
+
+ if (!pdata->en_signbit_msb_out && (st->devid == ID_AD9834))
+ st->control |= AD9834_SIGN_PIB;
+
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ if (ret) {
+ dev_err(&spi->dev, "device init failed\n");
+ goto error_free_device;
+ }
+
+ ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, pdata->freq0);
+ if (ret)
+ goto error_free_device;
+
+ ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, pdata->freq1);
+ if (ret)
+ goto error_free_device;
+
+ ret = ad9834_write_phase(st, AD9834_REG_PHASE0, pdata->phase0);
+ if (ret)
+ goto error_free_device;
+
+ ret = ad9834_write_phase(st, AD9834_REG_PHASE1, pdata->phase1);
+ if (ret)
+ goto error_free_device;
+
+ st->control &= ~AD9834_RESET;
+ st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
+ ret = spi_sync(st->spi, &st->msg);
+ if (ret)
+ goto error_free_device;
+
+ ret = iio_device_register(st->indio_dev);
+ if (ret)
+ goto error_free_device;
+
+ return 0;
+
+error_free_device:
+ iio_free_device(st->indio_dev);
+error_disable_reg:
+ if (!IS_ERR(st->reg))
+ regulator_disable(st->reg);
+error_put_reg:
+ if (!IS_ERR(st->reg))
+ regulator_put(st->reg);
+ kfree(st);
+error_ret:
+ return ret;
+}
+
+static int __devexit ad9834_remove(struct spi_device *spi)
+{
+ struct ad9834_state *st = spi_get_drvdata(spi);
+ struct iio_dev *indio_dev = st->indio_dev;
+
+ iio_device_unregister(indio_dev);
+ if (!IS_ERR(st->reg)) {
+ regulator_disable(st->reg);
+ regulator_put(st->reg);
+ }
+ kfree(st);
+ return 0;
+}
+
+static const struct spi_device_id ad9834_id[] = {
+ {"ad9833", ID_AD9833},
+ {"ad9834", ID_AD9834},
+ {}
+};
+
+static struct spi_driver ad9834_driver = {
+ .driver = {
+ .name = "ad9834",
+ .bus = &spi_bus_type,
+ .owner = THIS_MODULE,
+ },
+ .probe = ad9834_probe,
+ .remove = __devexit_p(ad9834_remove),
+ .id_table = ad9834_id,
+};
+
+static int __init ad9834_init(void)
+{
+ return spi_register_driver(&ad9834_driver);
+}
+module_init(ad9834_init);
+
+static void __exit ad9834_exit(void)
+{
+ spi_unregister_driver(&ad9834_driver);
+}
+module_exit(ad9834_exit);
+
+MODULE_AUTHOR("Michael Hennerich <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD9833/AD9834 DDS");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:ad9834");
diff --git a/drivers/staging/iio/dds/ad9834.h b/drivers/staging/iio/dds/ad9834.h
new file mode 100644
index 0000000..0fc3b88
--- /dev/null
+++ b/drivers/staging/iio/dds/ad9834.h
@@ -0,0 +1,112 @@
+/*
+ * AD9834 SPI DDS driver
+ *
+ * Copyright 2010 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+#ifndef IIO_DDS_AD9834_H_
+#define IIO_DDS_AD9834_H_
+
+/* Registers */
+
+#define AD9834_REG_CMD (0 << 14)
+#define AD9834_REG_FREQ0 (1 << 14)
+#define AD9834_REG_FREQ1 (2 << 14)
+#define AD9834_REG_PHASE0 (6 << 13)
+#define AD9834_REG_PHASE1 (7 << 13)
+
+/* Command Control Bits */
+
+#define AD9834_B28 (1 << 13)
+#define AD9834_HLB (1 << 12)
+#define AD9834_FSEL (1 << 11)
+#define AD9834_PSEL (1 << 10)
+#define AD9834_PIN_SW (1 << 9)
+#define AD9834_RESET (1 << 8)
+#define AD9834_SLEEP1 (1 << 7)
+#define AD9834_SLEEP12 (1 << 6)
+#define AD9834_OPBITEN (1 << 5)
+#define AD9834_SIGN_PIB (1 << 4)
+#define AD9834_DIV2 (1 << 3)
+#define AD9834_MODE (1 << 1)
+
+#define AD9834_FREQ_BITS 28
+#define AD9834_PHASE_BITS 12
+
+#define RES_MASK(bits) ((1 << (bits)) - 1)
+
+/**
+ * struct ad9834_state - driver instance specific data
+ * @indio_dev: the industrial I/O device
+ * @spi: spi_device
+ * @reg: supply regulator
+ * @mclk: external master clock
+ * @control: cached control word
+ * @xfer: default spi transfer
+ * @msg: default spi message
+ * @freq_xfer: tuning word spi transfer
+ * @freq_msg: tuning word spi message
+ * @data: spi transmit buffer
+ * @freq_data: tuning word spi transmit buffer
+ */
+
+struct ad9834_state {
+ struct iio_dev *indio_dev;
+ struct spi_device *spi;
+ struct regulator *reg;
+ unsigned int mclk;
+ unsigned short control;
+ unsigned short devid;
+ struct spi_transfer xfer;
+ struct spi_message msg;
+ struct spi_transfer freq_xfer[2];
+ struct spi_message freq_msg;
+
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ unsigned short data ____cacheline_aligned;
+ unsigned short freq_data[2] ;
+};
+
+
+/*
+ * TODO: struct ad7887_platform_data needs to go into include/linux/iio
+ */
+
+/**
+ * struct ad9834_platform_data - platform specific information
+ * @mclk: master clock in Hz
+ * @freq0: power up freq0 tuning word in Hz
+ * @freq1: power up freq1 tuning word in Hz
+ * @phase0: power up phase0 value [0..4095] correlates with 0..2PI
+ * @phase1: power up phase1 value [0..4095] correlates with 0..2PI
+ * @en_div2: digital output/2 is passed to the SIGN BIT OUT pin
+ * @en_signbit_msb_out: the MSB (or MSB/2) of the DAC data is connected to the
+ * SIGN BIT OUT pin. en_div2 controls whether it is the MSB
+ * or MSB/2 that is output. if en_signbit_msb_out=false,
+ * the on-board comparator is connected to SIGN BIT OUT
+ */
+
+struct ad9834_platform_data {
+ unsigned int mclk;
+ unsigned int freq0;
+ unsigned int freq1;
+ unsigned short phase0;
+ unsigned short phase1;
+ bool en_div2;
+ bool en_signbit_msb_out;
+};
+
+/**
+ * ad9834_supported_device_ids:
+ */
+
+enum ad9834_supported_device_ids {
+ ID_AD9833,
+ ID_AD9834,
+};
+
+#endif /* IIO_DDS_AD9834_H_ */
--
1.6.0.2
On 12/13/10 16:27, [email protected] wrote:
> From: Michael Hennerich <[email protected]>
>
> Changes since RFC/v1:
> IIO: Apply list review feedback:
>
> Apply list review feedback:
> Restructure documentation according to list feedback.
> Rename attributes to fit IIO convention used in other drivers.
> Fix typos.
> Provide ddsX_out_enable as opposed to ddsX_out_disable
Hi Michael,
This is more or less there. A few minor queries / suggestions
inline. The only major one is the mixture of X and [n] notation
for the various indicies. I 'think' all our docs have now moved
to the X notation. (if not then I have messed up!)
I am happy with all the actual attributes. All remaining issues
are to do with the descriptions of what they do!
Thanks,
Jonathan
>
> Signed-off-by: Michael Hennerich <[email protected]>
> Reviewed-by: Jonathan Cameron <[email protected]>
> ---
> .../staging/iio/Documentation/sysfs-bus-iio-dds | 94 ++++++++++++++++++++
> 1 files changed, 94 insertions(+), 0 deletions(-)
> create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-dds
>
> diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-dds b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
> new file mode 100644
> index 0000000..6791174
> --- /dev/null
> +++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
> @@ -0,0 +1,94 @@
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_freqY
We have a hybrid here of the old indexing schemed (device[n]) and the
new. Either just shorten then form to
/sys/bus/iio/.../ddsX_freqY or use
/sys/bus/iio/devices/deviceX/ddsY_freqZ and update below appropriately.
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Stores frequency into tuning word register Y.
Technically it might not be a register? Dropping 'register' doesn't
to my mind change the meaning but makes it marginally more general.
> + There can be more than one ddsX_freqY file, which allows for
If there is only one wouldn't it be ddsX_freq and correspond to direct
control rather than dependant on the tunning word? The upshot of this
is that there are always 0 or more than 1 ddsX_freqY file.
Given we don't have any direct control cases yet that can be documented
when/if the turn up. Hence change line above to..
"There will be more than one ddsX_freqY file, which allows for"
> + pin controlled FSK Frequency Shift Keying
> + (ddsX_pincontrol_freq_en is active) or the user can control
> + the desired active tuning word by writing Y to the
> + ddsX_freqsymbol file.
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_freqY_scale
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Scale to be applied to ddsX_freqY in order to obtain the
> + desired value in Hz. If shared across all frequency registers
> + Y is not present. It is also possible X is not present if
> + shared across all channels.
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_freqsymbol
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Specifies the active output frequency tuning word. The value
> + corresponds to the Y in ddsX_freqY. To exit this mode the user
> + can write ddsX_pincontrol_freq_en or ddsX_out_disable file.
Is it theoretically possible to read this value when pin control is on?
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_phaseY
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Stores phase into phase register Y.
Again, the mention of 'register' is a bit leading. It's always a register
so far but at least in theory it could be say several registers...
> + There can be more than one ddsX_phaseY file, which allows for
Again, 'There will be'
> + pin controlled PSK Phase Shift Keying
> + (ddsX_pincontrol_phase_en is active) or the user can
> + control the desired phase Y which is added to the phase
> + accumulator output by writing Y to the en_phase file.
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_phaseY_scale
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Scale to be applied to ddsX_phaseY in order to obtain the
> + desired value in rad. If shared across all phase registers
> + Y is not present. It is also possible X is not present if
> + shared across all channels.
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_phasesymbol
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Specifies the active phase Y which is added to the phase
> + accumulator output. The value corresponds to the Y in
> + ddsX_phaseY. To exit this mode the user can write
> + ddsX_pincontrol_phase_en or disable file.
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_en
> +What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_freq_en
> +What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_phase_en
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + ddsX_pincontrol_en: Both, the active frequency and phase is
> + controlled by the respective phase and frequency control inputs.
> + In case the device in question allows to independent controls,
> + then there are dedicated files (ddsX_pincontrol_freq_en,
> + ddsX_pincontrol_phase_en).
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_out_enable
> +What: /sys/bus/iio/devices/device[n]/ddsX_outY_enable
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + ddsX_out_enable: Enables signal generation on all outputs
> + of channel X.
> + ddsX_outY_enable: Enables signal generation on output Y,
> + of channel X.
This description takes a rather different form from similar ones above.
Perhaps it should read...
ddsX_outY_enable controls signal generation on output Y of channel X. Y may
be suppressed if all channels are controlled together.
(we can add supression of X to the description if it ever occurs!)
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Specifies the output waveform.
> + (sine, triangle, ramp, square, ...)
> + For a list of available output waveform options read
> + available_output_modes.
> +
> +What: /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype_available
> +KernelVersion: 2.6.37
> +Contact: [email protected]
> +Description:
> + Lists all available output waveform options.
> --
> 1.6.0.2
>
>
On 12/13/10 16:27, [email protected] wrote:
> From: Michael Hennerich <[email protected]>
>
> Changes since RFC/v1:
> IIO: Apply list review feedback
>
> Apply list review feedback:
> Rename attributes to fit IIO convention used in other drivers.
> Provide ddsX_out_enable as opposed to ddsX_out_disable.
> Fix typos.
> Hi Michael,
I'm afraid all the comments on docs carry over to this file. If you
aren't particularly attached to having docs in here I'd be tempted to
drop them purely so we don't end up with two differing sets in the
future. The docs file is more or less obligatory whereas nice docs
in here is just a bonus for driver devs. I really don't mind, but
we'll have to keep a close eye to ensure they don't get significantly
out of sync!
The other comments are almost all about restricting parameters to be
write only. I think this is a bit restrictive and some drivers may
allow much of this stuff to be read back (even if only from cache).
Jonathan
> Signed-off-by: Michael Hennerich <[email protected]>
> ---
> drivers/staging/iio/dds/dds.h | 152 +++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 152 insertions(+), 0 deletions(-)
> create mode 100644 drivers/staging/iio/dds/dds.h
>
> diff --git a/drivers/staging/iio/dds/dds.h b/drivers/staging/iio/dds/dds.h
> new file mode 100644
> index 0000000..a8dd7be
> --- /dev/null
> +++ b/drivers/staging/iio/dds/dds.h
> @@ -0,0 +1,152 @@
> +/*
> + * dds.h - sysfs attributes associated with DDS devices
> + *
> + * Copyright (c) 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_freqY
> + * Description:
> + * Stores frequency into tuning word register Y.
> + * There can be more than one ddsX_freqY file, which allows for pin controlled
> + * FSK Frequency Shift Keying (ddsX_pincontrol_freq_en is active) or the user
> + * can control the desired active tuning word by writing Y to the
> + * ddsX_freqsymbol file.
> + */
> +
> +#define IIO_DEV_ATTR_FREQ(_device, _num, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_freq##_num, S_IWUSR, NULL, _store, _addr)
Can envision some parts allowing this to be read back. So I'd be inclined
to make mode a parameter of the macro and allow a read function. Also is _device
a little misleading? Perhaps use _channel?
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_freqY_scale
> + * Description:
> + * Scale to be applied to ddsX_freqY in order to obtain the
> + * desired value in Hz. If shared across all frequency registers
> + * Y is not present.
> + */
> +
> +#define IIO_CONST_ATTR_FREQ_SCALE(_device, _string) \
> + IIO_CONST_ATTR(dds##_device##_freq_scale, _string)
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_freqsymbol
> + * Description:
> + * Specifies the active output frequency tuning word. The value corresponds
> + * to the Y in ddsX_freqY. To exit this mode the user can write
> + * ddsX_pincontrol_freq_en or ddsX_out_disable file.
> + */
> +
> +#define IIO_DEV_ATTR_FREQSYMBOL(_device, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_freqsymbol, \
> + S_IWUSR, NULL, _store, _addr);
Again I can envision parts for which this may be read back so this is probably
to restrictive.
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_phaseY
> + * Description:
> + * Stores phase into phase register Y.
> + * There can be more than one ddsX_phaseY file, which allows for pin controlled
> + * PSK Phase Shift Keying (ddsX_pincontrol_phase_en is active) or the user can
> + * control the desired phase Y which is added to the phase accumulator output
> + * by writing Y to the en_phase file.
> + */
> +
> +#define IIO_DEV_ATTR_PHASE(_device, _num, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_phase##_num, \
> + S_IWUSR, NULL, _store, _addr)
Another one that I think should allow reading (if the driver wants to provide
it anyway!)
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_phaseY_scale
> + * Description:
> + * Scale to be applied to ddsX_phaseY in order to obtain the
> + * desired value in rad. If shared across all phase registers
> + * Y is not present.
> + */
> +
> +#define IIO_CONST_ATTR_PHASE_SCALE(_device, _string) \
> + IIO_CONST_ATTR(dds##_device##_phase_scale, _string)
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_phasesymbol
> + * Description:
> + * Specifies the active phase Y which is added to the phase accumulator output.
> + * The value corresponds to the Y in ddsX_phaseY. To exit this mode the user
> + * can write ddsX_pincontrol_phase_en or disable file.
> + */
> +
> +#define IIO_DEV_ATTR_PHASESYMBOL(_device, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_phasesymbol, \
> + S_IWUSR, NULL, _store, _addr);
Another that I'd imagine some drivers may allow to be read back.
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_en
> + * Description:
> + * Both, the active frequency and phase is controlled by the respective
> + * phase and frequency control inputs.
> + */
> +
> +#define IIO_DEV_ATTR_PINCONTROL_EN(_device, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_pincontrol_en, \
> + S_IWUSR, NULL, _store, _addr);
ditto.
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_freq_en
> + * Description:
> + * The active frequency is controlled by the respective
> + * frequency control/select inputs.
> + */
> +
> +#define IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_device, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_pincontrol_freq_en, \
> + S_IWUSR, NULL, _store, _addr);
ditto.
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_phase_en
> + * Description:
> + * The active phase is controlled by the respective
> + * phase control/select inputs.
> + */
> +
> +#define IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_device, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_pincontrol_phase_en, \
> + S_IWUSR, NULL, _store, _addr);
ditto
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_out_enable
> + * Description:
> + * Disables any signal generation on all outputs.
> + */
> +
> +#define IIO_DEV_ATTR_OUT_ENABLE(_device, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_out_enable, \
> + S_IWUSR, NULL, _store, _addr);
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_outY_enable
> + * Description:
> + * Disables any signal generation on output Y.
Comment is lagging changes in the macro.
> + */
> +
> +#define IIO_DEV_ATTR_OUTY_ENABLE(_device, _output, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_out##_output##_enable, \
> + S_IWUSR, NULL, _store, _addr);
Why write only?
> +
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype
> + * Description:
> + * Specifies the output waveform. (sine, triangle, ramp, square, ...)
> + * For a list of available output waveform options read available_output_modes.
> + */
> +#define IIO_DEV_ATTR_OUT_WAVETYPE(_device, _output, _store, _addr) \
> + IIO_DEVICE_ATTR(dds##_device##_out##_output##_wavetype, \
> + S_IWUSR, NULL, _store, _addr);
> +/**
> + * /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype_available
> + * Description:
> + * Lists all available output waveform options.
> + */
> +#define IIO_CONST_ATTR_OUT_WAVETYPES_AVAILABLE(_device, _output, _modes)\
> + IIO_CONST_ATTR(dds##_device##_out##_output##_wavetype_available,\
> + _modes);
> --
> 1.6.0.2
>
>
On 12/13/10 16:27, [email protected] wrote:
> From: Michael Hennerich <[email protected]>
>
> Changes since RFC/v1:
> IIO: Apply list review feedback
>
> Apply list review feedback:
> Rename attributes to fit IIO convention used in other drivers.
> Fix typos.
> Provide ddsX_out_enable as opposed to ddsX_out_disable.
> Use proper __devexit marking.
> Use strict_strtoul() to avoid negatives.
>
Couple of nitpicks inline. Subject to changes due to changes in macros
suggested in previous patch (or you convincing me otherwise):
> Signed-off-by: Michael Hennerich <[email protected]>
> Reviewed-by: Datta Shubhrajyoti <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
> ---
> drivers/staging/iio/dds/Kconfig | 7 +
> drivers/staging/iio/dds/Makefile | 1 +
> drivers/staging/iio/dds/ad9834.c | 482 ++++++++++++++++++++++++++++++++++++++
> drivers/staging/iio/dds/ad9834.h | 112 +++++++++
> 4 files changed, 602 insertions(+), 0 deletions(-)
> create mode 100644 drivers/staging/iio/dds/ad9834.c
> create mode 100644 drivers/staging/iio/dds/ad9834.h
>
> diff --git a/drivers/staging/iio/dds/Kconfig b/drivers/staging/iio/dds/Kconfig
> index 7969be2..4c9cce3 100644
> --- a/drivers/staging/iio/dds/Kconfig
> +++ b/drivers/staging/iio/dds/Kconfig
> @@ -17,6 +17,13 @@ config AD9832
> Say yes here to build support for Analog Devices DDS chip
> ad9832 and ad9835, provides direct access via sysfs.
>
> +config AD9834
> + tristate "Analog Devices ad9833/4/ driver"
> + depends on SPI
> + help
> + Say yes here to build support for Analog Devices DDS chip
> + AD9833 and AD9834, provides direct access via sysfs.
> +
> config AD9850
> tristate "Analog Devices ad9850/1 driver"
> depends on SPI
> diff --git a/drivers/staging/iio/dds/Makefile b/drivers/staging/iio/dds/Makefile
> index 6f274ac..1477461 100644
> --- a/drivers/staging/iio/dds/Makefile
> +++ b/drivers/staging/iio/dds/Makefile
> @@ -4,6 +4,7 @@
>
> obj-$(CONFIG_AD5930) += ad5930.o
> obj-$(CONFIG_AD9832) += ad9832.o
> +obj-$(CONFIG_AD9834) += ad9834.o
> obj-$(CONFIG_AD9850) += ad9850.o
> obj-$(CONFIG_AD9852) += ad9852.o
> obj-$(CONFIG_AD9910) += ad9910.o
> diff --git a/drivers/staging/iio/dds/ad9834.c b/drivers/staging/iio/dds/ad9834.c
> new file mode 100644
> index 0000000..df3d68c
> --- /dev/null
> +++ b/drivers/staging/iio/dds/ad9834.c
> @@ -0,0 +1,482 @@
> +/*
> + * AD9834 SPI DAC driver
> + *
> + * Copyright 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/workqueue.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/list.h>
> +#include <linux/spi/spi.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/err.h>
> +#include <asm/div64.h>
> +
> +#include "../iio.h"
> +#include "../sysfs.h"
> +#include "dds.h"
> +
> +#include "ad9834.h"
> +
> +static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
> +{
> + unsigned long long freqreg = (u64) fout * (u64) (1 << AD9834_FREQ_BITS);
> + do_div(freqreg, mclk);
> + return freqreg;
> +}
> +
> +static int ad9834_write_frequency(struct ad9834_state *st,
> + unsigned long addr, unsigned long fout)
> +{
> + unsigned long regval;
> +
> + if (fout > (st->mclk / 2))
> + return -EINVAL;
> +
> + regval = ad9834_calc_freqreg(st->mclk, fout);
> +
> + st->freq_data[0] = cpu_to_be16(addr | (regval &
> + RES_MASK(AD9834_FREQ_BITS / 2)));
> + st->freq_data[1] = cpu_to_be16(addr | ((regval >>
> + (AD9834_FREQ_BITS / 2)) &
> + RES_MASK(AD9834_FREQ_BITS / 2)));
> +
> + return spi_sync(st->spi, &st->freq_msg);;
> +}
> +
> +static int ad9834_write_phase(struct ad9834_state *st,
> + unsigned long addr, unsigned long phase)
> +{
> + if (phase > (1 << AD9834_PHASE_BITS))
> + return -EINVAL;
> + st->data = cpu_to_be16(addr | phase);
> +
> + return spi_sync(st->spi, &st->msg);
> +}
> +
> +static ssize_t ad9834_write(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + struct iio_dev *dev_info = dev_get_drvdata(dev);
> + struct ad9834_state *st = dev_info->dev_data;
> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> + int ret;
> + long val;
> +
> + ret = strict_strtoul(buf, 10, &val);
> + if (ret)
> + goto error_ret;
> +
> + mutex_lock(&dev_info->mlock);
> + switch (this_attr->address) {
> + case AD9834_REG_FREQ0:
> + case AD9834_REG_FREQ1:
> + ret = ad9834_write_frequency(st, this_attr->address, val);
> + break;
> + case AD9834_REG_PHASE0:
> + case AD9834_REG_PHASE1:
> + ret = ad9834_write_phase(st, this_attr->address, val);
> + break;
> + case AD9834_OPBITEN:
> + if (st->control & AD9834_MODE) {
> + ret = -EINVAL; /* AD9843 reserved mode */
> + break;
> + }
> +
> + if (val)
> + st->control |= AD9834_OPBITEN;
> + else
> + st->control &= ~AD9834_OPBITEN;
> +
> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> + ret = spi_sync(st->spi, &st->msg);
> + break;
> + case AD9834_PIN_SW:
> + if (val)
> + st->control |= AD9834_PIN_SW;
> + else
> + st->control &= ~AD9834_PIN_SW;
> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> + ret = spi_sync(st->spi, &st->msg);
> + break;
> + case AD9834_FSEL:
> + case AD9834_PSEL:
> + if (val == 0)
> + st->control &= ~(this_attr->address | AD9834_PIN_SW);
> + else if (val == 1) {
> + st->control |= this_attr->address;
> + st->control &= ~AD9834_PIN_SW;
> + } else {
> + ret = -EINVAL;
> + break;
> + }
> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> + ret = spi_sync(st->spi, &st->msg);
> + break;
> + case AD9834_RESET:
> + if (val)
> + st->control &= ~AD9834_RESET;
> + else
> + st->control |= AD9834_RESET;
> +
> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> + ret = spi_sync(st->spi, &st->msg);
> + break;
> + default:
> + ret = -ENODEV;
> + }
> + mutex_unlock(&dev_info->mlock);
> +
> +error_ret:
> + return ret ? ret : len;
> +}
> +
> +static ssize_t ad9834_store_wavetype(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t len)
> +{
> + struct iio_dev *dev_info = dev_get_drvdata(dev);
> + struct ad9834_state *st = dev_info->dev_data;
> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> + int ret = 0;
> + bool is_ad9833 = st->devid == ID_AD9833;
> +
> + mutex_lock(&dev_info->mlock);
> +
> + switch (this_attr->address) {
> + case 0:
> + if (sysfs_streq(buf, "sine")) {
> + st->control &= ~AD9834_MODE;
> + if (is_ad9833)
> + st->control &= ~AD9834_OPBITEN;
> + } else if (sysfs_streq(buf, "triangle")) {
> + if (is_ad9833) {
> + st->control &= ~AD9834_OPBITEN;
> + st->control |= AD9834_MODE;
> + } else if (st->control & AD9834_OPBITEN) {
> + ret = -EINVAL; /* AD9843 reserved mode */
> + } else {
> + st->control |= AD9834_MODE;
> + }
> + } else if (is_ad9833 && sysfs_streq(buf, "square")) {
> + st->control &= ~AD9834_MODE;
> + st->control |= AD9834_OPBITEN;
> + } else {
> + ret = -EINVAL;
> + }
> +
> + break;
> + case 1:
> + if (sysfs_streq(buf, "square") &&
> + !(st->control & AD9834_MODE)) {
> + st->control &= ~AD9834_MODE;
> + st->control |= AD9834_OPBITEN;
> + } else {
> + ret = -EINVAL;
> + }
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (!ret) {
> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> + ret = spi_sync(st->spi, &st->msg);
> + }
> + mutex_unlock(&dev_info->mlock);
In correct tabbing on previous line? (or my email client has messed up)
> +
> + return ret ? ret : len;
> +}
> +
> +static ssize_t ad9834_show_name(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *dev_info = dev_get_drvdata(dev);
> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> +
> + return sprintf(buf, "%s\n", spi_get_device_id(st->spi)->name);
> +}
> +static IIO_DEVICE_ATTR(name, S_IRUGO, ad9834_show_name, NULL, 0);
> +
> +static ssize_t ad9834_show_out0_wavetype_available(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *dev_info = dev_get_drvdata(dev);
> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> + char *str;
> +
> + if (st->devid == ID_AD9833)
> + str = "sine triangle square";
> + else if (st->control & AD9834_OPBITEN)
> + str = "sine";
> + else
> + str = "sine triangle";
> +
> + return sprintf(buf, "%s\n", str);
> +}
> +
> +
> +static IIO_DEVICE_ATTR(dds0_out0_wavetype_available, S_IRUGO,
> + ad9834_show_out0_wavetype_available, NULL, 0);
> +
> +static ssize_t ad9834_show_out1_wavetype_available(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *dev_info = dev_get_drvdata(dev);
> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> + char *str;
> +
> + if (st->control & AD9834_MODE)
> + str = "";
I 'think' that the is_visible below makes this first condition impossible?
I've never used the is_visible stuff so not entirely sure I've understood
it correctly...
That probably means that you can use a const attr for this...
> + else
> + str = "square";
> +
> + return sprintf(buf, "%s\n", str);
> +}
> +
> +static IIO_DEVICE_ATTR(dds0_out1_wavetype_available, S_IRUGO,
> + ad9834_show_out1_wavetype_available, NULL, 0);
> +
> +/**
> + * see dds.h for further information
> + */
> +
> +static IIO_DEV_ATTR_FREQ(0, 0, ad9834_write, AD9834_REG_FREQ0);
> +static IIO_DEV_ATTR_FREQ(0, 1, ad9834_write, AD9834_REG_FREQ1);
> +static IIO_DEV_ATTR_FREQSYMBOL(0, ad9834_write, AD9834_FSEL);
> +static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
> +
> +static IIO_DEV_ATTR_PHASE(0, 0, ad9834_write, AD9834_REG_PHASE0);
> +static IIO_DEV_ATTR_PHASE(0, 1, ad9834_write, AD9834_REG_PHASE1);
> +static IIO_DEV_ATTR_PHASESYMBOL(0, ad9834_write, AD9834_PSEL);
> +static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12 rad*/
> +
> +static IIO_DEV_ATTR_PINCONTROL_EN(0, ad9834_write, AD9834_PIN_SW);
> +static IIO_DEV_ATTR_OUT_ENABLE(0, ad9834_write, AD9834_RESET);
> +static IIO_DEV_ATTR_OUTY_ENABLE(0, 1, ad9834_write, AD9834_OPBITEN);
> +static IIO_DEV_ATTR_OUT_WAVETYPE(0, 0, ad9834_store_wavetype, 0);
> +static IIO_DEV_ATTR_OUT_WAVETYPE(0, 1, ad9834_store_wavetype, 1);
> +
> +static struct attribute *ad9834_attributes[] = {
> + &iio_dev_attr_dds0_freq0.dev_attr.attr,
> + &iio_dev_attr_dds0_freq1.dev_attr.attr,
> + &iio_const_attr_dds0_freq_scale.dev_attr.attr,
> + &iio_dev_attr_dds0_phase0.dev_attr.attr,
> + &iio_dev_attr_dds0_phase1.dev_attr.attr,
> + &iio_const_attr_dds0_phase_scale.dev_attr.attr,
> + &iio_dev_attr_dds0_pincontrol_en.dev_attr.attr,
> + &iio_dev_attr_dds0_freqsymbol.dev_attr.attr,
> + &iio_dev_attr_dds0_phasesymbol.dev_attr.attr,
> + &iio_dev_attr_dds0_out_enable.dev_attr.attr,
> + &iio_dev_attr_dds0_out1_enable.dev_attr.attr,
> + &iio_dev_attr_dds0_out0_wavetype.dev_attr.attr,
> + &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr,
> + &iio_dev_attr_dds0_out0_wavetype_available.dev_attr.attr,
> + &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr,
> + &iio_dev_attr_name.dev_attr.attr,
> + NULL,
> +};
> +
> +static mode_t ad9834_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct device *dev = container_of(kobj, struct device, kobj);
> + struct iio_dev *dev_info = dev_get_drvdata(dev);
> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> +
> + mode_t mode = attr->mode;
> +
> + if (st->devid == ID_AD9834)
> + return mode;
> +
> + if ((attr == &iio_dev_attr_dds0_out1_enable.dev_attr.attr) ||
> + (attr == &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr) ||
> + (attr ==
> + &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr))
> + mode = 0;
> +
> + return mode;
> +}
> +
> +static const struct attribute_group ad9834_attribute_group = {
> + .attrs = ad9834_attributes,
> + .is_visible = ad9834_attr_is_visible,
> +};
> +
> +static int __devinit ad9834_probe(struct spi_device *spi)
> +{
> + struct ad9834_platform_data *pdata = spi->dev.platform_data;
> + struct ad9834_state *st;
> + int ret;
> +
> + if (!pdata) {
> + dev_dbg(&spi->dev, "no platform data?\n");
> + return -ENODEV;
> + }
> +
> + st = kzalloc(sizeof(*st), GFP_KERNEL);
> + if (st == NULL) {
> + ret = -ENOMEM;
> + goto error_ret;
> + }
> +
> + st->reg = regulator_get(&spi->dev, "vcc");
> + if (!IS_ERR(st->reg)) {
> + ret = regulator_enable(st->reg);
> + if (ret)
> + goto error_put_reg;
> + }
> +
> + st->mclk = pdata->mclk;
> +
> + spi_set_drvdata(spi, st);
> +
> + st->spi = spi;
> + st->devid = spi_get_device_id(spi)->driver_data;
> +
> + st->indio_dev = iio_allocate_device();
> + if (st->indio_dev == NULL) {
> + ret = -ENOMEM;
> + goto error_disable_reg;
> + }
> +
> + st->indio_dev->dev.parent = &spi->dev;
> + st->indio_dev->attrs = &ad9834_attribute_group;
> + st->indio_dev->dev_data = (void *)(st);
nitpick: technically superflous brackets...
> + st->indio_dev->driver_module = THIS_MODULE;
> + st->indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + /* Setup default messages */
> +
> + st->xfer.tx_buf = &st->data;
> + st->xfer.len = 2;
> +
> + spi_message_init(&st->msg);
> + spi_message_add_tail(&st->xfer, &st->msg);
> +
> + st->freq_xfer[0].tx_buf = &st->freq_data[0];
> + st->freq_xfer[0].len = 2;
> + st->freq_xfer[0].cs_change = 1;
> + st->freq_xfer[1].tx_buf = &st->freq_data[1];
> + st->freq_xfer[1].len = 2;
> +
> + spi_message_init(&st->freq_msg);
> + spi_message_add_tail(&st->freq_xfer[0], &st->freq_msg);
> + spi_message_add_tail(&st->freq_xfer[1], &st->freq_msg);
> +
> + st->control = AD9834_B28 | AD9834_RESET;
> +
> + if (!pdata->en_div2)
> + st->control |= AD9834_DIV2;
> +
> + if (!pdata->en_signbit_msb_out && (st->devid == ID_AD9834))
> + st->control |= AD9834_SIGN_PIB;
> +
> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> + ret = spi_sync(st->spi, &st->msg);
> + if (ret) {
> + dev_err(&spi->dev, "device init failed\n");
> + goto error_free_device;
> + }
> +
> + ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, pdata->freq0);
> + if (ret)
> + goto error_free_device;
> +
> + ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, pdata->freq1);
> + if (ret)
> + goto error_free_device;
> +
> + ret = ad9834_write_phase(st, AD9834_REG_PHASE0, pdata->phase0);
> + if (ret)
> + goto error_free_device;
> +
> + ret = ad9834_write_phase(st, AD9834_REG_PHASE1, pdata->phase1);
> + if (ret)
> + goto error_free_device;
> +
> + st->control &= ~AD9834_RESET;
> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> + ret = spi_sync(st->spi, &st->msg);
> + if (ret)
> + goto error_free_device;
> +
> + ret = iio_device_register(st->indio_dev);
> + if (ret)
> + goto error_free_device;
> +
> + return 0;
> +
> +error_free_device:
> + iio_free_device(st->indio_dev);
> +error_disable_reg:
> + if (!IS_ERR(st->reg))
> + regulator_disable(st->reg);
> +error_put_reg:
> + if (!IS_ERR(st->reg))
> + regulator_put(st->reg);
> + kfree(st);
> +error_ret:
> + return ret;
> +}
> +
> +static int __devexit ad9834_remove(struct spi_device *spi)
> +{
> + struct ad9834_state *st = spi_get_drvdata(spi);
> + struct iio_dev *indio_dev = st->indio_dev;
> +
nitpick: could just use st->indio_dev in the next call and lose the
line above. It's the only use in this function.
> + iio_device_unregister(indio_dev);
> + if (!IS_ERR(st->reg)) {
> + regulator_disable(st->reg);
> + regulator_put(st->reg);
> + }
> + kfree(st);
> + return 0;
> +}
> +
> +static const struct spi_device_id ad9834_id[] = {
> + {"ad9833", ID_AD9833},
> + {"ad9834", ID_AD9834},
> + {}
> +};
> +
> +static struct spi_driver ad9834_driver = {
> + .driver = {
> + .name = "ad9834",
> + .bus = &spi_bus_type,
> + .owner = THIS_MODULE,
> + },
> + .probe = ad9834_probe,
> + .remove = __devexit_p(ad9834_remove),
> + .id_table = ad9834_id,
> +};
> +
> +static int __init ad9834_init(void)
> +{
> + return spi_register_driver(&ad9834_driver);
> +}
> +module_init(ad9834_init);
> +
> +static void __exit ad9834_exit(void)
> +{
> + spi_unregister_driver(&ad9834_driver);
> +}
> +module_exit(ad9834_exit);
> +
> +MODULE_AUTHOR("Michael Hennerich <[email protected]>");
> +MODULE_DESCRIPTION("Analog Devices AD9833/AD9834 DDS");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("spi:ad9834");
> diff --git a/drivers/staging/iio/dds/ad9834.h b/drivers/staging/iio/dds/ad9834.h
> new file mode 100644
> index 0000000..0fc3b88
> --- /dev/null
> +++ b/drivers/staging/iio/dds/ad9834.h
> @@ -0,0 +1,112 @@
> +/*
> + * AD9834 SPI DDS driver
> + *
> + * Copyright 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +#ifndef IIO_DDS_AD9834_H_
> +#define IIO_DDS_AD9834_H_
> +
> +/* Registers */
> +
> +#define AD9834_REG_CMD (0 << 14)
> +#define AD9834_REG_FREQ0 (1 << 14)
> +#define AD9834_REG_FREQ1 (2 << 14)
> +#define AD9834_REG_PHASE0 (6 << 13)
> +#define AD9834_REG_PHASE1 (7 << 13)
> +
> +/* Command Control Bits */
> +
> +#define AD9834_B28 (1 << 13)
> +#define AD9834_HLB (1 << 12)
> +#define AD9834_FSEL (1 << 11)
> +#define AD9834_PSEL (1 << 10)
> +#define AD9834_PIN_SW (1 << 9)
> +#define AD9834_RESET (1 << 8)
> +#define AD9834_SLEEP1 (1 << 7)
> +#define AD9834_SLEEP12 (1 << 6)
> +#define AD9834_OPBITEN (1 << 5)
> +#define AD9834_SIGN_PIB (1 << 4)
> +#define AD9834_DIV2 (1 << 3)
> +#define AD9834_MODE (1 << 1)
> +
> +#define AD9834_FREQ_BITS 28
> +#define AD9834_PHASE_BITS 12
> +
> +#define RES_MASK(bits) ((1 << (bits)) - 1)
> +
> +/**
> + * struct ad9834_state - driver instance specific data
> + * @indio_dev: the industrial I/O device
> + * @spi: spi_device
> + * @reg: supply regulator
> + * @mclk: external master clock
> + * @control: cached control word
> + * @xfer: default spi transfer
> + * @msg: default spi message
> + * @freq_xfer: tuning word spi transfer
> + * @freq_msg: tuning word spi message
> + * @data: spi transmit buffer
> + * @freq_data: tuning word spi transmit buffer
> + */
> +
> +struct ad9834_state {
> + struct iio_dev *indio_dev;
> + struct spi_device *spi;
> + struct regulator *reg;
> + unsigned int mclk;
> + unsigned short control;
> + unsigned short devid;
> + struct spi_transfer xfer;
> + struct spi_message msg;
> + struct spi_transfer freq_xfer[2];
> + struct spi_message freq_msg;
> +
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + unsigned short data ____cacheline_aligned;
> + unsigned short freq_data[2] ;
> +};
> +
> +
> +/*
> + * TODO: struct ad7887_platform_data needs to go into include/linux/iio
> + */
> +
> +/**
> + * struct ad9834_platform_data - platform specific information
> + * @mclk: master clock in Hz
> + * @freq0: power up freq0 tuning word in Hz
> + * @freq1: power up freq1 tuning word in Hz
> + * @phase0: power up phase0 value [0..4095] correlates with 0..2PI
> + * @phase1: power up phase1 value [0..4095] correlates with 0..2PI
> + * @en_div2: digital output/2 is passed to the SIGN BIT OUT pin
> + * @en_signbit_msb_out: the MSB (or MSB/2) of the DAC data is connected to the
> + * SIGN BIT OUT pin. en_div2 controls whether it is the MSB
> + * or MSB/2 that is output. if en_signbit_msb_out=false,
> + * the on-board comparator is connected to SIGN BIT OUT
> + */
> +
> +struct ad9834_platform_data {
> + unsigned int mclk;
> + unsigned int freq0;
> + unsigned int freq1;
> + unsigned short phase0;
> + unsigned short phase1;
> + bool en_div2;
> + bool en_signbit_msb_out;
> +};
> +
> +/**
> + * ad9834_supported_device_ids:
> + */
> +
> +enum ad9834_supported_device_ids {
> + ID_AD9833,
> + ID_AD9834,
> +};
> +
> +#endif /* IIO_DDS_AD9834_H_ */
> --
> 1.6.0.2
>
>
> Jonathan Cameron wrote on 2010-12-13:
> On 12/13/10 16:27, [email protected] wrote:
> > From: Michael Hennerich <[email protected]>
> >
> > Changes since RFC/v1:
> > IIO: Apply list review feedback:
> >
> > Apply list review feedback:
> > Restructure documentation according to list feedback.
> > Rename attributes to fit IIO convention used in other drivers.
> > Fix typos.
> > Provide ddsX_out_enable as opposed to ddsX_out_disable
> Hi Michael,
>
> This is more or less there. A few minor queries / suggestions
> inline. The only major one is the mixture of X and [n] notation
> for the various indicies. I 'think' all our docs have now moved
> to the X notation. (if not then I have messed up!)
>
> I am happy with all the actual attributes. All remaining issues
> are to do with the descriptions of what they do!
>
> Thanks,
>
> Jonathan
> >
> > Signed-off-by: Michael Hennerich <[email protected]>
> > Reviewed-by: Jonathan Cameron <[email protected]>
> > ---
> > .../staging/iio/Documentation/sysfs-bus-iio-dds | 94
> ++++++++++++++++++++
> > 1 files changed, 94 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-
> dds
> >
> > diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
> b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
> > new file mode 100644
> > index 0000000..6791174
> > --- /dev/null
> > +++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
> > @@ -0,0 +1,94 @@
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_freqY
> We have a hybrid here of the old indexing schemed (device[n]) and the
> new. Either just shorten then form to
> /sys/bus/iio/.../ddsX_freqY or use
> /sys/bus/iio/devices/deviceX/ddsY_freqZ and update below appropriately.
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Stores frequency into tuning word register Y.
> Technically it might not be a register? Dropping 'register' doesn't
> to my mind change the meaning but makes it marginally more general.
ok
> > + There can be more than one ddsX_freqY file, which allows
> for
> If there is only one wouldn't it be ddsX_freq and correspond to direct
> control rather than dependant on the tunning word?
I don't understand. In case there is only one ddsX_freqY file.
Then there is certainly no ddsX_freqsymbol or ddsX_pincontrol_freq_en file.
> The upshot of this
> is that there are always 0 or more than 1 ddsX_freqY file.
> Given we don't have any direct control cases yet that can be documented
> when/if the turn up. Hence change line above to..
> "There will be more than one ddsX_freqY file, which allows for"
We can document the missing cases once we add parts that requires them.
> > + pin controlled FSK Frequency Shift Keying
> > + (ddsX_pincontrol_freq_en is active) or the user can control
> > + the desired active tuning word by writing Y to the
> > + ddsX_freqsymbol file.
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_freqY_scale
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Scale to be applied to ddsX_freqY in order to obtain the
> > + desired value in Hz. If shared across all frequency
> registers
> > + Y is not present. It is also possible X is not present if
> > + shared across all channels.
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_freqsymbol
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Specifies the active output frequency tuning word. The
> value
> > + corresponds to the Y in ddsX_freqY. To exit this mode the
> user
> > + can write ddsX_pincontrol_freq_en or ddsX_out_disable file.
> Is it theoretically possible to read this value when pin control is on?
Not with the AD9832/35 AD9833/34, some other DDS parts allow register readouts.
Depending on the implementation of the device in question. It might be possible
To read back the current hardware state.
I'll add the read method.
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_phaseY
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Stores phase into phase register Y.
> Again, the mention of 'register' is a bit leading. It's always a
> register
> so far but at least in theory it could be say several registers...
I'll change.
> > + There can be more than one ddsX_phaseY file, which allows
> for
> Again, 'There will be'
ok
> > + pin controlled PSK Phase Shift Keying
> > + (ddsX_pincontrol_phase_en is active) or the user can
> > + control the desired phase Y which is added to the phase
> > + accumulator output by writing Y to the en_phase file.
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_phaseY_scale
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Scale to be applied to ddsX_phaseY in order to obtain the
> > + desired value in rad. If shared across all phase registers
> > + Y is not present. It is also possible X is not present if
> > + shared across all channels.
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_phasesymbol
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Specifies the active phase Y which is added to the phase
> > + accumulator output. The value corresponds to the Y in
> > + ddsX_phaseY. To exit this mode the user can write
> > + ddsX_pincontrol_phase_en or disable file.
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_en
> > +What:
> /sys/bus/iio/devices/device[n]/ddsX_pincontrol_freq_en
> > +What:
> /sys/bus/iio/devices/device[n]/ddsX_pincontrol_phase_en
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + ddsX_pincontrol_en: Both, the active frequency and phase is
> > + controlled by the respective phase and frequency control
> inputs.
> > + In case the device in question allows to independent
> controls,
> > + then there are dedicated files (ddsX_pincontrol_freq_en,
> > + ddsX_pincontrol_phase_en).
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_out_enable
> > +What: /sys/bus/iio/devices/device[n]/ddsX_outY_enable
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + ddsX_out_enable: Enables signal generation on all outputs
> > + of channel X.
> > + ddsX_outY_enable: Enables signal generation on output Y,
> > + of channel X.
> This description takes a rather different form from similar ones above.
> Perhaps it should read...
>
> ddsX_outY_enable controls signal generation on output Y of channel X. Y
> may
> be suppressed if all channels are controlled together.
>
> (we can add supression of X to the description if it ever occurs!)
ok
> > +
> > +What: /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Specifies the output waveform.
> > + (sine, triangle, ramp, square, ...)
> > + For a list of available output waveform options read
> > + available_output_modes.
> > +
> > +What:
> /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype_available
> > +KernelVersion: 2.6.37
> > +Contact: [email protected]
> > +Description:
> > + Lists all available output waveform options.
> > --
> > 1.6.0.2
> >
> >
On 12/14/10 09:49, Hennerich, Michael wrote:
>> Jonathan Cameron wrote on 2010-12-13:
>> On 12/13/10 16:27, [email protected] wrote:
>>> From: Michael Hennerich <[email protected]>
>>>
>>> Changes since RFC/v1:
>>> IIO: Apply list review feedback:
>>>
>>> Apply list review feedback:
>>> Restructure documentation according to list feedback.
>>> Rename attributes to fit IIO convention used in other drivers.
>>> Fix typos.
>>> Provide ddsX_out_enable as opposed to ddsX_out_disable
>> Hi Michael,
>>
>> This is more or less there. A few minor queries / suggestions
>> inline. The only major one is the mixture of X and [n] notation
>> for the various indicies. I 'think' all our docs have now moved
>> to the X notation. (if not then I have messed up!)
>>
>> I am happy with all the actual attributes. All remaining issues
>> are to do with the descriptions of what they do!
>>
>> Thanks,
>>
>> Jonathan
>>>
>>> Signed-off-by: Michael Hennerich <[email protected]>
>>> Reviewed-by: Jonathan Cameron <[email protected]>
>>> ---
>>> .../staging/iio/Documentation/sysfs-bus-iio-dds | 94
>> ++++++++++++++++++++
>>> 1 files changed, 94 insertions(+), 0 deletions(-)
>>> create mode 100644 drivers/staging/iio/Documentation/sysfs-bus-iio-
>> dds
>>>
>>> diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
>> b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
>>> new file mode 100644
>>> index 0000000..6791174
>>> --- /dev/null
>>> +++ b/drivers/staging/iio/Documentation/sysfs-bus-iio-dds
>>> @@ -0,0 +1,94 @@
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_freqY
>> We have a hybrid here of the old indexing schemed (device[n]) and the
>> new. Either just shorten then form to
>> /sys/bus/iio/.../ddsX_freqY or use
>> /sys/bus/iio/devices/deviceX/ddsY_freqZ and update below appropriately.
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Stores frequency into tuning word register Y.
>> Technically it might not be a register? Dropping 'register' doesn't
>> to my mind change the meaning but makes it marginally more general.
>
> ok
>
>>> + There can be more than one ddsX_freqY file, which allows
>> for
>> If there is only one wouldn't it be ddsX_freq and correspond to direct
>> control rather than dependant on the tunning word?
>
> I don't understand. In case there is only one ddsX_freqY file.
> Then there is certainly no ddsX_freqsymbol or ddsX_pincontrol_freq_en file.
Then what is the Y index for? It's always 0 so why not suppress it and
just have ddsX_freq? Thus if there is a Y index there has to be more than 1.
>
>> The upshot of this
>> is that there are always 0 or more than 1 ddsX_freqY file.
>> Given we don't have any direct control cases yet that can be documented
>> when/if the turn up. Hence change line above to..
>> "There will be more than one ddsX_freqY file, which allows for"
>
> We can document the missing cases once we add parts that requires them.
>
>>> + pin controlled FSK Frequency Shift Keying
>>> + (ddsX_pincontrol_freq_en is active) or the user can control
>>> + the desired active tuning word by writing Y to the
>>> + ddsX_freqsymbol file.
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_freqY_scale
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Scale to be applied to ddsX_freqY in order to obtain the
>>> + desired value in Hz. If shared across all frequency
>> registers
>>> + Y is not present. It is also possible X is not present if
>>> + shared across all channels.
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_freqsymbol
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Specifies the active output frequency tuning word. The
>> value
>>> + corresponds to the Y in ddsX_freqY. To exit this mode the
>> user
>>> + can write ddsX_pincontrol_freq_en or ddsX_out_disable file.
>> Is it theoretically possible to read this value when pin control is on?
>
> Not with the AD9832/35 AD9833/34, some other DDS parts allow register readouts.
> Depending on the implementation of the device in question. It might be possible
> To read back the current hardware state.
>
> I'll add the read method.
>
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_phaseY
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Stores phase into phase register Y.
>> Again, the mention of 'register' is a bit leading. It's always a
>> register
>> so far but at least in theory it could be say several registers...
>
> I'll change.
>
>>> + There can be more than one ddsX_phaseY file, which allows
>> for
>> Again, 'There will be'
>
> ok
>
>>> + pin controlled PSK Phase Shift Keying
>>> + (ddsX_pincontrol_phase_en is active) or the user can
>>> + control the desired phase Y which is added to the phase
>>> + accumulator output by writing Y to the en_phase file.
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_phaseY_scale
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Scale to be applied to ddsX_phaseY in order to obtain the
>>> + desired value in rad. If shared across all phase registers
>>> + Y is not present. It is also possible X is not present if
>>> + shared across all channels.
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_phasesymbol
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Specifies the active phase Y which is added to the phase
>>> + accumulator output. The value corresponds to the Y in
>>> + ddsX_phaseY. To exit this mode the user can write
>>> + ddsX_pincontrol_phase_en or disable file.
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_pincontrol_en
>>> +What:
>> /sys/bus/iio/devices/device[n]/ddsX_pincontrol_freq_en
>>> +What:
>> /sys/bus/iio/devices/device[n]/ddsX_pincontrol_phase_en
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + ddsX_pincontrol_en: Both, the active frequency and phase is
>>> + controlled by the respective phase and frequency control
>> inputs.
>>> + In case the device in question allows to independent
>> controls,
>>> + then there are dedicated files (ddsX_pincontrol_freq_en,
>>> + ddsX_pincontrol_phase_en).
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_out_enable
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_outY_enable
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + ddsX_out_enable: Enables signal generation on all outputs
>>> + of channel X.
>>> + ddsX_outY_enable: Enables signal generation on output Y,
>>> + of channel X.
>> This description takes a rather different form from similar ones above.
>> Perhaps it should read...
>>
>> ddsX_outY_enable controls signal generation on output Y of channel X. Y
>> may
>> be suppressed if all channels are controlled together.
>>
>> (we can add supression of X to the description if it ever occurs!)
>
> ok
>
>>> +
>>> +What: /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Specifies the output waveform.
>>> + (sine, triangle, ramp, square, ...)
>>> + For a list of available output waveform options read
>>> + available_output_modes.
>>> +
>>> +What:
>> /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype_available
>>> +KernelVersion: 2.6.37
>>> +Contact: [email protected]
>>> +Description:
>>> + Lists all available output waveform options.
>>> --
>>> 1.6.0.2
>>>
>>>
>
>
> Jonathan Cameron wrote on 2010-12-13:
> > On 12/13/10 16:27, [email protected] wrote:
> > From: Michael Hennerich <[email protected]>
> >
> > Changes since RFC/v1:
> > IIO: Apply list review feedback
> >
> > Apply list review feedback:
> > Rename attributes to fit IIO convention used in other drivers.
> > Provide ddsX_out_enable as opposed to ddsX_out_disable.
> > Fix typos.
> > Hi Michael,
>
> I'm afraid all the comments on docs carry over to this file. If you
> aren't particularly attached to having docs in here I'd be tempted to
> drop them purely so we don't end up with two differing sets in the
> future. The docs file is more or less obligatory whereas nice docs
> in here is just a bonus for driver devs. I really don't mind, but
> we'll have to keep a close eye to ensure they don't get significantly
> out of sync!
I'll drop the descriptions within the comments, but keep the file names.
> The other comments are almost all about restricting parameters to be
> write only. I think this is a bit restrictive and some drivers may
> allow much of this stuff to be read back (even if only from cache).
The parts I looked at so far were write only parts, but right you are.
I'll add the read methods to the macros.
> Jonathan
> > Signed-off-by: Michael Hennerich <[email protected]>
> > ---
> > drivers/staging/iio/dds/dds.h | 152
> +++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 152 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/staging/iio/dds/dds.h
> >
> > diff --git a/drivers/staging/iio/dds/dds.h
> b/drivers/staging/iio/dds/dds.h
> > new file mode 100644
> > index 0000000..a8dd7be
> > --- /dev/null
> > +++ b/drivers/staging/iio/dds/dds.h
> > @@ -0,0 +1,152 @@
> > +/*
> > + * dds.h - sysfs attributes associated with DDS devices
> > + *
> > + * Copyright (c) 2010 Analog Devices Inc.
> > + *
> > + * Licensed under the GPL-2 or later.
> > + */
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_freqY
> > + * Description:
> > + * Stores frequency into tuning word register Y.
> > + * There can be more than one ddsX_freqY file, which allows for pin
> controlled
> > + * FSK Frequency Shift Keying (ddsX_pincontrol_freq_en is active) or
> the user
> > + * can control the desired active tuning word by writing Y to the
> > + * ddsX_freqsymbol file.
> > + */
> > +
> > +#define IIO_DEV_ATTR_FREQ(_device, _num, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_freq##_num, S_IWUSR, NULL, _store,
> _addr)
> Can envision some parts allowing this to be read back. So I'd be
> inclined
> to make mode a parameter of the macro and allow a read function. Also
> is _device
> a little misleading? Perhaps use _channel?
Right, channel fits better.
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_freqY_scale
> > + * Description:
> > + * Scale to be applied to ddsX_freqY in order to obtain the
> > + * desired value in Hz. If shared across all frequency registers
> > + * Y is not present.
> > + */
> > +
> > +#define IIO_CONST_ATTR_FREQ_SCALE(_device, _string)
> \
> > + IIO_CONST_ATTR(dds##_device##_freq_scale, _string)
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_freqsymbol
> > + * Description:
> > + * Specifies the active output frequency tuning word. The value
> corresponds
> > + * to the Y in ddsX_freqY. To exit this mode the user can write
> > + * ddsX_pincontrol_freq_en or ddsX_out_disable file.
> > + */
> > +
> > +#define IIO_DEV_ATTR_FREQSYMBOL(_device, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_freqsymbol, \
> > + S_IWUSR, NULL, _store, _addr);
> Again I can envision parts for which this may be read back so this is
> probably
> to restrictive.
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_phaseY
> > + * Description:
> > + * Stores phase into phase register Y.
> > + * There can be more than one ddsX_phaseY file, which allows for pin
> controlled
> > + * PSK Phase Shift Keying (ddsX_pincontrol_phase_en is active) or
> the user can
> > + * control the desired phase Y which is added to the phase
> accumulator output
> > + * by writing Y to the en_phase file.
> > + */
> > +
> > +#define IIO_DEV_ATTR_PHASE(_device, _num, _store, _addr) \
> > + IIO_DEVICE_ATTR(dds##_device##_phase##_num, \
> > + S_IWUSR, NULL, _store, _addr)
> Another one that I think should allow reading (if the driver wants to
> provide
> it anyway!)
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_phaseY_scale
> > + * Description:
> > + * Scale to be applied to ddsX_phaseY in order to obtain the
> > + * desired value in rad. If shared across all phase registers
> > + * Y is not present.
> > + */
> > +
> > +#define IIO_CONST_ATTR_PHASE_SCALE(_device, _string)
> \
> > + IIO_CONST_ATTR(dds##_device##_phase_scale, _string)
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_phasesymbol
> > + * Description:
> > + * Specifies the active phase Y which is added to the phase
> accumulator output.
> > + * The value corresponds to the Y in ddsX_phaseY. To exit this mode
> the user
> > + * can write ddsX_pincontrol_phase_en or disable file.
> > + */
> > +
> > +#define IIO_DEV_ATTR_PHASESYMBOL(_device, _store, _addr) \
> > + IIO_DEVICE_ATTR(dds##_device##_phasesymbol, \
> > + S_IWUSR, NULL, _store, _addr);
> Another that I'd imagine some drivers may allow to be read back.
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_en
> > + * Description:
> > + * Both, the active frequency and phase is controlled by the
> respective
> > + * phase and frequency control inputs.
> > + */
> > +
> > +#define IIO_DEV_ATTR_PINCONTROL_EN(_device, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_pincontrol_en, \
> > + S_IWUSR, NULL, _store, _addr);
> ditto.
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_freq_en
> > + * Description:
> > + * The active frequency is controlled by the respective
> > + * frequency control/select inputs.
> > + */
> > +
> > +#define IIO_DEV_ATTR_PINCONTROL_FREQ_EN(_device, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_pincontrol_freq_en, \
> > + S_IWUSR, NULL, _store, _addr);
> ditto.
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_pincontrol_phase_en
> > + * Description:
> > + * The active phase is controlled by the respective
> > + * phase control/select inputs.
> > + */
> > +
> > +#define IIO_DEV_ATTR_PINCONTROL_PHASE_EN(_device, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_pincontrol_phase_en, \
> > + S_IWUSR, NULL, _store, _addr);
> ditto
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_out_enable
> > + * Description:
> > + * Disables any signal generation on all outputs.
> > + */
> > +
> > +#define IIO_DEV_ATTR_OUT_ENABLE(_device, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_out_enable, \
> > + S_IWUSR, NULL, _store, _addr);
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_outY_enable
> > + * Description:
> > + * Disables any signal generation on output Y.
> Comment is lagging changes in the macro.
> > + */
> > +
> > +#define IIO_DEV_ATTR_OUTY_ENABLE(_device, _output, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_out##_output##_enable, \
> > + S_IWUSR, NULL, _store, _addr);
> Why write only?
> > +
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype
> > + * Description:
> > + * Specifies the output waveform. (sine, triangle, ramp, square,
> ...)
> > + * For a list of available output waveform options read
> available_output_modes.
> > + */
> > +#define IIO_DEV_ATTR_OUT_WAVETYPE(_device, _output, _store, _addr)
> \
> > + IIO_DEVICE_ATTR(dds##_device##_out##_output##_wavetype,
> \
> > + S_IWUSR, NULL, _store, _addr);
>
> > +/**
> > + * /sys/bus/iio/devices/device[n]/ddsX_outY_wavetype_available
> > + * Description:
> > + * Lists all available output waveform options.
> > + */
> > +#define IIO_CONST_ATTR_OUT_WAVETYPES_AVAILABLE(_device, _output,
> _modes)\
> > + IIO_CONST_ATTR(dds##_device##_out##_output##_wavetype_available,\
> > + _modes);
> > --
> > 1.6.0.2
> >
> >
> Jonathan Cameron wrote on 2010-12-13:
> > On 12/13/10 16:27, [email protected] wrote:
> > From: Michael Hennerich <[email protected]>
> >
> > Changes since RFC/v1:
> > IIO: Apply list review feedback
> >
> > Apply list review feedback:
> > Rename attributes to fit IIO convention used in other drivers.
> > Fix typos.
> > Provide ddsX_out_enable as opposed to ddsX_out_disable.
> > Use proper __devexit marking.
> > Use strict_strtoul() to avoid negatives.
> >
> Couple of nitpicks inline. Subject to changes due to changes in macros
> suggested in previous patch (or you convincing me otherwise):
> > Signed-off-by: Michael Hennerich <[email protected]>
> > Reviewed-by: Datta Shubhrajyoti <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>
> > ---
> > drivers/staging/iio/dds/Kconfig | 7 +
> > drivers/staging/iio/dds/Makefile | 1 +
> > drivers/staging/iio/dds/ad9834.c | 482
> ++++++++++++++++++++++++++++++++++++++
> > drivers/staging/iio/dds/ad9834.h | 112 +++++++++
> > 4 files changed, 602 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/staging/iio/dds/ad9834.c
> > create mode 100644 drivers/staging/iio/dds/ad9834.h
> >
> > diff --git a/drivers/staging/iio/dds/Kconfig
> b/drivers/staging/iio/dds/Kconfig
> > index 7969be2..4c9cce3 100644
> > --- a/drivers/staging/iio/dds/Kconfig
> > +++ b/drivers/staging/iio/dds/Kconfig
> > @@ -17,6 +17,13 @@ config AD9832
> > Say yes here to build support for Analog Devices DDS chip
> > ad9832 and ad9835, provides direct access via sysfs.
> >
> > +config AD9834
> > + tristate "Analog Devices ad9833/4/ driver"
> > + depends on SPI
> > + help
> > + Say yes here to build support for Analog Devices DDS chip
> > + AD9833 and AD9834, provides direct access via sysfs.
> > +
> > config AD9850
> > tristate "Analog Devices ad9850/1 driver"
> > depends on SPI
> > diff --git a/drivers/staging/iio/dds/Makefile
> b/drivers/staging/iio/dds/Makefile
> > index 6f274ac..1477461 100644
> > --- a/drivers/staging/iio/dds/Makefile
> > +++ b/drivers/staging/iio/dds/Makefile
> > @@ -4,6 +4,7 @@
> >
> > obj-$(CONFIG_AD5930) += ad5930.o
> > obj-$(CONFIG_AD9832) += ad9832.o
> > +obj-$(CONFIG_AD9834) += ad9834.o
> > obj-$(CONFIG_AD9850) += ad9850.o
> > obj-$(CONFIG_AD9852) += ad9852.o
> > obj-$(CONFIG_AD9910) += ad9910.o
> > diff --git a/drivers/staging/iio/dds/ad9834.c
> b/drivers/staging/iio/dds/ad9834.c
> > new file mode 100644
> > index 0000000..df3d68c
> > --- /dev/null
> > +++ b/drivers/staging/iio/dds/ad9834.c
> > @@ -0,0 +1,482 @@
> > +/*
> > + * AD9834 SPI DAC driver
> > + *
> > + * Copyright 2010 Analog Devices Inc.
> > + *
> > + * Licensed under the GPL-2 or later.
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/slab.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/list.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/err.h>
> > +#include <asm/div64.h>
> > +
> > +#include "../iio.h"
> > +#include "../sysfs.h"
> > +#include "dds.h"
> > +
> > +#include "ad9834.h"
> > +
> > +static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned
> long fout)
> > +{
> > + unsigned long long freqreg = (u64) fout * (u64) (1 <<
> AD9834_FREQ_BITS);
> > + do_div(freqreg, mclk);
> > + return freqreg;
> > +}
> > +
> > +static int ad9834_write_frequency(struct ad9834_state *st,
> > + unsigned long addr, unsigned long fout)
> > +{
> > + unsigned long regval;
> > +
> > + if (fout > (st->mclk / 2))
> > + return -EINVAL;
> > +
> > + regval = ad9834_calc_freqreg(st->mclk, fout);
> > +
> > + st->freq_data[0] = cpu_to_be16(addr | (regval &
> > + RES_MASK(AD9834_FREQ_BITS / 2)));
> > + st->freq_data[1] = cpu_to_be16(addr | ((regval >>
> > + (AD9834_FREQ_BITS / 2)) &
> > + RES_MASK(AD9834_FREQ_BITS / 2)));
> > +
> > + return spi_sync(st->spi, &st->freq_msg);;
> > +}
> > +
> > +static int ad9834_write_phase(struct ad9834_state *st,
> > + unsigned long addr, unsigned long phase)
> > +{
> > + if (phase > (1 << AD9834_PHASE_BITS))
> > + return -EINVAL;
> > + st->data = cpu_to_be16(addr | phase);
> > +
> > + return spi_sync(st->spi, &st->msg);
> > +}
> > +
> > +static ssize_t ad9834_write(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf,
> > + size_t len)
> > +{
> > + struct iio_dev *dev_info = dev_get_drvdata(dev);
> > + struct ad9834_state *st = dev_info->dev_data;
> > + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> > + int ret;
> > + long val;
> > +
> > + ret = strict_strtoul(buf, 10, &val);
> > + if (ret)
> > + goto error_ret;
> > +
> > + mutex_lock(&dev_info->mlock);
> > + switch (this_attr->address) {
> > + case AD9834_REG_FREQ0:
> > + case AD9834_REG_FREQ1:
> > + ret = ad9834_write_frequency(st, this_attr->address, val);
> > + break;
> > + case AD9834_REG_PHASE0:
> > + case AD9834_REG_PHASE1:
> > + ret = ad9834_write_phase(st, this_attr->address, val);
> > + break;
> > + case AD9834_OPBITEN:
> > + if (st->control & AD9834_MODE) {
> > + ret = -EINVAL; /* AD9843 reserved mode */
> > + break;
> > + }
> > +
> > + if (val)
> > + st->control |= AD9834_OPBITEN;
> > + else
> > + st->control &= ~AD9834_OPBITEN;
> > +
> > + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> > + ret = spi_sync(st->spi, &st->msg);
> > + break;
> > + case AD9834_PIN_SW:
> > + if (val)
> > + st->control |= AD9834_PIN_SW;
> > + else
> > + st->control &= ~AD9834_PIN_SW;
> > + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> > + ret = spi_sync(st->spi, &st->msg);
> > + break;
> > + case AD9834_FSEL:
> > + case AD9834_PSEL:
> > + if (val == 0)
> > + st->control &= ~(this_attr->address | AD9834_PIN_SW);
> > + else if (val == 1) {
> > + st->control |= this_attr->address;
> > + st->control &= ~AD9834_PIN_SW;
> > + } else {
> > + ret = -EINVAL;
> > + break;
> > + }
> > + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> > + ret = spi_sync(st->spi, &st->msg);
> > + break;
> > + case AD9834_RESET:
> > + if (val)
> > + st->control &= ~AD9834_RESET;
> > + else
> > + st->control |= AD9834_RESET;
> > +
> > + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> > + ret = spi_sync(st->spi, &st->msg);
> > + break;
> > + default:
> > + ret = -ENODEV;
> > + }
> > + mutex_unlock(&dev_info->mlock);
> > +
> > +error_ret:
> > + return ret ? ret : len;
> > +}
> > +
> > +static ssize_t ad9834_store_wavetype(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf,
> > + size_t len)
> > +{
> > + struct iio_dev *dev_info = dev_get_drvdata(dev);
> > + struct ad9834_state *st = dev_info->dev_data;
> > + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> > + int ret = 0;
> > + bool is_ad9833 = st->devid == ID_AD9833;
> > +
> > + mutex_lock(&dev_info->mlock);
> > +
> > + switch (this_attr->address) {
> > + case 0:
> > + if (sysfs_streq(buf, "sine")) {
> > + st->control &= ~AD9834_MODE;
> > + if (is_ad9833)
> > + st->control &= ~AD9834_OPBITEN;
> > + } else if (sysfs_streq(buf, "triangle")) {
> > + if (is_ad9833) {
> > + st->control &= ~AD9834_OPBITEN;
> > + st->control |= AD9834_MODE;
> > + } else if (st->control & AD9834_OPBITEN) {
> > + ret = -EINVAL; /* AD9843 reserved mode */
> > + } else {
> > + st->control |= AD9834_MODE;
> > + }
> > + } else if (is_ad9833 && sysfs_streq(buf, "square")) {
> > + st->control &= ~AD9834_MODE;
> > + st->control |= AD9834_OPBITEN;
> > + } else {
> > + ret = -EINVAL;
> > + }
> > +
> > + break;
> > + case 1:
> > + if (sysfs_streq(buf, "square") &&
> > + !(st->control & AD9834_MODE)) {
> > + st->control &= ~AD9834_MODE;
> > + st->control |= AD9834_OPBITEN;
> > + } else {
> > + ret = -EINVAL;
> > + }
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + break;
> > + }
> > +
> > + if (!ret) {
> > + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> > + ret = spi_sync(st->spi, &st->msg);
> > + }
> > + mutex_unlock(&dev_info->mlock);
> In correct tabbing on previous line? (or my email client has messed up)
Your email client doing the right thing.
> > +
> > + return ret ? ret : len;
> > +}
> > +
> > +static ssize_t ad9834_show_name(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct iio_dev *dev_info = dev_get_drvdata(dev);
> > + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> > +
> > + return sprintf(buf, "%s\n", spi_get_device_id(st->spi)->name);
> > +}
> > +static IIO_DEVICE_ATTR(name, S_IRUGO, ad9834_show_name, NULL, 0);
> > +
> > +static ssize_t ad9834_show_out0_wavetype_available(struct device
> *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct iio_dev *dev_info = dev_get_drvdata(dev);
> > + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> > + char *str;
> > +
> > + if (st->devid == ID_AD9833)
> > + str = "sine triangle square";
> > + else if (st->control & AD9834_OPBITEN)
> > + str = "sine";
> > + else
> > + str = "sine triangle";
> > +
> > + return sprintf(buf, "%s\n", str);
> > +}
> > +
> > +
> > +static IIO_DEVICE_ATTR(dds0_out0_wavetype_available, S_IRUGO,
> > + ad9834_show_out0_wavetype_available, NULL, 0);
> > +
> > +static ssize_t ad9834_show_out1_wavetype_available(struct device
> *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct iio_dev *dev_info = dev_get_drvdata(dev);
> > + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> > + char *str;
> > +
> > + if (st->control & AD9834_MODE)
> > + str = "";
> I 'think' that the is_visible below makes this first condition
> impossible?
No - the is_visible prevents this file to be available on the AD9833.
However this expresses a dependency on the AD9834 between the two outputs, where
the 'square' output on out1 is not available in case out0 is set to 'triangle'
> I've never used the is_visible stuff so not entirely sure I've
> understood
> it correctly...
> That probably means that you can use a const attr for this...
No
> > + else
> > + str = "square";
> > +
> > + return sprintf(buf, "%s\n", str);
> > +}
> > +
> > +static IIO_DEVICE_ATTR(dds0_out1_wavetype_available, S_IRUGO,
> > + ad9834_show_out1_wavetype_available, NULL, 0);
> > +
> > +/**
> > + * see dds.h for further information
> > + */
> > +
> > +static IIO_DEV_ATTR_FREQ(0, 0, ad9834_write, AD9834_REG_FREQ0);
> > +static IIO_DEV_ATTR_FREQ(0, 1, ad9834_write, AD9834_REG_FREQ1);
> > +static IIO_DEV_ATTR_FREQSYMBOL(0, ad9834_write, AD9834_FSEL);
> > +static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
> > +
> > +static IIO_DEV_ATTR_PHASE(0, 0, ad9834_write, AD9834_REG_PHASE0);
> > +static IIO_DEV_ATTR_PHASE(0, 1, ad9834_write, AD9834_REG_PHASE1);
> > +static IIO_DEV_ATTR_PHASESYMBOL(0, ad9834_write, AD9834_PSEL);
> > +static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12
> rad*/
> > +
> > +static IIO_DEV_ATTR_PINCONTROL_EN(0, ad9834_write, AD9834_PIN_SW);
> > +static IIO_DEV_ATTR_OUT_ENABLE(0, ad9834_write, AD9834_RESET);
> > +static IIO_DEV_ATTR_OUTY_ENABLE(0, 1, ad9834_write, AD9834_OPBITEN);
> > +static IIO_DEV_ATTR_OUT_WAVETYPE(0, 0, ad9834_store_wavetype, 0);
> > +static IIO_DEV_ATTR_OUT_WAVETYPE(0, 1, ad9834_store_wavetype, 1);
> > +
> > +static struct attribute *ad9834_attributes[] = {
> > + &iio_dev_attr_dds0_freq0.dev_attr.attr,
> > + &iio_dev_attr_dds0_freq1.dev_attr.attr,
> > + &iio_const_attr_dds0_freq_scale.dev_attr.attr,
> > + &iio_dev_attr_dds0_phase0.dev_attr.attr,
> > + &iio_dev_attr_dds0_phase1.dev_attr.attr,
> > + &iio_const_attr_dds0_phase_scale.dev_attr.attr,
> > + &iio_dev_attr_dds0_pincontrol_en.dev_attr.attr,
> > + &iio_dev_attr_dds0_freqsymbol.dev_attr.attr,
> > + &iio_dev_attr_dds0_phasesymbol.dev_attr.attr,
> > + &iio_dev_attr_dds0_out_enable.dev_attr.attr,
> > + &iio_dev_attr_dds0_out1_enable.dev_attr.attr,
> > + &iio_dev_attr_dds0_out0_wavetype.dev_attr.attr,
> > + &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr,
> > + &iio_dev_attr_dds0_out0_wavetype_available.dev_attr.attr,
> > + &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr,
> > + &iio_dev_attr_name.dev_attr.attr,
> > + NULL,
> > +};
> > +
> > +static mode_t ad9834_attr_is_visible(struct kobject *kobj,
> > + struct attribute *attr, int n)
> > +{
> > + struct device *dev = container_of(kobj, struct device, kobj);
> > + struct iio_dev *dev_info = dev_get_drvdata(dev);
> > + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
> > +
> > + mode_t mode = attr->mode;
> > +
> > + if (st->devid == ID_AD9834)
> > + return mode;
> > +
> > + if ((attr == &iio_dev_attr_dds0_out1_enable.dev_attr.attr) ||
> > + (attr == &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr) ||
> > + (attr ==
> > + &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr))
> > + mode = 0;
> > +
> > + return mode;
> > +}
> > +
> > +static const struct attribute_group ad9834_attribute_group = {
> > + .attrs = ad9834_attributes,
> > + .is_visible = ad9834_attr_is_visible,
> > +};
> > +
> > +static int __devinit ad9834_probe(struct spi_device *spi)
> > +{
> > + struct ad9834_platform_data *pdata = spi->dev.platform_data;
> > + struct ad9834_state *st;
> > + int ret;
> > +
> > + if (!pdata) {
> > + dev_dbg(&spi->dev, "no platform data?\n");
> > + return -ENODEV;
> > + }
> > +
> > + st = kzalloc(sizeof(*st), GFP_KERNEL);
> > + if (st == NULL) {
> > + ret = -ENOMEM;
> > + goto error_ret;
> > + }
> > +
> > + st->reg = regulator_get(&spi->dev, "vcc");
> > + if (!IS_ERR(st->reg)) {
> > + ret = regulator_enable(st->reg);
> > + if (ret)
> > + goto error_put_reg;
> > + }
> > +
> > + st->mclk = pdata->mclk;
> > +
> > + spi_set_drvdata(spi, st);
> > +
> > + st->spi = spi;
> > + st->devid = spi_get_device_id(spi)->driver_data;
> > +
> > + st->indio_dev = iio_allocate_device();
> > + if (st->indio_dev == NULL) {
> > + ret = -ENOMEM;
> > + goto error_disable_reg;
> > + }
> > +
> > + st->indio_dev->dev.parent = &spi->dev;
> > + st->indio_dev->attrs = &ad9834_attribute_group;
> > + st->indio_dev->dev_data = (void *)(st);
> nitpick: technically superflous brackets...
ok
> > + st->indio_dev->driver_module = THIS_MODULE;
> > + st->indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > + /* Setup default messages */
> > +
> > + st->xfer.tx_buf = &st->data;
> > + st->xfer.len = 2;
> > +
> > + spi_message_init(&st->msg);
> > + spi_message_add_tail(&st->xfer, &st->msg);
> > +
> > + st->freq_xfer[0].tx_buf = &st->freq_data[0];
> > + st->freq_xfer[0].len = 2;
> > + st->freq_xfer[0].cs_change = 1;
> > + st->freq_xfer[1].tx_buf = &st->freq_data[1];
> > + st->freq_xfer[1].len = 2;
> > +
> > + spi_message_init(&st->freq_msg);
> > + spi_message_add_tail(&st->freq_xfer[0], &st->freq_msg);
> > + spi_message_add_tail(&st->freq_xfer[1], &st->freq_msg);
> > +
> > + st->control = AD9834_B28 | AD9834_RESET;
> > +
> > + if (!pdata->en_div2)
> > + st->control |= AD9834_DIV2;
> > +
> > + if (!pdata->en_signbit_msb_out && (st->devid == ID_AD9834))
> > + st->control |= AD9834_SIGN_PIB;
> > +
> > + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> > + ret = spi_sync(st->spi, &st->msg);
> > + if (ret) {
> > + dev_err(&spi->dev, "device init failed\n");
> > + goto error_free_device;
> > + }
> > +
> > + ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, pdata->freq0);
> > + if (ret)
> > + goto error_free_device;
> > +
> > + ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, pdata->freq1);
> > + if (ret)
> > + goto error_free_device;
> > +
> > + ret = ad9834_write_phase(st, AD9834_REG_PHASE0, pdata->phase0);
> > + if (ret)
> > + goto error_free_device;
> > +
> > + ret = ad9834_write_phase(st, AD9834_REG_PHASE1, pdata->phase1);
> > + if (ret)
> > + goto error_free_device;
> > +
> > + st->control &= ~AD9834_RESET;
> > + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
> > + ret = spi_sync(st->spi, &st->msg);
> > + if (ret)
> > + goto error_free_device;
> > +
> > + ret = iio_device_register(st->indio_dev);
> > + if (ret)
> > + goto error_free_device;
> > +
> > + return 0;
> > +
> > +error_free_device:
> > + iio_free_device(st->indio_dev);
> > +error_disable_reg:
> > + if (!IS_ERR(st->reg))
> > + regulator_disable(st->reg);
> > +error_put_reg:
> > + if (!IS_ERR(st->reg))
> > + regulator_put(st->reg);
> > + kfree(st);
> > +error_ret:
> > + return ret;
> > +}
> > +
> > +static int __devexit ad9834_remove(struct spi_device *spi)
> > +{
> > + struct ad9834_state *st = spi_get_drvdata(spi);
> > + struct iio_dev *indio_dev = st->indio_dev;
> > +
> nitpick: could just use st->indio_dev in the next call and lose the
> line above. It's the only use in this function.
ok
> > + iio_device_unregister(indio_dev);
> > + if (!IS_ERR(st->reg)) {
> > + regulator_disable(st->reg);
> > + regulator_put(st->reg);
> > + }
> > + kfree(st);
> > + return 0;
> > +}
> > +
> > +static const struct spi_device_id ad9834_id[] = {
> > + {"ad9833", ID_AD9833},
> > + {"ad9834", ID_AD9834},
> > + {}
> > +};
> > +
> > +static struct spi_driver ad9834_driver = {
> > + .driver = {
> > + .name = "ad9834",
> > + .bus = &spi_bus_type,
> > + .owner = THIS_MODULE,
> > + },
> > + .probe = ad9834_probe,
> > + .remove = __devexit_p(ad9834_remove),
> > + .id_table = ad9834_id,
> > +};
> > +
> > +static int __init ad9834_init(void)
> > +{
> > + return spi_register_driver(&ad9834_driver);
> > +}
> > +module_init(ad9834_init);
> > +
> > +static void __exit ad9834_exit(void)
> > +{
> > + spi_unregister_driver(&ad9834_driver);
> > +}
> > +module_exit(ad9834_exit);
> > +
> > +MODULE_AUTHOR("Michael Hennerich <[email protected]>");
> > +MODULE_DESCRIPTION("Analog Devices AD9833/AD9834 DDS");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("spi:ad9834");
> > diff --git a/drivers/staging/iio/dds/ad9834.h
> b/drivers/staging/iio/dds/ad9834.h
> > new file mode 100644
> > index 0000000..0fc3b88
> > --- /dev/null
> > +++ b/drivers/staging/iio/dds/ad9834.h
> > @@ -0,0 +1,112 @@
> > +/*
> > + * AD9834 SPI DDS driver
> > + *
> > + * Copyright 2010 Analog Devices Inc.
> > + *
> > + * Licensed under the GPL-2 or later.
> > + */
> > +#ifndef IIO_DDS_AD9834_H_
> > +#define IIO_DDS_AD9834_H_
> > +
> > +/* Registers */
> > +
> > +#define AD9834_REG_CMD (0 << 14)
> > +#define AD9834_REG_FREQ0 (1 << 14)
> > +#define AD9834_REG_FREQ1 (2 << 14)
> > +#define AD9834_REG_PHASE0 (6 << 13)
> > +#define AD9834_REG_PHASE1 (7 << 13)
> > +
> > +/* Command Control Bits */
> > +
> > +#define AD9834_B28 (1 << 13)
> > +#define AD9834_HLB (1 << 12)
> > +#define AD9834_FSEL (1 << 11)
> > +#define AD9834_PSEL (1 << 10)
> > +#define AD9834_PIN_SW (1 << 9)
> > +#define AD9834_RESET (1 << 8)
> > +#define AD9834_SLEEP1 (1 << 7)
> > +#define AD9834_SLEEP12 (1 << 6)
> > +#define AD9834_OPBITEN (1 << 5)
> > +#define AD9834_SIGN_PIB (1 << 4)
> > +#define AD9834_DIV2 (1 << 3)
> > +#define AD9834_MODE (1 << 1)
> > +
> > +#define AD9834_FREQ_BITS 28
> > +#define AD9834_PHASE_BITS 12
> > +
> > +#define RES_MASK(bits) ((1 << (bits)) - 1)
> > +
> > +/**
> > + * struct ad9834_state - driver instance specific data
> > + * @indio_dev: the industrial I/O device
> > + * @spi: spi_device
> > + * @reg: supply regulator
> > + * @mclk: external master clock
> > + * @control: cached control word
> > + * @xfer: default spi transfer
> > + * @msg: default spi message
> > + * @freq_xfer: tuning word spi transfer
> > + * @freq_msg: tuning word spi message
> > + * @data: spi transmit buffer
> > + * @freq_data: tuning word spi transmit buffer
> > + */
> > +
> > +struct ad9834_state {
> > + struct iio_dev *indio_dev;
> > + struct spi_device *spi;
> > + struct regulator *reg;
> > + unsigned int mclk;
> > + unsigned short control;
> > + unsigned short devid;
> > + struct spi_transfer xfer;
> > + struct spi_message msg;
> > + struct spi_transfer freq_xfer[2];
> > + struct spi_message freq_msg;
> > +
> > + /*
> > + * DMA (thus cache coherency maintenance) requires the
> > + * transfer buffers to live in their own cache lines.
> > + */
> > + unsigned short data ____cacheline_aligned;
> > + unsigned short freq_data[2] ;
> > +};
> > +
> > +
> > +/*
> > + * TODO: struct ad7887_platform_data needs to go into
> include/linux/iio
> > + */
> > +
> > +/**
> > + * struct ad9834_platform_data - platform specific information
> > + * @mclk: master clock in Hz
> > + * @freq0: power up freq0 tuning word in Hz
> > + * @freq1: power up freq1 tuning word in Hz
> > + * @phase0: power up phase0 value [0..4095] correlates with
> 0..2PI
> > + * @phase1: power up phase1 value [0..4095] correlates with
> 0..2PI
> > + * @en_div2: digital output/2 is passed to the SIGN BIT OUT
> pin
> > + * @en_signbit_msb_out: the MSB (or MSB/2) of the DAC data is
> connected to the
> > + * SIGN BIT OUT pin. en_div2 controls whether it is the
> MSB
> > + * or MSB/2 that is output. if en_signbit_msb_out=false,
> > + * the on-board comparator is connected to SIGN BIT OUT
> > + */
> > +
> > +struct ad9834_platform_data {
> > + unsigned int mclk;
> > + unsigned int freq0;
> > + unsigned int freq1;
> > + unsigned short phase0;
> > + unsigned short phase1;
> > + bool en_div2;
> > + bool en_signbit_msb_out;
> > +};
> > +
> > +/**
> > + * ad9834_supported_device_ids:
> > + */
> > +
> > +enum ad9834_supported_device_ids {
> > + ID_AD9833,
> > + ID_AD9834,
> > +};
> > +
> > +#endif /* IIO_DDS_AD9834_H_ */
> > --
> > 1.6.0.2
> >
> >
On 12/14/10 11:01, Hennerich, Michael wrote:
>> Jonathan Cameron wrote on 2010-12-13:
>>> On 12/13/10 16:27, [email protected] wrote:
>>> From: Michael Hennerich <[email protected]>
>>>
>>> Changes since RFC/v1:
>>> IIO: Apply list review feedback
>>>
>>> Apply list review feedback:
>>> Rename attributes to fit IIO convention used in other drivers.
>>> Fix typos.
>>> Provide ddsX_out_enable as opposed to ddsX_out_disable.
>>> Use proper __devexit marking.
>>> Use strict_strtoul() to avoid negatives.
>>>
>> Couple of nitpicks inline. Subject to changes due to changes in macros
>> suggested in previous patch (or you convincing me otherwise):
>>> Signed-off-by: Michael Hennerich <[email protected]>
>>> Reviewed-by: Datta Shubhrajyoti <[email protected]>
>> Acked-by: Jonathan Cameron <[email protected]>
>>> ---
>>> drivers/staging/iio/dds/Kconfig | 7 +
>>> drivers/staging/iio/dds/Makefile | 1 +
>>> drivers/staging/iio/dds/ad9834.c | 482
>> ++++++++++++++++++++++++++++++++++++++
>>> drivers/staging/iio/dds/ad9834.h | 112 +++++++++
>>> 4 files changed, 602 insertions(+), 0 deletions(-)
>>> create mode 100644 drivers/staging/iio/dds/ad9834.c
>>> create mode 100644 drivers/staging/iio/dds/ad9834.h
>>>
>>> diff --git a/drivers/staging/iio/dds/Kconfig
>> b/drivers/staging/iio/dds/Kconfig
>>> index 7969be2..4c9cce3 100644
>>> --- a/drivers/staging/iio/dds/Kconfig
>>> +++ b/drivers/staging/iio/dds/Kconfig
>>> @@ -17,6 +17,13 @@ config AD9832
>>> Say yes here to build support for Analog Devices DDS chip
>>> ad9832 and ad9835, provides direct access via sysfs.
>>>
>>> +config AD9834
>>> + tristate "Analog Devices ad9833/4/ driver"
>>> + depends on SPI
>>> + help
>>> + Say yes here to build support for Analog Devices DDS chip
>>> + AD9833 and AD9834, provides direct access via sysfs.
>>> +
>>> config AD9850
>>> tristate "Analog Devices ad9850/1 driver"
>>> depends on SPI
>>> diff --git a/drivers/staging/iio/dds/Makefile
>> b/drivers/staging/iio/dds/Makefile
>>> index 6f274ac..1477461 100644
>>> --- a/drivers/staging/iio/dds/Makefile
>>> +++ b/drivers/staging/iio/dds/Makefile
>>> @@ -4,6 +4,7 @@
>>>
>>> obj-$(CONFIG_AD5930) += ad5930.o
>>> obj-$(CONFIG_AD9832) += ad9832.o
>>> +obj-$(CONFIG_AD9834) += ad9834.o
>>> obj-$(CONFIG_AD9850) += ad9850.o
>>> obj-$(CONFIG_AD9852) += ad9852.o
>>> obj-$(CONFIG_AD9910) += ad9910.o
>>> diff --git a/drivers/staging/iio/dds/ad9834.c
>> b/drivers/staging/iio/dds/ad9834.c
>>> new file mode 100644
>>> index 0000000..df3d68c
>>> --- /dev/null
>>> +++ b/drivers/staging/iio/dds/ad9834.c
>>> @@ -0,0 +1,482 @@
>>> +/*
>>> + * AD9834 SPI DAC driver
>>> + *
>>> + * Copyright 2010 Analog Devices Inc.
>>> + *
>>> + * Licensed under the GPL-2 or later.
>>> + */
>>> +
>>> +#include <linux/interrupt.h>
>>> +#include <linux/workqueue.h>
>>> +#include <linux/device.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/sysfs.h>
>>> +#include <linux/list.h>
>>> +#include <linux/spi/spi.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/err.h>
>>> +#include <asm/div64.h>
>>> +
>>> +#include "../iio.h"
>>> +#include "../sysfs.h"
>>> +#include "dds.h"
>>> +
>>> +#include "ad9834.h"
>>> +
>>> +static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned
>> long fout)
>>> +{
>>> + unsigned long long freqreg = (u64) fout * (u64) (1 <<
>> AD9834_FREQ_BITS);
>>> + do_div(freqreg, mclk);
>>> + return freqreg;
>>> +}
>>> +
>>> +static int ad9834_write_frequency(struct ad9834_state *st,
>>> + unsigned long addr, unsigned long fout)
>>> +{
>>> + unsigned long regval;
>>> +
>>> + if (fout > (st->mclk / 2))
>>> + return -EINVAL;
>>> +
>>> + regval = ad9834_calc_freqreg(st->mclk, fout);
>>> +
>>> + st->freq_data[0] = cpu_to_be16(addr | (regval &
>>> + RES_MASK(AD9834_FREQ_BITS / 2)));
>>> + st->freq_data[1] = cpu_to_be16(addr | ((regval >>
>>> + (AD9834_FREQ_BITS / 2)) &
>>> + RES_MASK(AD9834_FREQ_BITS / 2)));
>>> +
>>> + return spi_sync(st->spi, &st->freq_msg);;
>>> +}
>>> +
>>> +static int ad9834_write_phase(struct ad9834_state *st,
>>> + unsigned long addr, unsigned long phase)
>>> +{
>>> + if (phase > (1 << AD9834_PHASE_BITS))
>>> + return -EINVAL;
>>> + st->data = cpu_to_be16(addr | phase);
>>> +
>>> + return spi_sync(st->spi, &st->msg);
>>> +}
>>> +
>>> +static ssize_t ad9834_write(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf,
>>> + size_t len)
>>> +{
>>> + struct iio_dev *dev_info = dev_get_drvdata(dev);
>>> + struct ad9834_state *st = dev_info->dev_data;
>>> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>>> + int ret;
>>> + long val;
>>> +
>>> + ret = strict_strtoul(buf, 10, &val);
>>> + if (ret)
>>> + goto error_ret;
>>> +
>>> + mutex_lock(&dev_info->mlock);
>>> + switch (this_attr->address) {
>>> + case AD9834_REG_FREQ0:
>>> + case AD9834_REG_FREQ1:
>>> + ret = ad9834_write_frequency(st, this_attr->address, val);
>>> + break;
>>> + case AD9834_REG_PHASE0:
>>> + case AD9834_REG_PHASE1:
>>> + ret = ad9834_write_phase(st, this_attr->address, val);
>>> + break;
>>> + case AD9834_OPBITEN:
>>> + if (st->control & AD9834_MODE) {
>>> + ret = -EINVAL; /* AD9843 reserved mode */
>>> + break;
>>> + }
>>> +
>>> + if (val)
>>> + st->control |= AD9834_OPBITEN;
>>> + else
>>> + st->control &= ~AD9834_OPBITEN;
>>> +
>>> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>>> + ret = spi_sync(st->spi, &st->msg);
>>> + break;
>>> + case AD9834_PIN_SW:
>>> + if (val)
>>> + st->control |= AD9834_PIN_SW;
>>> + else
>>> + st->control &= ~AD9834_PIN_SW;
>>> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>>> + ret = spi_sync(st->spi, &st->msg);
>>> + break;
>>> + case AD9834_FSEL:
>>> + case AD9834_PSEL:
>>> + if (val == 0)
>>> + st->control &= ~(this_attr->address | AD9834_PIN_SW);
>>> + else if (val == 1) {
>>> + st->control |= this_attr->address;
>>> + st->control &= ~AD9834_PIN_SW;
>>> + } else {
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>>> + ret = spi_sync(st->spi, &st->msg);
>>> + break;
>>> + case AD9834_RESET:
>>> + if (val)
>>> + st->control &= ~AD9834_RESET;
>>> + else
>>> + st->control |= AD9834_RESET;
>>> +
>>> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>>> + ret = spi_sync(st->spi, &st->msg);
>>> + break;
>>> + default:
>>> + ret = -ENODEV;
>>> + }
>>> + mutex_unlock(&dev_info->mlock);
>>> +
>>> +error_ret:
>>> + return ret ? ret : len;
>>> +}
>>> +
>>> +static ssize_t ad9834_store_wavetype(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf,
>>> + size_t len)
>>> +{
>>> + struct iio_dev *dev_info = dev_get_drvdata(dev);
>>> + struct ad9834_state *st = dev_info->dev_data;
>>> + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>>> + int ret = 0;
>>> + bool is_ad9833 = st->devid == ID_AD9833;
>>> +
>>> + mutex_lock(&dev_info->mlock);
>>> +
>>> + switch (this_attr->address) {
>>> + case 0:
>>> + if (sysfs_streq(buf, "sine")) {
>>> + st->control &= ~AD9834_MODE;
>>> + if (is_ad9833)
>>> + st->control &= ~AD9834_OPBITEN;
>>> + } else if (sysfs_streq(buf, "triangle")) {
>>> + if (is_ad9833) {
>>> + st->control &= ~AD9834_OPBITEN;
>>> + st->control |= AD9834_MODE;
>>> + } else if (st->control & AD9834_OPBITEN) {
>>> + ret = -EINVAL; /* AD9843 reserved mode */
>>> + } else {
>>> + st->control |= AD9834_MODE;
>>> + }
>>> + } else if (is_ad9833 && sysfs_streq(buf, "square")) {
>>> + st->control &= ~AD9834_MODE;
>>> + st->control |= AD9834_OPBITEN;
>>> + } else {
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + break;
>>> + case 1:
>>> + if (sysfs_streq(buf, "square") &&
>>> + !(st->control & AD9834_MODE)) {
>>> + st->control &= ~AD9834_MODE;
>>> + st->control |= AD9834_OPBITEN;
>>> + } else {
>>> + ret = -EINVAL;
>>> + }
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + break;
>>> + }
>>> +
>>> + if (!ret) {
>>> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>>> + ret = spi_sync(st->spi, &st->msg);
>>> + }
>>> + mutex_unlock(&dev_info->mlock);
>> In correct tabbing on previous line? (or my email client has messed up)
>
> Your email client doing the right thing.
>
>>> +
>>> + return ret ? ret : len;
>>> +}
>>> +
>>> +static ssize_t ad9834_show_name(struct device *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct iio_dev *dev_info = dev_get_drvdata(dev);
>>> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
>>> +
>>> + return sprintf(buf, "%s\n", spi_get_device_id(st->spi)->name);
>>> +}
>>> +static IIO_DEVICE_ATTR(name, S_IRUGO, ad9834_show_name, NULL, 0);
>>> +
>>> +static ssize_t ad9834_show_out0_wavetype_available(struct device
>> *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct iio_dev *dev_info = dev_get_drvdata(dev);
>>> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
>>> + char *str;
>>> +
>>> + if (st->devid == ID_AD9833)
>>> + str = "sine triangle square";
>>> + else if (st->control & AD9834_OPBITEN)
>>> + str = "sine";
>>> + else
>>> + str = "sine triangle";
>>> +
>>> + return sprintf(buf, "%s\n", str);
>>> +}
>>> +
>>> +
>>> +static IIO_DEVICE_ATTR(dds0_out0_wavetype_available, S_IRUGO,
>>> + ad9834_show_out0_wavetype_available, NULL, 0);
>>> +
>>> +static ssize_t ad9834_show_out1_wavetype_available(struct device
>> *dev,
>>> + struct device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct iio_dev *dev_info = dev_get_drvdata(dev);
>>> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
>>> + char *str;
>>> +
>>> + if (st->control & AD9834_MODE)
>>> + str = "";
>> I 'think' that the is_visible below makes this first condition
>> impossible?
>
> No - the is_visible prevents this file to be available on the AD9833.
> However this expresses a dependency on the AD9834 between the two outputs, where
> the 'square' output on out1 is not available in case out0 is set to 'triangle'
ah, I'd missed that.
>
>> I've never used the is_visible stuff so not entirely sure I've
>> understood
>> it correctly...
>> That probably means that you can use a const attr for this...
>
> No
>
>>> + else
>>> + str = "square";
>>> +
>>> + return sprintf(buf, "%s\n", str);
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(dds0_out1_wavetype_available, S_IRUGO,
>>> + ad9834_show_out1_wavetype_available, NULL, 0);
>>> +
>>> +/**
>>> + * see dds.h for further information
>>> + */
>>> +
>>> +static IIO_DEV_ATTR_FREQ(0, 0, ad9834_write, AD9834_REG_FREQ0);
>>> +static IIO_DEV_ATTR_FREQ(0, 1, ad9834_write, AD9834_REG_FREQ1);
>>> +static IIO_DEV_ATTR_FREQSYMBOL(0, ad9834_write, AD9834_FSEL);
>>> +static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
>>> +
>>> +static IIO_DEV_ATTR_PHASE(0, 0, ad9834_write, AD9834_REG_PHASE0);
>>> +static IIO_DEV_ATTR_PHASE(0, 1, ad9834_write, AD9834_REG_PHASE1);
>>> +static IIO_DEV_ATTR_PHASESYMBOL(0, ad9834_write, AD9834_PSEL);
>>> +static IIO_CONST_ATTR_PHASE_SCALE(0, "0.0015339808"); /* 2PI/2^12
>> rad*/
>>> +
>>> +static IIO_DEV_ATTR_PINCONTROL_EN(0, ad9834_write, AD9834_PIN_SW);
>>> +static IIO_DEV_ATTR_OUT_ENABLE(0, ad9834_write, AD9834_RESET);
>>> +static IIO_DEV_ATTR_OUTY_ENABLE(0, 1, ad9834_write, AD9834_OPBITEN);
>>> +static IIO_DEV_ATTR_OUT_WAVETYPE(0, 0, ad9834_store_wavetype, 0);
>>> +static IIO_DEV_ATTR_OUT_WAVETYPE(0, 1, ad9834_store_wavetype, 1);
>>> +
>>> +static struct attribute *ad9834_attributes[] = {
>>> + &iio_dev_attr_dds0_freq0.dev_attr.attr,
>>> + &iio_dev_attr_dds0_freq1.dev_attr.attr,
>>> + &iio_const_attr_dds0_freq_scale.dev_attr.attr,
>>> + &iio_dev_attr_dds0_phase0.dev_attr.attr,
>>> + &iio_dev_attr_dds0_phase1.dev_attr.attr,
>>> + &iio_const_attr_dds0_phase_scale.dev_attr.attr,
>>> + &iio_dev_attr_dds0_pincontrol_en.dev_attr.attr,
>>> + &iio_dev_attr_dds0_freqsymbol.dev_attr.attr,
>>> + &iio_dev_attr_dds0_phasesymbol.dev_attr.attr,
>>> + &iio_dev_attr_dds0_out_enable.dev_attr.attr,
>>> + &iio_dev_attr_dds0_out1_enable.dev_attr.attr,
>>> + &iio_dev_attr_dds0_out0_wavetype.dev_attr.attr,
>>> + &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr,
>>> + &iio_dev_attr_dds0_out0_wavetype_available.dev_attr.attr,
>>> + &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr,
>>> + &iio_dev_attr_name.dev_attr.attr,
>>> + NULL,
>>> +};
>>> +
>>> +static mode_t ad9834_attr_is_visible(struct kobject *kobj,
>>> + struct attribute *attr, int n)
>>> +{
>>> + struct device *dev = container_of(kobj, struct device, kobj);
>>> + struct iio_dev *dev_info = dev_get_drvdata(dev);
>>> + struct ad9834_state *st = iio_dev_get_devdata(dev_info);
>>> +
>>> + mode_t mode = attr->mode;
>>> +
>>> + if (st->devid == ID_AD9834)
>>> + return mode;
>>> +
>>> + if ((attr == &iio_dev_attr_dds0_out1_enable.dev_attr.attr) ||
>>> + (attr == &iio_dev_attr_dds0_out1_wavetype.dev_attr.attr) ||
>>> + (attr ==
>>> + &iio_dev_attr_dds0_out1_wavetype_available.dev_attr.attr))
>>> + mode = 0;
>>> +
>>> + return mode;
>>> +}
>>> +
>>> +static const struct attribute_group ad9834_attribute_group = {
>>> + .attrs = ad9834_attributes,
>>> + .is_visible = ad9834_attr_is_visible,
>>> +};
>>> +
>>> +static int __devinit ad9834_probe(struct spi_device *spi)
>>> +{
>>> + struct ad9834_platform_data *pdata = spi->dev.platform_data;
>>> + struct ad9834_state *st;
>>> + int ret;
>>> +
>>> + if (!pdata) {
>>> + dev_dbg(&spi->dev, "no platform data?\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + st = kzalloc(sizeof(*st), GFP_KERNEL);
>>> + if (st == NULL) {
>>> + ret = -ENOMEM;
>>> + goto error_ret;
>>> + }
>>> +
>>> + st->reg = regulator_get(&spi->dev, "vcc");
>>> + if (!IS_ERR(st->reg)) {
>>> + ret = regulator_enable(st->reg);
>>> + if (ret)
>>> + goto error_put_reg;
>>> + }
>>> +
>>> + st->mclk = pdata->mclk;
>>> +
>>> + spi_set_drvdata(spi, st);
>>> +
>>> + st->spi = spi;
>>> + st->devid = spi_get_device_id(spi)->driver_data;
>>> +
>>> + st->indio_dev = iio_allocate_device();
>>> + if (st->indio_dev == NULL) {
>>> + ret = -ENOMEM;
>>> + goto error_disable_reg;
>>> + }
>>> +
>>> + st->indio_dev->dev.parent = &spi->dev;
>>> + st->indio_dev->attrs = &ad9834_attribute_group;
>>> + st->indio_dev->dev_data = (void *)(st);
>> nitpick: technically superflous brackets...
>
> ok
>
>>> + st->indio_dev->driver_module = THIS_MODULE;
>>> + st->indio_dev->modes = INDIO_DIRECT_MODE;
>>> +
>>> + /* Setup default messages */
>>> +
>>> + st->xfer.tx_buf = &st->data;
>>> + st->xfer.len = 2;
>>> +
>>> + spi_message_init(&st->msg);
>>> + spi_message_add_tail(&st->xfer, &st->msg);
>>> +
>>> + st->freq_xfer[0].tx_buf = &st->freq_data[0];
>>> + st->freq_xfer[0].len = 2;
>>> + st->freq_xfer[0].cs_change = 1;
>>> + st->freq_xfer[1].tx_buf = &st->freq_data[1];
>>> + st->freq_xfer[1].len = 2;
>>> +
>>> + spi_message_init(&st->freq_msg);
>>> + spi_message_add_tail(&st->freq_xfer[0], &st->freq_msg);
>>> + spi_message_add_tail(&st->freq_xfer[1], &st->freq_msg);
>>> +
>>> + st->control = AD9834_B28 | AD9834_RESET;
>>> +
>>> + if (!pdata->en_div2)
>>> + st->control |= AD9834_DIV2;
>>> +
>>> + if (!pdata->en_signbit_msb_out && (st->devid == ID_AD9834))
>>> + st->control |= AD9834_SIGN_PIB;
>>> +
>>> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>>> + ret = spi_sync(st->spi, &st->msg);
>>> + if (ret) {
>>> + dev_err(&spi->dev, "device init failed\n");
>>> + goto error_free_device;
>>> + }
>>> +
>>> + ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, pdata->freq0);
>>> + if (ret)
>>> + goto error_free_device;
>>> +
>>> + ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, pdata->freq1);
>>> + if (ret)
>>> + goto error_free_device;
>>> +
>>> + ret = ad9834_write_phase(st, AD9834_REG_PHASE0, pdata->phase0);
>>> + if (ret)
>>> + goto error_free_device;
>>> +
>>> + ret = ad9834_write_phase(st, AD9834_REG_PHASE1, pdata->phase1);
>>> + if (ret)
>>> + goto error_free_device;
>>> +
>>> + st->control &= ~AD9834_RESET;
>>> + st->data = cpu_to_be16(AD9834_REG_CMD | st->control);
>>> + ret = spi_sync(st->spi, &st->msg);
>>> + if (ret)
>>> + goto error_free_device;
>>> +
>>> + ret = iio_device_register(st->indio_dev);
>>> + if (ret)
>>> + goto error_free_device;
>>> +
>>> + return 0;
>>> +
>>> +error_free_device:
>>> + iio_free_device(st->indio_dev);
>>> +error_disable_reg:
>>> + if (!IS_ERR(st->reg))
>>> + regulator_disable(st->reg);
>>> +error_put_reg:
>>> + if (!IS_ERR(st->reg))
>>> + regulator_put(st->reg);
>>> + kfree(st);
>>> +error_ret:
>>> + return ret;
>>> +}
>>> +
>>> +static int __devexit ad9834_remove(struct spi_device *spi)
>>> +{
>>> + struct ad9834_state *st = spi_get_drvdata(spi);
>>> + struct iio_dev *indio_dev = st->indio_dev;
>>> +
>> nitpick: could just use st->indio_dev in the next call and lose the
>> line above. It's the only use in this function.
>
> ok
>
>>> + iio_device_unregister(indio_dev);
>>> + if (!IS_ERR(st->reg)) {
>>> + regulator_disable(st->reg);
>>> + regulator_put(st->reg);
>>> + }
>>> + kfree(st);
>>> + return 0;
>>> +}
>>> +
>>> +static const struct spi_device_id ad9834_id[] = {
>>> + {"ad9833", ID_AD9833},
>>> + {"ad9834", ID_AD9834},
>>> + {}
>>> +};
>>> +
>>> +static struct spi_driver ad9834_driver = {
>>> + .driver = {
>>> + .name = "ad9834",
>>> + .bus = &spi_bus_type,
>>> + .owner = THIS_MODULE,
>>> + },
>>> + .probe = ad9834_probe,
>>> + .remove = __devexit_p(ad9834_remove),
>>> + .id_table = ad9834_id,
>>> +};
>>> +
>>> +static int __init ad9834_init(void)
>>> +{
>>> + return spi_register_driver(&ad9834_driver);
>>> +}
>>> +module_init(ad9834_init);
>>> +
>>> +static void __exit ad9834_exit(void)
>>> +{
>>> + spi_unregister_driver(&ad9834_driver);
>>> +}
>>> +module_exit(ad9834_exit);
>>> +
>>> +MODULE_AUTHOR("Michael Hennerich <[email protected]>");
>>> +MODULE_DESCRIPTION("Analog Devices AD9833/AD9834 DDS");
>>> +MODULE_LICENSE("GPL v2");
>>> +MODULE_ALIAS("spi:ad9834");
>>> diff --git a/drivers/staging/iio/dds/ad9834.h
>> b/drivers/staging/iio/dds/ad9834.h
>>> new file mode 100644
>>> index 0000000..0fc3b88
>>> --- /dev/null
>>> +++ b/drivers/staging/iio/dds/ad9834.h
>>> @@ -0,0 +1,112 @@
>>> +/*
>>> + * AD9834 SPI DDS driver
>>> + *
>>> + * Copyright 2010 Analog Devices Inc.
>>> + *
>>> + * Licensed under the GPL-2 or later.
>>> + */
>>> +#ifndef IIO_DDS_AD9834_H_
>>> +#define IIO_DDS_AD9834_H_
>>> +
>>> +/* Registers */
>>> +
>>> +#define AD9834_REG_CMD (0 << 14)
>>> +#define AD9834_REG_FREQ0 (1 << 14)
>>> +#define AD9834_REG_FREQ1 (2 << 14)
>>> +#define AD9834_REG_PHASE0 (6 << 13)
>>> +#define AD9834_REG_PHASE1 (7 << 13)
>>> +
>>> +/* Command Control Bits */
>>> +
>>> +#define AD9834_B28 (1 << 13)
>>> +#define AD9834_HLB (1 << 12)
>>> +#define AD9834_FSEL (1 << 11)
>>> +#define AD9834_PSEL (1 << 10)
>>> +#define AD9834_PIN_SW (1 << 9)
>>> +#define AD9834_RESET (1 << 8)
>>> +#define AD9834_SLEEP1 (1 << 7)
>>> +#define AD9834_SLEEP12 (1 << 6)
>>> +#define AD9834_OPBITEN (1 << 5)
>>> +#define AD9834_SIGN_PIB (1 << 4)
>>> +#define AD9834_DIV2 (1 << 3)
>>> +#define AD9834_MODE (1 << 1)
>>> +
>>> +#define AD9834_FREQ_BITS 28
>>> +#define AD9834_PHASE_BITS 12
>>> +
>>> +#define RES_MASK(bits) ((1 << (bits)) - 1)
>>> +
>>> +/**
>>> + * struct ad9834_state - driver instance specific data
>>> + * @indio_dev: the industrial I/O device
>>> + * @spi: spi_device
>>> + * @reg: supply regulator
>>> + * @mclk: external master clock
>>> + * @control: cached control word
>>> + * @xfer: default spi transfer
>>> + * @msg: default spi message
>>> + * @freq_xfer: tuning word spi transfer
>>> + * @freq_msg: tuning word spi message
>>> + * @data: spi transmit buffer
>>> + * @freq_data: tuning word spi transmit buffer
>>> + */
>>> +
>>> +struct ad9834_state {
>>> + struct iio_dev *indio_dev;
>>> + struct spi_device *spi;
>>> + struct regulator *reg;
>>> + unsigned int mclk;
>>> + unsigned short control;
>>> + unsigned short devid;
>>> + struct spi_transfer xfer;
>>> + struct spi_message msg;
>>> + struct spi_transfer freq_xfer[2];
>>> + struct spi_message freq_msg;
>>> +
>>> + /*
>>> + * DMA (thus cache coherency maintenance) requires the
>>> + * transfer buffers to live in their own cache lines.
>>> + */
>>> + unsigned short data ____cacheline_aligned;
>>> + unsigned short freq_data[2] ;
>>> +};
>>> +
>>> +
>>> +/*
>>> + * TODO: struct ad7887_platform_data needs to go into
>> include/linux/iio
>>> + */
>>> +
>>> +/**
>>> + * struct ad9834_platform_data - platform specific information
>>> + * @mclk: master clock in Hz
>>> + * @freq0: power up freq0 tuning word in Hz
>>> + * @freq1: power up freq1 tuning word in Hz
>>> + * @phase0: power up phase0 value [0..4095] correlates with
>> 0..2PI
>>> + * @phase1: power up phase1 value [0..4095] correlates with
>> 0..2PI
>>> + * @en_div2: digital output/2 is passed to the SIGN BIT OUT
>> pin
>>> + * @en_signbit_msb_out: the MSB (or MSB/2) of the DAC data is
>> connected to the
>>> + * SIGN BIT OUT pin. en_div2 controls whether it is the
>> MSB
>>> + * or MSB/2 that is output. if en_signbit_msb_out=false,
>>> + * the on-board comparator is connected to SIGN BIT OUT
>>> + */
>>> +
>>> +struct ad9834_platform_data {
>>> + unsigned int mclk;
>>> + unsigned int freq0;
>>> + unsigned int freq1;
>>> + unsigned short phase0;
>>> + unsigned short phase1;
>>> + bool en_div2;
>>> + bool en_signbit_msb_out;
>>> +};
>>> +
>>> +/**
>>> + * ad9834_supported_device_ids:
>>> + */
>>> +
>>> +enum ad9834_supported_device_ids {
>>> + ID_AD9833,
>>> + ID_AD9834,
>>> +};
>>> +
>>> +#endif /* IIO_DDS_AD9834_H_ */
>>> --
>>> 1.6.0.2
>>>
>>>
>
>