[PATCH net 2/3] net: ipa: use state to determine channel command success

From: Alex Elder
Date: Tue Dec 22 2020 - 13:01:38 EST


The result of issuing a channel control command should be that the
channel changes state. If enabled, a completion interrupt signals
that the channel state has changed. This interrupt is enabled by
gsi_channel_command() and disabled again after the command has
completed (or we time out).

There is a window of time--after the completion interrupt is disabled
but before the channel state is read--during which the command could
complete successfully without interrupting. This would cause the
channel to transition to the desired new state.

So whether a channel command ends via completion interrupt or
timeout, we can consider the command successful if the channel
has entered the desired state (and a failure if it has not,
regardless of the cause).

Fixes: d6c9e3f506ae8 ("net: ipa: only enable generic command completion IRQ when needed");
Signed-off-by: Alex Elder <elder@xxxxxxxxxx>
---
drivers/net/ipa/gsi.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 4aee60d62ab09..4f0e791764237 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -505,15 +505,15 @@ static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)

ret = gsi_channel_command(channel, GSI_CH_ALLOCATE);

- /* Channel state will normally have been updated */
+ /* If successful the channel state will have changed */
state = gsi_channel_state(channel);
- if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED) {
- dev_err(dev, "channel %u bad state %u after alloc\n",
- channel_id, state);
- ret = -EIO;
- }
+ if (state == GSI_CHANNEL_STATE_ALLOCATED)
+ return 0;

- return ret;
+ dev_err(dev, "channel %u bad state %u after alloc\n",
+ channel_id, state);
+
+ return -EIO;
}

/* Start an ALLOCATED channel */
@@ -533,15 +533,15 @@ static int gsi_channel_start_command(struct gsi_channel *channel)

ret = gsi_channel_command(channel, GSI_CH_START);

- /* Channel state will normally have been updated */
+ /* If successful the channel state will have changed */
state = gsi_channel_state(channel);
- if (!ret && state != GSI_CHANNEL_STATE_STARTED) {
- dev_err(dev, "channel %u bad state %u after start\n",
- gsi_channel_id(channel), state);
- ret = -EIO;
- }
+ if (state == GSI_CHANNEL_STATE_STARTED)
+ return 0;

- return ret;
+ dev_err(dev, "channel %u bad state %u after start\n",
+ gsi_channel_id(channel), state);
+
+ return -EIO;
}

/* Stop a GSI channel in STARTED state */
@@ -568,10 +568,10 @@ static int gsi_channel_stop_command(struct gsi_channel *channel)

ret = gsi_channel_command(channel, GSI_CH_STOP);

- /* Channel state will normally have been updated */
+ /* If successful the channel state will have changed */
state = gsi_channel_state(channel);
- if (ret || state == GSI_CHANNEL_STATE_STOPPED)
- return ret;
+ if (state == GSI_CHANNEL_STATE_STOPPED)
+ return 0;

/* We may have to try again if stop is in progress */
if (state == GSI_CHANNEL_STATE_STOP_IN_PROC)
@@ -604,9 +604,9 @@ static void gsi_channel_reset_command(struct gsi_channel *channel)

ret = gsi_channel_command(channel, GSI_CH_RESET);

- /* Channel state will normally have been updated */
+ /* If successful the channel state will have changed */
state = gsi_channel_state(channel);
- if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED)
+ if (state != GSI_CHANNEL_STATE_ALLOCATED)
dev_err(dev, "channel %u bad state %u after reset\n",
gsi_channel_id(channel), state);
}
@@ -628,9 +628,10 @@ static void gsi_channel_de_alloc_command(struct gsi *gsi, u32 channel_id)

ret = gsi_channel_command(channel, GSI_CH_DE_ALLOC);

- /* Channel state will normally have been updated */
+ /* If successful the channel state will have changed */
state = gsi_channel_state(channel);
- if (!ret && state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
+
+ if (state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
dev_err(dev, "channel %u bad state %u after dealloc\n",
channel_id, state);
}
--
2.20.1