[WIP][RFC]Device control features #44693
Conversation
tbursztyka
added
RFC
|
Something's off: I get 0 device_context in the .map file in devcontext's section, though __used is set to all of them, same for the locks. Need to investigate this. [edit] The error was induced by what's being between the chair and the keyboard. |
github-actions
bot
added
area: Devicetree
area: Devicetree Binding
|
Last update: finally got a clean way to make the device lock optional per-device (thanks @mbolivar-nordic for the proposal!). Introducing a generic device property "zephyr,no-lock" which permits to disable the lock allocation on an instance basis. Current drawback: not all device instance in the system are DTS based, so these ones by default do not get any lock anyway. |
Rename existing struct device_state to struct device_context as this structure will hold more than just state stuff. Remove struct device's state attribute, saving some ROM space. Instead order the device_contexts sections the same way as the devices one so a device context can be easily retrieved from it's device pointer (same "index" in the list of device context). Signed-off-by: Tomasz Bursztyka <[email protected]>
This introduce a generic device concurrent access protection. Such feature already exist, scattered either through some driver domain API - SPI for instance - or most of the time per-driver when the maintainer has a good understanding on what to do. But not everywhere. Many drivers are not protected against concurrent access. Generalizing this will remove the burden for driver maintainers to do that on their own, but also will add it to all the drivers which are lacking this. Signed-off-by: Tomasz Bursztyka <[email protected]>
Make the lock as a pointer, and optional via a DTS property. Currently, all devices get the lock or not depending on CONFIG_DEVICE_CONCURRENT_ACCESS being set or not. That's too heavy as some devices may never need such lock. Thus making it optional via a DTS property. By default, all devices will get the lock. The instances that do not want it will only have to set zephyry,no-lock = false in the relevant DTS file. The obvious drawback is that it only works for DTS based device instances. Non DTS based instance are not getting any lock anyway then. Signed-off-by: Tomasz Bursztyka <[email protected]>
This adds a synchronization semaphore to generalize call synchronization among all device drivers. This will permit to avoid for drivers maintainers to implement their own solution. Signed-off-by: Tomasz Bursztyka <[email protected]>
| @@ -1,6 +1,6 @@ | |||
| # Common fields for all devices | |||
|
|
|||
| include: [pm.yaml] | |||
| include: [pm.yaml, "zephyr,device.yaml"] | |||
There was a problem hiding this comment.
One problem with the approach of putting this in base.yaml is that it will affect every binding, even those bindings which do not correspond to struct devices.
There was a problem hiding this comment.
I am little bit confused here, how this file may be used by bindings that will not generate any struct device instance? Seems counter intuitive, relatively to the comment telling "common fields for all devices". Do you have any example ?
PM stuff are fully tightened to struct device also.
Or should we have an intermediate device.yaml file that would include base, pm and the one I am adding then? And actual bindings generating struct device would include that one instead of base only.
| struct device_state { | ||
| struct device_context { |
There was a problem hiding this comment.
In my opinion this should be dropped. I think the existing name is OK and I think there should be a high bar to changing the core device model APIs.
There was a problem hiding this comment.
device_state is semantically quite off once you add more features in it. Plus, the change is currently minimal, there is only the statistics in i²c and can that directly use it. It's unrelated to pm_device_state
|
After running some numbers, I feel this is not going to be the best solution for limited RAM targets.
Anyway,I am addressing that from a different stand point on devices, keeping the improvements while solving the above issues. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
Reviving #24511 (See api channel in discord)
See #22941 as well for more background information.
This is a WORK IN PROGRESS
Current status:
More features to come