soundwire: cadence: Fixes for message completion interrupt race #5659
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Author
|
@bardliao @shumingfan Do you have soak/stress tests you could run on Realtek so we can get more confidence this doesn't cause any new problems? |
Collaborator
|
@rfvirgil There are some
That means Device 0 is attached. Not sure if it is a real issue. |
bardliao
reviewed
Feb 2, 2026
| CDNS_MCP_INT_SLAVE_MASK, 0); | ||
|
|
||
| int_status &= ~CDNS_MCP_INT_SLAVE_MASK; | ||
|
|
Collaborator
There was a problem hiding this comment.
@rfvirgil @shumingfan Is it possible that a Peripheral can generate a new event while handling a special event and lead to an infinite loop in a corner case?
rfvirgil
force-pushed
the
topic/sdw-cadence-irq-race-fix
branch
2 times, most recently
from
30adad5 to
03f9ba5
Compare
rfvirgil
changed the title
…thread
Clear the CDNS_MCP_INT_RX_WL interrupt before signalling completion.
This is to prevent the potential race where:
- The main thread is scheduled immediately the completion is signalled,
and starts a new message
- The RX_WL IRQ for this new message happens before sdw_cdns_irq() has
been re-scheduled.
- When sdw_cdns_irq() is re-scheduled it clears the new RX_WL interrupt.
MAIN THREAD | IRQ THREAD
|
_cdns_xfer_msg() |
{ |
write data to FIFO |
wait_for_completion_timeout() |
<BLOCKED> | <---- RX_WL IRQ
| sdw_cdns_irq()
| {
| signal completion
<== RESCHEDULE <==
Handle message completion |
} |
|
Start new message |
_cdns_xfer_msg() |
{ |
write data to FIFO |
wait_for_completion_timeout() |
<BLOCKED> | <---- RX_WL IRQ
==> RESCHEDULE ==>
| // New RX_WL IRQ is cleared before
| // it has been handled.
| clear CDNS_MCP_INTSTAT
| return IRQ_HANDLED;
| }
Before this change, this error message was sometimes seen on kernels
that have large amounts of debugging enabled:
SCP Msg trf timed out
This error indicates that the completion has not been signalled after
500ms.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 956baa1 ("soundwire: cdns: Add sdw_master_ops and IO transfer support")
rfvirgil
force-pushed
the
topic/sdw-cadence-irq-race-fix
branch
from
03f9ba5 to
015497c
Compare
Author
|
I've reduced this pull request to only the one patch to fix the race between interrupt thread handling RX_WL IRQ, and main thread starting a new message. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series is to fix three places in the cadence IRQ handling where interrupt evens could be lost.Two are variations of the same problem that the code cleared interrupts AFTER it handled them. This is not the correct way, because any interrupt that re-triggered while the irq handler function is running would then be cleared at the end of the function without ever having been handled.
The other case is to avoid masking the PING status change interrupt while waiting for the work function to handle it. There is a potential problem here if state changes that happen while masked are lost,New version, this pull is now only to fix the potential race caused by clearing the RX_WL interrupt AFTER signaling the completion to the main thread. The main thread could be scheduled immediately and start a new message which raises an IRQ that is then cleared by the IRQ thread before it is handled.