Skip to content

Commit

Permalink
net: ipa: kill ipa_cmd_pipeline_clear()
Browse files Browse the repository at this point in the history
Calling ipa_cmd_pipeline_clear() after stopping the channel
underlying the AP<-modem RX endpoint can lead to a deadlock.

This occurs in the ->runtime_suspend device power operation for the
IPA driver.  While this callback is in progress, any other requests
for power will block until the callback returns.

Stopping the AP<-modem RX channel does not prevent the modem from
sending another packet to this endpoint.  If a packet arrives for an
RX channel when the channel is stopped, an SUSPEND IPA interrupt
condition will be pending.  Handling an IPA interrupt requires
power, so ipa_isr_thread() calls pm_runtime_get_sync() first thing.

The problem occurs because a "pipeline clear" command will not
complete while such a SUSPEND interrupt condition exists.  So the
SUSPEND IPA interrupt handler won't proceed until it gets power;
that won't happen until the ->runtime_suspend callback (and its
"pipeline clear" command) completes; and that can't happen while
the SUSPEND interrupt condition exists.

It turns out that in this case there is no need to use the "pipeline
clear" command.  There are scenarios in which clearing the pipeline
is required while suspending, but those are not (yet) supported
upstream.  So a simple fix, avoiding the potential deadlock, is to
stop calling ipa_cmd_pipeline_clear() in ipa_endpoint_suspend().
This removes the only user of ipa_cmd_pipeline_clear(), so get rid
of that function.  It can be restored again whenever it's needed.

This is basically a manual revert along with an explanation for
commit 6cb63ea ("net: ipa: introduce ipa_cmd_tag_process()").

Fixes: 6cb63ea ("net: ipa: introduce ipa_cmd_tag_process()")
Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Alex Elder authored and David S. Miller committed Nov 23, 2021
1 parent a049a30 commit e4e9bfb
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 24 deletions.
16 changes: 0 additions & 16 deletions drivers/net/ipa/ipa_cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -661,22 +661,6 @@ void ipa_cmd_pipeline_clear_wait(struct ipa *ipa)
wait_for_completion(&ipa->completion);
}

void ipa_cmd_pipeline_clear(struct ipa *ipa)
{
u32 count = ipa_cmd_pipeline_clear_count();
struct gsi_trans *trans;

trans = ipa_cmd_trans_alloc(ipa, count);
if (trans) {
ipa_cmd_pipeline_clear_add(trans);
gsi_trans_commit_wait(trans);
ipa_cmd_pipeline_clear_wait(ipa);
} else {
dev_err(&ipa->pdev->dev,
"error allocating %u entry tag transaction\n", count);
}
}

static struct ipa_cmd_info *
ipa_cmd_info_alloc(struct ipa_endpoint *endpoint, u32 tre_count)
{
Expand Down
6 changes: 0 additions & 6 deletions drivers/net/ipa/ipa_cmd.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,6 @@ u32 ipa_cmd_pipeline_clear_count(void);
*/
void ipa_cmd_pipeline_clear_wait(struct ipa *ipa);

/**
* ipa_cmd_pipeline_clear() - Clear the hardware pipeline
* @ipa: - IPA pointer
*/
void ipa_cmd_pipeline_clear(struct ipa *ipa);

/**
* ipa_cmd_trans_alloc() - Allocate a transaction for the command TX endpoint
* @ipa: IPA pointer
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/ipa/ipa_endpoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1636,8 +1636,6 @@ void ipa_endpoint_suspend(struct ipa *ipa)
if (ipa->modem_netdev)
ipa_modem_suspend(ipa->modem_netdev);

ipa_cmd_pipeline_clear(ipa);

ipa_endpoint_suspend_one(ipa->name_map[IPA_ENDPOINT_AP_LAN_RX]);
ipa_endpoint_suspend_one(ipa->name_map[IPA_ENDPOINT_AP_COMMAND_TX]);
}
Expand Down

0 comments on commit e4e9bfb

Please sign in to comment.