| Message ID | 20211028101840.24632-7-andrea.merello@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Bosch BNO055 IMU | expand |
On Thu, 28 Oct 2021 12:18:36 +0200 Andrea Merello <andrea.merello@gmail.com> wrote: > This patch adds ABI documentation for bno055 driver private sysfs > attributes. Hohum. As normal I dislike custom attributes but reality is these don't map to anything 'standard' and I don't want them getting adopted in places where the 'standard' approach works. So thinking a bit more on this, I wonder if we can fit it into standard ABI. We can't use the normal range specification method of _scale because it's internal to the device and the output reading is unaffected. The range specification via _raw_available would let us know the range, but it is not writeable so.. A control that changes the internal scaling of the sensor in a fashion that is not visible to the outside world maps to calibscale. Whilst that was intended for little tweaks to the input signal (often front end amplifier gain tweak) it works here. It doesn't map through to anything userspace is expected to apply. That combined with _raw_available to let us know what the result is should work? What do you think of that approach? It's obviously a little more complex to handle in the driver, but it does map to existing ABI and avoids custom attributes etc. > > Signed-off-by: Andrea Merello <andrea.merello@iit.it> > --- > .../ABI/testing/sysfs-bus-iio-bno055 | 84 +++++++++++++++++++ > 1 file changed, 84 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055 > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-bno055 b/Documentation/ABI/testing/sysfs-bus-iio-bno055 > new file mode 100644 > index 000000000000..930a70c5a858 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-bno055 > @@ -0,0 +1,84 @@ > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Range for acceleration readings in G. Note that this does not > + affects the scale (which should be used when changing the > + maximum and minimum readable value affects also the reading > + scaling factor). Having this in G but the sensor output in m/s^2 seems inconsistent. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Range for angular velocity readings in dps. Note that this does > + not affects the scale (which should be used when changing the > + maximum and minimum readable value affects also the reading > + scaling factor). Again, units need to match or this is going to be really confusing. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range_available > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + List of allowed values for in_accel_range attribute > + > +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range_available > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + List of allowed values for in_anglvel_range attribute > + > +What: /sys/bus/iio/devices/iio:deviceX/fast_magnetometer_calibration_enable > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Can be 1 or 0. Enables/disables the "Fast Magnetometer > + Calibration" HW function. Naming needs to be consistent with the ABI. This is a channel type specific function and to match existing calibration related ABI naming it would be. in_magn_calibration_fast_enable Some of the others need renaming in a similar way. > + > +What: /sys/bus/iio/devices/iio:deviceX/fusion_enable > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Can be 1 or 0. Enables/disables the "sensor fusion" (a.k.a. > + NDOF) HW function. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_calibration_data > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Reports the binary calibration data blob for the IMU sensors. Why in_ ? What channels does this apply to? > + > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_accel > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > + Report the autocalibration status for the accelerometer sensor. For interfaces that really don't have any chance of generalising this one is terrible. Any hope at all of mapping this to something numeric? in_accel_calibration_auto_status > + > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_gyro > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > + Reports the autocalibration status for the gyroscope sensor. in_angvel_calibration_auto_status etc. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_magn > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > + Reports the autocalibration status for the magnetometer sensor. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_sys > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > + Reports the status for the IMU overall autocalibration. > + > +What: /sys/bus/iio/devices/iio:deviceX/unique_id Hmm. So normally we just dump these in the kernel log. I guess you need it here to associate a calibration blob with a particular sensor? We could put it in label, but that would stop us using that for things like positioning of the sensor. So perhaps this is something that we should add to the main ABI doc. Probably as serial_number rather than unique ID though. > +KernelVersion: 5.15 > +Contact: linux-iio@vger.kernel.org > +Description: > + 16-bytes, 2-digits-per-byte, HEX-string representing the sensor > + unique ID number.
Few inline comments; ok for the rest. Il giorno gio 28 ott 2021 alle ore 12:59 Jonathan Cameron <jic23@kernel.org> ha scritto: > > On Thu, 28 Oct 2021 12:18:36 +0200 > Andrea Merello <andrea.merello@gmail.com> wrote: > > > This patch adds ABI documentation for bno055 driver private sysfs > > attributes. > > Hohum. As normal I dislike custom attributes but reality is these > don't map to anything 'standard' and I don't want them getting adopted > in places where the 'standard' approach works. > > So thinking a bit more on this, I wonder if we can fit it into standard > ABI. > > We can't use the normal range specification method of > _scale because it's internal to the device and the output reading is > unaffected. The range specification via _raw_available would let us know > the range, but it is not writeable so.. > > A control that changes the internal scaling of the sensor in a fashion > that is not visible to the outside world maps to calibscale. Whilst > that was intended for little tweaks to the input signal (often front > end amplifier gain tweak) it works here. It doesn't map through to > anything userspace is expected to apply. That combined with > _raw_available to let us know what the result is should work? > > What do you think of that approach? It's obviously a little more complex > to handle in the driver, but it does map to existing ABI and avoids > custom attributes etc. If I read the ABI documentation, then I would say that calibscale has nothing to do with this, but I think you have obviously a better feeling than me about what calibscale is really for. To be honest I've probably not a clear idea about what calibscale is indeed... In general, I would say that is better to stick to standard attributes when possible, and of course to avoid having the same thing mapped on random custom attributes in each driver, but IMO only up to the extent which doesn't force something that is really something different to map on a standard thing just because of the sake of having as much standard things as possible... But all this is probably quite obvious, and it all depends on the above (i.e. is it calibscale fitting well in your opinion?) .. Up to you on this one.. BTW I'm missing why this should complicate the driver.. I guess I'll find out if I'll implement it :) > > > > Signed-off-by: Andrea Merello <andrea.merello@iit.it> > > --- > > .../ABI/testing/sysfs-bus-iio-bno055 | 84 +++++++++++++++++++ > > 1 file changed, 84 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055 > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-bno055 b/Documentation/ABI/testing/sysfs-bus-iio-bno055 > > new file mode 100644 > > index 000000000000..930a70c5a858 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-bno055 > > @@ -0,0 +1,84 @@ > > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Range for acceleration readings in G. Note that this does not > > + affects the scale (which should be used when changing the > > + maximum and minimum readable value affects also the reading > > + scaling factor). > > Having this in G but the sensor output in m/s^2 seems inconsistent. > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Range for angular velocity readings in dps. Note that this does > > + not affects the scale (which should be used when changing the > > + maximum and minimum readable value affects also the reading > > + scaling factor). > > Again, units need to match or this is going to be really confusing. > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range_available > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + List of allowed values for in_accel_range attribute > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range_available > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + List of allowed values for in_anglvel_range attribute > > + > > +What: /sys/bus/iio/devices/iio:deviceX/fast_magnetometer_calibration_enable > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Can be 1 or 0. Enables/disables the "Fast Magnetometer > > + Calibration" HW function. > > Naming needs to be consistent with the ABI. This is a channel type specific function > and to match existing calibration related ABI naming it would be. > > in_magn_calibration_fast_enable > > Some of the others need renaming in a similar way. > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/fusion_enable > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Can be 1 or 0. Enables/disables the "sensor fusion" (a.k.a. > > + NDOF) HW function. > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_calibration_data > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Reports the binary calibration data blob for the IMU sensors. > > Why in_ ? What channels does this apply to? > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_accel > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > + Report the autocalibration status for the accelerometer sensor. > > For interfaces that really don't have any chance of generalising this one is terrible. > Any hope at all of mapping this to something numeric? > > in_accel_calibration_auto_status > > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_gyro > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > + Reports the autocalibration status for the gyroscope sensor. > > in_angvel_calibration_auto_status > etc. > > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_magn > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > + Reports the autocalibration status for the magnetometer sensor. > > + > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_sys > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > + Reports the status for the IMU overall autocalibration. > > + > > +What: /sys/bus/iio/devices/iio:deviceX/unique_id > > Hmm. So normally we just dump these in the kernel log. I guess you need it > here to associate a calibration blob with a particular sensor? Well, it was originally in kernel log, but putting in an attribute was one of the changes that has been requested for V2. It is needed by the user who copies the calibration data to the calibration file, in order for her/him to be able to properly name it (in case of more than 1 sensor on the same setup). > We could put it in label, but that would stop us using that for things like > positioning of the sensor. So perhaps this is something that we should add > to the main ABI doc. Probably as serial_number rather than unique ID though. OK, for renaming to "serial_number". I'm not sure they are conceptually the same thing, but I think it works anyway. Of course I can move its doc to the main file. Do you want a separate patch for this? > > +KernelVersion: 5.15 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + 16-bytes, 2-digits-per-byte, HEX-string representing the sensor > > + unique ID number. >
On Tue, 9 Nov 2021 11:22:27 +0100 Andrea Merello <andrea.merello@gmail.com> wrote: > Few inline comments; ok for the rest. > > Il giorno gio 28 ott 2021 alle ore 12:59 Jonathan Cameron > <jic23@kernel.org> ha scritto: > > > > On Thu, 28 Oct 2021 12:18:36 +0200 > > Andrea Merello <andrea.merello@gmail.com> wrote: > > > > > This patch adds ABI documentation for bno055 driver private sysfs > > > attributes. > > > > Hohum. As normal I dislike custom attributes but reality is these > > don't map to anything 'standard' and I don't want them getting adopted > > in places where the 'standard' approach works. > > > > So thinking a bit more on this, I wonder if we can fit it into standard > > ABI. > > > > We can't use the normal range specification method of > > _scale because it's internal to the device and the output reading is > > unaffected. The range specification via _raw_available would let us know > > the range, but it is not writeable so.. > > > > A control that changes the internal scaling of the sensor in a fashion > > that is not visible to the outside world maps to calibscale. Whilst > > that was intended for little tweaks to the input signal (often front > > end amplifier gain tweak) it works here. It doesn't map through to > > anything userspace is expected to apply. That combined with > > _raw_available to let us know what the result is should work? > > > > What do you think of that approach? It's obviously a little more complex > > to handle in the driver, but it does map to existing ABI and avoids > > custom attributes etc. > > If I read the ABI documentation, then I would say that calibscale has > nothing to do with this, but I think you have obviously a better > feeling than me about what calibscale is really for. To be honest I've > probably not a clear idea about what calibscale is indeed... Original intent was that it was a tweak for input amplifiers on some sensor types that you'd set as part of a calibration process. These days, for many sensors that have this it's handled at factory anyway and these tweak values are rarely exposed to software. > > In general, I would say that is better to stick to standard attributes > when possible, and of course to avoid having the same thing mapped on > random custom attributes in each driver, but IMO only up to the extent > which doesn't force something that is really something different to > map on a standard thing just because of the sake of having as much > standard things as possible... But all this is probably quite obvious, > and it all depends on the above (i.e. is it calibscale fitting well in > your opinion?) .. Up to you on this one.. > > BTW I'm missing why this should complicate the driver.. I guess I'll > find out if I'll implement it :) Inverse of the range values which is always a mess without floating point. Ok, I'm persuaded that we have to go with range here even if it is a bit painful and might cause confusion if we start getting it in lots of drivers. *fingers crossed we don't* There is still a units question though. Should we express the ranges in _processed or _raw units? Or do we make it explicit and call it rangeprocessed for example? For some devices the range will naturally be expressed as the range of ADC raw values, so there is definite room for confusion if we don't make it clear in the name. I'm open to other suggestions of how we name this to avoid falling into any heffalump traps. > > > > > > > Signed-off-by: Andrea Merello <andrea.merello@iit.it> > > > --- > > > .../ABI/testing/sysfs-bus-iio-bno055 | 84 +++++++++++++++++++ > > > 1 file changed, 84 insertions(+) > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055 > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-bno055 b/Documentation/ABI/testing/sysfs-bus-iio-bno055 > > > new file mode 100644 > > > index 000000000000..930a70c5a858 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-bno055 > > > @@ -0,0 +1,84 @@ > > > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Range for acceleration readings in G. Note that this does not > > > + affects the scale (which should be used when changing the > > > + maximum and minimum readable value affects also the reading > > > + scaling factor). > > > > Having this in G but the sensor output in m/s^2 seems inconsistent. > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Range for angular velocity readings in dps. Note that this does > > > + not affects the scale (which should be used when changing the > > > + maximum and minimum readable value affects also the reading > > > + scaling factor). > > > > Again, units need to match or this is going to be really confusing. > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range_available > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + List of allowed values for in_accel_range attribute > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range_available > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + List of allowed values for in_anglvel_range attribute > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/fast_magnetometer_calibration_enable > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Can be 1 or 0. Enables/disables the "Fast Magnetometer > > > + Calibration" HW function. > > > > Naming needs to be consistent with the ABI. This is a channel type specific function > > and to match existing calibration related ABI naming it would be. > > > > in_magn_calibration_fast_enable > > > > Some of the others need renaming in a similar way. > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/fusion_enable > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Can be 1 or 0. Enables/disables the "sensor fusion" (a.k.a. > > > + NDOF) HW function. > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_calibration_data > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Reports the binary calibration data blob for the IMU sensors. > > > > Why in_ ? What channels does this apply to? > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_accel > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > > + Report the autocalibration status for the accelerometer sensor. > > > > For interfaces that really don't have any chance of generalising this one is terrible. > > Any hope at all of mapping this to something numeric? > > > > in_accel_calibration_auto_status > > > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_gyro > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > > + Reports the autocalibration status for the gyroscope sensor. > > > > in_angvel_calibration_auto_status > > etc. > > > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_magn > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > > + Reports the autocalibration status for the magnetometer sensor. > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_sys > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". > > > + Reports the status for the IMU overall autocalibration. > > > + > > > +What: /sys/bus/iio/devices/iio:deviceX/unique_id > > > > Hmm. So normally we just dump these in the kernel log. I guess you need it > > here to associate a calibration blob with a particular sensor? > > Well, it was originally in kernel log, but putting in an attribute was > one of the changes that has been requested for V2. Oops. :) Inconsistency is my middle name... > It is needed by the user who copies the calibration data to the > calibration file, in order for her/him to be able to properly name it > (in case of more than 1 sensor on the same setup). Fair enough that makes complete sense. > > > We could put it in label, but that would stop us using that for things like > > positioning of the sensor. So perhaps this is something that we should add > > to the main ABI doc. Probably as serial_number rather than unique ID though. > > OK, for renaming to "serial_number". I'm not sure they are > conceptually the same thing, but I think it works anyway. > Of course I can move its doc to the main file. Do you want a separate > patch for this? Separate patch would be great. Thanks, Jonathan > > > > +KernelVersion: 5.15 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + 16-bytes, 2-digits-per-byte, HEX-string representing the sensor > > > + unique ID number. > >
Sorry for the huge delay... > There is still a units question though. Should we express the ranges > in _processed or _raw units? Or do we make it explicit and call it > rangeprocessed for example? For some devices the range will naturally > be expressed as the range of ADC raw values, so there is definite room > for confusion if we don't make it clear in the name. > > I'm open to other suggestions of how we name this to avoid falling into > any heffalump traps. You are right: this might lead to confusion.. Making it explicit in the name seems a good idea. I've looked at other iio sysfs attributes in the DOC. It seems that "thesh" and "roc" attributes allows for both preprocessed and raw data: I found e.g. "<type>[Y][_name]_<raw|input>_thresh_value", but the related "what" entries written above all seem to omit both "_raw" and "_input"; I don't understand why. In any case, maybe we can stick to that already-existent naming schema? Assuming the pattern is correct, then wouldn't it be "in_accel_raw_range" (or "in_accel_x_raw_range", in case it could have different values for each axis) or "in_accel_input_range" in case range applies to preprocessed vals, etc ? Andrea
On Tue, 4 Jan 2022 12:42:40 +0100 Andrea Merello <andrea.merello@gmail.com> wrote: > Sorry for the huge delay... No problem though I may have forgotten some of the discussion! > > > There is still a units question though. Should we express the ranges > > in _processed or _raw units? Or do we make it explicit and call it > > rangeprocessed for example? For some devices the range will naturally > > be expressed as the range of ADC raw values, so there is definite room > > for confusion if we don't make it clear in the name. > > > > I'm open to other suggestions of how we name this to avoid falling into > > any heffalump traps. > > You are right: this might lead to confusion.. Making it explicit in > the name seems a good idea. > > I've looked at other iio sysfs attributes in the DOC. It seems that > "thesh" and "roc" attributes allows for both preprocessed and raw > data: I found e.g. "<type>[Y][_name]_<raw|input>_thresh_value", but > the related "what" entries written above all seem to omit both "_raw" > and "_input"; I don't understand why. Excellent point. That documentation is garbage. Events are meant to pick it up implicitly from the related channel _raw or _input. I don't remember them ever having raw or input in their naming but it's possible they did right at the beginning before the ABI was anywhere near stable. Gah. I dread to think how long that that has been wrong. > > In any case, maybe we can stick to that already-existent naming schema? It doesn't exist really the docs are wrong. > > Assuming the pattern is correct, then wouldn't it be > "in_accel_raw_range" (or "in_accel_x_raw_range", in case it could > have different values for each axis) or "in_accel_input_range" in case > range applies to preprocessed vals, etc ? Tricky corner but I'd go with no, because the pattern is direction_type_infotype and in this case the infotype is rangeraw. We've not been totally consistent on whether we allow spaces in infotype or not. Intially we always did but then some of the userspace folks asked us to stop doing so because it requires all userspace software to have an explicit list rather than just adding controls to some GUI based on generic parsing. Hohum. Historical decisions that lead to messy interfaces... *sigh* Nearest to what you have here though are peak_raw and mean_raw though those are odd in of themselves in that they are basically special forms of _raw rather than something else that is in _raw units... So I think range_raw postfix is the best bet. Jonathan > > > Andrea
Trivial inline comments below. Beside that, I've found another
pleasing issue with this "range" thing on this device..
One one hand, things seem to always work as we discussed for the
accelerometer (i.e. range doesn't affect the scale; the HW always
provides readings in the same scale, but with different range and
precision) on the other hand, the gyroscope behavior depends by the
internal IMU firmware version.. great..
Stock firmware has a bug[0], so that the "range" gyroscope registers
do change the scale indeed. AFAICT stock firmware is the one you find
in most (all?) breakout boards, which are usually available (and which
I'm using right now for this driver mainlining attempt). Upgrading
firmware looks like a rather obscure process that AFAICT can be done
only in some specific USB-stick demo-board ("shuttle board") or with
maybe with FAE assistance on custom developed boards [1] (i.e. maybe
can be done by some professional user; I would say not for most
people).
So, I'm now wondering how to handle this... I really want to support
the stock FW, which seems the most widespread, and the one I have
right now; I'd say this means: the accelerometer thing will still work
as we discussed (i.e. the range attribute thing), while the gyro will
have writeable scale, and a (ro) scale_available attrib. But what
about the gyro range thing? Should I drop it, or keep it as
informative read-only?
Then I could also support the new firmware (which I cannot test right
now with my actual breakout board, but I might see whether I could get
a board with an updated IMU), keeping also the current driver behavior
(i.e. range stuff).
But the question is: in either cases (new vs old fw) should the
non-necessary attributes disappear or they may just be RO or locked
(i.e. scale_available for new FW and range stuff for the old one)?
Any thoughts and advice on this whole thing would be very welcome :)
my current inclination anyway now tends to be: go on supporting only
the stock FW (i.e. the board I have here now) and eventually add
support for the new fw later on, after merge.
[0] https://community.bosch-sensortec.com/t5/MEMS-sensors-forum/BNO055-Wrong-sensitivity-resolution-in-datasheet/td-p/10266
[1] https://community.bosch-sensortec.com/t5/MEMS-sensors-forum/BNO055-Software-Version/td-p/14001
> > I've looked at other iio sysfs attributes in the DOC. It seems that
> > "thesh" and "roc" attributes allows for both preprocessed and raw
> > data: I found e.g. "<type>[Y][_name]_<raw|input>_thresh_value", but
> > the related "what" entries written above all seem to omit both "_raw"
> > and "_input"; I don't understand why.
>
> Excellent point. That documentation is garbage. Events are meant
> to pick it up implicitly from the related channel _raw or _input.
> I don't remember them ever having raw or input in their naming but
> it's possible they did right at the beginning before the ABI was anywhere
> near stable. Gah. I dread to think how long that that has been wrong.
Ok, great :)
> So I think range_raw postfix is the best bet.
Will go with this, thanks.
> Jonathan
>
>
>
>
>
> >
>
> >
> > Andrea
>
On Mon, 17 Jan 2022 10:37:33 +0100 Andrea Merello <andrea.merello@gmail.com> wrote: > Trivial inline comments below. Beside that, I've found another > pleasing issue with this "range" thing on this device.. > > One one hand, things seem to always work as we discussed for the > accelerometer (i.e. range doesn't affect the scale; the HW always > provides readings in the same scale, but with different range and > precision) on the other hand, the gyroscope behavior depends by the > internal IMU firmware version.. great.. *sigh* :) > > Stock firmware has a bug[0], so that the "range" gyroscope registers > do change the scale indeed. AFAICT stock firmware is the one you find > in most (all?) breakout boards, which are usually available (and which > I'm using right now for this driver mainlining attempt). Upgrading > firmware looks like a rather obscure process that AFAICT can be done > only in some specific USB-stick demo-board ("shuttle board") or with > maybe with FAE assistance on custom developed boards [1] (i.e. maybe > can be done by some professional user; I would say not for most > people). > > So, I'm now wondering how to handle this... I really want to support > the stock FW, which seems the most widespread, and the one I have > right now; I'd say this means: the accelerometer thing will still work > as we discussed (i.e. the range attribute thing), while the gyro will > have writeable scale, and a (ro) scale_available attrib. But what > about the gyro range thing? Should I drop it, or keep it as > informative read-only? I'd be cynical and for initial version at least, just hide it as 'too complex' with a comment in the driver code on why. > > Then I could also support the new firmware (which I cannot test right > now with my actual breakout board, but I might see whether I could get > a board with an updated IMU), keeping also the current driver behavior > (i.e. range stuff). > > But the question is: in either cases (new vs old fw) should the > non-necessary attributes disappear or they may just be RO or locked > (i.e. scale_available for new FW and range stuff for the old one)? If they don't have meaning then they should disappear, but it would also be valid to have the 'broken' one be read only if there is an appropriate value. > > Any thoughts and advice on this whole thing would be very welcome :) > my current inclination anyway now tends to be: go on supporting only > the stock FW (i.e. the board I have here now) and eventually add > support for the new fw later on, after merge. Sounds sensible - but.... Make sure you check the firmware version number (I hope it has one) and print a warning at least if you get one that you have strong reason to believe will handle this differently from whatever the driver is supporting. This is definitely going to be a case for detailed comments in the driver code so that we can 'recall' what on earth was going on here in N years time! > > [0] https://community.bosch-sensortec.com/t5/MEMS-sensors-forum/BNO055-Wrong-sensitivity-resolution-in-datasheet/td-p/10266 > [1] https://community.bosch-sensortec.com/t5/MEMS-sensors-forum/BNO055-Software-Version/td-p/14001 > > > > I've looked at other iio sysfs attributes in the DOC. It seems that > > > "thesh" and "roc" attributes allows for both preprocessed and raw > > > data: I found e.g. "<type>[Y][_name]_<raw|input>_thresh_value", but > > > the related "what" entries written above all seem to omit both "_raw" > > > and "_input"; I don't understand why. > > > > Excellent point. That documentation is garbage. Events are meant > > to pick it up implicitly from the related channel _raw or _input. > > I don't remember them ever having raw or input in their naming but > > it's possible they did right at the beginning before the ABI was anywhere > > near stable. Gah. I dread to think how long that that has been wrong. > > Ok, great :) > > > So I think range_raw postfix is the best bet. > > Will go with this, thanks. > > > Jonathan > > > > > > > > > > > > > > > > > > > > > Andrea > >
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-bno055 b/Documentation/ABI/testing/sysfs-bus-iio-bno055 new file mode 100644 index 000000000000..930a70c5a858 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-bno055 @@ -0,0 +1,84 @@ +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Range for acceleration readings in G. Note that this does not + affects the scale (which should be used when changing the + maximum and minimum readable value affects also the reading + scaling factor). + +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Range for angular velocity readings in dps. Note that this does + not affects the scale (which should be used when changing the + maximum and minimum readable value affects also the reading + scaling factor). + +What: /sys/bus/iio/devices/iio:deviceX/in_accel_range_available +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + List of allowed values for in_accel_range attribute + +What: /sys/bus/iio/devices/iio:deviceX/in_anglvel_range_available +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + List of allowed values for in_anglvel_range attribute + +What: /sys/bus/iio/devices/iio:deviceX/fast_magnetometer_calibration_enable +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Can be 1 or 0. Enables/disables the "Fast Magnetometer + Calibration" HW function. + +What: /sys/bus/iio/devices/iio:deviceX/fusion_enable +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Can be 1 or 0. Enables/disables the "sensor fusion" (a.k.a. + NDOF) HW function. + +What: /sys/bus/iio/devices/iio:deviceX/in_calibration_data +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Reports the binary calibration data blob for the IMU sensors. + +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_accel +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". + Report the autocalibration status for the accelerometer sensor. + +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_gyro +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". + Reports the autocalibration status for the gyroscope sensor. + +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_magn +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". + Reports the autocalibration status for the magnetometer sensor. + +What: /sys/bus/iio/devices/iio:deviceX/in_autocalibration_status_sys +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + Can be "Idle", "Bad", "Barely enough", "Fair", or "Good". + Reports the status for the IMU overall autocalibration. + +What: /sys/bus/iio/devices/iio:deviceX/unique_id +KernelVersion: 5.15 +Contact: linux-iio@vger.kernel.org +Description: + 16-bytes, 2-digits-per-byte, HEX-string representing the sensor + unique ID number.
This patch adds ABI documentation for bno055 driver private sysfs attributes. Signed-off-by: Andrea Merello <andrea.merello@iit.it> --- .../ABI/testing/sysfs-bus-iio-bno055 | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-bno055