Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755084Ab0B1XDv (ORCPT ); Sun, 28 Feb 2010 18:03:51 -0500 Received: from mga10.intel.com ([192.55.52.92]:20405 "EHLO fmsmga102.fm.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754806Ab0B1XDt (ORCPT ); Sun, 28 Feb 2010 18:03:49 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.49,557,1262592000"; d="scan'208";a="776684984" Date: Mon, 1 Mar 2010 00:05:00 +0100 From: Samuel Ortiz To: Mark Brown Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] mfd: Use completion interrupt for WM831x AUXADC Message-ID: <20100228230459.GC3391@sortiz.org> References: <1266923286-16791-1-git-send-email-broonie@opensource.wolfsonmicro.com> <1266923286-16791-2-git-send-email-broonie@opensource.wolfsonmicro.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1266923286-16791-2-git-send-email-broonie@opensource.wolfsonmicro.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4378 Lines: 136 On Tue, Feb 23, 2010 at 11:08:06AM +0000, Mark Brown wrote: > Use the completion interrupt generated by the device rather than > polling for conversions to complete. As a backup we still check > the status of the AUXADC if we don't get a completion, mostly for > systems that don't have the WM831x interrupt infrastructure hooked > up. Patch applied to my for-next branch, thanks a lot. Cheers, Samuel. > Also reduce the timeout for completion of conversions to 5ms from > the previous 10ms, the lower timeout should be sufficient. > > Signed-off-by: Mark Brown > --- > drivers/mfd/wm831x-core.c | 36 +++++++++++++++++++++++++++++------- > include/linux/mfd/wm831x/core.h | 2 ++ > 2 files changed, 31 insertions(+), 7 deletions(-) > > diff --git a/drivers/mfd/wm831x-core.c b/drivers/mfd/wm831x-core.c > index c428d9f..07101e9 100644 > --- a/drivers/mfd/wm831x-core.c > +++ b/drivers/mfd/wm831x-core.c > @@ -321,7 +321,6 @@ EXPORT_SYMBOL_GPL(wm831x_set_bits); > */ > int wm831x_auxadc_read(struct wm831x *wm831x, enum wm831x_auxadc input) > { > - int tries = 10; > int ret, src; > > mutex_lock(&wm831x->auxadc_lock); > @@ -349,13 +348,14 @@ int wm831x_auxadc_read(struct wm831x *wm831x, enum wm831x_auxadc input) > goto disable; > } > > - do { > - msleep(1); > + /* Ignore the result to allow us to soldier on without IRQ hookup */ > + wait_for_completion_timeout(&wm831x->auxadc_done, msecs_to_jiffies(5)); > > - ret = wm831x_reg_read(wm831x, WM831X_AUXADC_CONTROL); > - if (ret < 0) > - ret = WM831X_AUX_CVT_ENA; > - } while ((ret & WM831X_AUX_CVT_ENA) && --tries); > + ret = wm831x_reg_read(wm831x, WM831X_AUXADC_CONTROL); > + if (ret < 0) { > + dev_err(wm831x->dev, "AUXADC status read failed: %d\n", ret); > + goto disable; > + } > > if (ret & WM831X_AUX_CVT_ENA) { > dev_err(wm831x->dev, "Timed out reading AUXADC\n"); > @@ -390,6 +390,15 @@ out: > } > EXPORT_SYMBOL_GPL(wm831x_auxadc_read); > > +static irqreturn_t wm831x_auxadc_irq(int irq, void *irq_data) > +{ > + struct wm831x *wm831x = irq_data; > + > + complete(&wm831x->auxadc_done); > + > + return IRQ_HANDLED; > +} > + > /** > * wm831x_auxadc_read_uv: Read a voltage from the WM831x AUXADC > * > @@ -1411,6 +1420,7 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) > mutex_init(&wm831x->io_lock); > mutex_init(&wm831x->key_lock); > mutex_init(&wm831x->auxadc_lock); > + init_completion(&wm831x->auxadc_done); > dev_set_drvdata(wm831x->dev, wm831x); > > ret = wm831x_reg_read(wm831x, WM831X_PARENT_ID); > @@ -1523,6 +1533,16 @@ static int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq) > if (ret != 0) > goto err; > > + if (wm831x->irq_base) { > + ret = request_threaded_irq(wm831x->irq_base + > + WM831X_IRQ_AUXADC_DATA, > + NULL, wm831x_auxadc_irq, 0, > + "auxadc", wm831x); > + if (ret < 0) > + dev_err(wm831x->dev, "AUXADC IRQ request failed: %d\n", > + ret); > + } > + > /* The core device is up, instantiate the subdevices. */ > switch (parent) { > case WM8310: > @@ -1593,6 +1613,8 @@ static void wm831x_device_exit(struct wm831x *wm831x) > { > wm831x_otp_exit(wm831x); > mfd_remove_devices(wm831x->dev); > + if (wm831x->irq_base) > + free_irq(wm831x->irq_base + WM831X_IRQ_AUXADC_DATA, wm831x); > wm831x_irq_exit(wm831x); > kfree(wm831x); > } > diff --git a/include/linux/mfd/wm831x/core.h b/include/linux/mfd/wm831x/core.h > index 53580b5..5915f6e 100644 > --- a/include/linux/mfd/wm831x/core.h > +++ b/include/linux/mfd/wm831x/core.h > @@ -15,6 +15,7 @@ > #ifndef __MFD_WM831X_CORE_H__ > #define __MFD_WM831X_CORE_H__ > > +#include > #include > > /* > @@ -261,6 +262,7 @@ struct wm831x { > int num_gpio; > > struct mutex auxadc_lock; > + struct completion auxadc_done; > > /* The WM831x has a security key blocking access to certain > * registers. The mutex is taken by the accessors for locking > -- > 1.7.0 > -- 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/