net: ipa: do not cache channel state
authorAlex Elder <elder@linaro.org>
Thu, 30 Apr 2020 22:13:23 +0000 (17:13 -0500)
committerDavid S. Miller <davem@davemloft.net>
Fri, 1 May 2020 22:53:33 +0000 (15:53 -0700)
It is possible for a GSI channel's state to be changed as a result
of an action by a different execution environment.  Specifically,
the modem is able to issue a GSI generic command that causes a state
change on a GSI channel associated with the AP.

A channel's state only needs to be known when a channel is allocated
or deallocaed, started or stopped, or reset.  So there is little
value in caching the state anyway.

Stop recording a copy of the channel's last known state, and instead
fetch the true state from hardware whenever it's needed.  In such
cases, *do* record the state in a local variable, in case an error
message reports it (so the value reported is the value seen).

Signed-off-by: Alex Elder <elder@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ipa/gsi.c
drivers/net/ipa/gsi.h

index 6946c39b664ac64dfc01bdcd71f8b06221c4c1f5..8184d34124b7b2f4055b3d150b3e59b5705ea94a 100644 (file)
@@ -415,7 +415,7 @@ static void gsi_evt_ring_de_alloc_command(struct gsi *gsi, u32 evt_ring_id)
                        evt_ring->state);
 }
 
-/* Return the hardware's notion of the current state of a channel */
+/* Fetch the current state of a channel from hardware */
 static enum gsi_channel_state gsi_channel_state(struct gsi_channel *channel)
 {
        u32 channel_id = gsi_channel_id(channel);
@@ -433,16 +433,18 @@ gsi_channel_command(struct gsi_channel *channel, enum gsi_ch_cmd_opcode opcode)
 {
        struct completion *completion = &channel->completion;
        u32 channel_id = gsi_channel_id(channel);
+       struct gsi *gsi = channel->gsi;
        u32 val;
 
        val = u32_encode_bits(channel_id, CH_CHID_FMASK);
        val |= u32_encode_bits(opcode, CH_OPCODE_FMASK);
 
-       if (gsi_command(channel->gsi, GSI_CH_CMD_OFFSET, val, completion))
+       if (gsi_command(gsi, GSI_CH_CMD_OFFSET, val, completion))
                return 0;       /* Success! */
 
-       dev_err(channel->gsi->dev, "GSI command %u to channel %u timed out "
-               "(state is %u)\n", opcode, channel_id, channel->state);
+       dev_err(gsi->dev,
+               "GSI command %u to channel %u timed out (state is %u)\n",
+               opcode, channel_id, gsi_channel_state(channel));
 
        return -ETIMEDOUT;
 }
@@ -451,18 +453,21 @@ gsi_channel_command(struct gsi_channel *channel, enum gsi_ch_cmd_opcode opcode)
 static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)
 {
        struct gsi_channel *channel = &gsi->channel[channel_id];
+       enum gsi_channel_state state;
        int ret;
 
        /* Get initial channel state */
-       channel->state = gsi_channel_state(channel);
-
-       if (channel->state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
+       state = gsi_channel_state(channel);
+       if (state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
                return -EINVAL;
 
        ret = gsi_channel_command(channel, GSI_CH_ALLOCATE);
-       if (!ret && channel->state != GSI_CHANNEL_STATE_ALLOCATED) {
+
+       /* Channel state will normally have been updated */
+       state = gsi_channel_state(channel);
+       if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED) {
                dev_err(gsi->dev, "bad channel state (%u) after alloc\n",
-                       channel->state);
+                       state);
                ret = -EIO;
        }
 
@@ -472,18 +477,21 @@ static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)
 /* Start an ALLOCATED channel */
 static int gsi_channel_start_command(struct gsi_channel *channel)
 {
-       enum gsi_channel_state state = channel->state;
+       enum gsi_channel_state state;
        int ret;
 
+       state = gsi_channel_state(channel);
        if (state != GSI_CHANNEL_STATE_ALLOCATED &&
            state != GSI_CHANNEL_STATE_STOPPED)
                return -EINVAL;
 
        ret = gsi_channel_command(channel, GSI_CH_START);
-       if (!ret && channel->state != GSI_CHANNEL_STATE_STARTED) {
+
+       /* Channel state will normally have been updated */
+       state = gsi_channel_state(channel);
+       if (!ret && state != GSI_CHANNEL_STATE_STARTED) {
                dev_err(channel->gsi->dev,
-                       "bad channel state (%u) after start\n",
-                       channel->state);
+                       "bad channel state (%u) after start\n", state);
                ret = -EIO;
        }
 
@@ -493,23 +501,27 @@ static int gsi_channel_start_command(struct gsi_channel *channel)
 /* Stop a GSI channel in STARTED state */
 static int gsi_channel_stop_command(struct gsi_channel *channel)
 {
-       enum gsi_channel_state state = channel->state;
+       enum gsi_channel_state state;
        int ret;
 
+       state = gsi_channel_state(channel);
        if (state != GSI_CHANNEL_STATE_STARTED &&
            state != GSI_CHANNEL_STATE_STOP_IN_PROC)
                return -EINVAL;
 
        ret = gsi_channel_command(channel, GSI_CH_STOP);
-       if (ret || channel->state == GSI_CHANNEL_STATE_STOPPED)
+
+       /* Channel state will normally have been updated */
+       state = gsi_channel_state(channel);
+       if (ret || state == GSI_CHANNEL_STATE_STOPPED)
                return ret;
 
        /* We may have to try again if stop is in progress */
-       if (channel->state == GSI_CHANNEL_STATE_STOP_IN_PROC)
+       if (state == GSI_CHANNEL_STATE_STOP_IN_PROC)
                return -EAGAIN;
 
-       dev_err(channel->gsi->dev, "bad channel state (%u) after stop\n",
-               channel->state);
+       dev_err(channel->gsi->dev,
+               "bad channel state (%u) after stop\n", state);
 
        return -EIO;
 }
@@ -517,41 +529,49 @@ static int gsi_channel_stop_command(struct gsi_channel *channel)
 /* Reset a GSI channel in ALLOCATED or ERROR state. */
 static void gsi_channel_reset_command(struct gsi_channel *channel)
 {
+       enum gsi_channel_state state;
        int ret;
 
        msleep(1);      /* A short delay is required before a RESET command */
 
-       if (channel->state != GSI_CHANNEL_STATE_STOPPED &&
-           channel->state != GSI_CHANNEL_STATE_ERROR) {
+       state = gsi_channel_state(channel);
+       if (state != GSI_CHANNEL_STATE_STOPPED &&
+           state != GSI_CHANNEL_STATE_ERROR) {
                dev_err(channel->gsi->dev,
-                       "bad channel state (%u) before reset\n",
-                       channel->state);
+                       "bad channel state (%u) before reset\n", state);
                return;
        }
 
        ret = gsi_channel_command(channel, GSI_CH_RESET);
-       if (!ret && channel->state != GSI_CHANNEL_STATE_ALLOCATED)
+
+       /* Channel state will normally have been updated */
+       state = gsi_channel_state(channel);
+       if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED)
                dev_err(channel->gsi->dev,
-                       "bad channel state (%u) after reset\n",
-                       channel->state);
+                       "bad channel state (%u) after reset\n", state);
 }
 
 /* Deallocate an ALLOCATED GSI channel */
 static void gsi_channel_de_alloc_command(struct gsi *gsi, u32 channel_id)
 {
        struct gsi_channel *channel = &gsi->channel[channel_id];
+       enum gsi_channel_state state;
        int ret;
 
-       if (channel->state != GSI_CHANNEL_STATE_ALLOCATED) {
-               dev_err(gsi->dev, "bad channel state (%u) before dealloc\n",
-                       channel->state);
+       state = gsi_channel_state(channel);
+       if (state != GSI_CHANNEL_STATE_ALLOCATED) {
+               dev_err(gsi->dev,
+                       "bad channel state (%u) before dealloc\n", state);
                return;
        }
 
        ret = gsi_channel_command(channel, GSI_CH_DE_ALLOC);
-       if (!ret && channel->state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
-               dev_err(gsi->dev, "bad channel state (%u) after dealloc\n",
-                       channel->state);
+
+       /* Channel state will normally have been updated */
+       state = gsi_channel_state(channel);
+       if (!ret && state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
+               dev_err(gsi->dev,
+                       "bad channel state (%u) after dealloc\n", state);
 }
 
 /* Ring an event ring doorbell, reporting the last entry processed by the AP.
@@ -778,6 +798,7 @@ int gsi_channel_start(struct gsi *gsi, u32 channel_id)
 int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
 {
        struct gsi_channel *channel = &gsi->channel[channel_id];
+       enum gsi_channel_state state;
        u32 retries;
        int ret;
 
@@ -787,7 +808,8 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
         * STOP command timed out.  We won't stop a channel if stopping it
         * was successful previously (so we still want the freeze above).
         */
-       if (channel->state == GSI_CHANNEL_STATE_STOPPED)
+       state = gsi_channel_state(channel);
+       if (state == GSI_CHANNEL_STATE_STOPPED)
                return 0;
 
        /* RX channels might require a little time to enter STOPPED state */
@@ -941,7 +963,6 @@ static void gsi_isr_chan_ctrl(struct gsi *gsi)
                channel_mask ^= BIT(channel_id);
 
                channel = &gsi->channel[channel_id];
-               channel->state = gsi_channel_state(channel);
 
                complete(&channel->completion);
        }
index 0698ff1ae7a6c48f2a4c414074317cfe5ab9d2e7..19471017fadfd7a5bebeb828404e65847713c04f 100644 (file)
@@ -113,8 +113,7 @@ struct gsi_channel {
        u16 tre_count;
        u16 event_count;
 
-       struct completion completion;   /* signals channel state changes */
-       enum gsi_channel_state state;
+       struct completion completion;   /* signals channel command completion */
 
        struct gsi_ring tre_ring;
        u32 evt_ring_id;