linux-kernel - Re: [PATCH] iio/adc: ingenic: Fix AUX/VBAT readings when touchscreen is used

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bbd87ebab678e3033545fef749c3b22a@artur-rojek.eu>
Date:   Wed, 04 Nov 2020 23:25:12 +0100
From:   Artur Rojek <contact@...ur-rojek.eu>
To:     Paul Cercueil <paul@...pouillou.net>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>, od@...c.me,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH] iio/adc: ingenic: Fix AUX/VBAT readings when touchscreen
 is used

Hi Paul,

On 2020-11-03 21:12, Paul Cercueil wrote:
> When the command feature of the ADC is used, it is possible to program
> the ADC, and specify at each step what input should be processed, and 
> in
> comparison to what reference.
> 
> This broke the AUX and battery readings when the touchscreen was
> enabled, most likely because the CMD feature would change the VREF all
> the time.
> 
> Now, when AUX or battery are read, we temporarily disable the CMD
> feature, which means that we won't get touchscreen readings in that 
> time
> frame. But it now gives correct values for AUX / battery, and the
> touchscreen isn't disabled for long enough to be an actual issue.
> 
> Fixes: b96952f498db ("IIO: Ingenic JZ47xx: Add touchscreen mode.")
> Cc: <stable@...r.kernel.org>
> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
> ---
>  drivers/iio/adc/ingenic-adc.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/ingenic-adc.c 
> b/drivers/iio/adc/ingenic-adc.c
> index 92b25083e23f..ecaff6a9b716 100644
> --- a/drivers/iio/adc/ingenic-adc.c
> +++ b/drivers/iio/adc/ingenic-adc.c
> @@ -177,13 +177,12 @@ static void ingenic_adc_set_config(struct
> ingenic_adc *adc,
>  	mutex_unlock(&adc->lock);
>  }
> 
> -static void ingenic_adc_enable(struct ingenic_adc *adc,
> -			       int engine,
> -			       bool enabled)
> +static void ingenic_adc_enable_unlocked(struct ingenic_adc *adc,
> +					int engine,
> +					bool enabled)
>  {
>  	u8 val;
> 
> -	mutex_lock(&adc->lock);
>  	val = readb(adc->base + JZ_ADC_REG_ENABLE);
> 
>  	if (enabled)
> @@ -192,20 +191,42 @@ static void ingenic_adc_enable(struct ingenic_adc 
> *adc,
>  		val &= ~BIT(engine);
> 
>  	writeb(val, adc->base + JZ_ADC_REG_ENABLE);
> +}
> +
> +static void ingenic_adc_enable(struct ingenic_adc *adc,
> +			       int engine,
> +			       bool enabled)
> +{
> +	mutex_lock(&adc->lock);
> +	ingenic_adc_enable_unlocked(adc, engine, enabled);
>  	mutex_unlock(&adc->lock);
>  }
> 
>  static int ingenic_adc_capture(struct ingenic_adc *adc,
>  			       int engine)
>  {
> +	u32 cfg;
>  	u8 val;
>  	int ret;
> 
> -	ingenic_adc_enable(adc, engine, true);
> +	/*
> +	 * Disable CMD_SEL temporarily, because it causes wrong VBAT 
> readings,
> +	 * probably due to the switch of VREF. We must keep the lock here to
> +	 * avoid races with the buffer enable/disable functions.
> +	 */
> +	mutex_lock(&adc->lock);
> +	cfg = readl(adc->base + JZ_ADC_REG_CFG);
> +	writel(cfg & ~JZ_ADC_REG_CFG_CMD_SEL, adc->base + JZ_ADC_REG_CFG);
> +
> +
Redundant empty line.
> +	ingenic_adc_enable_unlocked(adc, engine, true);
>  	ret = readb_poll_timeout(adc->base + JZ_ADC_REG_ENABLE, val,
>  				 !(val & BIT(engine)), 250, 1000);
>  	if (ret)
> -		ingenic_adc_enable(adc, engine, false);
> +		ingenic_adc_enable_unlocked(adc, engine, false);
> +
> +	writel(cfg, adc->base + JZ_ADC_REG_CFG);
> +	mutex_unlock(&adc->lock);
> 
>  	return ret;
>  }

With the above nitpick corrected:
Acked-by: Artur Rojek <contact@...ur-rojek.eu>