8000 drivers: sensor: Refactor drivers to use SENSOR_DEVICE_DT_INST_DEFINE by MaureenHelm · Pull Request #51352 · zephyrproject-rtos/zephyr · GitHub
[go: up one dir, main page]
Skip to content

drivers: sensor: Refactor drivers to use SENSOR_DEVICE_DT_INST_DEFINE #51352

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
merged 1 commit into from
Oct 27, 2022
Merged

drivers: sensor: Refactor drivers to use SENSOR_DEVICE_DT_INST_DEFINE #51352

merged 1 commit into from
Oct 27, 2022

Conversation

MaureenHelm
Copy link
Member

Refactors all sensor drivers to use SENSOR_DEVICE_DT_INST_DEFINE, which is a sensor-specific variant of DEVICE_DT_INST_DEFINE that provides a common place to instantiate additional data structures for the future sensor subsystem and/or sensor driver stats.

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

cc: @huhebo

This PR is another piece of #49294

@MaureenHelm
drivers: sensor: Refactor drivers to use SENSOR_DEVICE_DT_INST_DEFINE
e844828 
Refactors all sensor drivers to use SENSOR_DEVICE_DT_INST_DEFINE, which
is a sensor-specific variant of DEVICE_DT_INST_DEFINE that provides a
common place to instantiate additional data structures for the future
sensor subsystem and/or sensor driver stats.

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>
gmarull
gmarull previously requested changes Oct 18, 2022
View reviewed changes
Copy link
Member
@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

This seems to be a 1:1 wrapper around DEVICE_DT_DEFINE, why is this needed now? I2C/CAN have a use case for it.

@MaureenHelm
Copy link
Member Author

This seems to be a 1:1 wrapper around DEVICE_DT_DEFINE, why is this needed now? I2C/CAN have a use case for it.

This is needed to enable the sensor info iterable section and shell command in #49294. That PR introduced several new concepts (vendor and model names, base sensor devicetree properties, sensor info section and shell command, and virtual sensors), so I've been cleaning up and resubmitting each part separately to reduce the scope and get into a mergeable state. See #49489, #50015, #50929

@MaureenHelm
Copy link
Member Author

@gmarull ping

galak
galak reviewed Oct 25, 2022
View reviewed changes
Copy link
Contributor
@galak galak left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can we move init level (POST_KERNEL) and priority CONFIG_SENSOR_INIT_PRIORITY to be hidden inside of SENSOR_DEVICE_DT_DEFINE?

galak
galak reviewed Oct 25, 2022
View reviewed changes
Copy link
Contributor
@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.

Should we even allow SENSOR_DEVICE_DT_DEFINE? Meaning should we just have everyone use SENSOR_DEVICE_DT_INST_DEFINE and thus have new sensor drivers be multi-instance to start?

@galak galak added the DNM This PR should not be merged (Do Not Merge) label Oct 26, 2022
@galak
Copy link
Contributor
galak commented Oct 26, 2022

Marked DNM to wait to get feedback on my questions.

@gmarull
Copy link
Member
gmarull commented Oct 26, 2022

Can we move init level (POST_KERNEL) and priority CONFIG_SENSOR_INIT_PRIORITY to be hidden inside of SENSOR_DEVICE_DT_DEFINE?

I'd not remove that unless we have a real alternative. If for whatever reason a sensor needs to be initialized before Kernel, there'll be no choice. There's actually one sensor in PRE_KERNEL_2.

@MaureenHelm
Copy link
Member Author

Can we move init level (POST_KERNEL) and priority CONFIG_SENSOR_INIT_PRIORITY to be hidden inside of SENSOR_DEVICE_DT_DEFINE?

I'd not remove that unless we have a real alternative. If for whatever reason a sensor needs to be initialized before Kernel, there'll be no choice. There's actually one sensor in PRE_KERNEL_2.

I think this would be going a step too far.

Should we even allow SENSOR_DEVICE_DT_DEFINE? Meaning should we just have everyone use SENSOR_DEVICE_DT_INST_DEFINE and thus have new sensor drivers be multi-instance to start?

I'm fine with this.

@henrikbrixandersen
Copy link
Member

Should we even allow SENSOR_DEVICE_DT_DEFINE? Meaning should we just have everyone use SENSOR_DEVICE_DT_INST_DEFINE and thus have new sensor drivers be multi-instance to start?

I don't see why we should limit sensor drivers in this regard. For some (e.g. on-die) sensor types will only make sense to support one single instance.

@galak galak removed the DNM This PR should not be merged (Do Not Merge) label Oct 26, 2022
@fabiobaltieri fabiobaltieri merged commit a9b223b into zephyrproject-rtos:main Oct 27, 2022
@MaureenHelm MaureenHelm deleted the sensor-dt-inst-define branch October 27, 2022 11:56
@MaureenHelm MaureenHelm mentioned this pull request Mar 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teburd teburd teburd approved these changes

@galak galak galak approved these changes

@keith-zephyr keith-zephyr keith-zephyr approved these changes

@gmarull gmarull gmarull left review comments

@avisconti avisconti Awaiting requested review from avisconti

@bbilas bbilas Awaiting requested review from bbilas

@alexanderwachter alexanderwachter Awaiting requested review from alexanderwachter

@ABOSTM ABOSTM Awaiting requested review from ABOSTM

@brockus-zephyr brockus-zephyr Awaiting requested review from brockus-zephyr

@erwango erwango Awaiting requested review from erwango

@FRASTM FRASTM Awaiting requested review from FRASTM

@GeorgeCGV GeorgeCGV Awaiting requested review from GeorgeCGV

@GTLin08 GTLin08 Awaiting requested review from GTLin08

@jackrosenthal jackrosenthal Awaiting requested review from jackrosenthal

@sjg20 sjg20 Awaiting requested review from sjg20

@yperess yperess Awaiting requested review from yperess

@henrikbrixandersen henrikbrixandersen Awaiting requested review from henrikbrixandersen

Assignees

@MaureenHelm MaureenHelm

Labels
area: Sensors Sensors platform: ITE ITE platform: STM32 ST Micro STM32
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@MaureenHelm @galak @gmarull @henrikbrixandersen @teburd @keith-zephyr @fabiobaltieri @zephyrbot
0