Re: [PATCH] mfd/mc13783: near complete rewrite

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

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?  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@pengutronix.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/  |