Hi Tomasz, Replies inline. On Sun, 26 Apr 2020 13:11:04 +0200 Tomasz Duszynski <tomasz.duszynski@xxxxxxxxxxx> wrote: > On Sat, Apr 25, 2020 at 08:20:57PM +0100, Jonathan Cameron wrote: > > On Thu, 23 Apr 2020 17:53:17 +0200 > > Tomasz Duszynski <tomasz.duszynski@xxxxxxxxxxx> wrote: > > > > > On Wed, Apr 22, 2020 at 06:40:17PM +0200, Peter Meerwald-Stadler wrote: > > > > On Wed, 22 Apr 2020, Tomasz Duszynski wrote: > > > > > > > > > Add documentation for sensor specific iio attributes. > > > > > > > > minor comments below > > > > > > Thanks. > > > > > > > > > > > > Signed-off-by: Tomasz Duszynski <tomasz.duszynski@xxxxxxxxxxx> > > > > > --- > > > > > Documentation/ABI/testing/sysfs-bus-iio-scd30 | 97 +++++++++++++++++++ > > > > > 1 file changed, 97 insertions(+) > > > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30 > > > > > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30 > > > > > new file mode 100644 > > > > > index 000000000000..0431a718447d > > > > > --- /dev/null > > > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30 > > > > > @@ -0,0 +1,97 @@ > > > > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + Given that sensor's CO2 measurement chamber has fixed volume > > > > > + pressure changes will affect concentration readings. Writing > > > > > + current ambient pressure here will allow senor to make necessary > > > > > > > > sensor > > > > > > > > > > Okay. > > > > > > > > + adjustments. Upon reading previously set value is returned. > > > > > + Units are millibars. > > > > > > > > unit for pressure in IIO is kilopascal (e.g. > > > > /sys/bus/iio/devices/iio:deviceX/in_pressure_raw) > > > > > > > > > > My thinking here was that since these are sensor specific attributes > > > they don't need to stick to iio conventions and millibars were somewhat > > > more natural to use. But I guess that's just matter of habit. > > > > You absolutely have to stick to standard units. Userspace programs > > aren't going to come read your docs... > > > > For other sensors that take a calibration value like this we've reported > > them via an output channel. For example the atlas-ph sensor has > > an 'output temp' channel used for this purpose. > > > > Fair enough. > > > It's not ideal or totally intuitive but it does let us avoid expanding > > the overall ABI. The argument was something along the lines of > > 1) Imagine your sensor could control the pressure in the measurement space... > > 2) An output channel would provide the value to set it to. > > 3) Now instead we provide a means of saying 'what it is' > > 4) End result is we write a value and the pressure in the chamber is > > that value :) > > > > As I said not ideal but the best we can do without having to define a lot > > of ABI just to deal with compensation factors. > > > > This is a rare case where I would document the 'standard' ABI in here > > to make the point that it is actually providing an estimate of the pressure > > not controlling it... > > > > > > > > So generally I am okay with reworking all attrs to accept values in iio > > > preferred units. > > > > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/pressure_comp_available > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + The range of available values in millibars represented as the > > > > > + minimum value, the step and the maximum value, all enclosed in > > > > > + square brackets. > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval > > > > > +Date: January 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + Amount of time between subsequent measurements. Writing this > > > > > + attribute will change measurement interval. Upon reading > > > > > + current measurement interval is returned. Units are seconds. > > > > Use the existing ABI sampling frequency which is sort of the inverse of this. > > > > Was thinking about it but long periods in Hz simply don't look appealing :). > > No other strong opinions so I'll rework that. Agreed it can look a bit odd, but we don't want to have multiple controls for the same thing so we are stuck with it. > > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/meas_interval_available > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + The range of available values in seconds represented as the > > > > > + minimum value, the step and the maximum value, all enclosed in > > > > > + square brackets. > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/asc > > Spends some characters to easy of understanding ;) > > > > auto_calib_proc_enable maybe? Or can we get away with the 'somewhat standard > > calibration (it's used in at least one other driver IIRC) > > > > Just self_calibration would do? I'll think a bit more on this one but probably fine. > > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + Writing 1 or 0 to this attribute will respectively activate or > > > > > + deactivate automatic self calibration procedure. Upon reading 1 > > > > > > > > deactivate automatic self calibration (asc) procedure > > > > > > > > > > That shouldn't be too difficult to realize what asc actually stands for after > > > reading this short description. > > > > > > > > + is returned if asc is ongoing, 0 otherwise. > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/frc > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + Forced recalibration is used to compensate for sensor drifts > > > > > + when a reference value of CO2 concentration in close proximity > > > > > + to the sensor is available. Writing attribute will set frc > > > > > + value. Upon reading current frc is returned. Units are > > > > > + millibars. > > > > Could we implement this by just writing to the main channel value? > > Bit of a clunky ABI but sort of logically fits in my head given we are basically > > forcing the value we read to be this one? > > > > So the similar to the pressure compensation. Okay. > > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/frc_available > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + The range of available values in millibars represented as the > > > > > + minimum value, the step and the maximum value, all enclosed in > > > > > + square brackets. > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + Sensor readings may be affected by ambient temperature. > > > > > + Writing temperature offset will compensate for unwanted changes. > > > > > + Note that written offset gets multiplied by a factor of 100 > > > > > + by a sensor internally. > > > > > + > > > > > + For example, writing 10 here will correspond to 0.1 degree > > > > > + Celsius. > > > > This sounds like a calibbias to me which is standard ABI. > > > > Right, that could work. > > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/temp_offset_available > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + The range of available values in degrees Celsius represented as > > > > > + the minimum value, the step and the maximum value, all enclosed > > > > > + in square brackets. > > > > Wrong units for temperature (which is an odd one as we > > lifted them from hwmon before learning the error of our ways and starting to use > > SI units as the base). > > > > Does calibbias have _available counterpart? It might not be documented yet (as not sure it's been used) but any attribute has an available counterpart. That's effectively standard ABI. > > > > > > > > + > > > > > +What: /sys/bus/iio/devices/iio:deviceX/reset > > > > > +Date: April 2020 > > > > > +KernelVersion: 5.8 > > > > > +Contact: linux-iio@xxxxxxxxxxxxxxx > > > > > +Description: > > > > > + Software reset mechanism forces sensor into the same state > > > > > + as after powering up without the need for removing power supply. > > > > > + Writing any value will reset sensor. > > > > Not seeing an argument here for why you might want to do that other than on > > power up or module probe to get the driver into a known state. > > So currently it's a no to this one - just don't expose it to userspace. > > > > If one writes some odd configuration (though allowed) into sensor, for example > out of sheer curiosity and then writes the sane values back sensor needs some > time to recover (i.e start reporting valid measurements again). > > So rationale here was that after reset sensor recovers immediately. I'd > say that reset is sometimes useful. Perhaps that could be exported by > means of iio debug api? Debugfs would be fine > > > > > > > > > > > > > > > > -- > > > > > > > > Peter Meerwald-Stadler > > > > Mobile: +43 664 24 44 418 > >
