Skip to content

Add single driver for both lis2dux12 AND lis2duxs12 #80142

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.

Sign up for GitHub

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

Jump to bottom
Merged
kartben merged 4 commits into from
Dec 27, 2024
Merged

Add single driver for both lis2dux12 AND lis2duxs12 #80142

kartben merged 4 commits into from
Dec 27, 2024

Conversation

@avisconti
Copy link
Contributor

@avisconti avisconti commented Oct 21, 2024 •
edited
Loading

lis2dux12 and lis2duxs12 are belonging to the same device family and are registers compatible. Hence they can be handled by the same driver. This work is done on the same basis as lps2xdf driver (#65416).

In particular:

  1. Introduce the lis2dux12_chip_api structure filled with device specific APIs. At the moment define following callback:
        - api_lis2dux12_set_odr_raw()
        - api_lis2dux12_set_range()
        - api_lis2dux12_sample_fetch_accel()
        - api_lis2dux12_sample_fetch_temp()
        - api_lis2dux12_handle_interrupt()
        - api_lis2dux12_init_interrupt()

  1. Prepare specific APIs for st_lis2dux12 device.

  2. Make DT instantiation on inst + name basis, and not only by inst. This commit only use "name = DT_DRV_COMPAT = st_lis2dux12".

@avisconti avisconti added area: Drivers area: Sensors Sensors area: Devicetree Binding PR modifies or adds a Device Tree binding labels Oct 21, 2024
@avisconti avisconti requested a review from ubieda October 21, 2024 11:57
@avisconti avisconti self-assigned this Oct 21, 2024
@zephyrbot zephyrbot added area: Shields Shields (add-on boards) area: Samples Samples labels Oct 21, 2024
@jfischer-no jfischer-no removed their request for review October 21, 2024 13:25
@avisconti avisconti requested a review from jonas-rem October 23, 2024 14:30
@avisconti
Copy link
Contributor Author

avisconti commented Oct 23, 2024

@jonas-rem as you can see I re-used your work also for the lis2dux12/lis2duxs12 driver. Here the naming is a bit confusing. Would you mind give a quick review to it? Thx

jonas-rem
jonas-rem requested changes Oct 24, 2024
View reviewed changes
Copy link

@jonas-rem jonas-rem left a comment

Choose a reason for hiding this comment

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

The separation via api looks good to me. Few minor points and questions for clarification.

  • Add the lis2duxs12 variant to testss/driverss/build_all/sensor/i2C
  • The currently used feature- and register-set of both variants is identical. The s variant supports analog hub and qvar sensing, so this separation allows to implement those features in the future?
  • I3C would be supported by both devices, I guess it's just ouf-of-scope for this PR?

@avisconti
Copy link
Contributor Author

avisconti commented Oct 25, 2024

The separation via api looks good to me. Few minor points and questions for clarification.

  • Add the lis2duxs12 variant to testss/driverss/build_all/sensor/i2C

ok, I will

  • The currently used feature- and register-set of both variants is identical. The s variant supports analog hub and qvar sensing, so this separation allows to implement those features in the future?

Yes, that's my idea. Not planned yet though.

  • I3C would be supported by both devices, I guess it's just ouf-of-scope for this PR?

Well, I can add a commit in this PR itself. I'm not sure I'm in a position to test it though. Thanks for suggesting it.

@avisconti avisconti force-pushed the add-single-driver-dux branch from 15713e1 to 87d3886 Compare October 28, 2024 14:21
@avisconti avisconti requested a review from jonas-rem October 28, 2024 14:22
@avisconti
Copy link
Contributor Author

avisconti commented Oct 29, 2024

@jonas-rem I think I forgot to add I3C part... I think I would do a separate PR

@avisconti
Copy link
Contributor Author

avisconti commented Nov 11, 2024

@jonas-rem I think I forgot to add I3C part... I think I would do a separate PR

@jonas-rem would it be acceptable the PR as it is at the moment?

@jonas-rem
Copy link

jonas-rem commented Nov 11, 2024

@jonas-rem I think I forgot to add I3C part... I think I would do a separate PR

@jonas-rem would it be acceptable the PR as it is at the moment?

There is one open comment, could you have a look at that? Apart from that the PR looks good. #80142 (comment)

@avisconti
Copy link
Contributor Author

avisconti commented Nov 15, 2024

@jonas-rem I think I forgot to add I3C part... I think I would do a separate PR

@jonas-rem would it be acceptable the PR as it is at the moment?

There is one open comment, could you have a look at that? Apart from that the PR looks good. #80142 (comment)

Yes sure. I missed it.

@avisconti avisconti force-pushed the add-single-driver-dux branch 3 times, most recently from d87c792 to 1963f79 Compare November 20, 2024 14:24
@avisconti avisconti force-pushed the add-single-driver-dux branch from 1963f79 to cf1d06d Compare November 20, 2024 14:34
@avisconti
Copy link
Contributor Author

avisconti commented Nov 22, 2024

@jonas-rem Is it fine now Jonas?

jonas-rem
jonas-rem previously approved these changes Nov 22, 2024
View reviewed changes
@avisconti avisconti force-pushed the add-single-driver-dux branch from cf1d06d to 75ca45d Compare December 17, 2024 13:21
@avisconti avisconti requested a review from jonas-rem December 17, 2024 13:21
@zephyrbot zephyrbot requested a review from asemjonovs December 17, 2024 13:21
@avisconti avisconti added this to the v4.1.0 milestone Dec 17, 2024
jonas-rem
jonas-rem previously approved these changes Dec 19, 2024
View reviewed changes
erwango
erwango previously approved these changes Dec 20, 2024
View reviewed changes
@kartben
Copy link
Contributor

kartben commented Dec 20, 2024

@avisconti unfortunately, this needs rebasing

@avisconti
drivers/sensor: lis2dux12: re-working to be DUX generic
7fc8fa5 
Prepare the driver to be able to handle other DUX software compatible
devices. Currently the driver handles the st_lis2dux12 compatible only.

Do following major changes:

1. Introduce the lis2dux12_chip_api structure filled with device specific
   APIs. At the moment define following callback:

    - api_lis2dux12_set_odr_raw()
    - api_lis2dux12_set_range()
    - api_lis2dux12_sample_fetch_accel()
    - api_lis2dux12_sample_fetch_temp()
    - api_lis2dux12_handle_interrupt()
    - api_lis2dux12_init_interrupt()

2. Prepare specific APIs for st_lis2dux12 device.

3. Make DT instantiation on inst + name basis, and not only by inst. This
   commit only use "name = DT_DRV_COMPAT = st_lis2dux12".

Signed-off-by: Armando Visconti <[email protected]>
@avisconti
drivers/sensor: lis2dux12: add lis2duxs12 support
252a24f 
The LIS2DUXS12 is a smart, digital, 3-axis linear accelerometer whose
MEMS and ASIC have been expressly designed to combine the lowest current
consumption possible with features such as always-on antialiasing
filtering, a finite state machine (FSM) and machine learning core (MLC)
with adaptive self-configuration (ASC), and an analog hub / Qvar sensing
channel.

(https://www.st.com/en/mems-and-sensors/lis2duxs12.html)

Signed-off-by: Armando Visconti <[email protected]>
@avisconti
boards/shields: x-nucleo-iks4a1: add support to LIS2DUXS12
6100148 
Add 3-axis linear accelerometer LIS2DUXS12.

Signed-off-by: Armando Visconti <[email protected]>
@avisconti
samples/shields: x-nucleo-iks4a1: Add lis2dux12 accel and temp test
5bd6586 
Add sample display for lis2duxs12 accel and temp sensors.

Signed-off-by: Armando Visconti <[email protected]>
@avisconti avisconti dismissed stale reviews from erwango and jonas-rem via 5bd6586 December 27, 2024 15:30
@avisconti avisconti force-pushed the add-single-driver-dux branch from 75ca45d to 5bd6586 Compare December 27, 2024 15:30
@avisconti
Copy link
Contributor Author

avisconti commented Dec 27, 2024

@avisconti unfortunately, this needs rebasing

Yes, the usual i2c.dtsi file...

@avisconti
Copy link
Contributor Author

avisconti commented Dec 27, 2024

Thanks @jonas-rem and @erwango for you review. Unfortunately I had to rebase the work and push it again...

@kartben kartben merged commit 02fa47f into zephyrproject-rtos:main Dec 27, 2024
24 checks passed
@avisconti avisconti deleted the add-single-driver-dux branch December 29, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kartben kartben kartben approved these changes

@jonas-rem jonas-rem jonas-rem approved these changes

@ubieda ubieda Awaiting requested review from ubieda

@jeppenodgaard jeppenodgaard Awaiting requested review from jeppenodgaard

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@nashif nashif Awaiting requested review from nashif

@teburd teburd Awaiting requested review from teburd

@tristan-google tristan-google Awaiting requested review from tristan-google

@yperess yperess Awaiting requested review from yperess

@asemjonovs asemjonovs Awaiting requested review from asemjonovs

@erwango erwango Awaiting requested review from erwango

Assignees

@avisconti avisconti

Labels

area: Devicetree Binding PR modifies or adds a Device Tree binding area: Drivers area: Samples Samples area: Sensors Sensors area: Shields Shields (add-on boards)

Projects

None yet

Milestone

v4.1.0

Development

Successfully merging this pull request may close these issues.

5 participants

@avisconti @jonas-rem @kartben @erwango @zephyrbot