* [PATCH] iio/adc: ingenic: Fix AUX/VBAT readings when touchscreen is used
@ 2020-11-03 20:12 Paul Cercueil
2020-11-04 22:25 ` Artur Rojek
0 siblings, 1 reply; 3+ messages in thread
From: Paul Cercueil @ 2020-11-03 20:12 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler,
Artur Rojek
Cc: od, linux-iio, linux-kernel, Paul Cercueil, stable
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@vger.kernel.org>
Signed-off-by: Paul Cercueil <paul@crapouillou.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);
+
+
+ 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;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio/adc: ingenic: Fix AUX/VBAT readings when touchscreen is used
2020-11-03 20:12 [PATCH] iio/adc: ingenic: Fix AUX/VBAT readings when touchscreen is used Paul Cercueil
@ 2020-11-04 22:25 ` Artur Rojek
2020-11-08 15:37 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Artur Rojek @ 2020-11-04 22:25 UTC (permalink / raw)
To: Paul Cercueil
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald-Stadler, od,
linux-iio, linux-kernel, stable
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@vger.kernel.org>
> Signed-off-by: Paul Cercueil <paul@crapouillou.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@artur-rojek.eu>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio/adc: ingenic: Fix AUX/VBAT readings when touchscreen is used
2020-11-04 22:25 ` Artur Rojek
@ 2020-11-08 15:37 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2020-11-08 15:37 UTC (permalink / raw)
To: Artur Rojek
Cc: Paul Cercueil, Lars-Peter Clausen, Peter Meerwald-Stadler, od,
linux-iio, linux-kernel, stable
On Wed, 04 Nov 2020 23:25:12 +0100
Artur Rojek <contact@artur-rojek.eu> wrote:
> 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@vger.kernel.org>
> > Signed-off-by: Paul Cercueil <paul@crapouillou.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@artur-rojek.eu>
>
fixed up whilst applying.
Applied to the fixes-togreg branch of iio.git. Thanks,
Jonathan
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-08 15:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 20:12 [PATCH] iio/adc: ingenic: Fix AUX/VBAT readings when touchscreen is used Paul Cercueil
2020-11-04 22:25 ` Artur Rojek
2020-11-08 15:37 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).