sensors: Sensor channel DT child nodes

[teburd]

A rough, WIP, idea of maybe describing sensor channels as child nodes of a sensor. Original idea was from @MaureenHelm

Some nice attributes of this... it maybe better matches up with what the sensor subsystem needs (a channel rather than sensor in terminology). It enables possibly adding optional channels of information for devices that represent sensor hubs like bosch's bhi260ap which has many potential channels of information.

A single numerical value could represent a channel, and the information about that channel could be obtained perhaps using DT macros statically without any allocation or C calls involved

Related Issues/PRs
#64478
#63830
#61163

[teburd]

So unfortunately, atleast today, there's no great way of patching up a tree given a compatible, or supplying a "default" subtree for a given compatible which would be really useful for this sort of thing but at least this builds and can sort of give some clues as to what I was thinking

I wouldn't release this requiring #include <some_sensor.dtsi> in the node as that's a bit awkward. Instead ideally the node would have some default child nodes created through the binding perhaps (would make sense to me anyways).

From here all the APIs would then reference a channel index rather than a channel type. The channel index for a given type could be obtained through some DT macros or sensor APIs perhaps? Or we could stick with the copy friendly chan spec as the unique identifier (so the pair of type, index) as #63830 noted.

It's a very rough idea, but at least its a place to tinker around perhaps. Each channel could specify an array of available ODR (associating a value for the register perhaps), available power modes, available filter modes, available sensitivities, and so on.

@avisconti was doing that at a sensor level which is cool as well for st sensors

[yperess]

I'm personally not a big fan of the subtree approach (specifically because we can't set a default). I don't envision that we'll need much data here and I think that having something like the following would be enough:

bmi160 {
  compatible = "bosch,bmi160";
  channels = <SENSOR_CHAN_ACCEL_XYZ, SENSOR_CHAN_GYRO_XYZ, SENSOR_CHAN_DIE_TEMP>;
  attributes = <
                SENSOR_CHAN_ALL       SENSOR_ATTR_BATCH_DURATION
                SENSOR_CHAN_ACCEL_XYZ SENSOR_ATTR_SAMPLING_FREQUENCY
                SENSOR_CHAN_ACCEL_XYZ SENSOR_ATTR_OFFSET
                ...
                >;
};

[teburd]

I'm personally not a big fan of the subtree approach (specifically because we can't set a default). I don't envision that we'll need much data here and I think that having something like the following would be enough:

bmi160 {
  compatible = "bosch,bmi160";
  channels = <SENSOR_CHAN_ACCEL_XYZ, SENSOR_CHAN_GYRO_XYZ, SENSOR_CHAN_DIE_TEMP>;
  attributes = <
                SENSOR_CHAN_ALL       SENSOR_ATTR_BATCH_DURATION
                SENSOR_CHAN_ACCEL_XYZ SENSOR_ATTR_SAMPLING_FREQUENCY
                SENSOR_CHAN_ACCEL_XYZ SENSOR_ATTR_OFFSET
                ...
                >;
};

How would you describe multiple channels of the same type in this scenario? This was the initial issue noted in some other PRs.

In case of multiple simple temperature channels this could be pretty easily done with child nodes.

There's also some potential benefit here in being able to configure and enable optional channels, e.g. some devices may act as sensor hubs with externally connected devices, or optional channels.

bhi260ap is the extreme end of this. Imagine a DT for this device with all the channels that you can then enable/disable/configure like SoC peripherals. That would be pretty nice.

I'm not opposed to using properties, this is a rough idea coming from Maureen initially and I think it warranted exploring. I like some attributes of this route.

But really I feel like its DT that's limiting the options here as I'd like to have an array of structured data by default for this just like an SoC sort of has a default tree for a board.

To do it in DT with defaults the way we do things today I'd need to set it up as a destructured set of arrays.. which has some other limitations, like we can't describe all the channels then set their status and configure them individually which for the most part otherwise would be fine to do.

bmi160 {
  compatible = "bosch,bmi160";
  /* position in the array here implies the channels index */
  channels = <SENSOR_CHANNEL_ACCEL, SENSOR_CHANNEL_GYRO, SENSOR_CHANNEL_TEMP>;
  channel-fields = <<SENSOR_FIELD_X, SENSOR_FIELD_Y, SENSOR_FIELD_Z>,
                    <SENSOR_FIELD_X, SENSOR_FIELD_Y, SENSOR_FIELD_Z>,
                    <SENSOR_FIELD_DIE>>;
};

I'd note that the way the ST sensors now work with enum ODR/filter/sensitivity options is quite nice but doesn't match up with attributes very well as you have them. That's something to look at harder seemingly.

[teburd]

Sensor WG: No one likes the include
Brix: Take a look at pinctl as a possible answer (index properties)
Yuval: Wants to use these in class templates, as long as an array of channels is accessible.
Brix: ADC controller binding uses child nodes for channel bindings

[lixuzha]

If there are multiple sensors of the same type in a chip, the sensing subsystem cannot use the existing sensor API to handle this situation. This pull request introduces channel index, which is a way to solve this problem. For me, this is a good start.

[github-actions]

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.