zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.64k stars 6.51k forks source link

ST Sensors: Driver factorization #7001

Closed erwango closed 5 years ago

erwango commented 6 years ago

As discussed in #6420, several sensors reuse the same bits and drivers that get them working could be optimized taking this fact into account.

Raise a dedicated topic to keep the discussion going on

erwango commented 6 years ago

@avisconti , @ydamigos , let's continue the discussion here.

avisconti commented 5 years ago

I would like to restart this discussion as I feel it is time to make some progress on the ST sensor support in zephyr. The aim is to reorganise the drivers in order to reuse as match as possible the s/w: this will help in both maintainance as well as support of new sensors (infact, some new sensors are just combos of already existing ones or with same interface).

Let's say that a given device usually has a who_am_i register with a value that is unique for that particular device. This value represent a sort of given register i/f to the s/w. It may happen, though, that two device with same who_am_i value (which means that have same register i/f) require a little different implementation because the mem sensor has some difference. But this case is rare and in any case may be handled thru ifdefs.

I would like to re-organize the drivers on the base of the who_am_i value. Current situation (including wai):

As you can see there are 3 devices with same wai (0x33): lis2dh/lis3dh/LSM303DLHC_accel. They can share same driver.

Moreover we should be able with little effort to add support for other sensors too. For example:

I'm studying a way to reuse the drivers in a smart way. In linux this is done using multiple 'compatible' strings in the common drivers.

avisconti commented 5 years ago

STEP1: Use same driver for all devices using 0x33 wai reg_if.

For common driver name we have at least two possibilities:

  1. using name of the most used device (i.e. lis3dh)
  2. use a name that makes clear that 0x33 is the reg_if.

The latter not always work and sometimes is not liked by marketing, but for the 0x33 case this is what we have in linux and I would suggest to use it. (linux: st_acc33_xxx.c).

Then I will use the most complete one and best written driver among the 3 existing ones as a common driver. I will delete the other two.

In the common driver I will specifiy in the Kconfig that the driver is used for all 3 devices (i.e. lis2dh, lis3dh, lsm303dlhc_accel). This will be the only place where it is documented the re-use.

From dts point of view there will be something like that:

lis3dh@19 {
compatible = "st,acc33"; reg = <0x19>; label = "LIS3DH"; };

What do you think?

I'm a little bit worried about the amount of work. Maybe it is not that much. Also if someone can help me out in testing as more boards as possible would be great!

EDIT:: The STEP1 can be further divided into two sub-steps:

  1. using lsm303dlhc_accel for all 3 devices (removing the other two)
  2. changing name into st_acc33 (if required)
avisconti commented 5 years ago

I'm closing down this issue as it has been addressed.

WilliamGFish commented 3 years ago

Hi, Did this effort stop, as I am looking at the Nano33 Sense and its sensors? The LSM9DS1 driver would be useful.

Billy..

avisconti commented 3 years ago

Hi, Did this effort stop, as I am looking at the Nano33 Sense and its sensors? The LSM9DS1 driver would be useful.

Billy..

@WilliamGFish The LSM9DS1 sensor is covered by this two drivers together: LSM6DS0 + LIS3MDL

You can watch https://github.com/zephyrproject-rtos/zephyr/blob/main/boards/shields/x_nucleo_iks01a1/x_nucleo_iks01a1.overlay DT as an example.

On the iks01a1 shields there are two separated devices, but if you have them combined together in LSM9DS1 device then DT would look exactly the same.

o7-machinehum commented 1 year ago

Hi, Did this effort stop, as I am looking at the Nano33 Sense and its sensors? The LSM9DS1 driver would be useful. Billy..

@WilliamGFish The LSM9DS1 sensor is covered by this two drivers together: LSM6DS0 + LIS3MDL

You can watch https://github.com/zephyrproject-rtos/zephyr/blob/main/boards/shields/x_nucleo_iks01a1/x_nucleo_iks01a1.overlay DT as an example.

On the iks01a1 shields there are two separated devices, but if you have them combined together in LSM9DS1 device then DT would look exactly the same.

Thanks for this!