diff mbox series

[net-next,4/6] net: ipa: support retries on generic GSI commands

Message ID 20201119224929.23819-5-elder@linaro.org
State New
Headers show
Series net: ipa: add a driver shutdown callback | expand

Commit Message

Alex Elder Nov. 19, 2020, 10:49 p.m. UTC
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(-)

-- 
2.20.1

Comments

Jakub Kicinski Nov. 21, 2020, 2:49 a.m. UTC | #1
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!
Alex Elder Nov. 21, 2020, 3:31 a.m. UTC | #2
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
Jakub Kicinski Nov. 21, 2020, 3:46 a.m. UTC | #3
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 mbox series

Patch

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 */
 };