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

sensors: BME280: obtain device pointers directly #30536

Merged

Conversation

mbolivar-nordic
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic commented Dec 8, 2020

The main point of this pull request is to show the advantage of declaring device structures using the DEVICE_DT_DEFINE() and DEVICE_DT_DEFINE_INST() macros introduced in e643ee26a71and 3a83f0e.

I'm using the BME280 sensor driver as a proof of concept. We used the same driver as a testing ground for the new DT API when that was introduced in zephyr v2.3.

Avoid calling device_get_binding() at boot time to grab the device. Instead, the BME280 driver can just use DEVICE_DT_GET() and device_is_ready() to make sure the relevant SPI, I2C, and GPIO devices it may depend on are available instead. This saves RAM by moving a struct device* and a struct spi_cs_control from the driver data to its config structure since all the devices are known at build time, and avoids error-prone boilerplate by reusing common macros from drivers/spi.h.

A secondary goal of this PR is to add more helper macros to the SPI API for initializing SPI device configuration data from devicetree nodes. This eliminates boilerplate in the BME280 driver in a way that could be reused elsewhere.

Update the sample likewise to use DEVICE_DT_GET to get the BME280 device instead of calling device_get_binding(). I re-worked the sample a bit as well so that it works on any board that has Arduino SPI and I2C pins while we're here, in hopes that makes this PR easier for others to try out.

@mbolivar-nordic mbolivar-nordic added area: SPI SPI bus area: Sensors Sensors area: Devicetree DNM This PR should not be merged (Do Not Merge) area: Devicetree Tooling PR modifies or adds a Device Tree tooling labels Dec 8, 2020
@github-actions github-actions bot added area: API Changes to public APIs area: Documentation area: Samples Samples area: Tests Issues related to a particular existing or missing test labels Dec 8, 2020
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

This looks good, pending actually trying it which I'll do soon. Hopefully other drivers won't require so much prep.

include/drivers/spi.h Show resolved Hide resolved
@erwango
Copy link
Member

erwango commented Dec 10, 2020

That's sweet!

@mbolivar-nordic
Copy link
Contributor Author

Rebased and addressed review feedback. I suppose this could be merged for Zephyr 2.5, but I'm not in a rush.

Adjust the documentation and devicetree overlays so the sample can be
built for any board with an Arduino I2C and SPI pinout, defaulting I2C
and SPI to y to make it easier to switch between the two without
requiring a pristine build.

The user has to choose an appropriate overlay or have a sensor built
in to the board.

Use the newly introduced DEVICE_DT_GET_ANY() in main.c to ask for a
bosch,bme280 without worrying over the details or exposing
DT_DRV_COMPAT-based functionality that is really meant for drivers.

Remove the no-longer-needed board specific overlay for nRF52840 DK;
this is covered by the generic Arduino overlays now.

Fix the datasheet link while we're here.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
@mbolivar-nordic
Copy link
Contributor Author

I see why a pointer-to-object is needed (because struct spi_config requires one), and it may be that we can't do anything else, but I now think we need to discuss this more.

Missed this comment in my above reply. Yes, this is the crux of the issue.

I've rebased and pushed a version with these macros ifdef'ed out on C++. No other changes.

@MaureenHelm
Copy link
Member

@avisconti please review, you may want to do this in the ST sensor drivers

@mbolivar-nordic
Copy link
Contributor Author

@avisconti please review, you may want to do this in the ST sensor drivers

@MaureenHelm he is waiting for it to go in before updating one of his PRs actually; see: #31775 (comment)

@MaureenHelm
Copy link
Member

@avisconti please review, you may want to do this in the ST sensor drivers

@MaureenHelm he is waiting for it to go in before updating one of his PRs actually; see: #31775 (comment)

@mbolivar-nordic Thanks for the reminder. I'd seen that comment earlier, but forgot about it.

@nashif nashif merged commit d090751 into zephyrproject-rtos:master Feb 23, 2021
@avisconti
Copy link
Collaborator

@avisconti please review, you may want to do this in the ST sensor drivers

@MaureenHelm he is waiting for it to go in before updating one of his PRs actually; see: #31775 (comment)

@mbolivar-nordic Thanks for the reminder. I'd seen that comment earlier, but forgot about it.

Yes, sorry for not reviewing it. I was busy with other stuff as well

avisconti added a commit to avisconti/zephyr that referenced this pull request Mar 9, 2021
This commit simplifies the Device Tree configuration by using
the new helpers introduced in zephyrproject-rtos#30536.

In particular:
    - get bus devices with DEVICE_DT_GET
    - get SPI information with SPI_CONFIG_DT_INST

Signed-off-by: Armando Visconti <armando.visconti@st.com>
nashif pushed a commit that referenced this pull request Mar 10, 2021
This commit simplifies the Device Tree configuration by using
the new helpers introduced in #30536.

In particular:
    - get bus devices with DEVICE_DT_GET
    - get SPI information with SPI_CONFIG_DT_INST

Signed-off-by: Armando Visconti <armando.visconti@st.com>
avisconti added a commit to avisconti/zephyr that referenced this pull request Mar 20, 2021
Make this driver multi-instance and use the new API.
This commit makes use of the new helpers introduced in zephyrproject-rtos#30536.

In particular:
    - get bus devices with DEVICE_DT_GET
    - get SPI information with SPI_CONFIG_DT_INST
    - get drdy gpios with GPIO_DT_SPEC_GET

Signed-off-by: Armando Visconti <armando.visconti@st.com>
avisconti added a commit to avisconti/zephyr that referenced this pull request Apr 15, 2021
Make this driver multi-instance and use the new API.
This commit makes use of the new helpers introduced in zephyrproject-rtos#30536.

In particular:
    - get bus devices with DEVICE_DT_GET
    - get SPI information with SPI_CONFIG_DT_INST
    - get drdy gpios with GPIO_DT_SPEC_GET

Signed-off-by: Armando Visconti <armando.visconti@st.com>
carlescufi pushed a commit that referenced this pull request Apr 16, 2021
Make this driver multi-instance and use the new API.
This commit makes use of the new helpers introduced in #30536.

In particular:
    - get bus devices with DEVICE_DT_GET
    - get SPI information with SPI_CONFIG_DT_INST
    - get drdy gpios with GPIO_DT_SPEC_GET

Signed-off-by: Armando Visconti <armando.visconti@st.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Devicetree Tooling PR modifies or adds a Device Tree tooling area: Devicetree area: Documentation area: I2C area: Samples Samples area: Sensors Sensors area: SPI SPI bus area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants