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@xxxxxxxxxx> wrote: > > > Hi! > > > > 2022-07-19 at 10:51, Jonathan Cameron wrote: > > > On Mon, 18 Jul 2022 13:43:12 -0500 > > > Chris Morgan <macroalpha82@xxxxxxxxx> wrote: > > > > > >> From: Chris Morgan <macromorgan@xxxxxxxxxxx> > > >> > > >> 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@xxxxxxxxxxx> > > > 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&data=05%7C01%7C%7Cfc07a056f5ae40ecc37908da6991a298%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637938371469723267%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tAT8scZvG9Z8OVSgwdxog%2Bz55dzQC2TFddDa%2BQEX6e4%3D&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; > > >> } > > >> > > > >
