On Sun, Jun 26, 2022 at 2:20 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > Changes since v2: Thanks to Andy Shevchenko for review > - Fix the horrible mess I made of a rebase that meant v2 didn't build > mid patch set. > - Fix various build issues I'd missed due to wrong make file (fails to > update variables after moving them, missing unaligned include, > failure to cleanup staging driver Kconfig / Makefile) > - Change new ABI to _zeropoint and add more the description to hopefully > add clarify to what this is. I don't think it's going to be a particularly > common bit of ABI, but still good to get it right. For non-commented patches (except 17th): Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> For commented, in case you address the comments I suggested, you may also add a tag. > Note that the --no-renames was used to preserve full code listing > for the actual move out of staging to aid review. The ad7746 file is > moved with no changes. > > diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/iio/cdc/ad7746.c > similarity index 100% > rename from drivers/staging/iio/cdc/ad7746.c > rename to drivers/iio/cdc/ad7746.c > > 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. Note that if you are using roadtest rebased to current > kernel, add CONFIG_UML_RANDOM as suggested by Vincent in reply > to v2 of this series. I still saw one freeze but with that config > element added roadtest runs the tests successfully the vast majority > of the time. > > 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) _zeropoint 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 userspace shouldn't not apply it when > 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 (_zeropoint) for this. > 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. Given roadtest isn't > upstream yet this test will have to wait. That doesn't block merging > the rest of the series. > > 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. > > [1] https://lore.kernel.org/all/20220311162445.346685-9-vincent.whitchurch@xxxxxxxx/ > > Jonathan Cameron (17): > iio: core: Increase precision of IIO_VAL_FRACTIONAL_LOG2 when possible > iio: ABI: Fix wrong format of differential capacitance channel ABI. > staging: iio: cdc: ad7746: Use explicit be24 handling. > staging: iio: cdc: ad7746: Push handling of supply voltage scale to > userspace. > staging: iio: cdc: ad7746: Use local buffer for multi byte reads. > staging: iio: cdc: ad7746: Factor out ad7746_read_channel() > staging: iio: cdc: ad7764: Push locking down into case statements in > read/write_raw > staging: iio: cdc: ad7746: Break up use of chan->address and use > FIELD_PREP etc > staging: iio: cdc: ad7746: Drop unused i2c_set_clientdata() > staging: iio: cdc: ad7746: Use _raw and _scale for temperature > channels. > iio: core: Introduce _zeropoint for differential channels > staging: iio: cdc: ad7746: Switch from _offset to _zeropoint for > differential channels. > staging: iio: cdc: ad7746: Use read_avail() rather than opencoding. > staging: iio: ad7746: White space cleanup > iio: cdc: ad7746: Add device specific ABI documentation. > iio: cdc: ad7746: Move driver out of staging. > RFC: iio: cdc: ad7746: Add roadtest > > Documentation/ABI/testing/sysfs-bus-iio | 21 +- > .../ABI/testing/sysfs-bus-iio-cdc-ad7746 | 11 + > drivers/iio/cdc/Kconfig | 10 + > drivers/iio/cdc/Makefile | 1 + > drivers/iio/cdc/ad7746.c | 820 ++++++++++++++++++ > drivers/iio/industrialio-core.c | 32 +- > drivers/staging/iio/Kconfig | 1 - > drivers/staging/iio/Makefile | 1 - > drivers/staging/iio/cdc/Kconfig | 17 - > drivers/staging/iio/cdc/Makefile | 6 - > drivers/staging/iio/cdc/ad7746.c | 767 ---------------- > include/linux/iio/types.h | 1 + > .../roadtest/tests/iio/cdc/__init__.py | 0 > .../roadtest/roadtest/tests/iio/cdc/config | 1 + > .../roadtest/tests/iio/cdc/test_ad7746.py | 323 +++++++ > 15 files changed, 1211 insertions(+), 801 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-cdc-ad7746 > create mode 100644 drivers/iio/cdc/ad7746.c > delete mode 100644 drivers/staging/iio/cdc/Kconfig > delete mode 100644 drivers/staging/iio/cdc/Makefile > delete mode 100644 drivers/staging/iio/cdc/ad7746.c > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/__init__.py > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/config > create mode 100644 tools/testing/roadtest/roadtest/tests/iio/cdc/test_ad7746.py > > -- > 2.36.1 > -- With Best Regards, Andy Shevchenko
