Hi Jonathan,

Le ven., août 19 2022 at 18:53:39 +0100, Jonathan Cameron 
[off-list ref] a écrit :

On Wed, 17 Aug 2022 12:56:43 +0200
Artur Rojek [off-list ref] wrote:

quoted

 Don't try to access buffer data of a channel by its scan index. 
Instead,
 use the newly introduced `iio_find_channel_offset_in_buffer` to get 
the
 correct data offset.

 The scan index of a channel does not represent its position in a 
buffer,
 as the buffer will contain data for enabled channels only, affecting
 data offsets and alignment.

 Fixes: 2c2b364fddd5 ("Input: joystick - add ADC attached joystick 
driver.")
 Reported-by: Chris Morgan [off-list ref]
 Tested-by: Paul Cercueil [off-list ref]
 Signed-off-by: Artur Rojek [off-list ref]
 ---
  drivers/input/joystick/adc-joystick.c | 26 
+++++++++++++++++---------
  1 file changed, 17 insertions(+), 9 deletions(-)

 diff --git a/drivers/input/joystick/adc-joystick.c 
b/drivers/input/joystick/adc-joystick.c
 index c0deff5d4282..aed853ebe1d1 100644
 --- a/drivers/input/joystick/adc-joystick.c
 +++ b/drivers/input/joystick/adc-joystick.c
 @@ -6,6 +6,7 @@
  #include <linux/ctype.h>
  #include <linux/input.h>
  #include <linux/iio/iio.h>
 +#include <linux/iio/buffer.h>
  #include <linux/iio/consumer.h>
  #include <linux/module.h>
  #include <linux/platform_device.h>
 @@ -46,36 +47,43 @@ static void adc_joystick_poll(struct input_dev 
*input)
  static int adc_joystick_handle(const void *data, void *private)
  {
  	struct adc_joystick *joy = private;
 +	struct iio_buffer *buffer;
  	enum iio_endian endianness;
 -	int bytes, msb, val, idx, i;
 -	const u16 *data_u16;
 +	int bytes, msb, val, off;
 +	const u8 *chan_data;
 +	unsigned int i;
  	bool sign;

  	bytes = joy->chans[0].channel->scan_type.storagebits >> 3;

  	for (i = 0; i < joy->num_chans; ++i) {
 -		idx = joy->chans[i].channel->scan_index;
  		endianness = joy->chans[i].channel->scan_type.endianness;
  		msb = joy->chans[i].channel->scan_type.realbits - 1;
  		sign = tolower(joy->chans[i].channel->scan_type.sign) == 's';
 +		buffer = iio_channel_cb_get_iio_buffer(joy->buffer);
 +		off = iio_find_channel_offset_in_buffer(joy->chans[i].indio_dev,
 +							joy->chans[i].channel,
 +							buffer);

With this call replaced with one that instead uses

		off = iio_find_channel_offset_in_buffer(joy->chans, i);

which I'm fairly sure is enough via the info in chans[x]->channel to 
establish this offset.

All is good, though you should probably cache it as doing that maths 
every
time seems excessive.

quoted

 +		if (off < 0)
 +			return off;
 +
 +		chan_data = (const u8 *)data + off;

  		switch (bytes) {
  		case 1:
 -			val = ((const u8 *)data)[idx];
 +			val = *chan_data;
  			break;
  		case 2:
 -			data_u16 = (const u16 *)data + idx;
 -
  			/*
  			 * Data is aligned to the sample size by IIO core.
  			 * Call `get_unaligned_xe16` to hide type casting.
  			 */
  			if (endianness == IIO_BE)
 -				val = get_unaligned_be16(data_u16);
 +				val = get_unaligned_be16(chan_data);

I obviously missed this previously but these are aligned so we don't 
need the
unaligned form.

Yes, the comment above says that it's used to hide type casting.

Cheers,
-Paul

quoted

  			else if (endianness == IIO_LE)
 -				val = get_unaligned_le16(data_u16);
 +				val = get_unaligned_le16(chan_data);
  			else /* IIO_CPU */
 -				val = *data_u16;
 +				val = *(const u16 *)chan_data;
  			break;
  		default:
  			return -EINVAL;