[PATCH net-next v2 0/7] net: ipa: don't disable NAPI in suspend

[PATCH net-next v2 0/7] net: ipa: don't disable NAPI in suspend

[Alex Elder]

This is version 2 of a series that reworks the order in which things
happen during channel stop and suspend (and start and resume), in
order to address a hang that has been observed during suspend.
The introductory message on the first version of the series gave
some history which is omitted here.

The end result of this series is that we only enable NAPI and the
I/O completion interrupt on a channel when we start the channel for
the first time.  And we only disable them when stopping the channel
"for good."  In other words, NAPI and the completion interrupt
remain enabled while a channel is stopped for suspend.

One comment on version 1 of the series suggested *not* returning
early on success in a function, instead having both success and
error paths return from the same point at the end of the function
block.  This has been addressed in this version.

In addition, this version consolidates things a little bit, but the
net result of the series is exactly the same as version 1 (with the
exception of the return fix mentioned above).

First, patch 6 in the first version was a small step to make patch 7
easier to understand.  The two have been combined now.

Second, previous version moved (and for suspend/resume, eliminated)
I/O completion interrupt and NAPI disable/enable control in separate
steps (patches).  Now both are moved around together in patch 5 and
6, which eliminates the need for the final (NAPI-only) patch.

I won't repeat the patch summaries provided in v1:
  https://lore.kernel.org/netdev/20210129202019.2099259-1-elder@linaro.org/

Many thanks to Willem de Bruijn for his thoughtful input.

					-Alex

Alex Elder (7):
  net: ipa: don't thaw channel if error starting
  net: ipa: introduce gsi_channel_stop_retry()
  net: ipa: introduce __gsi_channel_start()
  net: ipa: kill gsi_channel_freeze() and gsi_channel_thaw()
  net: ipa: disable interrupt and NAPI after channel stop
  net: ipa: don't disable interrupt on suspend
  net: ipa: expand last transaction check

 drivers/net/ipa/gsi.c | 138 ++++++++++++++++++++++++++----------------
 1 file changed, 85 insertions(+), 53 deletions(-)

-- 
2.27.0

[PATCH net-next v2 1/7] net: ipa: don't thaw channel if error starting

[Alex Elder]

If an error occurs starting a channel, don't "thaw" it.
We should assume the channel remains in a non-started state.

Update the comment in gsi_channel_stop(); calls to this function
are no longer retried.

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 f79cf3c327c1c..4a3e125e898f6 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -885,7 +885,9 @@ int gsi_channel_start(struct gsi *gsi, u32 channel_id)
 
 	mutex_unlock(&gsi->mutex);
 
-	gsi_channel_thaw(channel);
+	/* Thaw the channel if successful */
+	if (!ret)
+		gsi_channel_thaw(channel);
 
 	return ret;
 }
@@ -910,7 +912,7 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
 
 	mutex_unlock(&gsi->mutex);
 
-	/* Thaw the channel if we need to retry (or on error) */
+	/* Re-thaw the channel if an error occurred while stopping */
 	if (ret)
 		gsi_channel_thaw(channel);
 
-- 
2.27.0

[PATCH net-next v2 2/7] net: ipa: introduce gsi_channel_stop_retry()

[Alex Elder]

Create a new helper function that encapsulates issuing a set of
channel stop commands, retrying if appropriate, with a short delay
between attempts.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 4a3e125e898f6..bd1bf388d9892 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -892,15 +892,12 @@ int gsi_channel_start(struct gsi *gsi, u32 channel_id)
 	return ret;
 }
 
-/* Stop a started channel */
-int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
+static int gsi_channel_stop_retry(struct gsi_channel *channel)
 {
-	struct gsi_channel *channel = &gsi->channel[channel_id];
 	u32 retries = GSI_CHANNEL_STOP_RETRIES;
+	struct gsi *gsi = channel->gsi;
 	int ret;
 
-	gsi_channel_freeze(channel);
-
 	mutex_lock(&gsi->mutex);
 
 	do {
@@ -912,6 +909,19 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
 
 	mutex_unlock(&gsi->mutex);
 
+	return ret;
+}
+
+/* Stop a started channel */
+int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
+{
+	struct gsi_channel *channel = &gsi->channel[channel_id];
+	int ret;
+
+	gsi_channel_freeze(channel);
+
+	ret = gsi_channel_stop_retry(channel);
+
 	/* Re-thaw the channel if an error occurred while stopping */
 	if (ret)
 		gsi_channel_thaw(channel);
-- 
2.27.0

[PATCH net-next v2 3/7] net: ipa: introduce __gsi_channel_start()

[Alex Elder]

Create a new function that does most of the work of starting a
channel.  What's different is that it takes a flag indicating
whether the channel should really be started or not.  Create
another new function __gsi_channel_stop() that behaves similarly.

IPA v3.5.1 implements suspend using a special SUSPEND endpoint
setting.  If the endpoint is suspended when an I/O completes on the
underlying GSI channel, a SUSPEND interrupt is generated.

Newer versions of IPA do not implement the SUSPEND endpoint mode.
Instead, endpoint suspend is implemented by simply stopping the
underlying GSI channel.  In this case, a completing I/O on a
*stopped* channel causes the SUSPEND interrupt condition.

These new functions put all activity related to starting or stopping
a channel (including "thawing/freezing" the channel) in one place,
whether or not the channel is actually started or stopped.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index bd1bf388d9892..bba64887fe969 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -873,23 +873,30 @@ static void gsi_channel_deprogram(struct gsi_channel *channel)
 	/* Nothing to do */
 }
 
+static int __gsi_channel_start(struct gsi_channel *channel, bool start)
+{
+	struct gsi *gsi = channel->gsi;
+	int ret;
+
+	mutex_lock(&gsi->mutex);
+
+	ret = start ? gsi_channel_start_command(channel) : 0;
+
+	mutex_unlock(&gsi->mutex);
+
+	/* Thaw the channel if successful */
+	if (!ret)
+		gsi_channel_thaw(channel);
+
+	return ret;
+}
+
 /* Start an allocated GSI channel */
 int gsi_channel_start(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
-	int ret;
 
-	mutex_lock(&gsi->mutex);
-
-	ret = gsi_channel_start_command(channel);
-
-	mutex_unlock(&gsi->mutex);
-
-	/* Thaw the channel if successful */
-	if (!ret)
-		gsi_channel_thaw(channel);
-
-	return ret;
+	return __gsi_channel_start(channel, true);
 }
 
 static int gsi_channel_stop_retry(struct gsi_channel *channel)
@@ -912,21 +919,27 @@ static int gsi_channel_stop_retry(struct gsi_channel *channel)
 	return ret;
 }
 
+static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
+{
+	int ret;
+
+	gsi_channel_freeze(channel);
+
+	ret = stop ? gsi_channel_stop_retry(channel) : 0;
+
+	/* Re-thaw the channel if an error occurred while stopping */
+	if (ret)
+		gsi_channel_thaw(channel);
+
+	return ret;
+}
+
 /* Stop a started channel */
 int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
-	int ret;
 
-	gsi_channel_freeze(channel);
-
-	ret = gsi_channel_stop_retry(channel);
-
-	/* Re-thaw the channel if an error occurred while stopping */
-	if (ret)
-		gsi_channel_thaw(channel);
-
-	return ret;
+	return __gsi_channel_stop(channel, true);
 }
 
 /* Reset and reconfigure a channel, (possibly) enabling the doorbell engine */
@@ -952,12 +965,7 @@ int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 
-	if (stop)
-		return gsi_channel_stop(gsi, channel_id);
-
-	gsi_channel_freeze(channel);
-
-	return 0;
+	return __gsi_channel_stop(channel, stop);
 }
 
 /* Resume a suspended channel (starting will be requested if STOPPED) */
@@ -965,12 +973,7 @@ int gsi_channel_resume(struct gsi *gsi, u32 channel_id, bool start)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 
-	if (start)
-		return gsi_channel_start(gsi, channel_id);
-
-	gsi_channel_thaw(channel);
-
-	return 0;
+	return __gsi_channel_start(channel, start);
 }
 
 /**
-- 
2.27.0

[PATCH net-next v2 4/7] net: ipa: kill gsi_channel_freeze() and gsi_channel_thaw()

[Alex Elder]

Open-code gsi_channel_freeze() and gsi_channel_thaw() in all callers
and get rid of these two functions.  This is part of reworking the
sequence of things done during channel suspend/resume and start/stop.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index bba64887fe969..565c785e33a25 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -764,24 +764,6 @@ static void gsi_channel_trans_quiesce(struct gsi_channel *channel)
 	}
 }
 
-/* Stop channel activity.  Transactions may not be allocated until thawed. */
-static void gsi_channel_freeze(struct gsi_channel *channel)
-{
-	gsi_channel_trans_quiesce(channel);
-
-	napi_disable(&channel->napi);
-
-	gsi_irq_ieob_disable_one(channel->gsi, channel->evt_ring_id);
-}
-
-/* Allow transactions to be used on the channel again. */
-static void gsi_channel_thaw(struct gsi_channel *channel)
-{
-	gsi_irq_ieob_enable_one(channel->gsi, channel->evt_ring_id);
-
-	napi_enable(&channel->napi);
-}
-
 /* Program a channel for use */
 static void gsi_channel_program(struct gsi_channel *channel, bool doorbell)
 {
@@ -884,9 +866,10 @@ static int __gsi_channel_start(struct gsi_channel *channel, bool start)
 
 	mutex_unlock(&gsi->mutex);
 
-	/* Thaw the channel if successful */
-	if (!ret)
-		gsi_channel_thaw(channel);
+	if (!ret) {
+		gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
+		napi_enable(&channel->napi);
+	}
 
 	return ret;
 }
@@ -921,15 +904,19 @@ static int gsi_channel_stop_retry(struct gsi_channel *channel)
 
 static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
 {
+	struct gsi *gsi = channel->gsi;
 	int ret;
 
-	gsi_channel_freeze(channel);
+	gsi_channel_trans_quiesce(channel);
+	napi_disable(&channel->napi);
+	gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
 
 	ret = stop ? gsi_channel_stop_retry(channel) : 0;
 
-	/* Re-thaw the channel if an error occurred while stopping */
-	if (ret)
-		gsi_channel_thaw(channel);
+	if (ret) {
+		gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
+		napi_enable(&channel->napi);
+	}
 
 	return ret;
 }
-- 
2.27.0

[PATCH net-next v2 5/7] net: ipa: disable interrupt and NAPI after channel stop

[Alex Elder]

Disable both the I/O completion interrupt and NAPI polling on a
channel *after* we successfully stop it rather than before.  This
ensures a completion occurring just before the channel is stopped
gets processed.

Enable NAPI polling and the interrupt *before* starting a channel
rather than after, to be symmetric.  A stopped channel won't
generate any completion interrupts anyway.

Enable NAPI before the interrupt and disable it afterward.

Signed-off-by: Alex Elder <elder@linaro.org>
---
v2: Update code for *both* NAPI and the completion interrupt.

 drivers/net/ipa/gsi.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 565c785e33a25..93e1d29b28385 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -860,15 +860,18 @@ static int __gsi_channel_start(struct gsi_channel *channel, bool start)
 	struct gsi *gsi = channel->gsi;
 	int ret;
 
+	napi_enable(&channel->napi);
+	gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
+
 	mutex_lock(&gsi->mutex);
 
 	ret = start ? gsi_channel_start_command(channel) : 0;
 
 	mutex_unlock(&gsi->mutex);
 
-	if (!ret) {
-		gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
-		napi_enable(&channel->napi);
+	if (ret) {
+		gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
+		napi_disable(&channel->napi);
 	}
 
 	return ret;
@@ -908,14 +911,11 @@ static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
 	int ret;
 
 	gsi_channel_trans_quiesce(channel);
-	napi_disable(&channel->napi);
-	gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
 
 	ret = stop ? gsi_channel_stop_retry(channel) : 0;
-
-	if (ret) {
-		gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
-		napi_enable(&channel->napi);
+	if (!ret) {
+		gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
+		napi_disable(&channel->napi);
 	}
 
 	return ret;
-- 
2.27.0

[PATCH net-next v2 6/7] net: ipa: don't disable interrupt on suspend

[Alex Elder]

No completion interrupts will occur while an endpoint is suspended,
nor when a channel has been stopped for suspend.  So there's no need
to disable the interrupt during suspend and re-enable it when
resuming.  Without any interrupts occurring, there is no need to
disable/re-enable NAPI for channel suspend/resume either.

We'll only enable NAPI and the interrupt when we first start the
channel, and disable it again only when it's "really" stopped.

To accomplish this, move the enable/disable calls out of
__gsi_channel_start() and __gsi_channel_stop(), and into
gsi_channel_start() and gsi_channel_stop() instead.

Add a call to napi_synchronize() to gsi_channel_suspend(), to ensure
NAPI polling is done before moving on.

Signed-off-by: Alex Elder <elder@linaro.org>
---
v2: Consolidate preparatory patch into the "real" one.
v2: Update code for *both* NAPI and the completion interrupt.
v2: Use common return path in gsi_channel_start().

 drivers/net/ipa/gsi.c | 44 ++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 93e1d29b28385..03498182ad024 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -860,20 +860,15 @@ static int __gsi_channel_start(struct gsi_channel *channel, bool start)
 	struct gsi *gsi = channel->gsi;
 	int ret;
 
-	napi_enable(&channel->napi);
-	gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
+	if (!start)
+		return 0;
 
 	mutex_lock(&gsi->mutex);
 
-	ret = start ? gsi_channel_start_command(channel) : 0;
+	ret = gsi_channel_start_command(channel);
 
 	mutex_unlock(&gsi->mutex);
 
-	if (ret) {
-		gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
-		napi_disable(&channel->napi);
-	}
-
 	return ret;
 }
 
@@ -881,8 +876,19 @@ static int __gsi_channel_start(struct gsi_channel *channel, bool start)
 int gsi_channel_start(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
+	int ret;
 
-	return __gsi_channel_start(channel, true);
+	/* Enable NAPI and the completion interrupt */
+	napi_enable(&channel->napi);
+	gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
+
+	ret = __gsi_channel_start(channel, true);
+	if (ret) {
+		gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
+		napi_disable(&channel->napi);
+	}
+
+	return ret;
 }
 
 static int gsi_channel_stop_retry(struct gsi_channel *channel)
@@ -907,16 +913,15 @@ static int gsi_channel_stop_retry(struct gsi_channel *channel)
 
 static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
 {
-	struct gsi *gsi = channel->gsi;
 	int ret;
 
+	/* Wait for any underway transactions to complete before stopping. */
 	gsi_channel_trans_quiesce(channel);
 
 	ret = stop ? gsi_channel_stop_retry(channel) : 0;
-	if (!ret) {
-		gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
-		napi_disable(&channel->napi);
-	}
+	/* Finally, ensure NAPI polling has finished. */
+	if (!ret)
+		napi_synchronize(&channel->napi);
 
 	return ret;
 }
@@ -925,8 +930,17 @@ static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
 int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
+	int ret;
 
-	return __gsi_channel_stop(channel, true);
+	/* Only disable the completion interrupt if stop is successful */
+	ret = __gsi_channel_stop(channel, true);
+	if (ret)
+		return ret;
+
+	gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
+	napi_disable(&channel->napi);
+
+	return 0;
 }
 
 /* Reset and reconfigure a channel, (possibly) enabling the doorbell engine */
-- 
2.27.0

[PATCH net-next v2 7/7] net: ipa: expand last transaction check

[Alex Elder]

Transactions to send data for a network device can be allocated at
any time up until the point the TX queue is stopped.  It is possible
for ipa_start_xmit() to be called in one context just before a
the transmit queue is stopped in another.

Update gsi_channel_trans_last() so that for TX channels the
allocated and pending transaction lists are checked--in addition
to the completed and polled lists--to determine the "last"
transaction.  This means any transaction that has been allocated
before the TX queue is stopped will be allowed to complete before
we conclude the channel is quiesced.

Rework the function a bit to use a list pointer and gotos.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 03498182ad024..8b64cbe4737a4 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -725,22 +725,38 @@ static void gsi_evt_ring_program(struct gsi *gsi, u32 evt_ring_id)
 	gsi_evt_ring_doorbell(gsi, evt_ring_id, 0);
 }
 
-/* Return the last (most recent) transaction completed on a channel. */
+/* Find the transaction whose completion indicates a channel is quiesced */
 static struct gsi_trans *gsi_channel_trans_last(struct gsi_channel *channel)
 {
 	struct gsi_trans_info *trans_info = &channel->trans_info;
+	const struct list_head *list;
 	struct gsi_trans *trans;
 
 	spin_lock_bh(&trans_info->spinlock);
 
-	if (!list_empty(&trans_info->complete))
-		trans = list_last_entry(&trans_info->complete,
-					struct gsi_trans, links);
-	else if (!list_empty(&trans_info->polled))
-		trans = list_last_entry(&trans_info->polled,
-					struct gsi_trans, links);
-	else
-		trans = NULL;
+	/* There is a small chance a TX transaction got allocated just
+	 * before we disabled transmits, so check for that.
+	 */
+	if (channel->toward_ipa) {
+		list = &trans_info->alloc;
+		if (!list_empty(list))
+			goto done;
+		list = &trans_info->pending;
+		if (!list_empty(list))
+			goto done;
+	}
+
+	/* Otherwise (TX or RX) we want to wait for anything that
+	 * has completed, or has been polled but not released yet.
+	 */
+	list = &trans_info->complete;
+	if (!list_empty(list))
+		goto done;
+	list = &trans_info->polled;
+	if (list_empty(list))
+		list = NULL;
+done:
+	trans = list ? list_last_entry(list, struct gsi_trans, links) : NULL;
 
 	/* Caller will wait for this, so take a reference */
 	if (trans)
-- 
2.27.0

Re: [PATCH net-next v2 0/7] net: ipa: don't disable NAPI in suspend

[Willem de Bruijn]

On Mon, Feb 1, 2021 at 12:28 PM Alex Elder <elder@linaro.org> wrote:
>
> This is version 2 of a series that reworks the order in which things
> happen during channel stop and suspend (and start and resume), in
> order to address a hang that has been observed during suspend.
> The introductory message on the first version of the series gave
> some history which is omitted here.
>
> The end result of this series is that we only enable NAPI and the
> I/O completion interrupt on a channel when we start the channel for
> the first time.  And we only disable them when stopping the channel
> "for good."  In other words, NAPI and the completion interrupt
> remain enabled while a channel is stopped for suspend.
>
> One comment on version 1 of the series suggested *not* returning
> early on success in a function, instead having both success and
> error paths return from the same point at the end of the function
> block.  This has been addressed in this version.
>
> In addition, this version consolidates things a little bit, but the
> net result of the series is exactly the same as version 1 (with the
> exception of the return fix mentioned above).
>
> First, patch 6 in the first version was a small step to make patch 7
> easier to understand.  The two have been combined now.
>
> Second, previous version moved (and for suspend/resume, eliminated)
> I/O completion interrupt and NAPI disable/enable control in separate
> steps (patches).  Now both are moved around together in patch 5 and
> 6, which eliminates the need for the final (NAPI-only) patch.
>
> I won't repeat the patch summaries provided in v1:
>   https://lore.kernel.org/netdev/20210129202019.2099259-1-elder@linaro.org/
>
> Many thanks to Willem de Bruijn for his thoughtful input.
>
>                                         -Alex
>
> Alex Elder (7):
>   net: ipa: don't thaw channel if error starting
>   net: ipa: introduce gsi_channel_stop_retry()
>   net: ipa: introduce __gsi_channel_start()
>   net: ipa: kill gsi_channel_freeze() and gsi_channel_thaw()
>   net: ipa: disable interrupt and NAPI after channel stop
>   net: ipa: don't disable interrupt on suspend
>   net: ipa: expand last transaction check
>
>  drivers/net/ipa/gsi.c | 138 ++++++++++++++++++++++++++----------------
>  1 file changed, 85 insertions(+), 53 deletions(-)

Acked-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net-next v2 0/7] net: ipa: don't disable NAPI in suspend

[Jakub Kicinski]

On Mon, 1 Feb 2021 13:44:20 -0500 Willem de Bruijn wrote:
> On Mon, Feb 1, 2021 at 12:28 PM Alex Elder <elder@linaro.org> wrote:
> >
> > This is version 2 of a series that reworks the order in which things
> > happen during channel stop and suspend (and start and resume), in
> > order to address a hang that has been observed during suspend.
> > The introductory message on the first version of the series gave
> > some history which is omitted here.
> >
> > The end result of this series is that we only enable NAPI and the
> > I/O completion interrupt on a channel when we start the channel for
> > the first time.  And we only disable them when stopping the channel
> > "for good."  In other words, NAPI and the completion interrupt
> > remain enabled while a channel is stopped for suspend.
> >
> > One comment on version 1 of the series suggested *not* returning
> > early on success in a function, instead having both success and
> > error paths return from the same point at the end of the function
> > block.  This has been addressed in this version.
> >
> > In addition, this version consolidates things a little bit, but the
> > net result of the series is exactly the same as version 1 (with the
> > exception of the return fix mentioned above).
> >
> > First, patch 6 in the first version was a small step to make patch 7
> > easier to understand.  The two have been combined now.
> >
> > Second, previous version moved (and for suspend/resume, eliminated)
> > I/O completion interrupt and NAPI disable/enable control in separate
> > steps (patches).  Now both are moved around together in patch 5 and
> > 6, which eliminates the need for the final (NAPI-only) patch.
>
> Acked-by: Willem de Bruijn <willemb@google.com>

Applied, thanks!