[PATCH net-next 0/6] net: ipa: add a driver shutdown callback

[PATCH net-next 0/6] net: ipa: add a driver shutdown callback

[Alex Elder]

The final patch in this series adds a driver shutdown callback for
the IPA driver.  The patches leading up to that address some issues
encountered while ensuring that callback worked as expected:
  - The first just reports a little more information when channels
    or event rings are in unexpected states
  - The second patch recognizes a condition where an as-yet-unused
    channel does not require a reset during teardown
  - The third patch explicitly ignores a certain error condition,
    because it can't be avoided, and is harmless if it occurs
  - The fourth properly handles requests to retry a channel HALT
    request
  - The fifth makes a second attempt to stop modem activity during
    shutdown if it's busy

The shutdown callback is implemented by calling the existing remove
callback function (reporting if that returns an error).

					-Alex

Alex Elder (6):
  net: ipa: print channel/event ring number on error
  net: ipa: don't reset an ALLOCATED channel
  net: ipa: ignore CHANNEL_NOT_RUNNING errors
  net: ipa: support retries on generic GSI commands
  net: ipa: retry modem stop if busy
  net: ipa: add driver shutdown callback

 drivers/net/ipa/gsi.c      | 101 ++++++++++++++++++++++++++++---------
 drivers/net/ipa/gsi.h      |   1 +
 drivers/net/ipa/ipa_main.c |  19 ++++++-
 3 files changed, 94 insertions(+), 27 deletions(-)

-- 
2.20.1

[PATCH net-next 1/6] net: ipa: print channel/event ring number on error

[Alex Elder]

When a GSI command is used to change the state of a channel or event
ring we check the state before and after the command to ensure it is
as expected.  If not, we print an error message, but it does not
include the channel or event ring id, and it easily can.  Add the
channel or event ring id to these error messages.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 54 +++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 55151960a6985..2bc513c663396 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -365,15 +365,15 @@ static int gsi_evt_ring_alloc_command(struct gsi *gsi, u32 evt_ring_id)
 	/* Get initial event ring state */
 	evt_ring->state = gsi_evt_ring_state(gsi, evt_ring_id);
 	if (evt_ring->state != GSI_EVT_RING_STATE_NOT_ALLOCATED) {
-		dev_err(gsi->dev, "bad event ring state %u before alloc\n",
-			evt_ring->state);
+		dev_err(gsi->dev, "event ring %u bad state %u before alloc\n",
+			evt_ring_id, evt_ring->state);
 		return -EINVAL;
 	}
 
 	ret = evt_ring_command(gsi, evt_ring_id, GSI_EVT_ALLOCATE);
 	if (!ret && evt_ring->state != GSI_EVT_RING_STATE_ALLOCATED) {
-		dev_err(gsi->dev, "bad event ring state %u after alloc\n",
-			evt_ring->state);
+		dev_err(gsi->dev, "event ring %u bad state %u after alloc\n",
+			evt_ring_id, evt_ring->state);
 		ret = -EIO;
 	}
 
@@ -389,15 +389,15 @@ static void gsi_evt_ring_reset_command(struct gsi *gsi, u32 evt_ring_id)
 
 	if (state != GSI_EVT_RING_STATE_ALLOCATED &&
 	    state != GSI_EVT_RING_STATE_ERROR) {
-		dev_err(gsi->dev, "bad event ring state %u before reset\n",
-			evt_ring->state);
+		dev_err(gsi->dev, "event ring %u bad state %u before reset\n",
+			evt_ring_id, evt_ring->state);
 		return;
 	}
 
 	ret = evt_ring_command(gsi, evt_ring_id, GSI_EVT_RESET);
 	if (!ret && evt_ring->state != GSI_EVT_RING_STATE_ALLOCATED)
-		dev_err(gsi->dev, "bad event ring state %u after reset\n",
-			evt_ring->state);
+		dev_err(gsi->dev, "event ring %u bad state %u after reset\n",
+			evt_ring_id, evt_ring->state);
 }
 
 /* Issue a hardware de-allocation request for an allocated event ring */
@@ -407,15 +407,15 @@ static void gsi_evt_ring_de_alloc_command(struct gsi *gsi, u32 evt_ring_id)
 	int ret;
 
 	if (evt_ring->state != GSI_EVT_RING_STATE_ALLOCATED) {
-		dev_err(gsi->dev, "bad event ring state %u before dealloc\n",
-			evt_ring->state);
+		dev_err(gsi->dev, "event ring %u state %u before dealloc\n",
+			evt_ring_id, evt_ring->state);
 		return;
 	}
 
 	ret = evt_ring_command(gsi, evt_ring_id, GSI_EVT_DE_ALLOC);
 	if (!ret && evt_ring->state != GSI_EVT_RING_STATE_NOT_ALLOCATED)
-		dev_err(gsi->dev, "bad event ring state %u after dealloc\n",
-			evt_ring->state);
+		dev_err(gsi->dev, "event ring %u bad state %u after dealloc\n",
+			evt_ring_id, evt_ring->state);
 }
 
 /* Fetch the current state of a channel from hardware */
@@ -479,7 +479,8 @@ static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)
 	/* Get initial channel state */
 	state = gsi_channel_state(channel);
 	if (state != GSI_CHANNEL_STATE_NOT_ALLOCATED) {
-		dev_err(dev, "bad channel state %u before alloc\n", state);
+		dev_err(dev, "channel %u bad state %u before alloc\n",
+			channel_id, state);
 		return -EINVAL;
 	}
 
@@ -488,7 +489,8 @@ static int gsi_channel_alloc_command(struct gsi *gsi, u32 channel_id)
 	/* Channel state will normally have been updated */
 	state = gsi_channel_state(channel);
 	if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED) {
-		dev_err(dev, "bad channel state %u after alloc\n", state);
+		dev_err(dev, "channel %u bad state %u after alloc\n",
+			channel_id, state);
 		ret = -EIO;
 	}
 
@@ -505,7 +507,8 @@ static int gsi_channel_start_command(struct gsi_channel *channel)
 	state = gsi_channel_state(channel);
 	if (state != GSI_CHANNEL_STATE_ALLOCATED &&
 	    state != GSI_CHANNEL_STATE_STOPPED) {
-		dev_err(dev, "bad channel state %u before start\n", state);
+		dev_err(dev, "channel %u bad state %u before start\n",
+			gsi_channel_id(channel), state);
 		return -EINVAL;
 	}
 
@@ -514,7 +517,8 @@ static int gsi_channel_start_command(struct gsi_channel *channel)
 	/* Channel state will normally have been updated */
 	state = gsi_channel_state(channel);
 	if (!ret && state != GSI_CHANNEL_STATE_STARTED) {
-		dev_err(dev, "bad channel state %u after start\n", state);
+		dev_err(dev, "channel %u bad state %u after start\n",
+			gsi_channel_id(channel), state);
 		ret = -EIO;
 	}
 
@@ -538,7 +542,8 @@ static int gsi_channel_stop_command(struct gsi_channel *channel)
 
 	if (state != GSI_CHANNEL_STATE_STARTED &&
 	    state != GSI_CHANNEL_STATE_STOP_IN_PROC) {
-		dev_err(dev, "bad channel state %u before stop\n", state);
+		dev_err(dev, "channel %u bad state %u before stop\n",
+			gsi_channel_id(channel), state);
 		return -EINVAL;
 	}
 
@@ -553,7 +558,8 @@ static int gsi_channel_stop_command(struct gsi_channel *channel)
 	if (state == GSI_CHANNEL_STATE_STOP_IN_PROC)
 		return -EAGAIN;
 
-	dev_err(dev, "bad channel state %u after stop\n", state);
+	dev_err(dev, "channel %u bad state %u after stop\n",
+		gsi_channel_id(channel), state);
 
 	return -EIO;
 }
@@ -570,7 +576,8 @@ static void gsi_channel_reset_command(struct gsi_channel *channel)
 	state = gsi_channel_state(channel);
 	if (state != GSI_CHANNEL_STATE_STOPPED &&
 	    state != GSI_CHANNEL_STATE_ERROR) {
-		dev_err(dev, "bad channel state %u before reset\n", state);
+		dev_err(dev, "channel %u bad state %u before reset\n",
+			gsi_channel_id(channel), state);
 		return;
 	}
 
@@ -579,7 +586,8 @@ static void gsi_channel_reset_command(struct gsi_channel *channel)
 	/* Channel state will normally have been updated */
 	state = gsi_channel_state(channel);
 	if (!ret && state != GSI_CHANNEL_STATE_ALLOCATED)
-		dev_err(dev, "bad channel state %u after reset\n", state);
+		dev_err(dev, "channel %u bad state %u after reset\n",
+			gsi_channel_id(channel), state);
 }
 
 /* Deallocate an ALLOCATED GSI channel */
@@ -592,7 +600,8 @@ static void gsi_channel_de_alloc_command(struct gsi *gsi, u32 channel_id)
 
 	state = gsi_channel_state(channel);
 	if (state != GSI_CHANNEL_STATE_ALLOCATED) {
-		dev_err(dev, "bad channel state %u before dealloc\n", state);
+		dev_err(dev, "channel %u bad state %u before dealloc\n",
+			channel_id, state);
 		return;
 	}
 
@@ -601,7 +610,8 @@ static void gsi_channel_de_alloc_command(struct gsi *gsi, u32 channel_id)
 	/* Channel state will normally have been updated */
 	state = gsi_channel_state(channel);
 	if (!ret && state != GSI_CHANNEL_STATE_NOT_ALLOCATED)
-		dev_err(dev, "bad channel state %u after dealloc\n", state);
+		dev_err(dev, "channel %u bad state %u after dealloc\n",
+			channel_id, state);
 }
 
 /* Ring an event ring doorbell, reporting the last entry processed by the AP.
-- 
2.20.1

[PATCH net-next 2/6] net: ipa: don't reset an ALLOCATED channel

[Alex Elder]

If the rmnet_ipa0 network device has not been opened at the time
we remove or shut down the IPA driver, its underlying TX and RX
GSI channels will not have been started, and they will still be
in ALLOCATED state.

The RESET command on a channel is meant to return a channel to
ALLOCATED state after it's been stopped.  But if it was never
started, its state will still be ALLOCATED, the RESET command
is not required.

Quietly skip doing the reset without printing an error message if a
channel is already in ALLOCATED state when we request it be reset.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 2bc513c663396..58bec70db5ab4 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -576,8 +576,10 @@ static void gsi_channel_reset_command(struct gsi_channel *channel)
 	state = gsi_channel_state(channel);
 	if (state != GSI_CHANNEL_STATE_STOPPED &&
 	    state != GSI_CHANNEL_STATE_ERROR) {
-		dev_err(dev, "channel %u bad state %u before reset\n",
-			gsi_channel_id(channel), state);
+		/* No need to reset a channel already in ALLOCATED state */
+		if (state != GSI_CHANNEL_STATE_ALLOCATED)
+			dev_err(dev, "channel %u bad state %u before reset\n",
+				gsi_channel_id(channel), state);
 		return;
 	}
 
-- 
2.20.1

[PATCH net-next 3/6] net: ipa: ignore CHANNEL_NOT_RUNNING errors

[Alex Elder]

IPA v4.2 has a hardware quirk that requires the AP to allocate GSI
channels for the modem to use.  It is recommended that these modem
channels get stopped (with a HALT generic command) by the AP when
its IPA driver gets removed.

The AP has no way of knowing the current state of a modem channel.
So when the IPA driver issues a HALT command it's possible the
channel is not running, and in that case we get an error indication.
This error simply means we didn't need to stop the channel, so we
can ignore it.

This patch adds an explanation for this situation, and arranges for
this condition to *not* report an error message.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 58bec70db5ab4..7c2e820625590 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1087,10 +1087,32 @@ static void gsi_isr_gp_int1(struct gsi *gsi)
 	u32 result;
 	u32 val;
 
+	/* This interrupt is used to handle completions of the two GENERIC
+	 * GSI commands.  We use these to allocate and halt channels on
+	 * the modem's behalf due to a hardware quirk on IPA v4.2.  Once
+	 * allocated, the modem "owns" these channels, and as a result we
+	 * have no way of knowing the channel's state at any given time.
+	 *
+	 * It is recommended that we halt the modem channels we allocated
+	 * when shutting down, but it's possible the channel isn't running
+	 * at the time we issue the HALT command.  We'll get an error in
+	 * that case, but it's harmless (the channel is already halted).
+	 *
+	 * For this reason, we silently ignore a CHANNEL_NOT_RUNNING error
+	 * if we receive it.
+	 */
 	val = ioread32(gsi->virt + GSI_CNTXT_SCRATCH_0_OFFSET);
 	result = u32_get_bits(val, GENERIC_EE_RESULT_FMASK);
-	if (result != GENERIC_EE_SUCCESS)
+
+	switch (result) {
+	case GENERIC_EE_SUCCESS:
+	case GENERIC_EE_CHANNEL_NOT_RUNNING:
+		break;
+
+	default:
 		dev_err(gsi->dev, "global INT1 generic result %u\n", result);
+		break;
+	}
 
 	complete(&gsi->completion);
 }
-- 
2.20.1

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

[Alex Elder]

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(-)

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

[PATCH net-next 5/6] net: ipa: retry modem stop if busy

[Alex Elder]

The IPA driver remove callback, ipa_remove(), calls ipa_modem_stop()
if the setup stage of initialization is complete.  If a concurrent
call to ipa_modem_start() or ipa_modem_stop() has begin but not
completed, ipa_modem_stop() can return an error (-EBUSY).

The next patch adds a driver shutdown callback, which will simply
call ipa_remove().  We really want our shutdown callback to clean
things up.  So add a single retry to the ipa_modem_stop() call in
ipa_remove() after a short (millisecond) delay.  This offers no
guarantee the shutdown will complete successfully, but we'll at
least try a little harder before giving up.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
index 3fb9c5d90b70e..9f4bd822ac625 100644
--- a/drivers/net/ipa/ipa_main.c
+++ b/drivers/net/ipa/ipa_main.c
@@ -864,6 +864,11 @@ static int ipa_remove(struct platform_device *pdev)
 
 	if (ipa->setup_complete) {
 		ret = ipa_modem_stop(ipa);
+		/* If starting or stopping is in progress, try once more */
+		if (ret == -EBUSY) {
+			usleep_range(USEC_PER_MSEC, 2 * USEC_PER_MSEC);
+			ret = ipa_modem_stop(ipa);
+		}
 		if (ret)
 			return ret;
 
-- 
2.20.1

[PATCH net-next 6/6] net: ipa: add driver shutdown callback

[Alex Elder]

A system shutdown can happen at essentially any time, and it's
possible that the IPA driver is busy when a shutdown is underway.
IPA hardware accesses IMEM and SMEM memory regions using an IOMMU,
and at some point during shutdown, needed I/O mappings could become
invalid.  This could be disastrous for any "in flight" IPA activity.

Avoid this by defining a new driver shutdown callback that stops all
IPA activity and cleanly shuts down the driver.  It merely calls the
driver's existing remove callback, reporting the error if it returns
one.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
index 9f4bd822ac625..bbfc071fa2a60 100644
--- a/drivers/net/ipa/ipa_main.c
+++ b/drivers/net/ipa/ipa_main.c
@@ -889,6 +889,15 @@ static int ipa_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void ipa_shutdown(struct platform_device *pdev)
+{
+	int ret;
+
+	ret = ipa_remove(pdev);
+	if (ret)
+		dev_err(&pdev->dev, "shutdown: remove returned %d\n", ret);
+}
+
 /**
  * ipa_suspend() - Power management system suspend callback
  * @dev:	IPA device structure
@@ -946,8 +955,9 @@ static const struct dev_pm_ops ipa_pm_ops = {
 };
 
 static struct platform_driver ipa_driver = {
-	.probe	= ipa_probe,
-	.remove	= ipa_remove,
+	.probe		= ipa_probe,
+	.remove		= ipa_remove,
+	.shutdown	= ipa_shutdown,
 	.driver	= {
 		.name		= "ipa",
 		.pm		= &ipa_pm_ops,
-- 
2.20.1

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

[Jakub Kicinski]

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!

Re: [PATCH net-next 0/6] net: ipa: add a driver shutdown callback

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Thu, 19 Nov 2020 16:49:23 -0600 you wrote:
> The final patch in this series adds a driver shutdown callback for
> the IPA driver.  The patches leading up to that address some issues
> encountered while ensuring that callback worked as expected:
>   - The first just reports a little more information when channels
>     or event rings are in unexpected states
>   - The second patch recognizes a condition where an as-yet-unused
>     channel does not require a reset during teardown
>   - The third patch explicitly ignores a certain error condition,
>     because it can't be avoided, and is harmless if it occurs
>   - The fourth properly handles requests to retry a channel HALT
>     request
>   - The fifth makes a second attempt to stop modem activity during
>     shutdown if it's busy
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] net: ipa: print channel/event ring number on error
    https://git.kernel.org/netdev/net-next/c/f8d3bdd561a7
  - [net-next,2/6] net: ipa: don't reset an ALLOCATED channel
    https://git.kernel.org/netdev/net-next/c/5d28913d4ee6
  - [net-next,3/6] net: ipa: ignore CHANNEL_NOT_RUNNING errors
    https://git.kernel.org/netdev/net-next/c/f849afcc8c3b
  - [net-next,4/6] net: ipa: support retries on generic GSI commands
    https://git.kernel.org/netdev/net-next/c/1136145660f3
  - [net-next,5/6] net: ipa: retry modem stop if busy
    https://git.kernel.org/netdev/net-next/c/7c80e83829db
  - [net-next,6/6] net: ipa: add driver shutdown callback
    https://git.kernel.org/netdev/net-next/c/ae1d72f9779f

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[Alex Elder]

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

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

[Jakub Kicinski]

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 :)