linux-kernel - Re: [PATCH 3.14 37/96] iio: mxs-lradc: only update the buffer when its conversions have finished

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <550A0BED.8010300@gmail.com>
Date:	Thu, 19 Mar 2015 01:36:13 +0200
From:	Kristina Martšenko 
	<kristina.martsenko@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Marek Vasut <marex@...x.de>,
	Jonathan Cameron <jic23@...nel.org>
Subject: Re: [PATCH 3.14 37/96] iio: mxs-lradc: only update the buffer when
 its conversions have finished

On 16/03/15 16:09, Greg Kroah-Hartman wrote:
> 3.14-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Kristina Martšenko <kristina.martsenko@...il.com>
> 
> commit 89bb35e200bee745c539a96666e0792301ca40f1 upstream.
> 
> Using the touchscreen while running buffered capture results in the
> buffer reporting lots of wrong values, often just zeros. This is because
> we push readings to the buffer every time a touchscreen interrupt
> arrives, including when the buffer's own conversions have not yet
> finished. So let's only push to the buffer when its conversions are
> ready.
> 
> Signed-off-by: Kristina Martšenko <kristina.martsenko@...il.com>
> Reviewed-by: Marek Vasut <marex@...x.de>
> Signed-off-by: Jonathan Cameron <jic23@...nel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> 
> ---
>  drivers/staging/iio/adc/mxs-lradc.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -1160,9 +1160,12 @@ static irqreturn_t mxs_lradc_handle_irq(
>  	}
>  
>  	if (iio_buffer_enabled(iio))
> -		iio_trigger_poll(iio->trig, iio_get_time_ns());
> -	else if (reg & LRADC_CTRL1_LRADC_IRQ(0))
> +	if (iio_buffer_enabled(iio)) {
> +		if (reg & lradc->buffer_vchans)
> +			iio_trigger_poll(iio->trig, iio_get_time_ns());
> +	} else if (reg & LRADC_CTRL1_LRADC_IRQ(0)) {
>  		complete(&lradc->completion);
> +	}
>  
>  	mxs_lradc_reg_clear(lradc, reg & clr_irq, LRADC_CTRL1);

I think something got messed up here, this now has both these lines:

	if (iio_buffer_enabled(iio))
	if (iio_buffer_enabled(iio)) {

which makes the 'else' case unreachable, and breaks reading the ADC
through sysfs. It should just be the second line.

Greg, can you fix it up for the next version? Let me know if I need to
do something. And sorry for not looking at this sooner.

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