> From: Jonathan Cameron [mailto:jic23@xxxxxxxxx] > On 02/03/11 14:51, michael.hennerich@xxxxxxxxxx wrote: > > From: Michael Hennerich <michael.hennerich@xxxxxxxxxx> [--snip--] > > + > > +What: /sys/bus/iio/devices/deviceX/range_available > > +KernelVersion: 2.6.38 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Hardware dependent supported vales for ADC Full Scale > Range. > > + > > +What: /sys/bus/iio/devices/deviceX/oversampling > > +KernelVersion: 2.6.38 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Hardware dependent ADC oversamplimg. > Typo. > > Controls the sampling ratio > > + of the digital filter if available. > What are the units? The obvious choice is a multiplier, but could be > frequency. > Either way, needs to be specified here. 'Ratio' or 'rate' is the best you can use here. http://mathworld.wolfram.com/Oversampling.html > > + > > +What: /sys/bus/iio/devices/deviceX/oversampling_available > > +KernelVersion: 2.6.38 > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > +Description: > > + Hardware dependent supported vales > values (and should probably be Hardware dependent values supported by > the > oversampling filter.) > > by the oversampling filter. > > + > > What: /sys/bus/iio/devices/deviceX/inY_raw > > What: /sys/bus/iio/devices/deviceX/inY_supply_raw > > KernelVersion: 2.6.35 > > diff --git a/drivers/staging/iio/adc/Kconfig > b/drivers/staging/iio/adc/Kconfig > > index 86869cd..70767ff 100644 > > --- a/drivers/staging/iio/adc/Kconfig > > +++ b/drivers/staging/iio/adc/Kconfig > > @@ -62,6 +62,33 @@ config AD7314 > > Say yes here to build support for Analog Devices AD7314 > > temperature sensors. > > > > +config AD7606 > > + tristate "Analog Devices AD7606 ADC driver" > > + select IIO_RING_BUFFER > > + select IIO_TRIGGER > > + select IIO_SW_RING > Looks like a generic gpio dependency is need as well? Will do > > + help > > + Say yes here to build support for Analog Devices: > > + ad7606, ad7606-6, ad7606-4 analog to digital convertors (ADC). > > + > > + To compile this driver as a module, choose M here: the > > + module will be called ad7606. > > + > > +config AD7606_IFACE_PARALLEL > > + bool "parallel interface support" > > + depends on AD7606 > > + help > > + Say yes here to include parallel interface support on the > AD7606 > > + ADC driver. > > + > > +config AD7606_IFACE_SPI > > + bool "spi interface support" > > + depends on AD7606 > > + depends on SPI > > + help > > + Say yes here to include parallel interface support on the > AD7606 > > + ADC driver. > > + > > config AD799X > > tristate "Analog Devices AD799x ADC driver" > > depends on I2C > > diff --git a/drivers/staging/iio/adc/Makefile > b/drivers/staging/iio/adc/Makefile > > index 6f231a2..cb73346 100644 > > --- a/drivers/staging/iio/adc/Makefile > > +++ b/drivers/staging/iio/adc/Makefile > > @@ -7,6 +7,12 @@ max1363-y += max1363_ring.o > > > > obj-$(CONFIG_MAX1363) += max1363.o > > > > +ad7606-y := ad7606_core.o > > +ad7606-$(CONFIG_IIO_RING_BUFFER) += ad7606_ring.o > > +ad7606-$(CONFIG_AD7606_IFACE_PARALLEL) += ad7606_par.o > > +ad7606-$(CONFIG_AD7606_IFACE_SPI) += ad7606_spi.o > > +obj-$(CONFIG_AD7606) += ad7606.o > > + > > ad799x-y := ad799x_core.o > > ad799x-$(CONFIG_AD799X_RING_BUFFER) += ad799x_ring.o > > obj-$(CONFIG_AD799X) += ad799x.o > > diff --git a/drivers/staging/iio/adc/ad7606.h > b/drivers/staging/iio/adc/ad7606.h > > new file mode 100644 > > index 0000000..99f21c5 > > --- /dev/null > > +++ b/drivers/staging/iio/adc/ad7606.h > > @@ -0,0 +1,119 @@ > > +/* > > + * AD7606 ADC driver > > + * > > + * Copyright 2011 Analog Devices Inc. > > + * > > + * Licensed under the GPL-2. > > + */ > > + > > +#ifndef IIO_ADC_AD7606_H_ > > +#define IIO_ADC_AD7606_H_ > > + > Only one use. I'd put it inline. > > +#define DRVNAME "iio-ad7606" > > + > > +/* > > + * TODO: struct ad7606_platform_data needs to go into > include/linux/iio > > + */ > > + > > +/** > > + * struct ad7606_platform_data - platform/board specifc information > > + * @default_os: default oversampling value {0, 2, 4, 8, > 16, 32, 64} > > + * @default_range: default range +/-{5, 10} Volt > > + * @gpio_convst: number of gpio connected to the CONVST pin > > + * @gpio_reset: gpio connected to the RESET pin, if not > used set to -1 > > + * @gpio_range: gpio connected to the RANGE pin, if not > used set to -1 > > + * @gpio_os0: gpio connected to the OS0 pin, if not used set > to -1 > > + * @gpio_os1: gpio connected to the OS1 pin, if not used set > to -1 > > + * @gpio_os2: gpio connected to the OS2 pin, if not used set > to -1 > > + * @gpio_frstdata: gpio connected to the FRSTDAT pin, if not used > set to -1 > > + * @gpio_stby: gpio connected to the STBY pin, if not used set > to -1 > > + */ > > + > > +struct ad7606_platform_data { > > + unsigned default_os; > > + unsigned default_range; > > + unsigned gpio_convst; > > + unsigned gpio_reset; > > + unsigned gpio_range; > > + unsigned gpio_os0; > > + unsigned gpio_os1; > > + unsigned gpio_os2; > > + unsigned gpio_frstdata; > > + unsigned gpio_stby; > > +}; > > + > > +/** > > + * struct ad7606_chip_info - chip specifc information > > + * @name: indentification string for chip > > + * @bits: accuracy of the adc in bits > > + * @bits: output coding [s]igned or [u]nsigned > > + * @int_vref_mv: the internal reference voltage > > + * @num_channels: number of physical inputs on chip > > + */ > > + > > +struct ad7606_chip_info { > > + char name[10]; > > + u8 bits; > > + char sign; > > + u16 int_vref_mv; > > + unsigned num_channels; > > +}; > > + > > +/** > > + * struct ad7606_state - driver instance specific data > > + */ > > + > > +struct ad7606_state { > > + struct iio_dev *indio_dev; > > + struct device *dev; > > + const struct ad7606_chip_info *chip_info; > > + struct ad7606_platform_data *pdata; > > + struct regulator *reg; > > + struct work_struct poll_work; > > + wait_queue_head_t wq_data_avail; > > + atomic_t protect_ring; > > + size_t d_size; > > + const struct ad7606_bus_ops *bops; > > + int irq; > > + unsigned id; > > + unsigned range; > > + unsigned oversampling; > > + bool done; > > + bool have_frstdata; > > + bool have_os; > > + bool have_stby; > > + bool have_reset; > > + bool have_range; > > + void __iomem *base_address; > > + > > + /* > > + * DMA (thus cache coherency maintenance) requires the > > + * transfer buffers to live in their own cache lines. > > + */ > > + > > + unsigned short data[8] ____cacheline_aligned; > > +}; > > + > > +struct ad7606_bus_ops { > > + /* more methods added in future? */ > > + int (*read_block)(struct device *, int, void *); > > +}; > > + > > +void ad7606_suspend(struct ad7606_state *st); > > +void ad7606_resume(struct ad7606_state *st); > > +struct ad7606_state *ad7606_probe(struct device *dev, int irq, > > + void __iomem *base_address, unsigned id, > > + const struct ad7606_bus_ops *bops); > > +int ad7606_remove(struct ad7606_state *st); > > +int ad7606_reset(struct ad7606_state *st); > > + > > +enum ad7606_supported_device_ids { > > + ID_AD7606_8, > > + ID_AD7606_6, > > + ID_AD7606_4 > > +}; > > + > > +int ad7606_scan_from_ring(struct ad7606_state *st, unsigned ch); > > +int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev); > > +void ad7606_ring_cleanup(struct iio_dev *indio_dev); > > +#endif /* IIO_ADC_AD7606_H_ */ > > diff --git a/drivers/staging/iio/adc/ad7606_core.c > b/drivers/staging/iio/adc/ad7606_core.c > > new file mode 100644 > > index 0000000..21e5af8 > > --- /dev/null > > +++ b/drivers/staging/iio/adc/ad7606_core.c > > @@ -0,0 +1,560 @@ > > +/* > > + * AD7606 SPI ADC driver > > + * > > + * Copyright 2011 Analog Devices Inc. > > + * > > + * Licensed under the GPL-2. > > + */ > > + > > +#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/regulator/consumer.h> > > +#include <linux/err.h> > > +#include <linux/gpio.h> > > +#include <linux/delay.h> > > + > > +#include "../iio.h" > > +#include "../sysfs.h" > > +#include "../ring_generic.h" > > +#include "adc.h" > > + > > +#include "ad7606.h" > > + > > +int ad7606_reset(struct ad7606_state *st) > > +{ > > + if (st->have_reset) { > > + gpio_set_value(st->pdata->gpio_reset, 1); > > + ndelay(100); /* t_reset >= 100ns */ > could it be a sleep? Only 100ns are required! Why would I go to sleep? > > + gpio_set_value(st->pdata->gpio_reset, 0); > > + return 0; > > + } > > + > > + return -ENODEV; > > +} > > + > > +static int ad7606_scan_direct(struct ad7606_state *st, unsigned ch) > > +{ > > + int ret; > > + > > + gpio_set_value(st->pdata->gpio_convst, 1); > > + st->done = false; > > + > > + ret = wait_event_interruptible(st->wq_data_avail, st->done); > > + if (ret) > > + goto error_ret; > > + > > + if (st->have_frstdata) { > > + st->bops->read_block(st->dev, 1, st->data); > > + if (!gpio_get_value(st->pdata->gpio_frstdata)) { > > + /* This should never happen */ > As below.... does it and if so why? In case it happens we're out of sync. Most people won't use FRSTDATA checks at all. However some signal glitch caused perhaps by some electrostatic discharge, could cause an extra read or clock. This allows recovery. > > + ad7606_reset(st); > > + ret = -EIO; > > + goto error_ret; > > + } > > + st->bops->read_block(st->dev, > > + st->chip_info->num_channels - 1, &st->data[1]); > > + } else { > > + st->bops->read_block(st->dev, > > + st->chip_info->num_channels, st->data); > > + } > > + > > + ret = st->data[ch]; > > + > > +error_ret: > > + gpio_set_value(st->pdata->gpio_convst, 0); > > + > > + return ret; > > +} > > + > > +static ssize_t ad7606_scan(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct iio_dev *dev_info = dev_get_drvdata(dev); > > + struct ad7606_state *st = dev_info->dev_data; > > + struct iio_dev_attr *this_attr = to_iio_dev_attr(attr); > > + int ret; > > + > > + mutex_lock(&dev_info->mlock); > > + if (iio_ring_enabled(dev_info)) > > + ret = ad7606_scan_from_ring(st, this_attr->address); > > + else > > + ret = ad7606_scan_direct(st, this_attr->address); > > + mutex_unlock(&dev_info->mlock); > > + > > + if (ret < 0) > > + return ret; > > + > > + return sprintf(buf, "%d\n", (short) ret); > > +} > > + > > +static IIO_DEV_ATTR_IN_RAW(0, ad7606_scan, 0); > > +static IIO_DEV_ATTR_IN_RAW(1, ad7606_scan, 1); > > +static IIO_DEV_ATTR_IN_RAW(2, ad7606_scan, 2); > > +static IIO_DEV_ATTR_IN_RAW(3, ad7606_scan, 3); > > +static IIO_DEV_ATTR_IN_RAW(4, ad7606_scan, 4); > > +static IIO_DEV_ATTR_IN_RAW(5, ad7606_scan, 5); > > +static IIO_DEV_ATTR_IN_RAW(6, ad7606_scan, 6); > > +static IIO_DEV_ATTR_IN_RAW(7, ad7606_scan, 7); > > + > > +static ssize_t ad7606_show_scale(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + /* Driver currently only support internal vref */ > > + struct iio_dev *dev_info = dev_get_drvdata(dev); > > + struct ad7606_state *st = iio_dev_get_devdata(dev_info); > > + unsigned int scale_uv = (st->range * 1000000) >> st->chip_info- > >bits; > > + > > + return sprintf(buf, "%d.%03d\n", scale_uv / 1000, scale_uv % > 1000); > > +} > > +static IIO_DEVICE_ATTR(in_scale, S_IRUGO, ad7606_show_scale, NULL, > 0); > > + > > +static ssize_t ad7606_show_name(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct iio_dev *dev_info = dev_get_drvdata(dev); > > + struct ad7606_state *st = iio_dev_get_devdata(dev_info); > > + > > + return sprintf(buf, "%s\n", st->chip_info->name); > > +} > > + > > +static IIO_DEVICE_ATTR(name, S_IRUGO, ad7606_show_name, NULL, 0); > > + > > +static ssize_t ad7606_show_range(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct iio_dev *dev_info = dev_get_drvdata(dev); > > + struct ad7606_state *st = iio_dev_get_devdata(dev_info); > > + > > + return sprintf(buf, "%u\n", st->range); > > +} > > + > > +static ssize_t ad7606_store_range(struct device *dev, > > + struct device_attribute *attr, const char *buf, size_t > count) > > +{ > > + struct iio_dev *dev_info = dev_get_drvdata(dev); > > + struct ad7606_state *st = iio_dev_get_devdata(dev_info); > > + unsigned long lval; > > + > These are voltage values, so should be in millivolts? (as > we lift the nearest interface from hwmon) > > + if (strict_strtoul(buf, 10, &lval)) > > + return -EINVAL; > > + if (!(lval == 5 || lval == 10)) { > > + dev_err(dev, "range is not supported\n"); > > + return -EINVAL; > > + } > > + mutex_lock(&dev_info->mlock); > > + gpio_set_value(st->pdata->gpio_range, lval == 10); > > + st->range = lval; > > + mutex_unlock(&dev_info->mlock); > > + > > + return count; > > +} > > + > > +static IIO_DEVICE_ATTR(range, S_IRUGO | S_IWUSR, \ > > + ad7606_show_range, ad7606_store_range, 0); > > +static IIO_CONST_ATTR(range_available, "5 10"); > > + > > +static ssize_t ad7606_show_oversampling(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct iio_dev *dev_info = dev_get_drvdata(dev); > > + struct ad7606_state *st = iio_dev_get_devdata(dev_info); > > + > > + return sprintf(buf, "%u\n", st->oversampling); > > +} > > + > Name is a little missleading. It is actually finding the index and name > should reflect that. ok > > +static int ad7606_check_oversampling(unsigned val) > > +{ > > + unsigned char supported[] = {0, 2, 4, 8, 16, 32, 64}; > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(supported); i++) > > + if (val == supported[i]) > > + return i; > > + > > + return -EINVAL; > > +} > > + > > +static ssize_t ad7606_store_oversampling(struct device *dev, > > + struct device_attribute *attr, const char *buf, size_t > count) > > +{ > > + struct iio_dev *dev_info = dev_get_drvdata(dev); > > + struct ad7606_state *st = iio_dev_get_devdata(dev_info); > > + unsigned long lval; > > + int ret; > > + > > + if (strict_strtoul(buf, 10, &lval)) > > + return -EINVAL; > > + > > + ret = ad7606_check_oversampling(lval); > > + if (ret < 0) { > > + dev_err(dev, "oversampling %lu is not supported\n", lval); > > + return ret; > > + } > > + > > + mutex_lock(&dev_info->mlock); > > + gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1); > > + gpio_set_value(st->pdata->gpio_os1, (ret >> 1) & 1); > > + gpio_set_value(st->pdata->gpio_os1, (ret >> 2) & 1); > > + st->oversampling = lval; > > + mutex_unlock(&dev_info->mlock); > > + > > + return count; > > +} > > + > > +static IIO_DEVICE_ATTR(oversampling, S_IRUGO | S_IWUSR, \ > Need the '\' ? No > > + ad7606_show_oversampling, ad7606_store_oversampling, > 0); > > +static IIO_CONST_ATTR(oversampling_available, "0 2 4 8 16 32 64"); > > + > > +static struct attribute *ad7606_attributes[] = { > > + &iio_dev_attr_in0_raw.dev_attr.attr, > > + &iio_dev_attr_in1_raw.dev_attr.attr, > > + &iio_dev_attr_in2_raw.dev_attr.attr, > > + &iio_dev_attr_in3_raw.dev_attr.attr, > > + &iio_dev_attr_in4_raw.dev_attr.attr, > > + &iio_dev_attr_in5_raw.dev_attr.attr, > > + &iio_dev_attr_in6_raw.dev_attr.attr, > > + &iio_dev_attr_in7_raw.dev_attr.attr, > > + &iio_dev_attr_in_scale.dev_attr.attr, > > + &iio_dev_attr_name.dev_attr.attr, > > + &iio_dev_attr_range.dev_attr.attr, > > + &iio_const_attr_range_available.dev_attr.attr, > > + &iio_dev_attr_oversampling.dev_attr.attr, > > + &iio_const_attr_oversampling_available.dev_attr.attr, > > + NULL, > > +}; > > + > > +static mode_t ad7606_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 ad7606_state *st = iio_dev_get_devdata(dev_info); > > + > > + mode_t mode = attr->mode; > > + > As with the other is_visible function, I'd rather see this use the > chip_info->num_channels element instead of the ids. Makes it very > obvious what is happening. > > Then becomes > > if (st->chip_info->num_channels < 6 & > (attr == &iio_dev_attr_in7_raw.dev_attr.attr || > attr == &iio_dev_attr_in6_raw.dev_attr.attr) > mode = 0; > if (st->chip_info->num_channels < 4 & > (attr == &iio_dev_attr_in5_raw.dev_attr.attr || > attr == &iio_dev_attr_in4_raw.dev_attr.attr) > mode = 0; > > (other the same - though I'd reorder so first thing one > sees is the cause, then the attributes. More readable in > my opinion). > > Probably not worth optimizing the order in this function. ok > > > + if ((attr == &iio_dev_attr_in7_raw.dev_attr.attr) && > > + ((st->id == ID_AD7606_6) || (st->id == ID_AD7606_4))) > > + mode = 0; > > + else if ((attr == &iio_dev_attr_in6_raw.dev_attr.attr) && > > + ((st->id == ID_AD7606_6) || (st->id == ID_AD7606_4))) > > + mode = 0; > > + else if ((attr == &iio_dev_attr_in5_raw.dev_attr.attr) && > > + (st->id == ID_AD7606_4)) > > + mode = 0; > > + else if ((attr == &iio_dev_attr_in4_raw.dev_attr.attr) && > > + (st->id == ID_AD7606_4)) > > + mode = 0; > > + else if ((attr == &iio_dev_attr_oversampling.dev_attr.attr || > > + attr == > &iio_const_attr_oversampling_available.dev_attr.attr) && > > + !st->have_os) > > + mode = 0; > > + else if ((attr == &iio_dev_attr_range.dev_attr.attr || > > + attr == &iio_const_attr_range_available.dev_attr.attr) && > > + !st->have_range) > > + mode = 0; > > + > > + return mode; > > +} > > + > > +static const struct attribute_group ad7606_attribute_group = { > > + .attrs = ad7606_attributes, > > + .is_visible = ad7606_attr_is_visible, > > +}; > > + > > +static const struct ad7606_chip_info ad7606_chip_info_tbl[] = { > > + /* > > + * More devices added in future > Good. Otherwise I'd moan about the fact that these are nearly > identical and you could hard code most of them into the driver. > > > + */ > > + [ID_AD7606_8] = { > > + .name = "ad7606", > > + .bits = 16, > > + .sign = IIO_SCAN_EL_TYPE_SIGNED, > > + .int_vref_mv = 2500, > > + .num_channels = 8, > > + }, > > + [ID_AD7606_6] = { > > + .name = "ad7606-6", > > + .bits = 16, > > + .sign = IIO_SCAN_EL_TYPE_SIGNED, > > + .int_vref_mv = 2500, > > + .num_channels = 6, > > + }, > > + [ID_AD7606_4] = { > > + .name = "ad7606-4", > > + .bits = 16, > > + .sign = IIO_SCAN_EL_TYPE_SIGNED, > > + .int_vref_mv = 2500, > > + .num_channels = 4, > > + }, > > +}; > > + > > +static int ad7606_request_gpio_out(struct ad7606_state *st, > > + unsigned gpio, unsigned val) > > +{ > > + int ret = -EINVAL; > > + > > + if (gpio_is_valid(gpio)) { > > + ret = gpio_request(gpio, st->chip_info->name); > > + if (!ret) > > + gpio_direction_output(gpio, val); > > + } > > + return ret; > > +} > > + > > +static int ad7606_request_gpios(struct ad7606_state *st) > > +{ > > + int ret; > > + > > + ret = ad7606_request_gpio_out(st, st->pdata->gpio_convst, 0); > > + if (ret) { > > + dev_err(st->dev, "failed to request GPIO CONVST\n"); > > + return ret; > > + } > > + > > + ret = ad7606_request_gpio_out(st, st->pdata->gpio_os0, > > + (st->oversampling >> 0) & 1); > > + if (ret < 0) { > > + st->have_os = false; > > + } else { > > + ret = ad7606_request_gpio_out(st, st->pdata->gpio_os1, > > + (st->oversampling >> 1) & 1); > st->oversampling & 2 ? ok > > + if (ret < 0) { > > + gpio_free(st->pdata->gpio_os0); > > + st->have_os = false; > > + } else { > > + ret = ad7606_request_gpio_out(st, st->pdata- > >gpio_os2, > > + (st->oversampling >> 2) & 1); > st->oversampling & 4 or st->oversampling & (1 << 2) if you want to make > it explicity. > Compiler will neatly clean that up into a constant whereas it may not > this way around. > > + if (ret < 0) { > > + gpio_free(st->pdata->gpio_os0); > > + gpio_free(st->pdata->gpio_os1); > > + st->have_os = false; > > + } else { > > + st->have_os = true; > > + } > > + } > > + } > > + > > + ret = ad7606_request_gpio_out(st, st->pdata->gpio_reset, 0); > > + if (!ret) > > + st->have_reset = true; > > + > > + ret = ad7606_request_gpio_out(st, st->pdata->gpio_range, > > + st->range == 10); > > + if (!ret) > > + st->have_range = true; > > + > > + ret = ad7606_request_gpio_out(st, st->pdata->gpio_stby, 1); > > + if (!ret) > > + st->have_stby = true; > > + > > + if (gpio_is_valid(st->pdata->gpio_frstdata)) { > > + ret = gpio_request(st->pdata->gpio_frstdata, > > + st->chip_info->name); > Looks like you register a number of gpio's with the same name. Would be > nice to be a little more specific on what they are... ok > > + if (!ret) { > > + gpio_direction_input(st->pdata->gpio_frstdata); > > + st->have_frstdata = true; > > + } > > + } > > + > > + return 0; > > +} > > + > > +static void ad7606_free_gpios(struct ad7606_state *st) > > +{ > > + if (st->have_range) > > + gpio_free(st->pdata->gpio_range); > > + > > + if (st->have_stby) > > + gpio_free(st->pdata->gpio_stby); > > + > > + if (st->have_os) { > > + gpio_free(st->pdata->gpio_os0); > > + gpio_free(st->pdata->gpio_os1); > > + gpio_free(st->pdata->gpio_os2); > > + } > > + > > + if (st->have_reset) > > + gpio_free(st->pdata->gpio_reset); > > + > > + if (st->have_frstdata) > > + gpio_free(st->pdata->gpio_frstdata); > > + > > + gpio_free(st->pdata->gpio_convst); > > +} > > + > > +/** > > + * Interrupt handler > > + */ > > +static irqreturn_t ad7606_interrupt(int irq, void *dev_id) > > +{ > > + struct ad7606_state *st = dev_id; > > + > > + if (iio_ring_enabled(st->indio_dev)) { > > + if (!work_pending(&st->poll_work)) > > + schedule_work(&st->poll_work); > > + } else { > > + st->done = true; > > + wake_up_interruptible(&st->wq_data_avail); > > + } > > + > > + return IRQ_HANDLED; > > +}; > > + > > +struct ad7606_state *ad7606_probe(struct device *dev, int irq, > > + void __iomem *base_address, > > + unsigned id, > > + const struct ad7606_bus_ops *bops) > > +{ > > + struct ad7606_platform_data *pdata = dev->platform_data; > > + struct ad7606_state *st; > > + int ret; > > + > > + st = kzalloc(sizeof(*st), GFP_KERNEL); > > + if (st == NULL) { > > + ret = -ENOMEM; > > + goto error_ret; > > + } > > + > > + st->dev = dev; > > + st->id = id; > > + st->irq = irq; > > + st->bops = bops; > > + st->base_address = base_address; > > + st->range = pdata->default_range == 10 ? 10 : 5; > > + > > + ret = ad7606_check_oversampling(pdata->default_os); > > + if (ret < 0) { > > + dev_warn(dev, "oversampling %d is not supported\n", > > + pdata->default_os); > > + st->oversampling = 0; > > + } else { > > + st->oversampling = pdata->default_os; > > + } > > + > > + st->reg = regulator_get(dev, "vcc"); > > + if (!IS_ERR(st->reg)) { > > + ret = regulator_enable(st->reg); > > + if (ret) > > + goto error_put_reg; > > + } > > + > > + st->pdata = pdata; > > + st->chip_info = &ad7606_chip_info_tbl[id]; > > + > > + atomic_set(&st->protect_ring, 0); > > + > > + st->indio_dev = iio_allocate_device(); > > + if (st->indio_dev == NULL) { > > + ret = -ENOMEM; > > + goto error_disable_reg; > > + } > > + > > + st->indio_dev->dev.parent = dev; > > + st->indio_dev->attrs = &ad7606_attribute_group; > > + st->indio_dev->dev_data = (void *)(st); > > + st->indio_dev->driver_module = THIS_MODULE; > > + st->indio_dev->modes = INDIO_DIRECT_MODE; > > + > > + init_waitqueue_head(&st->wq_data_avail); > > + > > + ret = ad7606_request_gpios(st); > > + if (ret) > > + goto error_free_device; > > + > > + ret = ad7606_reset(st); > > + if (ret) > > + dev_warn(st->dev, "failed to RESET: no RESET GPIO > specified\n"); > > + > > + ret = request_irq(st->irq, ad7606_interrupt, > > + IRQF_TRIGGER_FALLING, st->chip_info->name, st); > > + if (ret) > > + goto error_free_gpios; > > + > > + ret = ad7606_register_ring_funcs_and_init(st->indio_dev); > > + if (ret) > > + goto error_free_irq; > > + > > + ret = iio_device_register(st->indio_dev); > > + if (ret) > > + goto error_free_irq; > > + > > + ret = iio_ring_buffer_register(st->indio_dev->ring, 0); > > + if (ret) > > + goto error_cleanup_ring; > > + > > + return st; > > + > > +error_cleanup_ring: > > + ad7606_ring_cleanup(st->indio_dev); > > + iio_device_unregister(st->indio_dev); > > + > > +error_free_irq: > > + free_irq(st->irq, st); > > + > > +error_free_gpios: > > + ad7606_free_gpios(st); > > + > > +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 ERR_PTR(ret); > > +} > > + > > +int ad7606_remove(struct ad7606_state *st) > > +{ > > + struct iio_dev *indio_dev = st->indio_dev; > > + iio_ring_buffer_unregister(indio_dev->ring); > > + ad7606_ring_cleanup(indio_dev); > > + iio_device_unregister(indio_dev); > > + free_irq(st->irq, st); > > + if (!IS_ERR(st->reg)) { > > + regulator_disable(st->reg); > > + regulator_put(st->reg); > > + } > > + > > + ad7606_free_gpios(st); > > + > > + kfree(st); > > + return 0; > > +} > > + > > +void ad7606_suspend(struct ad7606_state *st) > > +{ > > + if (st->have_stby) { > > + if (st->have_range) > > + gpio_set_value(st->pdata->gpio_range, 1); > > + gpio_set_value(st->pdata->gpio_stby, 0); > > + > Bonus white line (I'm getting fussy). > > + } > > +} > > + > > +void ad7606_resume(struct ad7606_state *st) > > +{ > > + if (st->have_stby) { > > + if (st->have_range) > > + gpio_set_value(st->pdata->gpio_range, st->range == > 10); > > + > > + gpio_set_value(st->pdata->gpio_stby, 1); > > + ad7606_reset(st); > > + } > > +} > > + > > +MODULE_AUTHOR("Michael Hennerich <hennerich@xxxxxxxxxxxxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Analog Devices AD7606 ADC"); > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/staging/iio/adc/ad7606_par.c > b/drivers/staging/iio/adc/ad7606_par.c > > new file mode 100644 > > index 0000000..d787c06 > > --- /dev/null > > +++ b/drivers/staging/iio/adc/ad7606_par.c > > @@ -0,0 +1,191 @@ > > +/* > > + * AD7606 Parallel Interface ADC driver > > + * > > + * Copyright 2011 Analog Devices Inc. > > + * > > + * Licensed under the GPL-2. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/types.h> > > +#include <linux/err.h> > > +#include <linux/io.h> > > + > > +#include "ad7606.h" > > + > > +static int ad7606_par16_read_block(struct device *dev, > > + int count, void *buf) > > +{ > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ad7606_state *st = platform_get_drvdata(pdev); > > + > > + insw((unsigned long) st->base_address, buf, count); > > + > > + return 0; > > +} > > + > > +static const struct ad7606_bus_ops ad7606_par16_bops = { > > + .read_block = ad7606_par16_read_block, > > +}; > > + > > +static int ad7606_par8_read_block(struct device *dev, > > + int count, void *buf) > > +{ > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ad7606_state *st = platform_get_drvdata(pdev); > > + > > + insb((unsigned long) st->base_address, buf, count * 2); > > + > > + return 0; > > +} > > + > > +static const struct ad7606_bus_ops ad7606_par8_bops = { > > + .read_block = ad7606_par8_read_block, > > +}; > > + > > +static int __devinit ad7606_par_probe(struct platform_device *pdev) > > +{ > > + struct resource *res; > > + struct ad7606_state *st; > > + void __iomem *addr; > > + resource_size_t remap_size; > > + int ret, irq; > > + > > + irq = platform_get_irq(pdev, 0); > > + if (irq < 0) { > > + dev_err(&pdev->dev, "no irq\n"); > > + return -ENODEV; > > + } > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -ENODEV; > > + > > + remap_size = resource_size(res); > > + > Nitpick. I think this the only time you use this particular syntax > for comments. Please stick to /* ... */ on one line. > > + /* > > + * Request the regions. > > + */ > > + if (!request_mem_region(res->start, remap_size, DRVNAME)) { > > + ret = -EBUSY; > > + goto out1; > > + } > > + addr = ioremap(res->start, remap_size); > > + if (!addr) { > > + ret = -ENOMEM; > > + goto out1; > > + } > > + > > + st = ad7606_probe(&pdev->dev, irq, addr, > > + platform_get_device_id(pdev)->driver_data, > > + remap_size > 1 ? &ad7606_par16_bops : > > + &ad7606_par8_bops); > > + > > + if (IS_ERR(st)) { > > + ret = PTR_ERR(st); > > + goto out2; > > + } > > + > > + platform_set_drvdata(pdev, st); > > + > > + return 0; > > + > > +out2: > > + iounmap(addr); > > +out1: > > + release_mem_region(res->start, remap_size); > > + > > + return ret; > > +} > > + > > +static int __devexit ad7606_par_remove(struct platform_device *pdev) > > +{ > > + struct ad7606_state *st = platform_get_drvdata(pdev); > > + struct resource *res; > > + > > + ad7606_remove(st); > > + > > + iounmap(st->base_address); > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + release_mem_region(res->start, resource_size(res)); > > + > > + platform_set_drvdata(pdev, NULL); > > + > > + return 0; > > +} > > + > > +#ifdef CONFIG_PM > > +static int ad7606_par_suspend(struct device *dev) > > +{ > > + struct ad7606_state *st = dev_get_drvdata(dev); > > + > > + ad7606_suspend(st); > > + > > + return 0; > > +} > > + > > +static int ad7606_par_resume(struct device *dev) > > +{ > > + struct ad7606_state *st = dev_get_drvdata(dev); > > + > > + ad7606_resume(st); > > + > > + return 0; > > +} > > + > > +static const struct dev_pm_ops ad7606_pm_ops = { > > + .suspend = ad7606_par_suspend, > > + .resume = ad7606_par_resume, > > +}; > > +#define AD7606_PAR_PM_OPS (&ad7606_pm_ops) > > + > > +#else > > +#define AD7606_PAR_PM_OPS NULL > > +#endif /* CONFIG_PM */ > > + > Nitpick, Bonus empty line. > > + > > +static struct platform_device_id ad7606_driver_ids[] = { > > + { > > + .name = "ad7606-8", > > + .driver_data = ID_AD7606_8, > > + }, { > > + .name = "ad7606-6", > > + .driver_data = ID_AD7606_6, > > + }, { > > + .name = "ad7606-4", > > + .driver_data = ID_AD7606_4, > > + }, > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(platform, ad7606_driver_ids); > > + > > +static struct platform_driver ad7606_driver = { > > + .probe = ad7606_par_probe, > > + .remove = __devexit_p(ad7606_par_remove), > > + .id_table = ad7606_driver_ids, > > + .driver = { > > + .name = "ad7606", > > + .owner = THIS_MODULE, > > + .pm = AD7606_PAR_PM_OPS, > > + }, > > +}; > > + > > +static int __init ad7606_init(void) > > +{ > > + return platform_driver_register(&ad7606_driver); > > +} > > + > > +static void __exit ad7606_cleanup(void) > > +{ > > + platform_driver_unregister(&ad7606_driver); > > +} > > + > > +module_init(ad7606_init); > > +module_exit(ad7606_cleanup); > > + > > +MODULE_AUTHOR("Michael Hennerich <hennerich@xxxxxxxxxxxxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Analog Devices AD7606 ADC"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_ALIAS("platform:ad7606-par"); > > diff --git a/drivers/staging/iio/adc/ad7606_ring.c > b/drivers/staging/iio/adc/ad7606_ring.c > > new file mode 100644 > > index 0000000..ab06503 > > --- /dev/null > > +++ b/drivers/staging/iio/adc/ad7606_ring.c > > @@ -0,0 +1,261 @@ > > +/* > > + * Copyright 2011 Analog Devices Inc. > > + * > > + * Licensed under the GPL-2. > > + * > > + */ > > + > > +#include <linux/interrupt.h> > > +#include <linux/gpio.h> > > +#include <linux/workqueue.h> > > +#include <linux/device.h> > > +#include <linux/kernel.h> > > +#include <linux/slab.h> > > +#include <linux/sysfs.h> > > Do we need this? > > +#include <linux/list.h> > > + > > +#include "../iio.h" > > +#include "../ring_generic.h" > > +#include "../ring_sw.h" > > +#include "../trigger.h" > > +#include "../sysfs.h" > > + > > +#include "ad7606.h" > > + > > +static IIO_SCAN_EL_C(in0, 0, 0, NULL); > > +static IIO_SCAN_EL_C(in1, 1, 0, NULL); > > +static IIO_SCAN_EL_C(in2, 2, 0, NULL); > > +static IIO_SCAN_EL_C(in3, 3, 0, NULL); > > +static IIO_SCAN_EL_C(in4, 4, 0, NULL); > > +static IIO_SCAN_EL_C(in5, 5, 0, NULL); > > +static IIO_SCAN_EL_C(in6, 6, 0, NULL); > > +static IIO_SCAN_EL_C(in7, 7, 0, NULL); > > + > > +static ssize_t ad7606_show_type(struct device *dev, > > + struct device_attribute *attr, > > + char *buf) > > +{ > > + struct iio_ring_buffer *ring = dev_get_drvdata(dev); > > + struct iio_dev *indio_dev = ring->indio_dev; > > + struct ad7606_state *st = indio_dev->dev_data; > > + > > + return sprintf(buf, "%c%d/%d\n", st->chip_info->sign, > > + st->chip_info->bits, st->chip_info->bits); > > +} > > +static IIO_DEVICE_ATTR(in_type, S_IRUGO, ad7606_show_type, NULL, 0); > > + > > +static struct attribute *ad7606_scan_el_attrs[] = { > > + &iio_scan_el_in0.dev_attr.attr, > > + &iio_const_attr_in0_index.dev_attr.attr, > > + &iio_scan_el_in1.dev_attr.attr, > > + &iio_const_attr_in1_index.dev_attr.attr, > > + &iio_scan_el_in2.dev_attr.attr, > > + &iio_const_attr_in2_index.dev_attr.attr, > > + &iio_scan_el_in3.dev_attr.attr, > > + &iio_const_attr_in3_index.dev_attr.attr, > > + &iio_scan_el_in4.dev_attr.attr, > > + &iio_const_attr_in4_index.dev_attr.attr, > > + &iio_scan_el_in5.dev_attr.attr, > > + &iio_const_attr_in5_index.dev_attr.attr, > > + &iio_scan_el_in6.dev_attr.attr, > > + &iio_const_attr_in6_index.dev_attr.attr, > > + &iio_scan_el_in7.dev_attr.attr, > > + &iio_const_attr_in7_index.dev_attr.attr, > > + &iio_dev_attr_in_type.dev_attr.attr, > > + NULL, > > +}; > > + > > +static mode_t ad7606_scan_el_attr_is_visible(struct kobject *kobj, > > + struct attribute *attr, int n) > > +{ > > + struct device *dev = container_of(kobj, struct device, kobj); > > + struct iio_ring_buffer *ring = dev_get_drvdata(dev); > > + struct iio_dev *indio_dev = ring->indio_dev; > > + struct ad7606_state *st = indio_dev->dev_data; > > + > > + mode_t mode = attr->mode; > > + > Ouch. I think this works out as the same, perhaps simpler to read? > > if ((st->id == ID_AD7606_6) || (st->id == ID_AD7606_4) && > attr == &iio_scan_el_in7.dev_attr.attr || > attr == &iio_const_attr_in7_index.dev_attr.attr || > attr == &iio_scan_el_in6.dev_attr.attr || > attr == &iio_const_attr_in6_index.dev_attr.attr) > mode = 0; > if (st->id == ID_AD7606_4 && > (attr == &iio_scan_el_in4.dev_attr.attr || > attr == &iio_const_attr_in4_index.dev_attr.attr || > attr == &iio_scan_el_in5.dev_attr.attr || > attr == &iio_const_attr_in5_index.dev_attr.attr)) > mode = 0; > > Or could use st->id != ID_AD7606_8 for first condition. > > Actually I'd personally the tests to be on the chip_info->num_channels > as then is more obvious what is going on. > > > + if ((attr == &iio_scan_el_in7.dev_attr.attr || > > + attr == &iio_const_attr_in7_index.dev_attr.attr) && > > + ((st->id == ID_AD7606_6) || (st->id == ID_AD7606_4))) > > + mode = 0; > > + else if ((attr == &iio_scan_el_in6.dev_attr.attr || > > + attr == &iio_const_attr_in6_index.dev_attr.attr) && > > + ((st->id == ID_AD7606_6) || (st->id == ID_AD7606_4))) > > + mode = 0; > > + else if ((attr == &iio_scan_el_in5.dev_attr.attr || > > + attr == &iio_const_attr_in5_index.dev_attr.attr) && > > + (st->id == ID_AD7606_4)) > > + mode = 0; > > + else if ((attr == &iio_scan_el_in4.dev_attr.attr || > > + attr == &iio_const_attr_in4_index.dev_attr.attr) && > > + (st->id == ID_AD7606_4)) > > + mode = 0; > > + > > + return mode; > > +} > > + > > +static struct attribute_group ad7606_scan_el_group = { > > + .name = "scan_elements", > > + .attrs = ad7606_scan_el_attrs, > > + .is_visible = ad7606_scan_el_attr_is_visible, > > +}; > > + > > +int ad7606_scan_from_ring(struct ad7606_state *st, unsigned ch) > > +{ > > + struct iio_ring_buffer *ring = st->indio_dev->ring; > > + int ret; > > + u16 *ring_data; > > + > > + ring_data = kmalloc(ring->access.get_bytes_per_datum(ring), > GFP_KERNEL); > > + if (ring_data == NULL) { > > + ret = -ENOMEM; > > + goto error_ret; > > + } > > + ret = ring->access.read_last(ring, (u8 *) ring_data); > > + if (ret) > > + goto error_free_ring_data; > > + > > + ret = ring_data[ch]; > > + > > +error_free_ring_data: > > + kfree(ring_data); > > +error_ret: > > + return ret; > > +} > > + > > +/** > > + * ad7606_ring_preenable() setup the parameters of the ring before > enabling > > + * > > + * The complex nature of the setting of the nuber of bytes per datum > is due > > + * to this driver currently ensuring that the timestamp is stored at > an 8 > > + * byte boundary. > > + **/ > > +static int ad7606_ring_preenable(struct iio_dev *indio_dev) > > +{ > > + struct ad7606_state *st = indio_dev->dev_data; > > + struct iio_ring_buffer *ring = indio_dev->ring; > > + size_t d_size; > > + > > + d_size = st->chip_info->num_channels * > > + st->chip_info->bits / 8 + sizeof(s64); > > + > > + if (d_size % sizeof(s64)) > > + d_size += sizeof(s64) - (d_size % sizeof(s64)); > > + > > + if (ring->access.set_bytes_per_datum) > > + ring->access.set_bytes_per_datum(ring, d_size); > > + > > + st->d_size = d_size; > > + > > + return 0; > > +} > > + > > +/** > > + * ad7606_poll_func_th() th of trigger launched polling to ring > buffer > > + * > > + **/ > > +static void ad7606_poll_func_th(struct iio_dev *indio_dev, s64 time) > > +{ > > + struct ad7606_state *st = indio_dev->dev_data; > > + gpio_set_value(st->pdata->gpio_convst, 1); > > + > > + return; > > +} > > +/** > > + * ad7606_poll_bh_to_ring() bh of trigger launched polling to ring > buffer > > + * @work_s: the work struct through which this was scheduled > > + * > > + * Currently there is no option in this driver to disable the saving > of > > + * timestamps within the ring. > > + * I think the one copy of this at a time was to avoid problems if > the > > + * trigger was set far too high and the reads then locked up the > computer. > > + **/ > > +static void ad7606_poll_bh_to_ring(struct work_struct *work_s) > > +{ > > + struct ad7606_state *st = container_of(work_s, struct > ad7606_state, > > + poll_work); > > + struct iio_dev *indio_dev = st->indio_dev; > > + struct iio_sw_ring_buffer *sw_ring = iio_to_sw_ring(indio_dev- > >ring); > > + struct iio_ring_buffer *ring = indio_dev->ring; > > + s64 time_ns; > > + __u8 *buf; > > + > > + /* Ensure only one copy of this function running at a time */ > > + if (atomic_inc_return(&st->protect_ring) > 1) > > + return; > > + > > + buf = kzalloc(st->d_size, GFP_KERNEL); > > + if (buf == NULL) > > + return; > > + > > + if (st->have_frstdata) { > > + st->bops->read_block(st->dev, 1, buf); > > + if (!gpio_get_value(st->pdata->gpio_frstdata)) { > > + /* This should never happen */ > With that comment... The question is 'Does it?' If not why is this > check here? Some explanation needed! > > + ad7606_reset(st); > > + goto done; > > + } > > + st->bops->read_block(st->dev, > > + st->chip_info->num_channels - 1, buf + 2); > > + } else { > > + st->bops->read_block(st->dev, > > + st->chip_info->num_channels, buf); > > + } > > + > > + time_ns = iio_get_time_ns(); > > + memcpy(buf + st->d_size - sizeof(s64), &time_ns, > sizeof(time_ns)); > > + > > + ring->access.store_to(&sw_ring->buf, buf, time_ns); > > +done: > > + gpio_set_value(st->pdata->gpio_convst, 0); > > + kfree(buf); > > + atomic_dec(&st->protect_ring); > > +} > > + > > +int ad7606_register_ring_funcs_and_init(struct iio_dev *indio_dev) > > +{ > > + struct ad7606_state *st = indio_dev->dev_data; > > + int ret; > > + > > + indio_dev->ring = iio_sw_rb_allocate(indio_dev); > > + if (!indio_dev->ring) { > > + ret = -ENOMEM; > > + goto error_ret; > > + } > > + > > + /* Effectively select the ring buffer implementation */ > > + iio_ring_sw_register_funcs(&indio_dev->ring->access); > > + ret = iio_alloc_pollfunc(indio_dev, NULL, &ad7606_poll_func_th); > > + if (ret) > > + goto error_deallocate_sw_rb; > > + > > + /* Ring buffer functions - here trigger setup related */ > > + > > + indio_dev->ring->preenable = &ad7606_ring_preenable; > > + indio_dev->ring->postenable = &iio_triggered_ring_postenable; > > + indio_dev->ring->predisable = &iio_triggered_ring_predisable; > > + indio_dev->ring->scan_el_attrs = &ad7606_scan_el_group; > > + > > + INIT_WORK(&st->poll_work, &ad7606_poll_bh_to_ring); > > + > > + /* Flag that polled ring buffering is possible */ > > + indio_dev->modes |= INDIO_RING_TRIGGERED; > > + return 0; > > +error_deallocate_sw_rb: > > + iio_sw_rb_free(indio_dev->ring); > > +error_ret: > > + return ret; > > +} > > + > > +void ad7606_ring_cleanup(struct iio_dev *indio_dev) > > +{ > > + /* ensure that the trigger has been detached */ > This does look suspiciously like a work around for a deficiency in > the core. Is it? > > > + if (indio_dev->trig) { > > + iio_put_trigger(indio_dev->trig); > > + iio_trigger_dettach_poll_func(indio_dev->trig, > > + indio_dev->pollfunc); > > + } > > + kfree(indio_dev->pollfunc); > > + iio_sw_rb_free(indio_dev->ring); > > +} > > diff --git a/drivers/staging/iio/adc/ad7606_spi.c > b/drivers/staging/iio/adc/ad7606_spi.c > > new file mode 100644 > > index 0000000..fb69bba > > --- /dev/null > > +++ b/drivers/staging/iio/adc/ad7606_spi.c > > @@ -0,0 +1,133 @@ > > +/* > > + * AD7606 SPI ADC driver > > + * > > + * Copyright 2011 Analog Devices Inc. > > + * > > + * Licensed under the GPL-2. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/spi/spi.h> > > +#include <linux/types.h> > > +#include <linux/err.h> > > +#include "ad7606.h" > > + > > +#define MAX_SPI_FREQ_HZ 23500000 /* VDRIVE above 4.75 V > */ > > + > > +static int ad7606_spi_read_block(struct device *dev, > > + int count, void *buf) > > +{ > > + struct spi_device *spi = to_spi_device(dev); > > + int i, ret; > > + unsigned short *data = buf; > > + > > + ret = spi_read(spi, (u8 *)buf, count * 2); > > + if (ret < 0) { > > + dev_err(&spi->dev, "SPI read error\n"); > > + return ret; > > + } > > + > > + for (i = 0; i < count; i++) > > + data[i] = be16_to_cpu(data[i]); > > + > > + return 0; > > +} > > + > > +static const struct ad7606_bus_ops ad7606_spi_bops = { > > + .read_block = ad7606_spi_read_block, > > +}; > > + > > +static int __devinit ad7606_spi_probe(struct spi_device *spi) > > +{ > > + struct ad7606_state *st; > > + > I'd as a general rule be cynical and say its the job of the board > file writer to get this right, so could loose this test. Guess > there is no harm in being sure though. > > + /* don't exceed max specified SPI CLK frequency */ > > + if (spi->max_speed_hz > MAX_SPI_FREQ_HZ) { > > + dev_err(&spi->dev, "SPI CLK %d Hz too fast\n", > > + spi->max_speed_hz); > > + return -EINVAL; > > + } > > + > > + st = ad7606_probe(&spi->dev, spi->irq, NULL, > > + spi_get_device_id(spi)->driver_data, > > + &ad7606_spi_bops); > > + > > + if (IS_ERR(st)) > > + return PTR_ERR(st); > > + > > + spi_set_drvdata(spi, st); > > + > > + return 0; > > +} > > + > > +static int __devexit ad7606_spi_remove(struct spi_device *spi) > > +{ > > + struct ad7606_state *st = dev_get_drvdata(&spi->dev); > > + > > + return ad7606_remove(st); > > +} > > + > > +#ifdef CONFIG_PM > > These are identical for the two buses. Admittedly they are trivial, > but can we easily share them between them (i.e. move them into > the core driver code?) > > +static int ad7606_spi_suspend(struct device *dev) > > +{ > > + struct ad7606_state *st = dev_get_drvdata(dev); > > + > > + ad7606_suspend(st); > > + > > + return 0; > > +} > > + > > +static int ad7606_spi_resume(struct device *dev) > > +{ > > + struct ad7606_state *st = dev_get_drvdata(dev); > > + > > + ad7606_resume(st); > > + > > + return 0; > > +} > > + > > +static const struct dev_pm_ops ad7606_pm_ops = { > > + .suspend = ad7606_spi_suspend, > > + .resume = ad7606_spi_resume, > > +}; > > +#define AD7606_SPI_PM_OPS (&ad7606_pm_ops) > > + > > +#else > > +#define AD7606_SPI_PM_OPS NULL > > +#endif > > + > > +static const struct spi_device_id ad7606_id[] = { > > + {"ad7606-8", ID_AD7606_8}, > > + {"ad7606-6", ID_AD7606_6}, > > + {"ad7606-4", ID_AD7606_4}, > > + {} > > +}; > > + > > +static struct spi_driver ad7606_driver = { > > + .driver = { > > + .name = "ad7606", > > + .bus = &spi_bus_type, > > + .owner = THIS_MODULE, > > + .pm = AD7606_SPI_PM_OPS, > > + }, > > + .probe = ad7606_spi_probe, > > + .remove = __devexit_p(ad7606_spi_remove), > > + .id_table = ad7606_id, > > +}; > > + > > +static int __init ad7606_spi_init(void) > > +{ > > + return spi_register_driver(&ad7606_driver); > > +} > > +module_init(ad7606_spi_init); > > + > > +static void __exit ad7606_spi_exit(void) > > +{ > > + spi_unregister_driver(&ad7606_driver); > > +} > > +module_exit(ad7606_spi_exit); > > + > > +MODULE_AUTHOR("Michael Hennerich <hennerich@xxxxxxxxxxxxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Analog Devices AD7606 ADC"); > > +MODULE_LICENSE("GPL v2"); > > +MODULE_ALIAS("spi:ad7606-spi"); -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
