[RFC] iio: multiplexer: Copy scan_type details from parent to child

[RFC] iio: multiplexer: Copy scan_type details from parent to child

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Copy the scan_type details from the parent iio channel to the child.
The scan_type is otherwise empty and things like the storagebits are
zero (which causes a problem for the adc-joystick driver which
validates the storagebits when used through a mux). I'm submitting this
as an RFC because I'm not sure if this is the correct way to handle
this scenario (a driver that checks the storagebits used with the iio
multiplexer).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 drivers/iio/multiplexer/iio-mux.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
index 93558fddfa9b..1de01ec878c4 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -322,6 +322,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
 	if (page)
 		devm_kfree(dev, page);
 
+	/* Copy scan type from parent to mux child. */
+	chan->scan_type = pchan->scan_type;
+
 	return 0;
 }
 
-- 
2.25.1

Re: [RFC] iio: multiplexer: Copy scan_type details from parent to child

[Jonathan Cameron]

On Mon, 18 Jul 2022 13:43:12 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Copy the scan_type details from the parent iio channel to the child.
> The scan_type is otherwise empty and things like the storagebits are
> zero (which causes a problem for the adc-joystick driver which
> validates the storagebits when used through a mux). I'm submitting this
> as an RFC because I'm not sure if this is the correct way to handle
> this scenario (a driver that checks the storagebits used with the iio
> multiplexer).
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Seems sensible to me. Though Peter is expert on this one.

One comment on the comment inline...
> ---
>  drivers/iio/multiplexer/iio-mux.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> index 93558fddfa9b..1de01ec878c4 100644
> --- a/drivers/iio/multiplexer/iio-mux.c
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -322,6 +322,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
>  	if (page)
>  		devm_kfree(dev, page);
>  
> +	/* Copy scan type from parent to mux child. */
Comment is in the obvious category, so drop that.

> +	chan->scan_type = pchan->scan_type;
> +
>  	return 0;
>  }
>  

Re: [RFC] iio: multiplexer: Copy scan_type details from parent to child

[Peter Rosin]

Hi!

2022-07-19 at 10:51, Jonathan Cameron wrote:
> On Mon, 18 Jul 2022 13:43:12 -0500
> Chris Morgan <macroalpha82@gmail.com> wrote:
> 
>> From: Chris Morgan <macromorgan@hotmail.com>
>>
>> Copy the scan_type details from the parent iio channel to the child.
>> The scan_type is otherwise empty and things like the storagebits are
>> zero (which causes a problem for the adc-joystick driver which
>> validates the storagebits when used through a mux). I'm submitting this
>> as an RFC because I'm not sure if this is the correct way to handle
>> this scenario (a driver that checks the storagebits used with the iio
>> multiplexer).
>>
>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Seems sensible to me. Though Peter is expert on this one.

I really doubt that it is this simple. I'm no expert on buffered channels,
but after a quick look it certainly looks like adc-joystick requires
buffered channels. And iio-multiplexer does not support that at all. I
think it could be supported, but in that case the only obvious support
would be to support buffered channels one at a time, and the way I
read adc-joystick it expects to get one sample for each axes in one
go (i.e. the way "normal" ADCs deliver multiple parallel samples). See
e.g. the call to iio_channel_get_all_cb in adc-joystick, which dives
right into the buffered iio department.

So, the stuff in scan_type (along with scan_index, which is highly
related) is only relevant for buffered channels, and iio_multiplexer is
currently limited by its

	indio_dev->modes = INDIO_DIRECT_MODE;

which is ... not buffered. :-(

The simplest way forward is probably to lift the requirement of buffered
channels from adc-joystick, but that might be a hard sell as that driver
would then probably be a lot bigger and more complex.

That said, I would of course love to see the generic solution with support
for buffered channels in the mux, but my guess is that that gets much
more difficult, at least if you need the kind of buffered support
expected by adc-joystick.

Or, am I misreading the whole situation and you have actually gotten it
to work with the below one-liner? If it works, fine by me, but I think
you also need to do something with scan_index?

Cheers,
Peter

> One comment on the comment inline...
>> ---
>>  drivers/iio/multiplexer/iio-mux.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
>> index 93558fddfa9b..1de01ec878c4 100644
>> --- a/drivers/iio/multiplexer/iio-mux.c
>> +++ b/drivers/iio/multiplexer/iio-mux.c
>> @@ -322,6 +322,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
>>  	if (page)
>>  		devm_kfree(dev, page);
>>  
>> +	/* Copy scan type from parent to mux child. */
> Comment is in the obvious category, so drop that.
> 
>> +	chan->scan_type = pchan->scan_type;
>> +
>>  	return 0;
>>  }
>>  
> 

Re: [RFC] iio: multiplexer: Copy scan_type details from parent to child

[Jonathan Cameron]

On Tue, 19 Jul 2022 15:27:24 +0200
Peter Rosin <peda@axentia.se> wrote:

> Hi!
> 
> 2022-07-19 at 10:51, Jonathan Cameron wrote:
> > On Mon, 18 Jul 2022 13:43:12 -0500
> > Chris Morgan <macroalpha82@gmail.com> wrote:
> >   
> >> From: Chris Morgan <macromorgan@hotmail.com>
> >>
> >> Copy the scan_type details from the parent iio channel to the child.
> >> The scan_type is otherwise empty and things like the storagebits are
> >> zero (which causes a problem for the adc-joystick driver which
> >> validates the storagebits when used through a mux). I'm submitting this
> >> as an RFC because I'm not sure if this is the correct way to handle
> >> this scenario (a driver that checks the storagebits used with the iio
> >> multiplexer).
> >>
> >> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>  
> > Seems sensible to me. Though Peter is expert on this one.  
> 
> I really doubt that it is this simple. I'm no expert on buffered channels,
> but after a quick look it certainly looks like adc-joystick requires
> buffered channels. And iio-multiplexer does not support that at all. I
> think it could be supported, but in that case the only obvious support
> would be to support buffered channels one at a time, and the way I
> read adc-joystick it expects to get one sample for each axes in one
> go (i.e. the way "normal" ADCs deliver multiple parallel samples). See
> e.g. the call to iio_channel_get_all_cb in adc-joystick, which dives
> right into the buffered iio department.
> 
> So, the stuff in scan_type (along with scan_index, which is highly
> related) is only relevant for buffered channels, and iio_multiplexer is
> currently limited by its
> 
> 	indio_dev->modes = INDIO_DIRECT_MODE;
> 
> which is ... not buffered. :-(
> 
> The simplest way forward is probably to lift the requirement of buffered
> channels from adc-joystick, but that might be a hard sell as that driver
> would then probably be a lot bigger and more complex.
>

Got it in one ;)
There is a recent series from Chris adding polled support to adc_joystick (not yet merged)
https://lore.kernel.org/all/20220705190354.69263-1-macromorgan@hotmail.com/
That just reads the channels one at a time via same interfaces used for
sysfs reads so would work with the iio-mux I think.

Jonathan

> That said, I would of course love to see the generic solution with support
> for buffered channels in the mux, but my guess is that that gets much
> more difficult, at least if you need the kind of buffered support
> expected by adc-joystick.
> 
> Or, am I misreading the whole situation and you have actually gotten it
> to work with the below one-liner? If it works, fine by me, but I think
> you also need to do something with scan_index?
> 



> Cheers,
> Peter
> 
> > One comment on the comment inline...  
> >> ---
> >>  drivers/iio/multiplexer/iio-mux.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> >> index 93558fddfa9b..1de01ec878c4 100644
> >> --- a/drivers/iio/multiplexer/iio-mux.c
> >> +++ b/drivers/iio/multiplexer/iio-mux.c
> >> @@ -322,6 +322,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
> >>  	if (page)
> >>  		devm_kfree(dev, page);
> >>  
> >> +	/* Copy scan type from parent to mux child. */  
> > Comment is in the obvious category, so drop that.
> >   
> >> +	chan->scan_type = pchan->scan_type;
> >> +
> >>  	return 0;
> >>  }
> >>    
> >   

Re: [RFC] iio: multiplexer: Copy scan_type details from parent to child

[Chris Morgan]

On Tue, Jul 19, 2022 at 03:19:01PM +0100, Jonathan Cameron wrote:
> On Tue, 19 Jul 2022 15:27:24 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
> > Hi!
> > 
> > 2022-07-19 at 10:51, Jonathan Cameron wrote:
> > > On Mon, 18 Jul 2022 13:43:12 -0500
> > > Chris Morgan <macroalpha82@gmail.com> wrote:
> > >   
> > >> From: Chris Morgan <macromorgan@hotmail.com>
> > >>
> > >> Copy the scan_type details from the parent iio channel to the child.
> > >> The scan_type is otherwise empty and things like the storagebits are
> > >> zero (which causes a problem for the adc-joystick driver which
> > >> validates the storagebits when used through a mux). I'm submitting this
> > >> as an RFC because I'm not sure if this is the correct way to handle
> > >> this scenario (a driver that checks the storagebits used with the iio
> > >> multiplexer).
> > >>
> > >> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>  
> > > Seems sensible to me. Though Peter is expert on this one.  
> > 
> > I really doubt that it is this simple. I'm no expert on buffered channels,
> > but after a quick look it certainly looks like adc-joystick requires
> > buffered channels. And iio-multiplexer does not support that at all. I
> > think it could be supported, but in that case the only obvious support
> > would be to support buffered channels one at a time, and the way I
> > read adc-joystick it expects to get one sample for each axes in one
> > go (i.e. the way "normal" ADCs deliver multiple parallel samples). See
> > e.g. the call to iio_channel_get_all_cb in adc-joystick, which dives
> > right into the buffered iio department.
> > 
> > So, the stuff in scan_type (along with scan_index, which is highly
> > related) is only relevant for buffered channels, and iio_multiplexer is
> > currently limited by its
> > 
> > 	indio_dev->modes = INDIO_DIRECT_MODE;
> > 
> > which is ... not buffered. :-(
> > 
> > The simplest way forward is probably to lift the requirement of buffered
> > channels from adc-joystick, but that might be a hard sell as that driver
> > would then probably be a lot bigger and more complex.
> >
> 
> Got it in one ;)
> There is a recent series from Chris adding polled support to adc_joystick (not yet merged)
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220705190354.69263-1-macromorgan%40hotmail.com%2F&amp;data=05%7C01%7C%7Cfc07a056f5ae40ecc37908da6991a298%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637938371469723267%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=tAT8scZvG9Z8OVSgwdxog%2Bz55dzQC2TFddDa%2BQEX6e4%3D&amp;reserved=0
> That just reads the channels one at a time via same interfaces used for
> sysfs reads so would work with the iio-mux I think.
> 
> Jonathan

Right, correct that these two "work" together. Honestly the main thing
is that the adc-joystick driver checks for the storagebits, and that's
failing for me in this case when I use a mux. I added a one-liner to
the multiplexer code which added the parent information and that fixed
it. It's also possible that I could change the adc-joystick driver to
either not look for the storagebits or to change it to only look when
not doing polling mode. I just assumed that a missing scan_type was
an issue, so I added it and marked the patch as RFC because I wasn't
sure. If you want me to relax the adc-joystick check instead for polled
devices, I can.

Note that the adc-joystick driver does work perfectly fine with a mux,
I'm in the very early stages of upstreaming some devices which use it
in such a manner (the Anbernic RG353, Anbernic RG503, Odroid Go Super
are example devices that use the adc-joystick with a mux downstream).

Thank you.

> 
> > That said, I would of course love to see the generic solution with support
> > for buffered channels in the mux, but my guess is that that gets much
> > more difficult, at least if you need the kind of buffered support
> > expected by adc-joystick.
> > 
> > Or, am I misreading the whole situation and you have actually gotten it
> > to work with the below one-liner? If it works, fine by me, but I think
> > you also need to do something with scan_index?
> > 
> 
> 
> 
> > Cheers,
> > Peter
> > 
> > > One comment on the comment inline...  
> > >> ---
> > >>  drivers/iio/multiplexer/iio-mux.c | 3 +++
> > >>  1 file changed, 3 insertions(+)
> > >>
> > >> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> > >> index 93558fddfa9b..1de01ec878c4 100644
> > >> --- a/drivers/iio/multiplexer/iio-mux.c
> > >> +++ b/drivers/iio/multiplexer/iio-mux.c
> > >> @@ -322,6 +322,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
> > >>  	if (page)
> > >>  		devm_kfree(dev, page);
> > >>  
> > >> +	/* Copy scan type from parent to mux child. */  
> > > Comment is in the obvious category, so drop that.
> > >   
> > >> +	chan->scan_type = pchan->scan_type;
> > >> +
> > >>  	return 0;
> > >>  }
> > >>    
> > >   
> 

Re: [RFC] iio: multiplexer: Copy scan_type details from parent to child

[Peter Rosin]

2022-07-19 at 16:44, Chris Morgan wrote:
> On Tue, Jul 19, 2022 at 03:19:01PM +0100, Jonathan Cameron wrote:
>> On Tue, 19 Jul 2022 15:27:24 +0200
>> Peter Rosin <peda@axentia.se> wrote:
>>
>>> Hi!
>>>
>>> 2022-07-19 at 10:51, Jonathan Cameron wrote:
>>>> On Mon, 18 Jul 2022 13:43:12 -0500
>>>> Chris Morgan <macroalpha82@gmail.com> wrote:
>>>>   
>>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>>>
>>>>> Copy the scan_type details from the parent iio channel to the child.
>>>>> The scan_type is otherwise empty and things like the storagebits are
>>>>> zero (which causes a problem for the adc-joystick driver which
>>>>> validates the storagebits when used through a mux). I'm submitting this
>>>>> as an RFC because I'm not sure if this is the correct way to handle
>>>>> this scenario (a driver that checks the storagebits used with the iio
>>>>> multiplexer).
>>>>>
>>>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>  
>>>> Seems sensible to me. Though Peter is expert on this one.  
>>>
>>> I really doubt that it is this simple. I'm no expert on buffered channels,
>>> but after a quick look it certainly looks like adc-joystick requires
>>> buffered channels. And iio-multiplexer does not support that at all. I
>>> think it could be supported, but in that case the only obvious support
>>> would be to support buffered channels one at a time, and the way I
>>> read adc-joystick it expects to get one sample for each axes in one
>>> go (i.e. the way "normal" ADCs deliver multiple parallel samples). See
>>> e.g. the call to iio_channel_get_all_cb in adc-joystick, which dives
>>> right into the buffered iio department.
>>>
>>> So, the stuff in scan_type (along with scan_index, which is highly
>>> related) is only relevant for buffered channels, and iio_multiplexer is
>>> currently limited by its
>>>
>>> 	indio_dev->modes = INDIO_DIRECT_MODE;
>>>
>>> which is ... not buffered. :-(
>>>
>>> The simplest way forward is probably to lift the requirement of buffered
>>> channels from adc-joystick, but that might be a hard sell as that driver
>>> would then probably be a lot bigger and more complex.
>>>
>>
>> Got it in one ;)

I guess I'm old and can therefore allow myself to blather :-)

>> There is a recent series from Chris adding polled support to adc_joystick (not yet merged)
>> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220705190354.69263-1-macromorgan%40hotmail.com%2F&amp;data=05%7C01%7C%7Cfc07a056f5ae40ecc37908da6991a298%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637938371469723267%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=tAT8scZvG9Z8OVSgwdxog%2Bz55dzQC2TFddDa%2BQEX6e4%3D&amp;reserved=0
>> That just reads the channels one at a time via same interfaces used for
>> sysfs reads so would work with the iio-mux I think.
>>
>> Jonathan
> 
> Right, correct that these two "work" together. Honestly the main thing
> is that the adc-joystick driver checks for the storagebits, and that's
> failing for me in this case when I use a mux. I added a one-liner to
> the multiplexer code which added the parent information and that fixed
> it. It's also possible that I could change the adc-joystick driver to
> either not look for the storagebits or to change it to only look when
> not doing polling mode. I just assumed that a missing scan_type was
> an issue, so I added it and marked the patch as RFC because I wasn't
> sure. If you want me to relax the adc-joystick check instead for polled
> devices, I can.

Ah, ok, there's been new development. Nice!

As I said, I'm by no means a iio-buffer expert. On the contrary, I'm a
total noob. So, be sure to take the following with a good pinch of salt,
but I think that the adc-joystick driver in a mode that does not use
iio-buffers should not then have artificial requirement on the buffer
layout (scan_type). Don't forget the salt... :-)

> Note that the adc-joystick driver does work perfectly fine with a mux,
> I'm in the very early stages of upstreaming some devices which use it
> in such a manner (the Anbernic RG353, Anbernic RG503, Odroid Go Super
> are example devices that use the adc-joystick with a mux downstream).

Cool!

Cheers,
Peter

Re: [RFC] iio: multiplexer: Copy scan_type details from parent to child

[Chris Morgan]

On Tue, Jul 19, 2022 at 07:54:18PM +0200, Peter Rosin wrote:
> 
> 
> 2022-07-19 at 16:44, Chris Morgan wrote:
> > On Tue, Jul 19, 2022 at 03:19:01PM +0100, Jonathan Cameron wrote:
> >> On Tue, 19 Jul 2022 15:27:24 +0200
> >> Peter Rosin <peda@axentia.se> wrote:
> >>
> >>> Hi!
> >>>
> >>> 2022-07-19 at 10:51, Jonathan Cameron wrote:
> >>>> On Mon, 18 Jul 2022 13:43:12 -0500
> >>>> Chris Morgan <macroalpha82@gmail.com> wrote:
> >>>>   
> >>>>> From: Chris Morgan <macromorgan@hotmail.com>
> >>>>>
> >>>>> Copy the scan_type details from the parent iio channel to the child.
> >>>>> The scan_type is otherwise empty and things like the storagebits are
> >>>>> zero (which causes a problem for the adc-joystick driver which
> >>>>> validates the storagebits when used through a mux). I'm submitting this
> >>>>> as an RFC because I'm not sure if this is the correct way to handle
> >>>>> this scenario (a driver that checks the storagebits used with the iio
> >>>>> multiplexer).
> >>>>>
> >>>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>  
> >>>> Seems sensible to me. Though Peter is expert on this one.  
> >>>
> >>> I really doubt that it is this simple. I'm no expert on buffered channels,
> >>> but after a quick look it certainly looks like adc-joystick requires
> >>> buffered channels. And iio-multiplexer does not support that at all. I
> >>> think it could be supported, but in that case the only obvious support
> >>> would be to support buffered channels one at a time, and the way I
> >>> read adc-joystick it expects to get one sample for each axes in one
> >>> go (i.e. the way "normal" ADCs deliver multiple parallel samples). See
> >>> e.g. the call to iio_channel_get_all_cb in adc-joystick, which dives
> >>> right into the buffered iio department.
> >>>
> >>> So, the stuff in scan_type (along with scan_index, which is highly
> >>> related) is only relevant for buffered channels, and iio_multiplexer is
> >>> currently limited by its
> >>>
> >>> 	indio_dev->modes = INDIO_DIRECT_MODE;
> >>>
> >>> which is ... not buffered. :-(
> >>>
> >>> The simplest way forward is probably to lift the requirement of buffered
> >>> channels from adc-joystick, but that might be a hard sell as that driver
> >>> would then probably be a lot bigger and more complex.
> >>>
> >>
> >> Got it in one ;)
> 
> I guess I'm old and can therefore allow myself to blather :-)
> 
> >> There is a recent series from Chris adding polled support to adc_joystick (not yet merged)
> >> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220705190354.69263-1-macromorgan%40hotmail.com%2F&amp;data=05%7C01%7C%7Ccb22e6634f844f68126e08da69afae8c%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637938500513929763%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=00HdVZvnHQNbQ1wFEON%2F9i%2B5b%2BicYb6gRhAJuKooZlA%3D&amp;reserved=0
> >> That just reads the channels one at a time via same interfaces used for
> >> sysfs reads so would work with the iio-mux I think.
> >>
> >> Jonathan
> > 
> > Right, correct that these two "work" together. Honestly the main thing
> > is that the adc-joystick driver checks for the storagebits, and that's
> > failing for me in this case when I use a mux. I added a one-liner to
> > the multiplexer code which added the parent information and that fixed
> > it. It's also possible that I could change the adc-joystick driver to
> > either not look for the storagebits or to change it to only look when
> > not doing polling mode. I just assumed that a missing scan_type was
> > an issue, so I added it and marked the patch as RFC because I wasn't
> > sure. If you want me to relax the adc-joystick check instead for polled
> > devices, I can.
> 
> Ah, ok, there's been new development. Nice!
> 
> As I said, I'm by no means a iio-buffer expert. On the contrary, I'm a
> total noob. So, be sure to take the following with a good pinch of salt,
> but I think that the adc-joystick driver in a mode that does not use
> iio-buffers should not then have artificial requirement on the buffer
> layout (scan_type). Don't forget the salt... :-)

This sounds like it makes the most sense to me now, since I wasn't aware
of the "scan_type" being associated with a buffer. I'll modify the
joystick driver and we can disregard this then.  Thank you.

> 
> > Note that the adc-joystick driver does work perfectly fine with a mux,
> > I'm in the very early stages of upstreaming some devices which use it
> > in such a manner (the Anbernic RG353, Anbernic RG503, Odroid Go Super
> > are example devices that use the adc-joystick with a mux downstream).
> 
> Cool!
> 
> Cheers,
> Peter

Re: [RFC] iio: multiplexer: Copy scan_type details from parent to child

[Jonathan Cameron]

On Tue, 19 Jul 2022 14:22:48 -0500
Chris Morgan <macromorgan@hotmail.com> wrote:

> On Tue, Jul 19, 2022 at 07:54:18PM +0200, Peter Rosin wrote:
> > 
> > 
> > 2022-07-19 at 16:44, Chris Morgan wrote:  
> > > On Tue, Jul 19, 2022 at 03:19:01PM +0100, Jonathan Cameron wrote:  
> > >> On Tue, 19 Jul 2022 15:27:24 +0200
> > >> Peter Rosin <peda@axentia.se> wrote:
> > >>  
> > >>> Hi!
> > >>>
> > >>> 2022-07-19 at 10:51, Jonathan Cameron wrote:  
> > >>>> On Mon, 18 Jul 2022 13:43:12 -0500
> > >>>> Chris Morgan <macroalpha82@gmail.com> wrote:
> > >>>>     
> > >>>>> From: Chris Morgan <macromorgan@hotmail.com>
> > >>>>>
> > >>>>> Copy the scan_type details from the parent iio channel to the child.
> > >>>>> The scan_type is otherwise empty and things like the storagebits are
> > >>>>> zero (which causes a problem for the adc-joystick driver which
> > >>>>> validates the storagebits when used through a mux). I'm submitting this
> > >>>>> as an RFC because I'm not sure if this is the correct way to handle
> > >>>>> this scenario (a driver that checks the storagebits used with the iio
> > >>>>> multiplexer).
> > >>>>>
> > >>>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>    
> > >>>> Seems sensible to me. Though Peter is expert on this one.    
> > >>>
> > >>> I really doubt that it is this simple. I'm no expert on buffered channels,
> > >>> but after a quick look it certainly looks like adc-joystick requires
> > >>> buffered channels. And iio-multiplexer does not support that at all. I
> > >>> think it could be supported, but in that case the only obvious support
> > >>> would be to support buffered channels one at a time, and the way I
> > >>> read adc-joystick it expects to get one sample for each axes in one
> > >>> go (i.e. the way "normal" ADCs deliver multiple parallel samples). See
> > >>> e.g. the call to iio_channel_get_all_cb in adc-joystick, which dives
> > >>> right into the buffered iio department.
> > >>>
> > >>> So, the stuff in scan_type (along with scan_index, which is highly
> > >>> related) is only relevant for buffered channels, and iio_multiplexer is
> > >>> currently limited by its
> > >>>
> > >>> 	indio_dev->modes = INDIO_DIRECT_MODE;
> > >>>
> > >>> which is ... not buffered. :-(
> > >>>
> > >>> The simplest way forward is probably to lift the requirement of buffered
> > >>> channels from adc-joystick, but that might be a hard sell as that driver
> > >>> would then probably be a lot bigger and more complex.
> > >>>  
> > >>
> > >> Got it in one ;)  
> > 
> > I guess I'm old and can therefore allow myself to blather :-)
> >   
> > >> There is a recent series from Chris adding polled support to adc_joystick (not yet merged)
> > >> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20220705190354.69263-1-macromorgan%40hotmail.com%2F&amp;data=05%7C01%7C%7Ccb22e6634f844f68126e08da69afae8c%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637938500513929763%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=00HdVZvnHQNbQ1wFEON%2F9i%2B5b%2BicYb6gRhAJuKooZlA%3D&amp;reserved=0
> > >> That just reads the channels one at a time via same interfaces used for
> > >> sysfs reads so would work with the iio-mux I think.
> > >>
> > >> Jonathan  
> > > 
> > > Right, correct that these two "work" together. Honestly the main thing
> > > is that the adc-joystick driver checks for the storagebits, and that's
> > > failing for me in this case when I use a mux. I added a one-liner to
> > > the multiplexer code which added the parent information and that fixed
> > > it. It's also possible that I could change the adc-joystick driver to
> > > either not look for the storagebits or to change it to only look when
> > > not doing polling mode. I just assumed that a missing scan_type was
> > > an issue, so I added it and marked the patch as RFC because I wasn't
> > > sure. If you want me to relax the adc-joystick check instead for polled
> > > devices, I can.  
> > 
> > Ah, ok, there's been new development. Nice!
> > 
> > As I said, I'm by no means a iio-buffer expert. On the contrary, I'm a
> > total noob. So, be sure to take the following with a good pinch of salt,
> > but I think that the adc-joystick driver in a mode that does not use
> > iio-buffers should not then have artificial requirement on the buffer
> > layout (scan_type). Don't forget the salt... :-)  
> 
> This sounds like it makes the most sense to me now, since I wasn't aware
> of the "scan_type" being associated with a buffer. I'll modify the
> joystick driver and we can disregard this then.  Thank you.


Agreed. It shouldn't be using this.  If there is a necessity to know the
'range' then the ADC should provide _raw_available and the adc-joystick driver
should query that in polled mode rather than anything from scan_type.
We have a bunch of DACs that do this for consumer usecases but not sure we have
many ADCs providing it yet. However, easy to add to a given driver when it's
needed!

J
> 
> >   
> > > Note that the adc-joystick driver does work perfectly fine with a mux,
> > > I'm in the very early stages of upstreaming some devices which use it
> > > in such a manner (the Anbernic RG353, Anbernic RG503, Odroid Go Super
> > > are example devices that use the adc-joystick with a mux downstream).  
> > 
> > Cool!
> > 
> > Cheers,
> > Peter