-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
drivers: serial: nrfx_uarte: Improve pin retention #81273
base: main
Are you sure you want to change the base?
Conversation
nordic-krch
commented
Nov 12, 2024
- Fix case when device PM is not used but low power mode is used and UARTE is disabled when idle. In that case pins retention was not enabled.
- Perform pin retention only when needed (always for UARTE120 and optional for other instances). Pin retention requires 4 register access to the slow domain which will take ~2us and it is done with interrupts locked so we want to avoid it if possible.
pin-retention: | ||
type: boolean | ||
description: | | ||
UARTE pin states must be retained when peripheral is disabled. | ||
It is required when CTRLSEL is used to route UARTE pins. It is always the | ||
case for fast peripheral and in special cases for slow peripherals. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be inferred from the power domain UART resides?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but there are special cases where for example UARTE130 can use P9.5 for TX and that configuration goes through CTRLSEL so I added this property so that there is a possibility to add pin-retention
to uart130 node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm worried that expecting users to remember to add this in special cases may lead to bug reports.
Is there any alternative to automate this? Or at least throw an error when property is missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing that comes to my mind is to add build assert in code where there are fixed location checked but we try to avoid putting soc specific stuff in the code.
Maybe we could come up with a way of describing that in DT. e.g. create a description of pool of available pins with some specific properties that are required for a given pool. This could be useful not only in this case but also in case of Lumos where peripherals are fixed to a single gpio port.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should likely review the pinctrl model we have, as new socs have changed it. For example, 54H20 has a mix of PSEL (peripheral-level) and CTRLSEL (classical multiplexer).
Add property to nrfx_uarte which indicates that pins must be retained for the given instance when it is disabled. Add that property to uart120 instances as those always require that since pins are routed through CTRLSEL. For other instances there are exceptions when CTRLSEL is used (see the specification). Signed-off-by: Krzysztof Chruściński <[email protected]>
Since pin retantion is done as a part of suspend/resume which is done in the critical section, it shall be done only when needed. Use DT flag to determine which UARTE instance needs it. Signed-off-by: Krzysztof Chruściński <[email protected]>
f2d4c1e
to
c31750c
Compare