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

[Samuel Ortiz <sameo@linux.intel.com>]

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@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/  |

-- 
Intel Open Source Technology Centre
http://oss.intel.com/