* [PATCH net-next 0/2] net: ipa: don't cache channel state
@ 2020-04-30 22:13 Alex Elder
2020-04-30 22:13 ` [PATCH net-next 1/2] net: ipa: pass channel pointer to gsi_channel_state() Alex Elder
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Alex Elder @ 2020-04-30 22:13 UTC (permalink / raw)
To: davem; +Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
This series removes a field that holds a copy of a channel's state
at the time it was last fetched. In principle the state can change
at any time, so it's better to just fetch it whenever needed. The
first patch is just preparatory, simplifying the arguments to
gsi_channel_state().
-Alex
Alex Elder (2):
net: ipa: pass channel pointer to gsi_channel_state()
net: ipa: do not cache channel state
drivers/net/ipa/gsi.c | 94 ++++++++++++++++++++++++++-----------------
drivers/net/ipa/gsi.h | 3 +-
2 files changed, 59 insertions(+), 38 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net-next 1/2] net: ipa: pass channel pointer to gsi_channel_state()
2020-04-30 22:13 [PATCH net-next 0/2] net: ipa: don't cache channel state Alex Elder
@ 2020-04-30 22:13 ` Alex Elder
2020-04-30 22:13 ` [PATCH net-next 2/2] net: ipa: do not cache channel state Alex Elder
2020-05-01 22:53 ` [PATCH net-next 0/2] net: ipa: don't " David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Alex Elder @ 2020-04-30 22:13 UTC (permalink / raw)
To: davem; +Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
Pass a channel pointer rather than a GSI pointer and channel ID to
gsi_channel_state().
Signed-off-by: Alex Elder <elder@linaro.org>
---
drivers/net/ipa/gsi.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 845478a19a4f..6946c39b664a 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -416,12 +416,13 @@ static void gsi_evt_ring_de_alloc_command(struct gsi *gsi, u32 evt_ring_id)
}
/* Return the hardware's notion of the current state of a channel */
-static enum gsi_channel_state
-gsi_channel_state(struct gsi *gsi, u32 channel_id)
+static enum gsi_channel_state gsi_channel_state(struct gsi_channel *channel)
{
+ u32 channel_id = gsi_channel_id(channel);
+ void *virt = channel->gsi->virt;
u32 val;
- val = ioread32(gsi->virt + GSI_CH_C_CNTXT_0_OFFSET(channel_id));
+ val = ioread32(virt + GSI_CH_C_CNTXT_0_OFFSET(channel_id));
return u32_get_bits(val, CHSTATE_FMASK);
}
@@ -453,7 +454,7 @@ static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)
int ret;
/* Get initial channel state */
- channel->state = gsi_channel_state(gsi, channel_id);
+ channel->state = gsi_channel_state(channel);
if (channel->state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
return -EINVAL;
@@ -940,7 +941,7 @@ static void gsi_isr_chan_ctrl(struct gsi *gsi)
channel_mask ^= BIT(channel_id);
channel = &gsi->channel[channel_id];
- channel->state = gsi_channel_state(gsi, channel_id);
+ channel->state = gsi_channel_state(channel);
complete(&channel->completion);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH net-next 2/2] net: ipa: do not cache channel state
2020-04-30 22:13 [PATCH net-next 0/2] net: ipa: don't cache channel state Alex Elder
2020-04-30 22:13 ` [PATCH net-next 1/2] net: ipa: pass channel pointer to gsi_channel_state() Alex Elder
@ 2020-04-30 22:13 ` Alex Elder
2020-05-01 22:53 ` [PATCH net-next 0/2] net: ipa: don't " David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Alex Elder @ 2020-04-30 22:13 UTC (permalink / raw)
To: davem; +Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
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>
---
drivers/net/ipa/gsi.c | 87 +++++++++++++++++++++++++++----------------
drivers/net/ipa/gsi.h | 3 +-
2 files changed, 55 insertions(+), 35 deletions(-)
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;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next 0/2] net: ipa: don't cache channel state
2020-04-30 22:13 [PATCH net-next 0/2] net: ipa: don't cache channel state Alex Elder
2020-04-30 22:13 ` [PATCH net-next 1/2] net: ipa: pass channel pointer to gsi_channel_state() Alex Elder
2020-04-30 22:13 ` [PATCH net-next 2/2] net: ipa: do not cache channel state Alex Elder
@ 2020-05-01 22:53 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-05-01 22:53 UTC (permalink / raw)
To: elder; +Cc: evgreen, subashab, cpratapa, bjorn.andersson, netdev, linux-kernel
From: Alex Elder <elder@linaro.org>
Date: Thu, 30 Apr 2020 17:13:21 -0500
> This series removes a field that holds a copy of a channel's state
> at the time it was last fetched. In principle the state can change
> at any time, so it's better to just fetch it whenever needed. The
> first patch is just preparatory, simplifying the arguments to
> gsi_channel_state().
Series applied, thanks Alex.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-01 22:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 22:13 [PATCH net-next 0/2] net: ipa: don't cache channel state Alex Elder
2020-04-30 22:13 ` [PATCH net-next 1/2] net: ipa: pass channel pointer to gsi_channel_state() Alex Elder
2020-04-30 22:13 ` [PATCH net-next 2/2] net: ipa: do not cache channel state Alex Elder
2020-05-01 22:53 ` [PATCH net-next 0/2] net: ipa: don't " David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).