Skip to content

Conversation

@rfvirgil
Copy link

@rfvirgil rfvirgil commented Jan 3, 2026

Replace the wait_for_completion_timeout() in sdw_prep_deprep_slave_ports() with a read_poll_timeout().

The original intent of the wait_for_completion_timeout() was to wait for the port prepare interrupt. But at this time the code is holding the bus_lock, which prevents the interrupt handler from running. Because of this, the port_pre completion will not be signaled and the wait_for_completion_timeout() will always timeout.

Rewriting the code to avoid taking the bus_lock carries risks, and needs careful consideration of the consequences. It is safer and simpler to replace the completion with a simple register poll.

As the code is holding the bus_lock, it is already blocking other activity so consuming control channel bandwidth for polling isn't really a concern.

Replace the wait_for_completion_timeout() in sdw_prep_deprep_slave_ports()
with a read_poll_timeout().

The original intent of the wait_for_completion_timeout() was to wait for
the port prepare interrupt. But at this time the code is holding the
bus_lock, which prevents the interrupt handler from running. Because of
this, the port_pre completion will not be signaled and the
wait_for_completion_timeout() will always timeout.

Rewriting the code to avoid taking the bus_lock carries risks, and
needs careful consideration of the consequences. It is safer and simpler
to replace the completion with a simple register poll.

As the code is holding the bus_lock, it is already blocking other activity
so consuming control channel bandwidth for polling isn't really a concern.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
SDW_PORT_PREP_POLL_USEC, ch_prep_timeout * USEC_PER_MSEC,
false, s_rt->slave, SDW_DPN_PREPARESTATUS(p_rt->num));
if (ret || (val < 0) || (val & p_rt->ch_mask)) {
ret = (val < 0) ? val : -ETIMEDOUT;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we not convert ret if it is not 0? In that case val could be 0

prep_ch.bank = bus->params.next_bank;

if (imp_def_interrupts || !simple_ch_prep_sm ||
if (imp_def_interrupts ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why this change is needed.

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.

2 participants