DT_SPI_DEV_CS_GPIOS_DT_SPEC_GET is a layering violation that shouldn't exist #42149
Labels
bug
The issue is a bug, or the PR is fixing a bug
priority: high
High impact/importance bug
Release Blocker
Use this label for justified release blockers
Milestone
Comments
mbolivar-nordic
added
bug
mbolivar-nordic
added
the
Release Blocker
mbolivar-nordic
added a commit
to mbolivar-nordic/zephyr
that referenced
this issue
Jan 26, 2022
The DT_SPI_DEV_CS_GPIOS_DT_SPEC_GET macro belongs in drivers/spi.h, not devicetree.h. It is creating a struct gpio_dt_spec, but the devicetree.h API does not (other than in this case) and should not depend on structures that are defined in the GPIO API. This is because the GPIO API already depends on the devicetree.h API, so making a dependency in the reverse direction creates a needless circular dependency. This macro should have been added to the drivers/spi.h API from the beginning. Move it there under a new name, SPI_CS_GPIOS_DT_SPEC_GET. We haven't created a Zephyr release with DT_SPI_DEV_CS_GPIOS_DT_SPEC_GET in it, so there is no need to go through the stable API change process for devicetree.h to deprecate and eventually remove it. We can just remove it directly. Fixes: zephyrproject-rtos#42149 Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
carlescufi
pushed a commit
that referenced
this issue
Jan 26, 2022
The DT_SPI_DEV_CS_GPIOS_DT_SPEC_GET macro belongs in drivers/spi.h, not devicetree.h. It is creating a struct gpio_dt_spec, but the devicetree.h API does not (other than in this case) and should not depend on structures that are defined in the GPIO API. This is because the GPIO API already depends on the devicetree.h API, so making a dependency in the reverse direction creates a needless circular dependency. This macro should have been added to the drivers/spi.h API from the beginning. Move it there under a new name, SPI_CS_GPIOS_DT_SPEC_GET. We haven't created a Zephyr release with DT_SPI_DEV_CS_GPIOS_DT_SPEC_GET in it, so there is no need to go through the stable API change process for devicetree.h to deprecate and eventually remove it. We can just remove it directly. Fixes: #42149 Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
This macro, introduced in commit 4d1c90e (pull request #39116), should not have been added to the devicetree API.
It is a layering violation that introduces an unnecessary circular dependency:
struct gpio_dt_specis defined by the GPIO API (drivers/gpio.h)devicetree.h), and uses this dependency to definestruct gpio_dt_specstruct gpio_dt_specThis is bad design. We don't add such things to the DT API, in order to keep our API dependencies clean.
This should have been added to the SPI API, not the devicetree API.
Impact
DT showstopper for v3.0, since devicetree.h is a stable API, and we really should not have this macro added to it in any release.
We haven't created a release with DT_SPI_DEV_CS_GPIOS_DT_SPEC_GET in it yet as far as I can tell, so there's still time to fix it without having to go through a deprecation cycle and stable API change.
The text was updated successfully, but these errors were encountered: