On Sat, Apr 09, 2022 at 12:08:57PM +0200, Artur Rojek wrote:
> Hi Chris & Dmitry,
>
> On 2022-04-09 04:07, Dmitry Torokhov wrote:
> > Hi Chris,
> >
> > On Fri, Apr 08, 2022 at 04:28:57PM -0500, Chris Morgan wrote:
> > > From: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > >
> > > For my Odroid Go Advance I noticed that the adc-joystick driver was
> > > only reporting the y channel and on the x axis. After debugging, I
> > > found that the driver was trying to read values from channels 0 and
> > > 1 even though my device is using channels 1 and 2. By changing the
> > > code
> > > to use the axis index instead of the scan index when unpacking the
> > > data
> > > from the buffer, the joystick begins working as expected.
> >
> > This sounds like some sort of misconfiguration, as your change
> > effectively removes the ability of using just some ADC channels for
> > joystick functionality...
>
> I agree, this sounds like either a case of misconfiguration, or an issue in
> the ADC driver that this device is using.
> The axis index corresponds to the iio channel associated with the joystick,
> but NOT to the order at which data is sampled by ADC.
> That's why each channel has a `scan_index` field. It sounds like in Chris'
> case the channels have wrong scan indices.
> I'd start by verifying that in the ADC driver that is being used.
>
> In any case, this patch is wrong and removes functionality that existing
> devices depend on.
I appreciate the feedback. If this driver is working as expected then
that means the issue I am experiencing is further up the stack. Based
on troubleshooting by getting the raw data that the rockchip-saradc
driver was putting into the triggered buffer and seeing what the
adc-joystick saw coming out of the triggered buffer I wonder if the
issue is with the rockchip-saradc driver? I noticed that the buffer
pushed by the driver's trigger handler would only (appear to) send the
channels that I was requesting data for. So basically the data buffer
would have the correct values in [0] and [1], but the adc-joystick
driver by using the idx would fetch values from [1] for x (which has
the y axis data) and [2] for y (which would have arbitrary data in
it, usually something around 65406 or so).
Do you think I should start looking at the rockchip-saradc driver then?
Should the saradc be putting stuff in the buffer for every channel with
empty data for channels that aren't to be reported?
Thank you.
>
> Cheers,
> Artur
>
> >
> > Let's add Jonathan and Arthur for their take on this.
> >
> > >
> > > Signed-off-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > > ---
> > > drivers/input/joystick/adc-joystick.c | 7 +++----
> > > 1 file changed, 3 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/input/joystick/adc-joystick.c
> > > b/drivers/input/joystick/adc-joystick.c
> > > index 78ebca7d400a..fe3bbd0d4566 100644
> > > --- a/drivers/input/joystick/adc-joystick.c
> > > +++ b/drivers/input/joystick/adc-joystick.c
> > > @@ -32,24 +32,23 @@ static int adc_joystick_handle(const void *data,
> > > void *private)
> > > {
> > > struct adc_joystick *joy = private;
> > > enum iio_endian endianness;
> > > - int bytes, msb, val, idx, i;
> > > + int bytes, msb, val, i;
> > > const u16 *data_u16;
> > > 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';
> > >
> > > switch (bytes) {
> > > case 1:
> > > - val = ((const u8 *)data)[idx];
> > > + val = ((const u8 *)data)[i];
> > > break;
> > > case 2:
> > > - data_u16 = (const u16 *)data + idx;
> > > + data_u16 = (const u16 *)data + i;
> > >
> > > /*
> > > * Data is aligned to the sample size by IIO core.
> > > --
> > > 2.25.1
> > >
> >
> > Thanks.
