On Thu, Sep 13, 2012 at 17:43:30, Jonathan Cameron wrote: > On 13/09/12 11:40, Patil, Rachna wrote: > > This patch adds support for TI's ADC driver. > > This is a multifunctional device. > > Analog input lines are provided on which voltage measurements can be > > carried out. > > You can have upto 8 input lines. > > > > Signed-off-by: Patil, Rachna <rachna@xxxxxx> > > There's a little fuzz in applying this due to other drivers that have gone in recently. > > Actually this is going to be 'interesting' to merge. Dmitry, Samuel thoughts on who takes this one and how? Maybe this is a case for a 'special' branch pulled into more than one tree? > > > One minor thing inline. I have an aversion to dynamic allocation of > things that are then constant. > > Also the module name is simply ti_adc. Does seem a little 'vague' > given the range of ADC's TI makes :) Perhaps keep the reference > to the tsc in there? Personally I'd have preferred the whole thing > being named after a particular part number (any one it support would > do) to avoid a clash in future with a new touch screen adc from TI. > Bit late for that though I guess ;) Yes, true. TI definitely might come up with more IP's of this type. This IP(TSC / ADC) is present on AM335x. If necessary we can rename the driver to ti_am335x_XXX. > > Jonathan > > --- > > Changes in v2: > > Addressed review comments from Matthias Kaehlcke > > > > Changes in v3: > > Addressed review comments from Jonathan Cameron. > > Added comments, new line appropriately. > > > > drivers/iio/adc/Kconfig | 7 + > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/ti_adc.c | 223 ++++++++++++++++++++++++++++++++++ > > drivers/mfd/ti_tscadc.c | 18 +++- > > include/linux/mfd/ti_tscadc.h | 9 ++- > > include/linux/platform_data/ti_adc.h | 14 ++ > > 6 files changed, 270 insertions(+), 2 deletions(-) > > create mode 100644 drivers/iio/adc/ti_adc.c > > create mode 100644 include/linux/platform_data/ti_adc.h > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 8a78b4f..ad32df8 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -22,4 +22,11 @@ config AT91_ADC > > help > > Say yes here to build support for Atmel AT91 ADC. > > > > +config TI_ADC > > + tristate "TI's ADC driver" > > + depends on ARCH_OMAP2PLUS > > + help > > + Say yes here to build support for Texas Instruments ADC > > + driver which is also a MFD client. > > + > > endmenu > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 52eec25..a930cee 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -4,3 +4,4 @@ > > > > obj-$(CONFIG_AD7266) += ad7266.o > > obj-$(CONFIG_AT91_ADC) += at91_adc.o > > +obj-$(CONFIG_TI_ADC) += ti_adc.o > > diff --git a/drivers/iio/adc/ti_adc.c b/drivers/iio/adc/ti_adc.c > > new file mode 100644 > > index 0000000..56f8af2 > > --- /dev/null > > +++ b/drivers/iio/adc/ti_adc.c > > @@ -0,0 +1,223 @@ > > +/* > > + * TI ADC MFD driver > > + * > > + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation version 2. > > + * > > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > > + * kind, whether express or implied; without even the implied warranty > > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#include <linux/init.h> > > +#include <linux/kernel.h> > > +#include <linux/err.h> > > +#include <linux/module.h> > > +#include <linux/slab.h> > > +#include <linux/interrupt.h> > > +#include <linux/platform_device.h> > > +#include <linux/io.h> > > +#include <linux/iio/iio.h> > > + > > +#include <linux/mfd/ti_tscadc.h> > > +#include <linux/platform_data/ti_adc.h> > > + > > +struct adc_device { > > + struct ti_tscadc_dev *mfd_tscadc; > > + struct iio_dev *idev; > > + int channels; > > +}; > > + > > +static unsigned int adc_readl(struct adc_device *adc, unsigned int reg) > > +{ > > + return readl(adc->mfd_tscadc->tscadc_base + reg); > > +} > > + > > +static void adc_writel(struct adc_device *adc, unsigned int reg, > > + unsigned int val) > > +{ > > + writel(val, adc->mfd_tscadc->tscadc_base + reg); > > +} > > + > > +static void adc_step_config(struct adc_device *adc_dev) > > +{ > > + unsigned int stepconfig; > > + int i, channels = 0, steps; > > + > > + /* > > + * There are 16 configurable steps and 8 analog input > > + * lines available which are shared between Touchscreen and ADC. > > + * > > + * Steps backwards i.e. from 16 towards 0 are used by ADC > > + * depending on number of input lines needed. > > + * Channel would represent which analog input > > + * needs to be given to ADC to digitalize data. > > + */ > > + > > + steps = TOTAL_STEPS - adc_dev->channels; > > + channels = TOTAL_CHANNELS - adc_dev->channels; > > + > > + stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1; > > + > > + for (i = (steps + 1); i <= TOTAL_STEPS; i++) { > > + adc_writel(adc_dev, REG_STEPCONFIG(i), > > + stepconfig | STEPCONFIG_INP(channels)); > > + adc_writel(adc_dev, REG_STEPDELAY(i), > > + STEPCONFIG_OPENDLY); > > + channels++; > > + } > > + adc_writel(adc_dev, REG_SE, STPENB_STEPENB); > > +} > > + > > +static int tiadc_channel_init(struct iio_dev *idev, int channels) > > +{ > > + struct iio_chan_spec *chan_array; > > + int i; > > + > > + idev->num_channels = channels; > > + chan_array = kcalloc(idev->num_channels, sizeof(struct iio_chan_spec), > > + GFP_KERNEL); > > + > > + if (chan_array == NULL) > > + return -ENOMEM; > > + > Minor point, but I'd be tempted to do this as a static const array and > then just set num_channels appropriately. Still it's such a simple > driver that I'm not that fussed. I looked at some other drivers, they seem to be doing the same way. I would like to go with the existing convention. > > + for (i = 0; i < (idev->num_channels); i++) { > > + struct iio_chan_spec *chan = chan_array + i; > > + chan->type = IIO_VOLTAGE; > > + chan->indexed = 1; > > + chan->channel = i; > > + chan->info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT; > > + } > > + > > + idev->channels = chan_array; > > + > > + return idev->num_channels; > > +} > > + Regards, Rachna -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
