On 02/04/11 14:27, Hennerich, Michael wrote: >> 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 Then lets call it oversampling_ratio and make this very obvious. > >>> + >>> +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. >>> + <snip> >>> 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? Fair point. > > >>> + 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. Can you add that to the comment please for future readers of the code. > >>> + ad7606_reset(st); >>> + ret = -EIO; >>> + goto error_ret; >>> + } <snip> -- 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
