| Message ID | 20201119224929.23819-5-elder@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1136145660f3116cb92794c1a7571bf49e4a1938 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ipa: add a driver shutdown callback | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 56 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Thu, 19 Nov 2020 16:49:27 -0600 Alex Elder wrote: > + do > + ret = gsi_generic_command(gsi, channel_id, > + GSI_GENERIC_HALT_CHANNEL); > + while (ret == -EAGAIN && retries--); This may well be the first time I've seen someone write a do while loop without the curly brackets!
On 11/20/20 8:49 PM, Jakub Kicinski wrote: > On Thu, 19 Nov 2020 16:49:27 -0600 Alex Elder wrote: >> + do >> + ret = gsi_generic_command(gsi, channel_id, >> + GSI_GENERIC_HALT_CHANNEL); >> + while (ret == -EAGAIN && retries--); > > This may well be the first time I've seen someone write a do while loop > without the curly brackets! I had them at one time, then saw I could get away without them. I don't have a preference but I see you accepted it as-is. I really appreciate your timely responses. -Alex
On Fri, 20 Nov 2020 21:31:09 -0600 Alex Elder wrote: > On 11/20/20 8:49 PM, Jakub Kicinski wrote: > > On Thu, 19 Nov 2020 16:49:27 -0600 Alex Elder wrote: > >> + do > >> + ret = gsi_generic_command(gsi, channel_id, > >> + GSI_GENERIC_HALT_CHANNEL); > >> + while (ret == -EAGAIN && retries--); > > > > This may well be the first time I've seen someone write a do while loop > > without the curly brackets! > > I had them at one time, then saw I could get away > without them. I don't have a preference but I see > you accepted it as-is. It was just an offhand comment, I don't have anything against it :)
diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c index 7c2e820625590..eb4c5d408a835 100644 --- a/drivers/net/ipa/gsi.c +++ b/drivers/net/ipa/gsi.c @@ -92,6 +92,7 @@ #define GSI_CMD_TIMEOUT 5 /* seconds */ #define GSI_CHANNEL_STOP_RX_RETRIES 10 +#define GSI_CHANNEL_MODEM_HALT_RETRIES 10 #define GSI_MHI_EVENT_ID_START 10 /* 1st reserved event id */ #define GSI_MHI_EVENT_ID_END 16 /* Last reserved event id */ @@ -1107,10 +1108,16 @@ static void gsi_isr_gp_int1(struct gsi *gsi) switch (result) { case GENERIC_EE_SUCCESS: case GENERIC_EE_CHANNEL_NOT_RUNNING: + gsi->result = 0; + break; + + case GENERIC_EE_RETRY: + gsi->result = -EAGAIN; break; default: dev_err(gsi->dev, "global INT1 generic result %u\n", result); + gsi->result = -EIO; break; } @@ -1624,7 +1631,7 @@ static int gsi_generic_command(struct gsi *gsi, u32 channel_id, iowrite32(BIT(ERROR_INT), gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET); if (success) - return 0; + return gsi->result; dev_err(gsi->dev, "GSI generic command %u to channel %u timed out\n", opcode, channel_id); @@ -1640,7 +1647,17 @@ static int gsi_modem_channel_alloc(struct gsi *gsi, u32 channel_id) static void gsi_modem_channel_halt(struct gsi *gsi, u32 channel_id) { - (void)gsi_generic_command(gsi, channel_id, GSI_GENERIC_HALT_CHANNEL); + u32 retries = GSI_CHANNEL_MODEM_HALT_RETRIES; + int ret; + + do + ret = gsi_generic_command(gsi, channel_id, + GSI_GENERIC_HALT_CHANNEL); + while (ret == -EAGAIN && retries--); + + if (ret) + dev_err(gsi->dev, "error %d halting modem channel %u\n", + ret, channel_id); } /* Setup function for channels */ diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h index ecc784e3a8127..96c9aed397aad 100644 --- a/drivers/net/ipa/gsi.h +++ b/drivers/net/ipa/gsi.h @@ -161,6 +161,7 @@ struct gsi { u32 type_enabled_bitmap; /* GSI IRQ types enabled */ u32 ieob_enabled_bitmap; /* IEOB IRQ enabled (event rings) */ struct completion completion; /* for global EE commands */ + int result; /* Negative errno (generic commands) */ struct mutex mutex; /* protects commands, programming */ };
When stopping an AP RX channel, there can be a transient period while the channel enters STOP_IN_PROC state before reaching the final STOPPED state. In that case we make another attempt to stop the channel. Similarly, when stopping a modem channel (using a GSI generic command issued from the AP), it's possible that multiple attempts will be required before the channel reaches STOPPED state. Add a field to the GSI structure to record an errno representing the result code provided when a generic command completes. If the result learned in gsi_isr_gp_int1() is RETRY, record -EAGAIN in the result code, otherwise record 0 for success, or -EIO for any other result. If we time out nf gsi_generic_command() waiting for the command to complete, return -ETIMEDOUT (as before). Otherwise return the result stashed by gsi_isr_gp_int1(). Add a loop in gsi_modem_channel_halt() to reissue the HALT command if the result code indicates -EAGAIN. Limit this to 10 retries (after the initial attempt). Signed-off-by: Alex Elder <elder@linaro.org> --- drivers/net/ipa/gsi.c | 21 +++++++++++++++++++-- drivers/net/ipa/gsi.h | 1 + 2 files changed, 20 insertions(+), 2 deletions(-)