On Fri, 9 Dec 2022 08:46:40 +0000 "Tanislav, Cosmin" <Cosmin.Tanislav@xxxxxxxxxx> wrote: > > -----Original Message----- > > From: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> > > Sent: Thursday, December 8, 2022 3:34 PM > > To: Nuno Sá <noname.nuno@xxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx > > Cc: Jonathan Cameron <jic23@xxxxxxxxxx>; Tanislav, Cosmin > > <Cosmin.Tanislav@xxxxxxxxxx>; Hennerich, Michael > > <Michael.Hennerich@xxxxxxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>; > > Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> > > Subject: [POC] iio: ad74413: allow channel configuration to be given via > > module parameters > > > > [External] > > > > Just to see how it would look, I tried doing the below. Since a board > > may have more than one ad74412/ad74413, one needs to be able to > > differentiate between them. So for now each module parameter channelX > > (X=0,1,2,3) accepts a space-separated list of <label>:<function>, > > where label is matched against the label property in device tree, but > > also allowing * to match any, which is more convenient when one knows > > there is only one device. > > > > Aside from the missing documentation (MODULE_PARM_DESC), there are of > > course various details to hash out. E.g., should the function be > > specified with a raw integer as here, or should we take a text string > > "voltage-output", "current-input-ext-power" etc. and translate those? > > Should we use space or comma or semicolon as separator? And so on. > > > > I also considered whether instead of the label one should instead > > match on the OF_FULLNAME, > > e.g. /soc@0/bus@30800000/spi@30840000/ad74412r@0, but that's a lot > > more complicated, and I assume that anybody that has more than one of > > these chips would anyway assign a label so that they can distinguish > > their /sys/bus/iio/... directories. > > > > I should also note that it is not unprecedented for modules to take > > parameters that do some sort of (ad hoc) parsing to apply settings > > per-device. For example: > > > > - ignore_wake in drivers/gpio/gpiolib-acpi.c > > - mtdparts in drivers/mtd/parsers/cmdlinepart.c > > - pci_devs_to_hide in drivers/xen/xen-pciback/pci_stub.c > > - quirks in drivers/hid/usbhid/hid-core.c > > > > So the question is, is there any chance that anything like this could > > be accepted? If so, I'll of course spin this into a real patch with > > proper MODULE_PARM_DESC and commit log etc. > > > > This has been tested doing > > > > insmod ad74413r.ko 'channel0=*:1' 'channel1=*:3' 'channel2=*:2' > > 'channel3=*:4' > > > > and seeing that the channels did indeed come up as expected, where the > > device tree specified CH_FUNC_HIGH_IMPEDANCE for all of them. > > > > Nuno previously mentioned dynamically loading device tree overlays, > which seems like a much cleaner solution to me. Have you looked into > that? > We are unlikely to take anything else I'm afraid. So hopefully device tree overlays will work for you. Dynamic configuration of pretty much anything about input OR output would be potentially fine (we'd probably want to add limits on output settings which I think we've done for some devices). A userspace interface to switch between those is much more of a corner case and could lead to hardware damage depending on exactly what is connected to the pins. Jonathan
