On 2/10/2012 11:17 AM, Marek Vasut wrote:
Logic is clean enough. Never said anything about the implementation and the complexity of doing it ;)On 2/10/2012 5:52 AM, Marek Vasut wrote:Signed-off-by: Marek Vasut<marek.vasut@xxxxxxxxx> Cc: Wolfgang Denk<wd@xxxxxxx> Cc: Stefano Babic<sbabic@xxxxxxx> Cc: Fabio Estevam<festevam@xxxxxxxxx> --- arch/arm/mach-mxs/mach-m28evk.c | 35 ++- drivers/staging/iio/adc/Kconfig | 10 + drivers/staging/iio/adc/Makefile | 1 + drivers/staging/iio/adc/mxs-lradc.c | 666 ++++++++++++++++++++++++++++++++ 4 files changed, 700 insertions(+), 4 deletions(-) create mode 100644 drivers/staging/iio/adc/mxs-lradc.c NOTE: The quality of code is still really crap here, I'm more interested in knowing whether I'm going in the right direction about this.I may well point out stuff you'd fix anyway, but its easier for me that way ;) Anyhow, having taken a look I like the general approach. The 'claim' logic is nice and clean.Really? Or what that irony ? ;-)
The problem with our buffered route is that it is done as one buffer per iio device. This makes sense from the point of view of most devices which have quick means of dumpling a whole 'scan' across the channels. The snag here is you can get different numbers of readings from different channels even when driven by theOne nasty bit I hadn't realized is that you get separate interrupts per channel. This is going to make the data aggregation and demuxing used with buffers interesting if you add that support. So thinking ahead, lets say you have 4 channels with oversampling as ax2,bx1,cx4,dx8But the oversampling happens in hardware. And I get a single interrupt at the end of the sampling.
same underlying trigger signal.
Yes. This would make an interesting test case for our various in kernel interfaces though I appreciate you may not want to take that path as there will be a few corners requiring changes in the IIO core.It will interrupt on each channel as xxxxxxxx a a a a a bbbbbbb c c d Easiest option will be to build up this as a single described 'scan' and push that to the demux. Fiddly and if there is a big oversampling value, everything gets delayed. Maybe we send scans with a flag indicating no data for some fields and demux accordingly. Probably reasonable to restrict a consumer driver to having only one oversampling so as far as they know it will come through at that lower rate. Still all that stuff is for another day. Your driver is fine in as far as it goes!Great, now I can continue to adding touchscreen, temp etc.
Only do that it if it is needed elsewhere, otherwise it should just stay in here even if it looks a little messy.Thanks!Thanks! diff --git a/arch/arm/mach-mxs/mach-m28evk.c b/arch/arm/mach-mxs/mach-m28evk.c index 2f27582..7a37d29 100644 --- a/arch/arm/mach-mxs/mach-m28evk.c +++ b/arch/arm/mach-mxs/mach-m28evk.c @@ -320,6 +320,31 @@ static struct mxs_mmc_platform_data m28evk_mmc_pdata[] __initdata = { }, }; +#define RES_IRQ(id, res) { .name = id, .start = (res), .end =(res),.flags = IORESOURCE_IRQ } + +static struct resource mxs_lradc_rsrc[] = { + [0] = { + .start = 0x80050000, + .end = 0x80050000 + SZ_8K - 1, + .flags = IORESOURCE_MEM, + }, + RES_IRQ("LRADC CH0", MX28_INT_LRADC_CH0), + RES_IRQ("LRADC CH1", MX28_INT_LRADC_CH1), + RES_IRQ("LRADC CH2", MX28_INT_LRADC_CH2), + RES_IRQ("LRADC CH3", MX28_INT_LRADC_CH3), + RES_IRQ("LRADC CH4", MX28_INT_LRADC_CH4), + RES_IRQ("LRADC CH5", MX28_INT_LRADC_CH5), + RES_IRQ("LRADC CH6", MX28_INT_LRADC_CH6), + RES_IRQ("LRADC CH7", MX28_INT_LRADC_CH7), +}; + +static struct platform_device mxs_lradc = { + .name = "mxs-lradc", + .id = -1, + .resource = mxs_lradc_rsrc, + .num_resources = ARRAY_SIZE(mxs_lradc_rsrc), +}; + static void __init m28evk_init(void) { mxs_iomux_setup_multiple_pads(m28evk_pads, ARRAY_SIZE(m28evk_pads)); @@ -347,6 +372,8 @@ static void __init m28evk_init(void) mx28_add_mxs_i2c(0); i2c_register_board_info(0, m28_stk5v3_i2c_boardinfo, ARRAY_SIZE(m28_stk5v3_i2c_boardinfo)); + + platform_device_register(&mxs_lradc); } static void __init m28evk_timer_init(void) diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig index d9decea..7f33032 100644 --- a/drivers/staging/iio/adc/Kconfig +++ b/drivers/staging/iio/adc/Kconfig @@ -193,4 +193,14 @@ config MAX1363_RING_BUFFER Say yes here to include ring buffer support in the MAX1363 ADC driver. +config MXS_LRADC + tristate "Freescale i.MX23/i.MX28 LRADC" + depends on ARCH_MXS + help + Say yes here to build support for i.MX23/i.MX28 LRADC convertor + built into these chips. + + To compile this driver as a module, choose M here: the + module will be called mxs-lradc. + endmenu diff --git a/drivers/staging/iio/adc/Makefile b/drivers/staging/iio/adc/Makefile index ceee7f3..2d764ec 100644 --- a/drivers/staging/iio/adc/Makefile +++ b/drivers/staging/iio/adc/Makefile @@ -37,3 +37,4 @@ obj-$(CONFIG_AD7192) += ad7192.o obj-$(CONFIG_ADT7310) += adt7310.o obj-$(CONFIG_ADT7410) += adt7410.o obj-$(CONFIG_AD7280) += ad7280a.o +obj-$(CONFIG_MXS_LRADC) += mxs-lradc.o diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c new file mode 100644 index 0000000..da50088 --- /dev/null +++ b/drivers/staging/iio/adc/mxs-lradc.c @@ -0,0 +1,666 @@ +/* + * ADT7410 digital temperature sensor driver supporting ADT7410 + * + * Copyright 2010 Analog Devices Inc.Obvious bit to fix here ;) Doesn't bode that well that you have the copyright notice for a driver I've been trying to get the heck out of IIO. *fingers crossed*Heh, thanks for pointing this out.+ * + * Licensed under the GPL-2 or later. + */ + +#include<linux/interrupt.h> +#include<linux/device.h> +#include<linux/kernel.h> +#include<linux/slab.h> +#include<linux/sysfs.h> +#include<linux/list.h> +#include<linux/io.h> +#include<linux/module.h> +#include<linux/platform_device.h> +#include<linux/spinlock.h> +#include<linux/wait.h> +#include<linux/sched.h> + +#include<mach/mxs.h> +#include<mach/common.h> + +#include "../iio.h" +#include "../sysfs.h" + +#define MAPPING_USED (1<< 7) +#define MAPPING_XXX (1<< 6) +#define MAPPING_SET_SLOT(m) (((m)& 0x3)<< 4) +#define MAPPING_GET_SLOT(m) (((m)>> 4)& 0x3) +#define MAPPING_CHAN(m) ((m)& 0xf) + +struct mxs_lradc_mapped_channel { + wait_queue_head_t wq; + bool wq_done; + uint16_t chan_data; + uint16_t mapping; + uint32_t users; +}; + +struct mxs_lradc_drv_data { + struct iio_dev *iio[4]; + void __iomem *mmio_base; + + spinlock_t lock; + + uint16_t claimed; + + struct mxs_lradc_mapped_channel ch[8];yikes. Individual oversample controls. I was assuming these were per sample trigger source. That's going to make for some fiddly data management when you add buffers. We'll have to define the ordering of data very carefully and the demux unit will need adapting to handle this. Fun fun fun. (note the demux unit becomes very important for in kernel consumers of iio buffered data). I really need to get that code out there in a more up to date form. Now I think Greg has been talked round to taking the 'pull interface' set we can hopefully make real progress on that front. I'm never sure if I like 'interesting' hardware or if it just gives me a headache!Right!+ uint8_t ch_oversample[16]; +}; + +struct mxs_lradc_data { + struct mxs_lradc_drv_data *drv_data; + int id; + spinlock_t lock; + + uint16_t claimed; + + uint32_t loop_interval; +}; + + +#define LRADC_CH(n) (0x50 + (0x10 * (n))) +#define LRADC_CH_ACCUMULATE (1<< 29) +#define LRADC_CH_NUM_SAMPLES_MASK (0x1f<< 24) +#define LRADC_CH_NUM_SAMPLES_OFFSET 24 +#define LRADC_CH_VALUE_MASK 0x3ffff +#define LRADC_CH_VALUE_OFFSET 0 + +#define LRADC_DELAY(n) (0xd0 + (0x10 * (n))) +#define LRADC_DELAY_TRIGGER_LRADCS_MASK (0xff<< 24) +#define LRADC_DELAY_TRIGGER_LRADCS_OFFSET 24 +#define LRADC_DELAY_KICK (1<< 20) +#define LRADC_DELAY_TRIGGER_DELAYS_MASK (0xf<< 16) +#define LRADC_DELAY_TRIGGER_DELAYS_OFFSET 16 +#define LRADC_DELAY_LOOP_COUNT_MASK (0x1f<< 11) +#define LRADC_DELAY_LOOP_COUNT_OFFSET 11 +#define LRADC_DELAY_DELAY_MASK 0x7ff +#define LRADC_DELAY_DELAY_OFFSET 0 + +#define LRADC_CTRL0 0x00 + +#define LRADC_CTRL1 0x10 +#define LRADC_CTRL1_LRADC_IRQ(n) (1<< (n)) +#define LRADC_CTRL1_LRADC_IRQ_EN(n) (1<< ((n) + 16)) + +#define LRADC_CTRL2 0x20 +#define LRADC_CTRL2_TEMPSENSE_PWD (1<< 15) + +#define LRADC_CTRL3 0x30 + +#define LRADC_CTRL4 0x140 +#define LRADC_CTRL4_LRADCSELECT_MASK(n) (0xf<< ((n) * 4)) +#define LRADC_CTRL4_LRADCSELECT_OFFSET(n) ((n) * 4)The mess above will go into separate include.
Pretty much as you have done, just with stuff in hz. Getting frequency control to a point where we can get to it from client drivers is on the todo list, but right now what you have is the only way of doing it.+ +/* + * Global IIO attributes + */ +static ssize_t mxs_lradc_show_sampling_rate(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct iio_dev *iio_dev = dev_get_drvdata(dev); + struct mxs_lradc_data *data = iio_priv(iio_dev); + + return sprintf(buf, "%u\n", data->loop_interval); +} + +static ssize_t mxs_lradc_store_sampling_rate(struct device *dev, + struct device_attribute *attr, const char *buf, size_t count) +{ + struct iio_dev *iio_dev = dev_get_drvdata(dev); + struct mxs_lradc_data *data = iio_priv(iio_dev); + struct mxs_lradc_drv_data *drv_data = data->drv_data; + uint32_t reg; + unsigned long lval; + unsigned long lflags; + + if (strict_strtoul(buf, 10,&lval)) + return -EINVAL; + + /* Check if the value is in requested range */ + if (lval>= 2048) + return -EINVAL; + + /* + * Noone is accessing our delay trigger in the LRADC reg space so we + * don't need to claim top-level spinlock. + */ + spin_lock_irqsave(&data->lock, lflags); + + if (data->loop_interval != lval) { + data->loop_interval = lval; + + /* Update the delay channel */ + reg = readl(drv_data->mmio_base + LRADC_DELAY(data->id)); + reg&= ~LRADC_DELAY_DELAY_MASK; + reg |= data->loop_interval; + writel(reg, drv_data->mmio_base + LRADC_DELAY(data->id)); + } + + spin_unlock_irqrestore(&data->lock, lflags); + + return count; +} + +static IIO_DEVICE_ATTR(sampling_rate, S_IRUGO | S_IWUSR, + mxs_lradc_show_sampling_rate, + mxs_lradc_store_sampling_rate, 0);Hmm.. Not abi compliant, we probably do need to come up with a format for specifying such range restrictions. Also note sampling rate is always in hz. This will require some fixed point fiddling but the interface has to be predictable.Yep, I didn't know how to actually handle this. Since this configuration is per- iio device, unlike the channel oversampling. Any pointers welcome.
As mentioned above, that would be easy without those pesky oversampling controls.+static IIO_CONST_ATTR(sampling_rate_available, + "0...2047 (in 1/2000 second steps)"); + +static struct attribute *mxs_lradc_attributes[] = { + &iio_dev_attr_sampling_rate.dev_attr.attr, + &iio_const_attr_sampling_rate_available.dev_attr.attr, + NULL, +}; + +static int mxs_lradc_can_claim_channel(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan) +{ + struct mxs_lradc_data *data = iio_priv(iio_dev); + struct mxs_lradc_drv_data *drv_data = data->drv_data; + int i, count = 0; + + /* The channel is already claimed by us. */ + if (data->claimed& (1<< chan->address)) + return 0; + + /* Check if someone else didn't claim this */ + if (drv_data->claimed& (1<< chan->address)) + return -EINVAL; + + for (i = 0; i< 16; i++) + if (drv_data->claimed& (1<< i)) + count++; + + /* Too many channels claimed */ + if (count == 8) + return -EINVAL; + + return 0; +} + +static int mxs_lradc_claim_channel(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan) +{ + struct mxs_lradc_data *data = iio_priv(iio_dev); + struct mxs_lradc_drv_data *drv_data = data->drv_data; + int i, ret; + unsigned long gflags; + uint32_t overspl = 0; + + spin_lock_irqsave(&drv_data->lock, gflags); + + ret = mxs_lradc_can_claim_channel(iio_dev, chan); + if (ret) + goto err; + + /* Claim the channel */ + drv_data->claimed |= 1<< chan->address; + data->claimed |= 1<< chan->address; + + /* Map the channel */ + for (i = 0; i< 8; i++) + if (!(drv_data->ch[i].mapping& MAPPING_USED)) + break; + + drv_data->ch[i].mapping = MAPPING_USED | + MAPPING_SET_SLOT(data->id) | + MAPPING_CHAN(chan->address); + + /* Setup the mapping */ + __mxs_clrl(LRADC_CTRL4_LRADCSELECT_MASK(i), + drv_data->mmio_base + LRADC_CTRL4); + __mxs_setl(chan->address<< LRADC_CTRL4_LRADCSELECT_OFFSET(i), + drv_data->mmio_base + LRADC_CTRL4); + + spin_unlock_irqrestore(&drv_data->lock, gflags); + + overspl = + ((drv_data->ch_oversample[chan->address] ? 1 : 0) + * LRADC_CH_ACCUMULATE) | + (drv_data->ch_oversample[chan->address] + << LRADC_CH_NUM_SAMPLES_OFFSET); + writel(overspl, drv_data->mmio_base + LRADC_CH(i)); + + /* Enable IRQ on the channel */ + __mxs_clrl(LRADC_CTRL1_LRADC_IRQ(i), + drv_data->mmio_base + LRADC_CTRL1); + __mxs_setl(LRADC_CTRL1_LRADC_IRQ_EN(i), + drv_data->mmio_base + LRADC_CTRL1); + + /* Set out channel to be triggers by this delay queue */ + __mxs_setl(1<< (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i), + drv_data->mmio_base + LRADC_DELAY(data->id)); + + return 0; + +err: + spin_unlock_irqrestore(&drv_data->lock, gflags); + return -EINVAL; +} + +static void mxs_lradc_relinquish_channel(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan, int chanidx) +{ + struct mxs_lradc_data *data = iio_priv(iio_dev); + struct mxs_lradc_drv_data *drv_data = data->drv_data; + unsigned long gflags; + int i; + + drv_data->ch[chanidx].users--; + if (drv_data->ch[chanidx].users == 0) { + /* No more users for this channel, stop generating interrupts */ + __mxs_setl(LRADC_CTRL1_LRADC_IRQ(i) | + LRADC_CTRL1_LRADC_IRQ_EN(i), + drv_data->mmio_base + LRADC_CTRL1); + + /* Don't trigger this channel */ + __mxs_clrl(1<< (LRADC_DELAY_TRIGGER_LRADCS_OFFSET + i), + drv_data->mmio_base + LRADC_DELAY(data->id)); + + spin_lock_irqsave(&drv_data->lock, gflags); + + /* Relinquish this channel */ + drv_data->claimed&= ~(1<< chan->address); + data->claimed&= ~(1<< chan->address); + drv_data->ch[chanidx].mapping = 0; + + spin_unlock_irqrestore(&drv_data->lock, gflags); + } +} + +/* + * I/O operations + */ +static int mxs_lradc_read_raw(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan, + int *val, int *val2, long m) +{ + struct mxs_lradc_data *data = iio_priv(iio_dev); + struct mxs_lradc_drv_data *drv_data = data->drv_data; + unsigned long lflags; + int i, ret; + + switch (m) { + case 0: + spin_lock_irqsave(&data->lock, lflags); +So we claim the channel for this IIO dev (which is one of the 4 timed triggers). Then wait for the next sample.CorrectGuess that does the job, though this is all faking a kind of polled device whereas this will fit much better as a interrupt driven device outputting via buffers (probably the next step once this is fine).This is actually a good idea since the sampling is constantly running anyway and it'd allow for the data to be readily available without the delay.Note that we typically disable this sort of access if the device is doing interrupt driven reading. I have been thinking about adding a buffer implementation that would provide just the last value and hence allowing this sort of single channel read, but that requires some infrastructure that hasn't merged yet.Heh. Maybe I should add buffer support into the driver and simply push the stuff into the buffers. I'd need to study this.
Yes. If you register buffers you get such a chrdev. But note that the data coming out is raw scans across the channels. All the IIO_ST stuff in the chan_spec and scan_index values etc are about describing to userspace what it is getting. Take a look at the example userspace code in the documentation directory. That is probably the best way of understanding the outputs.+ ret = mxs_lradc_claim_channel(iio_dev, chan); + if (ret) { + spin_unlock_irqrestore(&data->lock, lflags); + return ret; + } + + /* + * Once we are here, the channel is mapped by us already. + * Find the mapping. + */ + for (i = 0; i< 8; i++) { + if (!(drv_data->ch[i].mapping& MAPPING_USED)) + continue; + + if (MAPPING_CHAN(drv_data->ch[i].mapping) == + chan->address) + break; + } + + /* Wait until sampling is done */ + drv_data->ch[i].wq_done = false; + + drv_data->ch[i].users++; + + spin_unlock_irqrestore(&data->lock, lflags); + + ret = wait_event_interruptible(drv_data->ch[i].wq, + drv_data->ch[i].wq_done); + if (ret) + ret = -EINTR; + else { + *val = readl(drv_data->mmio_base + LRADC_CH(i))& + LRADC_CH_VALUE_MASK; + *val /= (drv_data->ch_oversample[chan->address] + 1); + ret = IIO_VAL_INT; + } + + spin_lock_irqsave(&data->lock, lflags); + + mxs_lradc_relinquish_channel(iio_dev, chan, i); + + spin_unlock_irqrestore(&data->lock, lflags); + + return ret; + + case IIO_CHAN_INFO_OVERSAMPLE_COUNT: + *val = drv_data->ch_oversample[chan->address]; + return IIO_VAL_INT; + } + + return -EINVAL; +} + +static int mxs_lradc_write_raw(struct iio_dev *iio_dev, + const struct iio_chan_spec *chan, + int val, int val2, long m) +{ + struct mxs_lradc_data *data = iio_priv(iio_dev); + struct mxs_lradc_drv_data *drv_data = data->drv_data; + + switch (m) { + case IIO_CHAN_INFO_OVERSAMPLE_COUNT: + if ((val<= 0) || (val>= 32)) + return -EINVAL; + + drv_data->ch_oversample[chan->address] = val - 1; + return 0; + } + + return -EINVAL; +} + +static irqreturn_t mxs_lradc_handle_irq(int irq, void *data) +{ + struct mxs_lradc_drv_data *drv_data = data; + uint32_t reg = readl(drv_data->mmio_base + LRADC_CTRL1); + int i; + + for (i = 0; i< 8; i++) + if (reg& LRADC_CTRL1_LRADC_IRQ(i)) { + drv_data->ch[i].wq_done = true; + wake_up_interruptible(&drv_data->ch[i].wq); + } + + __mxs_clrl(0xffff, drv_data->mmio_base + LRADC_CTRL1); + + return IRQ_HANDLED; +} + +static const struct iio_info mxs_lradc_iio_info = { + .driver_module = THIS_MODULE, + .read_raw = mxs_lradc_read_raw, + .write_raw = mxs_lradc_write_raw, +}; +These all look good... However, one thing you weren't to know is that we are trying to deprecate the IIO_CHAN macro. It's become a bit of a maintenance nightmare given the underlying structure keeps getting more complex as new requirements turn up. Arnd warned me about this when I originally put it in, but it took me a while to realise how right he was!Roger that, I can even add a much simpler one.Either define a local macro filling just those fields that matter here or fill the structure directly. Also you could leave out the IIO_ST stuff for now as it only means anything in a driver providing buffered access via the chrdev. Same is true of scan_index.This is something I still don't clearly understand (and I know I'm asking for the third time or so). How do I do continuous sampling (aka. opening a file and reading stuff from it)? Since if I do sysfs read, this triggers raw_read() which opens the device and closes it. So is there some /dev/ representation of the iio device or something?
Agreed. I was just observing that the local copy of iio_priv is only used here so you could copy each one in turn into the same local variable rather than using different elements of+static const struct iio_chan_spec mxs_lradc_chan_spec[] = { + [0] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 0, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 0, 0, IIO_ST('u', 18, 32, 0), 0), + [1] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 1, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 1, 1, IIO_ST('u', 18, 32, 0), 0), + [2] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 2, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 2, 2, IIO_ST('u', 18, 32, 0), 0), + [3] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 3, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 3, 3, IIO_ST('u', 18, 32, 0), 0), + [4] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 4, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 4, 4, IIO_ST('u', 18, 32, 0), 0), + [5] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 5, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 5, 5, IIO_ST('u', 18, 32, 0), 0), + [6] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 6, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 6, 6, IIO_ST('u', 18, 32, 0), 0), + /* VBATT */ + [7] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 7, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 7, 7, IIO_ST('u', 18, 32, 0), 0), + /* Temp sense 0 */ + [8] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 8, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 8, 8, IIO_ST('u', 18, 32, 0), 0), + /* Temp sense 1 */ + [9] = IIO_CHAN(IIO_TEMP, IIO_NO_MOD, 1, IIO_RAW, NULL, 9, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 9, 9, IIO_ST('u', 18, 32, 0), 0), + /* VDDIO */ + [10] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 10, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 10, 10, IIO_ST('u', 18, 32, 0), 0), + /* VTH */ + [11] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 11, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 11, 11, IIO_ST('u', 18, 32, 0), 0), + /* VDDA */ + [12] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 12, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 12, 12, IIO_ST('u', 18, 32, 0), 0), + /* VDDD */ + [13] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 13, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 13, 13, IIO_ST('u', 18, 32, 0), 0), + /* VBG */ + [14] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 14, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 14, 14, IIO_ST('u', 18, 32, 0), 0), + /* VDD5V */ + [15] = IIO_CHAN(IIO_VOLTAGE, IIO_NO_MOD, 1, IIO_RAW, NULL, 15, 0, + IIO_CHAN_INFO_SCALE_SEPARATE_BIT | + IIO_CHAN_INFO_OVERSAMPLE_COUNT_SEPARATE_BIT, + 15, 15, IIO_ST('u', 18, 32, 0), 0), +}; + +static void mxs_lradc_config(struct mxs_lradc_drv_data *drv_data) +{ + /* FIXME */ + int freq = 0x3; /* 6MHz */ + int onchip_ground_ref = 0; + + int i; + + mxs_reset_block(drv_data->mmio_base + LRADC_CTRL0); + + if (onchip_ground_ref) + __mxs_setl(1<< 26, drv_data->mmio_base + LRADC_CTRL0); + else + __mxs_clrl(1<< 26, drv_data->mmio_base + LRADC_CTRL0); + + __mxs_clrl(0x3<< 8, drv_data->mmio_base + LRADC_CTRL3); + __mxs_setl(freq, drv_data->mmio_base + LRADC_CTRL3); + + /* The delay channels constantly retrigger themself */ + for (i = 0; i< 4; i++) + __mxs_setl(LRADC_DELAY_KICK | + (1<< (LRADC_DELAY_TRIGGER_DELAYS_OFFSET + i)) | + 0x7ff, /* FIXME */ + drv_data->mmio_base + LRADC_DELAY(i)); + + /* Start temperature sensing */ + writel(0, drv_data->mmio_base + LRADC_CTRL2); +} + +/*static void mxs_lradc_config(struct mxs_lradc_pdata *pdata) +{ + +} +*/ +static int __devinit mxs_lradc_probe(struct platform_device *pdev) +{ + struct mxs_lradc_data *data[4]; + struct mxs_lradc_drv_data *drv_data; + struct iio_dev *iio; + struct resource *r; + int ret = 0; + int irq; + int i; + + /* + * DEVM management + */ + if (!devres_open_group(&pdev->dev, mxs_lradc_probe, GFP_KERNEL)) { + dev_err(&pdev->dev, "Can't open resource group\n"); + goto err0; + } + + drv_data = devm_kzalloc(&pdev->dev, sizeof(*drv_data), GFP_KERNEL); + if (!drv_data) { + dev_err(&pdev->dev, "Failed to allocate driver data\n"); + ret = -ENOMEM; + goto err0; + } + + spin_lock_init(&drv_data->lock); + + /* + * IIO ops + */ + for (i = 0; i< 4; i++) { + iio = iio_allocate_device(sizeof(*data)); + if (!iio) { + dev_err(&pdev->dev, + "Failed to allocate IIO device %i\n", i); + ret = -ENOMEM; + goto err1; + } + + iio->name = pdev->name;You will probably want to add an id to that name to distinguish between the difference instances. Even better would be to have a text description of each one.+ iio->dev.parent =&pdev->dev; + iio->info =&mxs_lradc_iio_info; + iio->modes = INDIO_DIRECT_MODE; + /* Channels */ + iio->channels = mxs_lradc_chan_spec; + iio->num_channels = ARRAY_SIZE(mxs_lradc_chan_spec); + iio->attrs = mxs_lradc_attributes; +Given data[i] is only used in here, why not just have one of them?These are actually four distinct private data for four distinct iio-devs.
a local array of pointers.
+ data[i] = iio_priv(iio); + data[i]->drv_data = drv_data; + data[i]->id = i; + + spin_lock_init(&data[i]->lock); + + drv_data->iio[i] = iio; + } + + for (i = 0; i< 8; i++) + init_waitqueue_head(&drv_data->ch[i].wq); + + dev_set_drvdata(&pdev->dev, drv_data); + + /* + * Allocate address space + */ + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (r == NULL) { + dev_err(&pdev->dev, "No I/O memory resource defined\n"); + ret = -ENODEV; + goto err1; + } + + r = devm_request_mem_region(&pdev->dev, r->start, + resource_size(r), pdev->name); + if (r == NULL) { + dev_err(&pdev->dev, "Failed to request I/O memory\n"); + ret = -EBUSY; + goto err1; + } + + drv_data->mmio_base = devm_ioremap(&pdev->dev, r->start, resource_size(r)); + if (!drv_data->mmio_base) { + dev_err(&pdev->dev, "Failed to map I/O memory\n"); + ret = -EBUSY; + goto err1; + } + + /* + * Allocate IRQ + */ + for (irq = 0; irq< 8; irq++) { + r = platform_get_resource(pdev, IORESOURCE_IRQ, irq); + if (r == NULL) { + dev_err(&pdev->dev, "No IRQ resource defined\n"); + ret = -ENODEV; + goto err1; + } + + ret = request_irq(r->start, mxs_lradc_handle_irq, 0, r->name, drv_data); + if (ret) { + dev_err(&pdev->dev, "request_irq %i failed: %d\n", irq,ret);+ ret = -EBUSY; + goto err1; + } + } + + /* + * Register IIO device + */ + for (i = 0; i< 4; i++) { + ret = iio_device_register(drv_data->iio[i]); + if (ret) { + dev_err(&pdev->dev, "Failed to register IIO device\n"); + ret = -EBUSY; + goto err2; + } + } + + devres_remove_group(&pdev->dev, mxs_lradc_probe); + + mxs_lradc_config(drv_data); + + return 0; + +err2: + while (--i>= 0) +Clearing out irqs?Missed that one with all the devm stuff, thanks :)iio_device_unregister(drv_data->iio[i]); +err1: + for (i = 0; i< 4; i++) + if (drv_data->iio[i]) + iio_free_device(drv_data->iio[i]); +err0: + devres_release_group(&pdev->dev, mxs_lradc_probe); + return ret; +} + +static int __devexit mxs_lradc_remove(struct platform_device *pdev) +{ + struct mxs_lradc_drv_data *drv_data = dev_get_drvdata(&pdev->dev); + struct resource *r; + int i, irq; +Obvious but use ARRAY_SIZE(drv_data->iio) instead of 4.This stupidity is not present only here. The driver is still full of arbitrary constants and crud. Thanks for the guidance! M+ for (i = 0; i< 4; i++) + iio_device_unregister(drv_data->iio[i]); + + for (irq = 0; irq< 8; irq++) { + r = platform_get_resource(pdev, IORESOURCE_IRQ, irq); + if (r != NULL) + free_irq(r->start, drv_data); + } + + for (i = 0; i< 4; i++) + iio_free_device(drv_data->iio[i]); + + return 0; +} + +static struct platform_driver mxs_lradc_driver = { + .driver = { + .name = "mxs-lradc", + }, + .probe = mxs_lradc_probe, + .remove = __devexit_p(mxs_lradc_remove), +}; + +module_platform_driver(mxs_lradc_driver); + +MODULE_AUTHOR("Marek Vasut<marek.vasut@xxxxxxxxx>"); +MODULE_DESCRIPTION("Freescale i.MX23/i.MX28 LRADC driver"); +MODULE_LICENSE("GPL v2");
-- 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
