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

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

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;
> >>  }
> >>    
> >