net: ipa: use state to determine channel command success

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@linaro.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 4aee60d..4f0e791 100644 (file)
--- 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);
 }