[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20091124232602.GH8290@sortiz.org>
Date: Wed, 25 Nov 2009 00:26:03 +0100
From: Samuel Ortiz <sameo@...ux.intel.com>
To: Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>
Cc: linux-kernel@...r.kernel.org,
Sascha Hauer <s.hauer@...gutronix.de>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>
Subject: Re: [PATCH] mfd/mc13783: near complete rewrite
Hi Uwe,
On Tue, Nov 24, 2009 at 10:44:52PM +0100, Uwe Kleine-König wrote:
> Hi Samuel,
>
> On Fri, Nov 06, 2009 at 01:28:50AM +0100, Samuel Ortiz wrote:
> > On Fri, Nov 06, 2009 at 12:56:08AM +0100, Uwe Kleine-König wrote:
> > > This fixes several things while still providing the old API:
> > >
> > > - simplify and fix locking
> > > - better error handling
> > > - don't ack all irqs making it impossible to detect a reset of the
> > > rtc
> > > - use a timeout variant to wait for completion of ADC conversion
> > > - provide platform-data to regulator subdevice (This allows making
> > > struct mc13783 opaque for other drivers after the regulator driver is
> > > updated to use its platform_data.)
> > > - expose all interrupts
> > > - use threaded irq
> > Thanks Uwe, patch applied to my for-next branch.
>
> I now worked on the touch interface for the mc13783 and found some
> non-critical but non-nice things in the mc13783_adc_do_conversion
> function.
>
> If it looks OK for you, can you please squash the patch below into the
> original commit?
That's fine with me, I'll do it.
Cheers,
Samuel.
> I think the changes are not worth to be mentioned in
> the commit log.
>
> Intensions are:
> - Request and ack irq before provoking the next to behave if there was
> already an ADCDONE irq pending.
>
> - restore TSMOD only after conversions are read out
>
> - assert return value is 0 or -E...
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> ---
> drivers/mfd/mc13783-core.c | 20 ++++++++++++--------
> 1 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mfd/mc13783-core.c b/drivers/mfd/mc13783-core.c
> index 99267ed..dc1add0 100644
> --- a/drivers/mfd/mc13783-core.c
> +++ b/drivers/mfd/mc13783-core.c
> @@ -483,12 +483,13 @@ int mc13783_adc_do_conversion(struct mc13783 *mc13783, unsigned int mode,
> return -EINVAL;
> }
>
> - mc13783_reg_write(mc13783, MC13783_REG_ADC_0, adc0);
> - mc13783_reg_write(mc13783, MC13783_REG_ADC_1, adc1);
> -
> dev_dbg(&mc13783->spidev->dev, "%s: request irq\n", __func__);
> mc13783_irq_request(mc13783, MC13783_IRQ_ADCDONE,
> mc13783_handler_adcdone, __func__, &adcdone_data);
> + mc13783_ackirq(mc13783, MC13783_IRQ_ADCDONE);
> +
> + mc13783_reg_write(mc13783, MC13783_REG_ADC_0, adc0);
> + mc13783_reg_write(mc13783, MC13783_REG_ADC_1, adc1);
>
> mc13783_unlock(mc13783);
>
> @@ -501,15 +502,18 @@ int mc13783_adc_do_conversion(struct mc13783 *mc13783, unsigned int mode,
>
> mc13783_irq_free(mc13783, MC13783_IRQ_ADCDONE, &adcdone_data);
>
> + if (ret > 0)
> + for (i = 0; i < 4; ++i) {
> + ret = mc13783_reg_read(mc13783,
> + MC13783_REG_ADC_2, &sample[i]);
> + if (ret)
> + break;
> + }
> +
> if (mode == MC13783_ADC_MODE_TS)
> /* restore TSMOD */
> mc13783_reg_write(mc13783, MC13783_REG_ADC_0, old_adc0);
>
> - if (ret > 0)
> - for (i = 0; i < 4; ++i)
> - mc13783_reg_read(mc13783,
> - MC13783_REG_ADC_2, &sample[i]);
> -
> mc13783->flags &= ~MC13783_ADC_WORKING;
> out:
> mc13783_unlock(mc13783);
> --
> 1.6.5.2
>
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
--
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists