On Mon, 29 May 2023 22:04:18 +0200 Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> wrote: > Hi George, > > On Sun, May 28, 2023 at 11:52 PM George Stark <GNStark@xxxxxxxxxxxxxx> wrote: > [...] > > > Based on the limited description we have here, I'm not understanding why > > > you don't just express this as a set of channels. One channel per mux > > > setting, with the in_voltageX_label providing the information on what the > > > channel is connected to. > > > > > > This is an interesting facility, so good to enable for high precision calibration > > > but we still want to map it to standards signals. Userspace doesn't > > > care that these are all being measured via the same input 7 - which > > > is itself probably an input to a MUX. > > > > > > Jonathan > > > > Hello Jonathan > > > > Thanks for the review. > > > > Your idea of exposing the mux setting as iio channels is very > > interesting and at least worth trying. > > The sysfs approach was chosen because of the code changes are simple and > > neat (compare to channels approach). > > Also calibration by using those mux inputs are already supported in the > > driver (performed at probe stage) so I expect very special usecases for > > those mux settings like debug or device production stage tests. In those > > usescases hardware specific knowledge is required anyway. Agreed that using channels for this is more complex code wise, but the avoidance of custom ABI is usually worthwhile. However I get your point that this is weird debug / corner case paths anyway. However... > Another downside to the debugfs approach is user support: > If someone reports odd values on ADC channel 7 then we need to make > sure to double check if the mux has been altered from userspace (the > calibration during initialization ensures to leave channel 7 in a > consistent state, while a user may change the mux, forget about it and > then complain that values are wrong). This problem raises red flags for me. If the channel wasn't otherwise useful then treating it as a weird debug thing is fine, but if a normal read can end up returning weird answers because of leftover configuration that is custom ABI, so standard code isn't even aware of it, that's bad. Hence I would not want to see any path under which that can happen. A read of in_voltage7_raw should continue to return the normal value whatever this patch adds. Jonathan > > > Best regards, > Martin
