Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dts: bindings: Introduce base sensor device properties #49294

Closed

Conversation

MaureenHelm
Copy link
Member

@MaureenHelm MaureenHelm commented Aug 19, 2022

Introduces an initial set of devicetree properties to be inherited by
all sensor devices, similar to how we define a base set of devicetree
properties for I2C and SPI devices. These properties will be used by the
future sensor subsystem to manage and expose sensors to a host operating
system, through HID or another protocol.

Signed-off-by: Maureen Helm maureen.helm@intel.com

Fixes #48148
cc: @huhebo

A key difference between this proposal and those discussed in #48148 and #45370 is that existing sensor drivers inherit base sensor properties and don't require corresponding shim nodes for the sensor subsystem. This is one less level of indirection and puts properties like vendor and model name with the physical device. Virtual sensors also inherit base sensor properties, but unlike physical sensors, they don't inherit communication bus properties from i2c-device.yaml or spi-device.yaml.

Instantiation of data structures that contain these properties takes some inspiration from network interfaces and utilizes iterable sections to allow enumeration of all sensor drivers in the system.

Further details are documented in commit messages in this PR. If this approach is accepted, I'll extend it to all the other vendors (currently I only instrumented ST and NXP sensors).

Example 1: Physical and mocked virtual sensors

 $ west build -b frdm_k64f -p auto samples/sensor/sensor_shell/ -- -DSHIELD=x_nucleo_iks01a3
uart:~$ sensor info
0xc6e8: device: 0xc2e8, vendor: STMicroelectronics, model: LPS22HH, friendly_name: 
0xc6f8: device: 0xc318, vendor: STMicroelectronics, model: STTS751, friendly_name: 
0xc708: device: 0xc300, vendor: STMicroelectronics, model: LSM6DSO, friendly_name: Base Accelerometer/Gyroscope
0xc718: device: 0xc2b8, vendor: STMicroelectronics, model: LIS2DW12, friendly_name: Lid Accelerometer
0xc728: device: 0xc270, vendor: Vendor, model: Hingle Angle, friendly_name: Hinge Angle
0xc738: device: 0xc2d0, vendor: STMicroelectronics, model: LIS2MDL, friendly_name: Base Magnetometer
0xc748: device: 0xc258, vendor: Vendor, model: Kalman Filter, friendly_name: Orientation
0xc758: device: 0xc288, vendor: NXP, model: FXOS8700, friendly_name: 
0xc768: device: 0xc2a0, vendor: STMicroelectronics, model: HTS221, friendly_name: 

Example 2: Physical sensors on a different board

$ west build -b reel_board -p auto samples/sensor/sensor_shell/
uart:~$ sensor info
0xbda0: device: 0xbb84, vendor: NXP, model: FXOS8700, friendly_name: 

Note that other sensors on the reel board don't yet appear in the console because I've only instrumented ST and NXP sensors.

Introduces an initial set of devicetree properties to be inherited by
all sensor devices, similar to how we define a base set of devicetree
properties for I2C and SPI devices. These properties will be used by the
future sensor subsystem to manage and expose sensors to a host operating
system, through HID or another protocol.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
Migrates all ST sensors to inherit base sensor device properties, and
sets vendor and model default values accordingly.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
Migrates all NXP sensors to inherit base sensor device properties, and
sets vendor and model default values accordingly.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
Updates NXP sensor drivers to use SENSOR_DEVICE_DT_INST_DEFINE, which is
a sensor-specific variant of DEVICE_DT_DEFINE that enables optional
instantiation of additional structures to be used by the future sensor
subsystem. Currently this defines an element in the sensor info iterable
section to hold the vendor and model name.

This approach was inspired by I2C_DEVICE_DT_INST_DEFINE to streamline
adding I2C stats support across all I2C drivers.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
Adds a new conditional shell command to the sensor shell to get data
from the sensor info iterable section, such as vendor and model name,
for all sensors.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
Extends the sensor shell sample application to demonstrate the sensor
info shell command and iterable section support. An application-level
devicetree overlay for the x_nucleo_iks01a3 shield sets sensor device
properties like friendly-name to identify which sensor is the "lid"
accelerometer and which is the "base" accelerometer.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
Adds an application-level devicetree overlay for the x_nucleo_iks01a3
shield to demonstrate virtual sensor nodes in the sensor shell sample. A
stubbed virtual sensor driver instantiates a sensor device and sensor
info structure for each enabled virtual sensor node in the devicetree.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
The lpcxpresso55s16, pan1781_evb, and pan1782_evb boards have some pins
missing in their arduino_header devicetree node, which makes them
incompatible with the x_nucleo_iks01a3 shield.

Signed-off-by: Maureen Helm <maureen.helm@intel.com>
Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we need vendor info, we should extract it from the compatible and expose in gen_defines.py. There's logic already in edtlib.py to parse vendor and compare against 'vendor-prefixes.txt' database. We can expose thus further if needed.

include: base.yaml

properties:
vendor:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want vendor and such. We can expose this from edtlib and gen_defines.py. We have a lot of the logic already in _check_compatible function in edtlib.py.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea, thanks.

Copy link
Collaborator

@yperess yperess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks for this!

@@ -4,6 +4,24 @@
* SPDX-License-Identifier: Apache-2.0
*/

/ {
virtual-sensors {
Copy link
Collaborator

@huhebo huhebo Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The virtual-sensors concepts can be leveraged by the proposed senss (#45370) for virtual sensor device tree node definitions.

But from senss's view, there're some more things need to consider:

  • senss can only enumerate and management senss sensors(implemented the senss sensor API)
    So, suggest define a device tree parent node for all senss sensors, such as sensor-subsystem, then senss can enumerate all senss sensors from sensor-subsystem node.
    This can also help isolate senss sensors nodes with zephyr sensor device nodes for existing API or V2 API (drivers: Add sensors v2 API #44098), and both of them can inherit the base sensor properties, such as vendor, model, friendly-name.

  • reporters phandle property for senss virtual sensor must also point to senss sensor(s) (either virtual or physical)

  • senss physical sensor is not a zephyr physical device sensor (implement the existing API, or V2 API).

    Unlike senss virtual sensor takes other senss sensor(s) (virtual or physical) as input reporters, a senss physical sensor has no senss sensors as inputs (reporters not defined or empty), it's just a data producer, collect underlying physical device's data etc.
    The underlying device can be zephyr physical sensor device (existing API or V2), or even can be any other devices, such as a wifi/ble device, or even pure sw which an produce data and can be abstract as a sensor. This can make senss more extensible and powerful.

    So, in senss, we defined the underlying-device for a senss physical sensor to reference a none senss sensor device.

    For example:

    /{
        &senss {
                hinge_angle : hinge-angle {
                       compatible = "senss,hinge-angle"
                       reporters =<&base_accel &lid_accel>
                }
                base_accel: base-accel {
                       compatible = "senss,phy-accel"
                       underlying-device = <&bmi_spi>
                }
                lid_accel: lid-accel {
                       compatible = "senss,phy-accel"
                       underlying-device = <&bmi_i2c>
                }
        }
    
        &spi0 {
               bmi_spi: bmi@3 {
                     compatible = "bosch,bmi160"
               }
        }
    
        &i2c0 {
               bmi_i2c: bmi@68 {
                      compatible = "bosch,bmi160"
               }
        }
    }
    

    bmi_i2c and bmi_spi are non senss sensors nodes.

@huhebo
Copy link
Collaborator

huhebo commented Aug 23, 2022

I really like the sensor info command here and the general changes to DT make sense to me. Are the vendor/model properties maybe more broadly useful beyond sensors? I would think so.

I of course like that custom macro for sensors here, and easily leads to hopefully some stats and fixing the sensor get shell command to only list sensors. Might be worth tacking on here or shortly after.

The virtual sensor stuff I'm not sure makes sense in the context of this PR but I get its important to show how it would work with senss/chre.

Is it required to use the SENSOR_ device define macros to avoid breaking things? If so this is a breaking API change (I broke i2c in a way..) for external trees/drivers. Something to note.

I also very like the sensor info command, and can be leveraged in senss also :)

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed about extracting the vendor / device from the compatible property for the matching binding in edtlib. A couple of other questions.

type: string
required: false

friendly-name:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe use label for this purpose?

Seems like a legitimate usage of label to me. @galak?

type: string
required: false

child-binding:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not disagreeing with your analysis, @MaureenHelm , but I am wondering whether you've looked at iio bindings when doing it. It seems to me like iio is the de facto standard for sensor bindings in Linux and I want to understand how decisions to deviate from its bindings, if any, were made, especially given @galak 's tepid enthusiasm for deviating from Linux (conveyed to me privately)

@bjarki-andreasen
Copy link
Collaborator

@MaureenHelm The last commit in this PR #49374 shows a POC implementation of how to assign vendor and model to sensors using the new properties files, and how to create a list of sensors and their properties at compile time using the multi api device model.

What do you think?

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Nov 15, 2022
@dleach02 dleach02 removed the Stale label Nov 22, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jan 22, 2023
@github-actions github-actions bot closed this Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sensor Subsystem: Base sensor DTS bindings
9 participants