MIME-Version: 1.0
In-Reply-To: <20091127193636.GA16866@sortiz.org>
References: <1259146071-2372-1-git-send-email-amit.kucheria@verdurent.com>
	 <20091127193636.GA16866@sortiz.org>
Date: Mon, 30 Nov 2009 15:58:39 +0200
Message-ID: <37786d4b0911300558i984d2bdj9b0c5617503c1950@mail.gmail.com>
Subject: Re: [PATCH] mfd: twl4030: Driver for twl4030 madc module
From: Amit Kucheria <amit.kucheria@verdurent.com>
To: Samuel Ortiz <sameo@linux.intel.com>
Cc: List Linux Kernel <linux-kernel@vger.kernel.org>,
       linux-omap@vger.kernel.org, Mikko Ylinen <mikko.k.ylinen@nokia.com>,
       khali@linux-fr.org
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 23251
Lines: 699

Hi Samuel,

On Fri, Nov 27, 2009 at 9:36 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Amit,
>
> On Wed, Nov 25, 2009 at 12:47:51PM +0200, Amit Kucheria wrote:
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index af0fc90..df1897b 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -25,8 +25,9 @@ obj-$(CONFIG_TPS65010) ? ? ? ? ? ? ?+= tps65010.o
>> ?obj-$(CONFIG_MENELAUS) ? ? ? ? ? ? ? += menelaus.o
>>
>> ?obj-$(CONFIG_TWL4030_CORE) ? += twl4030-core.o twl4030-irq.o
>> -obj-$(CONFIG_TWL4030_POWER) ? ?+= twl4030-power.o
>> +obj-$(CONFIG_TWL4030_POWER) ?+= twl4030-power.o
>> ?obj-$(CONFIG_TWL4030_CODEC) ?+= twl4030-codec.o
>> +obj-$(CONFIG_TWL4030_MADC) ? += twl4030-madc.o
>>
>> ?obj-$(CONFIG_MFD_MC13783) ? ?+= mc13783-core.o
>>
>> diff --git a/drivers/mfd/twl4030-madc.c b/drivers/mfd/twl4030-madc.c
>> new file mode 100644
>> index 0000000..90ce99a
>> --- /dev/null
>> +++ b/drivers/mfd/twl4030-madc.c
>> @@ -0,0 +1,548 @@
>> +/*
>> + * drivers/i2c/chips/twl4030-madc.c
> drivers/mfd/twl4030-madc.c
> By the way, have you considered moving this one to drivers/hwmon ?

I haven't. I moved it from i2c/chips/ to the most obvious place I
could think of - drivers/mfd. But wasn't this the point of mfd - that
various subcomponents drivers could live there instead of being
scattered across the driver tree?

Adding Jean to CC, if he would consider taking this driver in drivers/hwmon.

>> + * TWL4030 MADC module driver
>> + *
>> + * Copyright (C) 2008 Nokia Corporation
>> + * Mikko Ylinen <mikko.k.ylinen@nokia.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
>> + * 02110-1301 USA
>> + *
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/fs.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/i2c/twl4030.h>
>> +#include <linux/i2c/twl4030-madc.h>
>> +
>> +#include <asm/uaccess.h>
>> +
>> +#define TWL4030_MADC_PFX ? ? "twl4030-madc: "
>> +
>> +struct twl4030_madc_data {
>> + ? ? struct device ? ? ? ? ? *dev;
>> + ? ? struct mutex ? ? ? ? ? ?lock;
>> + ? ? struct work_struct ? ? ?ws;
>> + ? ? struct twl4030_madc_request ? ? requests[TWL4030_MADC_NUM_METHODS];
> Typically, I'd expect to have a list (limited in length) of requests here, but
> I guess you're happy with that array .
>
>
>
>> + ? ? int imr;
>> + ? ? int isr;
>> +};
>> +
>> +static struct twl4030_madc_data *the_madc;
> I dont particularly enjoy that. All of the twl4030 drivers have this bad habit
> of assuming they will be alone on a platform. Although it's certainly very
> likely, I still think having this kind of design is not so nice.

I agree. Unfortunately, twl4030-core.c is unable to handle multiple
devices currently. See note from line 779 in twl4030-core below:

/* NOTE:  this driver only handles a single twl4030/tps659x0 chip */
static int
twl4030_probe(struct i2c_client *client, const struct i2c_device_id *id)

>> +static int twl4030_madc_set_current_generator(struct twl4030_madc_data *madc, int chan, int on);
>> +
>> +static
>> +const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = {
>> + ? ? [TWL4030_MADC_RT] = {
>> + ? ? ? ? ? ? .sel ? ?= TWL4030_MADC_RTSELECT_LSB,
>> + ? ? ? ? ? ? .avg ? ?= TWL4030_MADC_RTAVERAGE_LSB,
>> + ? ? ? ? ? ? .rbase ?= TWL4030_MADC_RTCH0_LSB,
>> + ? ? },
>> + ? ? [TWL4030_MADC_SW1] = {
>> + ? ? ? ? ? ? .sel ? ?= TWL4030_MADC_SW1SELECT_LSB,
>> + ? ? ? ? ? ? .avg ? ?= TWL4030_MADC_SW1AVERAGE_LSB,
>> + ? ? ? ? ? ? .rbase ?= TWL4030_MADC_GPCH0_LSB,
>> + ? ? ? ? ? ? .ctrl ? = TWL4030_MADC_CTRL_SW1,
>> + ? ? },
>> + ? ? [TWL4030_MADC_SW2] = {
>> + ? ? ? ? ? ? .sel ? ?= TWL4030_MADC_SW2SELECT_LSB,
>> + ? ? ? ? ? ? .avg ? ?= TWL4030_MADC_SW2AVERAGE_LSB,
>> + ? ? ? ? ? ? .rbase ?= TWL4030_MADC_GPCH0_LSB,
>> + ? ? ? ? ? ? .ctrl ? = TWL4030_MADC_CTRL_SW2,
>> + ? ? },
>> +};
>> +
>> +static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg)
>> +{
>> + ? ? int ret;
>> + ? ? u8 val;
>> +
>> + ? ? ret = twl4030_i2c_read_u8(TWL4030_MODULE_MADC, &val, reg);
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(madc->dev, "unable to read register 0x%X\n", reg);
>> + ? ? ? ? ? ? return ret;
>> + ? ? }
>> +
>> + ? ? return val;
>> +}
> FWIW, you're not checking the return value on any of the madc_read calls
> below.

I've changed the dev_dbg above to dev_err now. If we see those error
messages, then anything that follows from the higher level functions
is overhead IMHO.
The higher level functions in this case aren't adding any more useful
information to the error. E.g. I could check the return value again in
twl4030_madc_channel_raw_read() below. But if would simply repeat the
same error message we show in twl4030_madc_read().

Hmm, perhaps twl4030_madc_read should return void?

>> +static void twl4030_madc_write(struct twl4030_madc_data *madc, u8 reg, u8 val)
>> +{
>> + ? ? int ret;
>> +
>> + ? ? ret = twl4030_i2c_write_u8(TWL4030_MODULE_MADC, val, reg);
>> + ? ? if (ret)
>> + ? ? ? ? ? ? dev_err(madc->dev, "unable to write register 0x%X\n", reg);
>> +}
>> +
>> +static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
>> +{
>> + ? ? u8 msb, lsb;
>> +
>> + ? ? /* For each ADC channel, we have MSB and LSB register pair. MSB address
>> + ? ? ?* is always LSB address+1. reg parameter is the addr of LSB register */
>> + ? ? msb = twl4030_madc_read(madc, reg + 1);
>> + ? ? lsb = twl4030_madc_read(madc, reg);
>> +
>> + ? ? return (int)(((msb << 8) | lsb) >> 6);
>> +}
>> +
>> +static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
>> + ? ? ? ? ? ? u8 reg_base, u16 channels, int *buf)
>> +{
>> + ? ? int count = 0;
>> + ? ? u8 reg, i;
>> +
>> + ? ? if (unlikely(!buf))
>> + ? ? ? ? ? ? return 0;
>> +
>> + ? ? for (i = 0; i < TWL4030_MADC_MAX_CHANNELS; i++) {
>> + ? ? ? ? ? ? if (channels & (1<<i)) {
>> + ? ? ? ? ? ? ? ? ? ? reg = reg_base + 2*i;
>> + ? ? ? ? ? ? ? ? ? ? buf[i] = twl4030_madc_channel_raw_read(madc, reg);
>> + ? ? ? ? ? ? ? ? ? ? count++;
>> + ? ? ? ? ? ? }
>> + ? ? }
>> + ? ? return count;
>> +}
>> +
>> +static void twl4030_madc_enable_irq(struct twl4030_madc_data *madc, int id)
>> +{
>> + ? ? u8 val;
>> +
>> + ? ? val = twl4030_madc_read(madc, madc->imr);
>> + ? ? val &= ~(1 << id);
>> + ? ? twl4030_madc_write(madc, madc->imr, val);
>> +}
>> +
>> +static void twl4030_madc_disable_irq(struct twl4030_madc_data *madc, int id)
>> +{
>> + ? ? u8 val;
>> +
>> + ? ? val = twl4030_madc_read(madc, madc->imr);
>> + ? ? val |= (1 << id);
>> + ? ? twl4030_madc_write(madc, madc->imr, val);
>> +}
>> +
>> +static irqreturn_t twl4030_madc_irq_handler(int irq, void *_madc)
>> +{
>> + ? ? struct twl4030_madc_data *madc = _madc;
>> + ? ? u8 isr_val, imr_val;
>> + ? ? int i;
>> +
>> +#ifdef CONFIG_LOCKDEP
>> + ? ? /* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
>> + ? ? ?* we don't want and can't tolerate. ?Although it might be
>> + ? ? ?* friendlier not to borrow this thread context...
>> + ? ? ?*/
>> + ? ? local_irq_enable();
>> +#endif
> I'm curious, what's special about madc so that it can't tolerate that ?

This was part of some bugfixes by David Brownell:
http://markmail.org/message/zlisiurni3ei6gst
TBH, I am not upto speed on the working of LOCKDEP.

>> + ? ? /* Use COR to ack interrupts since we have no shared IRQs in ISRx */
>> + ? ? isr_val = twl4030_madc_read(madc, madc->isr);
>> + ? ? imr_val = twl4030_madc_read(madc, madc->imr);
>> +
>> + ? ? isr_val &= ~imr_val;
>> +
>> + ? ? for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
>> +
>> + ? ? ? ? ? ? if (!(isr_val & (1<<i)))
>> + ? ? ? ? ? ? ? ? ? ? continue;
>> +
>> + ? ? ? ? ? ? twl4030_madc_disable_irq(madc, i);
>> + ? ? ? ? ? ? madc->requests[i].result_pending = 1;
>> + ? ? }
>> +
>> + ? ? schedule_work(&madc->ws);
>> +
>> + ? ? return IRQ_HANDLED;
>> +}
>> +
>> +static void twl4030_madc_work(struct work_struct *ws)
>> +{
>> + ? ? const struct twl4030_madc_conversion_method *method;
>> + ? ? struct twl4030_madc_data *madc;
>> + ? ? struct twl4030_madc_request *r;
>> + ? ? int len, i;
>> +
>> + ? ? madc = container_of(ws, struct twl4030_madc_data, ws);
>> + ? ? mutex_lock(&madc->lock);
>> +
>> + ? ? for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
>> +
>> + ? ? ? ? ? ? r = &madc->requests[i];
>> +
>> + ? ? ? ? ? ? /* No pending results for this method, move to next one */
>> + ? ? ? ? ? ? if (!r->result_pending)
>> + ? ? ? ? ? ? ? ? ? ? continue;
>> +
>> + ? ? ? ? ? ? method = &twl4030_conversion_methods[r->method];
>> +
>> + ? ? ? ? ? ? /* Read results */
>> + ? ? ? ? ? ? len = twl4030_madc_read_channels(madc, method->rbase,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?r->channels, r->rbuf);
>> +
>> + ? ? ? ? ? ? /* Return results to caller */
>> + ? ? ? ? ? ? if (r->func_cb != NULL) {
>> + ? ? ? ? ? ? ? ? ? ? r->func_cb(len, r->channels, r->rbuf);
>> + ? ? ? ? ? ? ? ? ? ? r->func_cb = NULL;
>> + ? ? ? ? ? ? }
>> +
>> + ? ? ? ? ? ? /* Free request */
>> + ? ? ? ? ? ? r->result_pending = 0;
>> + ? ? ? ? ? ? r->active ? ? ? ? = 0;
>> + ? ? }
>> +
>> + ? ? mutex_unlock(&madc->lock);
>> +}
> I think you may want to consider using a threaded irq here, see
> request_threaded_irq().

I am working on moving the entire twl* driver set to use threaded irqs
on the side. Will you consider merging this code with the work_struct
since it is known to work while I work on the conversion?

>> +static int twl4030_madc_set_irq(struct twl4030_madc_data *madc,
>> + ? ? ? ? ? ? struct twl4030_madc_request *req)
>> +{
>> + ? ? struct twl4030_madc_request *p;
>> +
>> + ? ? p = &madc->requests[req->method];
>> +
>> + ? ? memcpy(p, req, sizeof *req);
>> +
>> + ? ? twl4030_madc_enable_irq(madc, req->method);
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static inline void twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
>> + ? ? ? ? ? ? int conv_method)
>> +{
>> + ? ? const struct twl4030_madc_conversion_method *method;
>> +
>> + ? ? method = &twl4030_conversion_methods[conv_method];
>> +
>> + ? ? switch (conv_method) {
>> + ? ? case TWL4030_MADC_SW1:
>> + ? ? case TWL4030_MADC_SW2:
>> + ? ? ? ? ? ? twl4030_madc_write(madc, method->ctrl, TWL4030_MADC_SW_START);
>> + ? ? ? ? ? ? break;
>> + ? ? case TWL4030_MADC_RT:
>> + ? ? default:
>> + ? ? ? ? ? ? break;
>> + ? ? }
>> +}
>> +
>> +static int twl4030_madc_wait_conversion_ready(
>> + ? ? ? ? ? ? struct twl4030_madc_data *madc,
>> + ? ? ? ? ? ? unsigned int timeout_ms, u8 status_reg)
>> +{
>> + ? ? unsigned long timeout;
>> +
>> + ? ? timeout = jiffies + msecs_to_jiffies(timeout_ms);
>> + ? ? do {
>> + ? ? ? ? ? ? int reg;
>> +
>> + ? ? ? ? ? ? reg = twl4030_madc_read(madc, status_reg);
>> + ? ? ? ? ? ? if (unlikely(reg < 0))
>> + ? ? ? ? ? ? ? ? ? ? return reg;
>> + ? ? ? ? ? ? if (!(reg & TWL4030_MADC_BUSY) && (reg & TWL4030_MADC_EOC_SW))
>> + ? ? ? ? ? ? ? ? ? ? return 0;
>> + ? ? } while (!time_after(jiffies, timeout));
>> +
>> + ? ? return -EAGAIN;
>> +}
>> +
>> +static int twl4030_madc_set_power(struct twl4030_madc_data *madc, int on);
>> +int twl4030_madc_conversion(struct twl4030_madc_request *req)
>> +{
>> + ? ? const struct twl4030_madc_conversion_method *method;
>> + ? ? u8 ch_msb, ch_lsb;
>> + ? ? int ret;
>> +
>> + ? ? if (unlikely(!req))
>> + ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? mutex_lock(&the_madc->lock);
>> +
>> + ? ? twl4030_madc_set_power(the_madc, 1);
>> +
>> + ? ? /* Do we have a conversion request ongoing */
>> + ? ? if (the_madc->requests[req->method].active) {
>> + ? ? ? ? ? ? ret = -EBUSY;
>> + ? ? ? ? ? ? goto out;
>> + ? ? }
>> +
>> + ? ? ch_msb = (req->channels >> 8) & 0xff;
>> + ? ? ch_lsb = req->channels & 0xff;
>> +
>> + ? ? method = &twl4030_conversion_methods[req->method];
>> +
>> + ? ? /* Select channels to be converted */
>> + ? ? twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
>> + ? ? twl4030_madc_write(the_madc, method->sel, ch_lsb);
>> +
>> + ? ? /* Select averaging for all channels if do_avg is set */
>> + ? ? if (req->do_avg) {
>> + ? ? ? ? ? ? twl4030_madc_write(the_madc, method->avg + 1, ch_msb);
>> + ? ? ? ? ? ? twl4030_madc_write(the_madc, method->avg, ch_lsb);
>> + ? ? }
>> +
>> + ? ? if ((req->type == TWL4030_MADC_IRQ_ONESHOT) && (req->func_cb != NULL)) {
>> + ? ? ? ? ? ? twl4030_madc_set_irq(the_madc, req);
>> + ? ? ? ? ? ? twl4030_madc_start_conversion(the_madc, req->method);
>> + ? ? ? ? ? ? the_madc->requests[req->method].active = 1;
>> + ? ? ? ? ? ? ret = 0;
>> + ? ? ? ? ? ? goto out;
>> + ? ? }
>> +
>> + ? ? /* With RT method we should not be here anymore */
>> + ? ? if (req->method == TWL4030_MADC_RT) {
>> + ? ? ? ? ? ? ret = -EINVAL;
>> + ? ? ? ? ? ? goto out;
>> + ? ? }
>> +
>> + ? ? twl4030_madc_start_conversion(the_madc, req->method);
>> + ? ? the_madc->requests[req->method].active = 1;
>> +
>> + ? ? /* Wait until conversion is ready (ctrl register returns EOC) */
>> + ? ? ret = twl4030_madc_wait_conversion_ready(the_madc, 5, method->ctrl);
> So, you're busy looping with all conversions ? I have no idea how often this
> is called, but putting the caller to sleep on e.g. a waitqueue would be more
> elegant.

I suspect this was done for latency reasons since this is used for
battery charging software.
Mikko can you comment on this?

>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(the_madc->dev, "conversion timeout!\n");
>> + ? ? ? ? ? ? the_madc->requests[req->method].active = 0;
>> + ? ? ? ? ? ? goto out;
>> + ? ? }
>> +
>> + ? ? ret = twl4030_madc_read_channels(the_madc, method->rbase, req->channels,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?req->rbuf);
>> +
>> + ? ? the_madc->requests[req->method].active = 0;
>> +
>> + ? ? twl4030_madc_set_power(the_madc, 0);
>> +
>> +out:
>> + ? ? mutex_unlock(&the_madc->lock);
>> +
>> + ? ? return ret;
>> +}
>> +EXPORT_SYMBOL(twl4030_madc_conversion);
> Who's supposed to use this one ?

Nokia AV accessory detection code uses this. So does the BCI battery
driver from TI. The BCI driver has been removed from the omap tree
pending the merge of the madc driver upstream.

>> +static int twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
>> + ? ? ? ? ? ? int chan, int on)
>> +{
>> + ? ? int ret;
>> + ? ? u8 regval;
>> +
>> + ? ? /* Current generator is only available for ADCIN0 and ADCIN1. NB:
>> + ? ? ?* ADCIN1 current generator only works when AC or VBUS is present */
>> + ? ? if (chan > 1)
>> + ? ? ? ? ? ? return EINVAL;
>> +
>> + ? ? ret = twl4030_i2c_read_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &regval, TWL4030_BCI_BCICTL1);
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(madc->dev, "unable to read register 0x%X\n", TWL4030_BCI_BCICTL1);
>> + ? ? ? ? ? ? return ret;
>> + ? ? }
>> +
>> + ? ? if (on) {
>> + ? ? ? ? ? ? regval |= (chan) ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
>> + ? ? ? ? ? ? regval |= TWL4030_BCI_MESBAT;
>> + ? ? }
>> + ? ? else {
>> + ? ? ? ? ? ? regval &= (chan) ? ~TWL4030_BCI_ITHEN : ~TWL4030_BCI_TYPEN;
>> + ? ? ? ? ? ? regval &= ~TWL4030_BCI_MESBAT;
>> + ? ? }
>> +
>> + ? ? ret = twl4030_i2c_write_u8(TWL4030_MODULE_MAIN_CHARGE,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?regval, TWL4030_BCI_BCICTL1);
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(madc->dev, "unable to write register 0x%X\n", TWL4030_BCI_BCICTL1);
>> + ? ? }
>> + ? ? return ret;
>> +}
>> +
>> +static int twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
>> +{
>> + ? ? int ret = 0;
>> + ? ? u8 regval;
>> +
>> + ? ? if (on) {
>> + ? ? ? ? ? ? regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
>> + ? ? ? ? ? ? regval |= TWL4030_MADC_MADCON;
>> + ? ? ? ? ? ? twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
>> +
>> + ? ? ? ? ? ? ret |= twl4030_madc_set_current_generator(madc, 0, 1);
>> +
>> + ? ? } else {
>> + ? ? ? ? ? ? ret |= twl4030_madc_set_current_generator(madc, 0, 0);
>> +
>> + ? ? ? ? ? ? regval = twl4030_madc_read(madc, TWL4030_MADC_CTRL1);
>> + ? ? ? ? ? ? regval &= ~TWL4030_MADC_MADCON;
>> + ? ? ? ? ? ? twl4030_madc_write(madc, TWL4030_MADC_CTRL1, regval);
>> + ? ? }
>> + ? ? return ret;
>> +}
>> +
>> +static long twl4030_madc_ioctl(struct file *filp, unsigned int cmd,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned long arg)
>> +{
>> + ? ? struct twl4030_madc_user_parms par;
>> + ? ? int val, ret;
>> +
>> + ? ? ret = copy_from_user(&par, (void __user *) arg, sizeof(par));
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(the_madc->dev, "copy_from_user: %d\n", ret);
>> + ? ? ? ? ? ? return -EACCES;
>> + ? ? }
>> +
>> + ? ? switch (cmd) {
>> + ? ? case TWL4030_MADC_IOCX_ADC_RAW_READ: {
>> + ? ? ? ? ? ? struct twl4030_madc_request req;
>> + ? ? ? ? ? ? if (par.channel >= TWL4030_MADC_MAX_CHANNELS)
>> + ? ? ? ? ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? ? ? ? ? req.channels = (1 << par.channel);
>> + ? ? ? ? ? ? req.do_avg ? ? ?= par.average;
>> + ? ? ? ? ? ? req.method ? ? ?= TWL4030_MADC_SW1;
>> + ? ? ? ? ? ? req.func_cb ? ? = NULL;
>> + ? ? ? ? ? ? req.type ? ? ? ?= TWL4030_MADC_WAIT;
>> +
>> + ? ? ? ? ? ? val = twl4030_madc_conversion(&req);
>> + ? ? ? ? ? ? if (likely(val > 0)) {
>> + ? ? ? ? ? ? ? ? ? ? par.status = 0;
>> + ? ? ? ? ? ? ? ? ? ? par.result = (u16)req.rbuf[par.channel];
>> + ? ? ? ? ? ? } else if (val == 0) {
>> + ? ? ? ? ? ? ? ? ? ? par.status = -ENODATA;
>> + ? ? ? ? ? ? } else {
>> + ? ? ? ? ? ? ? ? ? ? par.status = val;
>> + ? ? ? ? ? ? }
>> + ? ? ? ? ? ? break;
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?}
>> + ? ? default:
>> + ? ? ? ? ? ? return -EINVAL;
>> + ? ? }
>> +
>> + ? ? ret = copy_to_user((void __user *) arg, &par, sizeof(par));
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(the_madc->dev, "copy_to_user: %d\n", ret);
>> + ? ? ? ? ? ? return -EACCES;
>> + ? ? }
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static struct file_operations twl4030_madc_fileops = {
>> + ? ? .owner = THIS_MODULE,
>> + ? ? .unlocked_ioctl = twl4030_madc_ioctl
>> +};
>> +
>> +static struct miscdevice twl4030_madc_device = {
>> + ? ? .minor = MISC_DYNAMIC_MINOR,
>> + ? ? .name = "twl4030-adc",
>> + ? ? .fops = &twl4030_madc_fileops
>> +};
>> +
>> +static int __init twl4030_madc_probe(struct platform_device *pdev)
>> +{
>> + ? ? struct twl4030_madc_data *madc;
>> + ? ? struct twl4030_madc_platform_data *pdata = pdev->dev.platform_data;
>> +
>> + ? ? int ret;
>> + ? ? u8 regval;
>> +
>> + ? ? madc = kzalloc(sizeof *madc, GFP_KERNEL);
>> + ? ? if (!madc)
>> + ? ? ? ? ? ? return -ENOMEM;
>> +
>> + ? ? if (!pdata) {
>> + ? ? ? ? ? ? dev_dbg(&pdev->dev, "platform_data not available\n");
>> + ? ? ? ? ? ? ret = -EINVAL;
>> + ? ? ? ? ? ? goto err_pdata;
>> + ? ? }
>> +
>> + ? ? madc->imr = (pdata->irq_line == 1) ? TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
>> + ? ? madc->isr = (pdata->irq_line == 1) ? TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
>> +
>> + ? ? ret = misc_register(&twl4030_madc_device);
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(&pdev->dev, "could not register misc_device\n");
>> + ? ? ? ? ? ? goto err_misc;
>> + ? ? }
>> +
>> + ? ? ret = request_irq(platform_get_irq(pdev, 0), twl4030_madc_irq_handler,
>> + ? ? ? ? ? ? ? ? ? ? ? 0, "twl4030_madc", madc);
>> + ? ? if (ret) {
>> + ? ? ? ? ? ? dev_dbg(&pdev->dev, "could not request irq\n");
>> + ? ? ? ? ? ? goto err_irq;
>> + ? ? }
>> +
>> + ? ? platform_set_drvdata(pdev, madc);
>> + ? ? mutex_init(&madc->lock);
>> + ? ? INIT_WORK(&madc->ws, twl4030_madc_work);
>> +
>> + ? ? the_madc = madc;
>> +
>> + ? ? return 0;
>> +
>> +err_irq:
>> + ? ? misc_deregister(&twl4030_madc_device);
>> +
>> +err_misc:
>> +err_pdata:
>> + ? ? kfree(madc);
>> +
>> + ? ? return ret;
>> +}
>> +
>> +static int __exit twl4030_madc_remove(struct platform_device *pdev)
>> +{
>> + ? ? struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
>> +
>> + ? ? free_irq(platform_get_irq(pdev, 0), madc);
>> + ? ? cancel_work_sync(&madc->ws);
>> + ? ? misc_deregister(&twl4030_madc_device);
>> +
>> + ? ? return 0;
>> +}
>> +
>> +static struct platform_driver twl4030_madc_driver = {
>> + ? ? .probe ? ? ? ? ?= twl4030_madc_probe,
>> + ? ? .remove ? ? ? ? = __exit_p(twl4030_madc_remove),
>> + ? ? .driver ? ? ? ? = {
>> + ? ? ? ? ? ? .name ? = "twl4030_madc",
>> + ? ? ? ? ? ? .owner ?= THIS_MODULE,
>> + ? ? },
>> +};
>> +
>> +static int __init twl4030_madc_init(void)
>> +{
>> + ? ? return platform_driver_register(&twl4030_madc_driver);
>> +}
>> +module_init(twl4030_madc_init);
>> +
>> +static void __exit twl4030_madc_exit(void)
>> +{
>> + ? ? platform_driver_unregister(&twl4030_madc_driver);
>> +}
>> +module_exit(twl4030_madc_exit);
>> +
>> +MODULE_ALIAS("platform:twl4030-madc");
>> +MODULE_AUTHOR("Nokia Corporation");
>> +MODULE_DESCRIPTION("twl4030 ADC driver");
>> +MODULE_LICENSE("GPL");
>> +
>> diff --git a/include/linux/i2c/twl4030-madc.h b/include/linux/i2c/twl4030-madc.h
>> new file mode 100644
>> index 0000000..24523b5
>> --- /dev/null
>> +++ b/include/linux/i2c/twl4030-madc.h
>> @@ -0,0 +1,126 @@
>> +/*
>> + * include/linux/i2c/twl4030-madc.h
>> + *
>> + * TWL4030 MADC module driver header
>> + *
>> + * Copyright (C) 2008 Nokia Corporation
>> + * Mikko Ylinen <mikko.k.ylinen@nokia.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but
>> + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the GNU
>> + * General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
>> + * 02110-1301 USA
>> + *
>> + */
>> +
>> +#ifndef _TWL4030_MADC_H
>> +#define _TWL4030_MADC_H
>> +
>> +struct twl4030_madc_conversion_method {
>> + ? ? u8 sel;
>> + ? ? u8 avg;
>> + ? ? u8 rbase;
>> + ? ? u8 ctrl;
>> +};
>> +
>> +#define TWL4030_MADC_MAX_CHANNELS 16
>> +
>> +struct twl4030_madc_request {
>> + ? ? u16 channels;
>> + ? ? u16 do_avg;
>> + ? ? u16 method;
>> + ? ? u16 type;
>> + ? ? int active;
>> + ? ? int result_pending;
> active and result_pending can be bool.
>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/