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

From: Uwe Kleine-König
Date: Tue Nov 24 2009 - 16:45:00 EST


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@xxxxxxxxxxxxxx>
---
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/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/