Consumers expect the buffer to only contain enabled channels. While
preparing the buffer, the driver also (incorrectly) inserts empty data
for disabled channels, causing the enabled channels to appear at wrong
offsets. Fix that.

Fixes: b96952f498db ("IIO: Ingenic JZ47xx: Add touchscreen mode.")
Tested-by: Paul Cercueil <paul@crapouillou.net>
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---
 drivers/iio/adc/ingenic-adc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index a7325dbbb99a..5a932c375a89 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -804,11 +804,10 @@ static irqreturn_t ingenic_adc_irq(int irq, void *data)
 	unsigned int i;
 	u32 tdat[3];
 
-	for (i = 0; i < ARRAY_SIZE(tdat); mask >>= 2, i++) {
+	memset(tdat, 0, ARRAY_SIZE(tdat));
+	for (i = 0; mask && i < ARRAY_SIZE(tdat); mask >>= 2) {
 		if (mask & 0x3)
-			tdat[i] = readl(adc->base + JZ_ADC_REG_ADTCH);
-		else
-			tdat[i] = 0;
+			tdat[i++] = readl(adc->base + JZ_ADC_REG_ADTCH);
 	}
 
 	iio_push_to_buffers(iio_dev, tdat);
-- 
2.37.2

On Wed, Aug 17, 2022 at 1:58 PM Artur Rojek <contact@artur-rojek.eu> wrote:
>
> Consumers expect the buffer to only contain enabled channels. While
> preparing the buffer, the driver also (incorrectly) inserts empty data
> for disabled channels, causing the enabled channels to appear at wrong
> offsets. Fix that.

What consumers? Have you tested on all of them? Please, elaborate. It
might be that some of them have to be fixed. In such case you need to
report the issue to their respective channels and put the
corresponding links here.

P.S. It doesn't mean I'm against the patch.

-- 
With Best Regards,
Andy Shevchenko

Hi Andy,

Le ven., août 19 2022 at 11:12:38 +0300, Andy Shevchenko 
<andy.shevchenko@gmail.com> a écrit :
> On Wed, Aug 17, 2022 at 1:58 PM Artur Rojek <contact@artur-rojek.eu> 
> wrote:
>> 
>>  Consumers expect the buffer to only contain enabled channels. While
>>  preparing the buffer, the driver also (incorrectly) inserts empty 
>> data
>>  for disabled channels, causing the enabled channels to appear at 
>> wrong
>>  offsets. Fix that.
> 
> What consumers? Have you tested on all of them? Please, elaborate. It
> might be that some of them have to be fixed. In such case you need to
> report the issue to their respective channels and put the
> corresponding links here.

There are no consumers to fix, only this driver. I believe it  wasn't 
noticed until now because all consumers were only using channels 0 and 
1.

Cheers,
-Paul

> P.S. It doesn't mean I'm against the patch.
> 
> --
> With Best Regards,
> Andy Shevchenko

On Fri, Aug 19, 2022 at 1:07 PM Paul Cercueil <paul@crapouillou.net> wrote:
>
> Hi Andy,
>
> Le ven., août 19 2022 at 11:12:38 +0300, Andy Shevchenko
> <andy.shevchenko@gmail.com> a écrit :
> > On Wed, Aug 17, 2022 at 1:58 PM Artur Rojek <contact@artur-rojek.eu>
> > wrote:
> >>
> >>  Consumers expect the buffer to only contain enabled channels. While
> >>  preparing the buffer, the driver also (incorrectly) inserts empty
> >> data
> >>  for disabled channels, causing the enabled channels to appear at
> >> wrong
> >>  offsets. Fix that.
> >
> > What consumers? Have you tested on all of them? Please, elaborate. It
> > might be that some of them have to be fixed. In such case you need to
> > report the issue to their respective channels and put the
> > corresponding links here.
>
> There are no consumers to fix, only this driver. I believe it  wasn't
> noticed until now because all consumers were only using channels 0 and
> 1.

Something like this explanation is missed in the commit message, with that
added (in the above or similar form)

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> > P.S. It doesn't mean I'm against the patch.


-- 
With Best Regards,
Andy Shevchenko