On Sun, Jun 19, 2022 at 8:58 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > Vincent: I'm getting some instability with roadtest after forwards porting to > current mainline. Tests run 'sometimes'. Other times I get a crash > in um_set_signal. Seems unrelated to the test this series adds. > > Changes since v1: Thanks to Andy and Peter for reviews. > - Use sizeof(data) for be24 buffer data[3] > - Make the precision patch fall back to old maths if overflow would > have occured. > > This started out as an experiment with Vincent Whitchurch's roadtest > testing framework [1] and that worked well so I carried on cleaning up the > driver. > > Mostly this is standard tidy up, move to new interfaces, then move the driver > out of staging, but there are a few other things in here. > > 1) Precision improvement for IIO_VAL_FRACTIONAL_LOG2. > The ad7746 is a 24 bit sensor and this highlighted that 9 decimal > places wasn't enough to keep the error introduced by > _raw * _scale from growing too large over the whole range (I couldn't > write a sensible test for it). So I've proposed increasing the precision > of the calculation used to provide the sysfs attributes with this value > type and printing a few more decimal places (12), but only if overflow > will not occur due to val[0] > ULLONG_MAX / PICO > 2) _inputoffset ABI addition. This driver had an odd use of _offset for > the case of differential channels that applied the offset to both > of the differential pair (hence usespace shouldn't not apply it when userspace? > converting to the base units. That isn't inline with the existing > documentation for _offset and it wasn't clear to me that it made sense > at all. To avoid confusion I've added this new ABI (_inputoffset) for t > 3) roadtest file - note this is not a complete test set for the driver and > mainly focused on the main channel reads and places I thought I might > have broken things whilst working on the driver. > > My conclusion on roadtest - Very useful indeed. I'd encourage others to > consider developing some basic sanity tests for drivers they are working on. > Hopefully my python code isn't too hideous to understand at least! > Vincent, it might be worth thinking about some generic code to handle the > 'variants' on correct ABI like I introduce here because I switched from > a shared by type scale to an individual one per channel for the voltages. > Both were ABI compliant so that sort of change is fine most of the time > though we have to be careful with it. > > All comments welcome. Note there may be changes that make more sense > to do after moving this out of staging as long as there are no ABI changes involved > etc. Feel free to highlight those sorts of changes as well as anything more > significant. Overall it looks good to me. One Q though, do you plan to switch it to regmap APIs? ... > staging: iio: cdc: ad7746: Drop usused i2c_set_clientdata() unused -- With Best Regards, Andy Shevchenko
