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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

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