Skip to content

device: refactor declare/define macros to support device definition and reference from devicetree nodes #30196

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 13 commits into from
Dec 1, 2020
Merged

device: refactor declare/define macros to support device definition and reference from devicetree nodes #30196

merged 13 commits into from
Dec 1, 2020

Conversation

@pabigot
Copy link
Contributor

@pabigot pabigot commented Nov 23, 2020

This pulls the macro refactorization out of #29644, allow driver implementations to define devices using information from devicetree nodes directly. Ultimately this will enable storing device dependencies and other metadata inferred from the devicetree source.

Now it allows a well-defined unique public identifier for device structures that can be used to reference the device object at build time, avoiding the need for device_get_binding() and ultimately eliminating many uses of the label property as a device identifier.

The devicetree binding API for gpio has been updated to support this, and the blinky example to use the new capability.

@github-actions github-actions bot added area: API Changes to public APIs area: Documentation area: I2C area: Samples Samples area: SPI SPI bus area: Tests Issues related to a particular existing or missing test labels Nov 23, 2020
@pabigot pabigot mentioned this pull request Nov 23, 2020
Merged
@pabigot
Copy link
Contributor Author

pabigot commented Nov 23, 2020 •
edited
Loading

Note that the compliance failures related to macro style are inappropriate and should be ignored.

galak
galak reviewed Nov 23, 2020
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.

Do we care about having a convention of using device_pm_control_nop vs NULL?

mbolivar-nordic
mbolivar-nordic reviewed Nov 23, 2020
View reviewed changes
@pabigot
Copy link
Contributor Author

pabigot commented Nov 23, 2020

Do we care about having a convention of using device_pm_control_nop vs NULL?

I don't know. ebe20f9 added it for a reason, and I remember finding it useful to have it explicit in another situation (but not why, or whether that effort got anywhere). It'll all be reworked when we get moving on device power management anyway.

@pabigot pabigot mentioned this pull request Nov 23, 2020
Closed
@galak
Copy link
Contributor

galak commented Nov 23, 2020

I don't know. ebe20f9 added it for a reason, and I remember finding it useful to have it explicit in another situation (but not why, or whether that effort got anywhere). It'll all be reworked when we get moving on device power management anyway.

Any sense if its better/more useful for it to be NULL or device_pm_control_nop?

@pabigot
Copy link
Contributor Author

pabigot commented Nov 24, 2020

Any sense if its better/more useful for it to be NULL or device_pm_control_nop?

Right now, no. I don't want to deal with this until I get to device_pm.

@pabigot
Copy link
Contributor Author

pabigot commented Nov 30, 2020

@nashif with my explanation and other people thinking it's ok, do you still insist on replacing device_if_ready() with something else?

I'd like to get this wrapped up.

@carlescufi carlescufi requested a review from nashif November 30, 2020 17:38
@nashif
Copy link
Member

nashif commented Nov 30, 2020 •
edited
Loading

@nashif with my explanation and other people thinking it's ok, do you still insist on replacing device_if_ready() with something else?

I still think we need a better API, I dislike having "if" in the API name. device_ready_get() could be an option. Everywhere else in Zephyr _if_ is being used for "Interface". _ready() at the end of an API usually indicates that we are expecting a boolean...

@pabigot pabigot requested a review from jakub-uC as a code owner November 30, 2020 18:23
@github-actions github-actions bot added the area: Shell Shell subsystem label Nov 30, 2020
@pabigot
Copy link
Contributor Author

pabigot commented Dec 1, 2020

OK, so here are the options I've considered to be acceptable:

  • const struct device *device_if_ready(const struct device *dev); original: dev if dev can be used else NULL
  • bool device_is_ready(const struct device *dev) current : true iff dev can be used
  • bool device_ready(const struct device *dev) alternative: true iff dev can be used

I don't consider these to be acceptable:

  • const struct device *device_ready(const struct device *dev); nak: ready suggests boolean result
  • const struct device *device_get_ready(const struct device *dev); nak: per above.

Comment if there are others so we know what to discuss.

pabigot added 13 commits December 1, 2020 12:52
@pabigot
device: provide API that uses the devicetree node as identifier
8fc7696 
The dev_name is the canonical DT node identifier encoding the
devicetree path; the drv_name is the label of the corresponding node.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
device: redefine devicetree device definition using a more generic API
5d8c344 
For devices defined using devicetree nodes we need to pass the
devicetree node, and to infer names for the device and the driver from
it.  Devices defined in the classical way don't need this.  Introduce
another level of private abstraction that accepts the information from
either source, falling back to the old parameters when the provided
node is invalid.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: clock_control: convert nrf to dt device defines
62f68a1 
Use the clock devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: flash: convert spi-nor drivers to dt device defines
063476b 
Use the devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: gpio: convert nrf driver to dt device defines
ea0370d 
Use the devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: i2c: convert nrf drivers to dt device defines
4421cb7 
Use the devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: regulator: convert to dt device defines
275b214 
Use the devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: sensor: ccs811: convert to dt device defines
fa79ec9 
Use the devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: serial: convert nrf drivers to dt device defines
89d6ac5 
Use the devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
drivers: spi: convert nrf drivers to dt device defines
466a536 
Use the devicetree node as the source of object name and other
information used when defining the device structure.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
device: refactor to allow direct access to devicetree device structures
f8cc97f 
When using a devicetree node as an identifier we know the identifier
used to define the device structure.  This allows code to directly
reference that structure, avoiding the need to look it up by label at
runtime.

Change the macros so DEVICE_DT_* device objects are globally visible
using the node identifier as the object identifier.

Also add the necessary API to verify that a device that was captured
at build time successfully completed initialization and any other
steps necessary before it can be safely used.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
shell: use public API to check device readiness
b81969b 
Avoid the private API now that there's a public equivalent.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@pabigot
devicetree: gpio: provide accessors for controller phandle
1e84fe0 
Provide a helper to extract the devicetree node_id for a GPIO
controller from a gpio phandle array.  This can be used with
DEVICE_DT_GET() to directly reference the corresponding controller
device.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
@nashif nashif merged commit 6918e3d into zephyrproject-rtos:master Dec 1, 2020
@mbolivar-nordic mbolivar-nordic mentioned this pull request Dec 1, 2020
Merged
@pabigot pabigot mentioned this pull request Dec 6, 2020
Closed
@mbolivar-nordic mbolivar-nordic mentioned this pull request Dec 8, 2020
Merged
@galak galak mentioned this pull request Dec 16, 2020
Merged
@de-nordic de-nordic mentioned this pull request Apr 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@galak galak galak approved these changes

@nashif nashif nashif approved these changes

@tbursztyka tbursztyka tbursztyka approved these changes

@carlescufi carlescufi carlescufi approved these changes

@anangl anangl Awaiting requested review from anangl

@MaureenHelm MaureenHelm Awaiting requested review from MaureenHelm

@Mierunski Mierunski Awaiting requested review from Mierunski

@mnkp mnkp Awaiting requested review from mnkp

@nordic-krch nordic-krch Awaiting requested review from nordic-krch

@wentongwu wentongwu Awaiting requested review from wentongwu

@jakub-uC jakub-uC Awaiting requested review from jakub-uC

+2 more reviewers

@nvlsianpu nvlsianpu nvlsianpu approved these changes

@mbolivar-nordic mbolivar-nordic mbolivar-nordic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area: API Changes to public APIs area: Device Management area: Devicetree area: Documentation area: I2C area: Samples Samples area: Shell Shell subsystem area: SPI SPI bus area: Tests Issues related to a particular existing or missing test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@pabigot @galak @nashif @tbursztyka @carlescufi @nvlsianpu @mbolivar-nordic