From: Alex Elder Date: Thu, 30 Apr 2020 22:13:23 +0000 (-0500) Subject: net: ipa: do not cache channel state X-Git-Tag: v5.10.7~2469^2~347^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a2003b303875b41542bad1c2e81800fdd4c27c29;p=platform%2Fkernel%2Flinux-rpi.git net: ipa: do not cache channel state 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 Signed-off-by: David S. Miller --- diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 6946c39b664a..8184d34124b7 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -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); } diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h index 0698ff1ae7a6..19471017fadf 100644 --- a/drivers/net/ipa/gsi.h +++ b/drivers/net/ipa/gsi.h @@ -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;