Skip to content

soundwire: cadence: fix race condition between suspend and Slave devi… - #2354

Merged
bardliao merged 1 commit into
thesofproject:topic/sof-devfrom
plbossart:sdw/fix-alert-on-suspend
Aug 13, 2020
Merged

bardliao merged 1 commit into
thesofproject:topic/sof-devfrom
plbossart:sdw/fix-alert-on-suspend

Conversation

@plbossart

Copy link
Copy Markdown
Member

…ce alerts

In system suspend stress cases, the SOF CI reports timeouts. The root
cause is that an alert is generated while the system suspends, so on
resume the previous transaction times out.
This error doesn't seem too problematic since it happens in a work
queue, and the system recovers without issues.

Nevertheless, this should not happen. When doing a system suspend, or
when disabling interrupts, we should make sure the current transaction
can complete, and prevent new work from being queued.

BugLink: #2344
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

Comment thread drivers/soundwire/cadence_master.c Outdated
@plbossart
plbossart force-pushed the sdw/fix-alert-on-suspend branch from 16afa01 to d66e70f Compare August 11, 2020 17:52
@plbossart

Copy link
Copy Markdown
Member Author

SOFCI TEST

Comment thread drivers/soundwire/cadence_master.c
@plbossart
plbossart force-pushed the sdw/fix-alert-on-suspend branch from d66e70f to 8c83d7c Compare August 12, 2020 14:59
@plbossart plbossart changed the title [TEST] soundwire: cadence: fix race condition between suspend and Slave devi… soundwire: cadence: fix race condition between suspend and Slave devi… Aug 12, 2020
@plbossart

Copy link
Copy Markdown
Member Author

@ranj063 @bardliao @RanderWang updated code and comments

ranj063
ranj063 previously approved these changes Aug 12, 2020

@ranj063 ranj063 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart looks good now. I had to read it a few times to understand the check in sdw_cdns_irq(). I think "interrupt_enabled" might make it clearer but its minor. If it is not confusing for you not a big deal for me.

…ce alerts

In system suspend stress cases, the SOF CI reports timeouts. The root
cause is that an alert is generated while the system suspends. The
interrupt handling generates transactions on the bus that will never
be handled because the interrupts are disabled in parallel.

As a result, the transaction never completes and times out on resume.
This error doesn't seem too problematic since it happens in a work
queue, and the system recovers without issues.

Nevertheless, this race condition should not happen. When doing a
system suspend, or when disabling interrupts, we should make sure the
current transaction can complete, and prevent new work from being
queued.

BugLink: thesofproject#2344
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart

Copy link
Copy Markdown
Member Author

@plbossart looks good now. I had to read it a few times to understand the check in sdw_cdns_irq(). I think "interrupt_enabled" might make it clearer but its minor. If it is not confusing for you not a big deal for me.

yes, 'interrupt_enabled' sounds better, it's a state not an action. updated code, thanks for the suggestion.

@bardliao
bardliao merged commit a1d84e0 into thesofproject:topic/sof-dev Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants