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

[Peter Rosin <peda@axentia.se>]

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