[PATCH net-next 0/6] net: ipa: prepare GSI interrupts for runtime PM

[PATCH net-next 0/6] net: ipa: prepare GSI interrupts for runtime PM

[Alex Elder]

The last patch in this series arranges for GSI interrupts to be
disabled when the IPA hardware is suspended.  This ensures the clock
is always operational when a GSI interrupt fires.  Leading up to
that are patches that rearrange the code a bit to allow this to
be done.

The first two patches aren't *directly* related.  They remove some
flag arguments to some GSI suspend/resume related functions, using
the version field now present in the GSI structure.

					-Alex

Alex Elder (6):
  net: ipa: use gsi->version for channel suspend/resume
  net: ipa: move version check for channel suspend/resume
  net: ipa: move some GSI setup functions
  net: ipa: have gsi_irq_setup() return an error code
  net: ipa: move gsi_irq_init() code into setup
  net: ipa: disable GSI interrupts while suspended

 drivers/net/ipa/gsi.c          | 239 ++++++++++++++++++---------------
 drivers/net/ipa/gsi.h          |  31 ++++-
 drivers/net/ipa/ipa_endpoint.c |  14 +-
 drivers/net/ipa/ipa_main.c     |   5 +-
 4 files changed, 166 insertions(+), 123 deletions(-)

-- 
2.27.0

[PATCH net-next 1/6] net: ipa: use gsi->version for channel suspend/resume

[Alex Elder]

The GSI layer has the IPA version now, so there's no need for
version-specific flags to be passed from IPA.  One instance of
this is in gsi_channel_suspend() and gsi_channel_resume(), which
indicate whether or not the endpoint suspend is implemented by
GSI stopping the channel.  We can make that determination based
on gsi->version, eliminating the need for a Boolean flag in those
functions.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c          | 14 ++++++++------
 drivers/net/ipa/gsi.h          | 19 +++++++++++++++++--
 drivers/net/ipa/ipa_endpoint.c | 14 ++------------
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 3de67ba066a68..e143deddb7c09 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1026,13 +1026,14 @@ void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell)
 	mutex_unlock(&gsi->mutex);
 }
 
-/* Stop a STARTED channel for suspend (using stop if requested) */
-int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop)
+/* Stop a started channel for suspend */
+int gsi_channel_suspend(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 	int ret;
 
-	ret = __gsi_channel_stop(channel, stop);
+	/* Prior to IPA v4.0 suspend/resume is not implemented by GSI */
+	ret = __gsi_channel_stop(channel, gsi->version >= IPA_VERSION_4_0);
 	if (ret)
 		return ret;
 
@@ -1042,12 +1043,13 @@ int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop)
 	return 0;
 }
 
-/* Resume a suspended channel (starting will be requested if STOPPED) */
-int gsi_channel_resume(struct gsi *gsi, u32 channel_id, bool start)
+/* Resume a suspended channel (starting if stopped) */
+int gsi_channel_resume(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 
-	return __gsi_channel_start(channel, start);
+	/* Prior to IPA v4.0 suspend/resume is not implemented by GSI */
+	return __gsi_channel_start(channel, gsi->version >= IPA_VERSION_4_0);
 }
 
 /**
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 81cd7b07f6e14..97163b58b4ebc 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -232,8 +232,23 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id);
  */
 void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell);
 
-int gsi_channel_suspend(struct gsi *gsi, u32 channel_id, bool stop);
-int gsi_channel_resume(struct gsi *gsi, u32 channel_id, bool start);
+/**
+ * gsi_channel_suspend() - Suspend a GSI channel
+ * @gsi:	GSI pointer
+ * @channel_id:	Channel to suspend
+ *
+ * For IPA v4.0+, suspend is implemented by stopping the channel.
+ */
+int gsi_channel_suspend(struct gsi *gsi, u32 channel_id);
+
+/**
+ * gsi_channel_resume() - Resume a suspended GSI channel
+ * @gsi:	GSI pointer
+ * @channel_id:	Channel to resume
+ *
+ * For IPA v4.0+, the stopped channel is started again.
+ */
+int gsi_channel_resume(struct gsi *gsi, u32 channel_id);
 
 /**
  * gsi_init() - Initialize the GSI subsystem
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 8070d1a1d5dfd..08ee37ae28813 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -1587,7 +1587,6 @@ void ipa_endpoint_suspend_one(struct ipa_endpoint *endpoint)
 {
 	struct device *dev = &endpoint->ipa->pdev->dev;
 	struct gsi *gsi = &endpoint->ipa->gsi;
-	bool stop_channel;
 	int ret;
 
 	if (!(endpoint->ipa->enabled & BIT(endpoint->endpoint_id)))
@@ -1598,11 +1597,7 @@ void ipa_endpoint_suspend_one(struct ipa_endpoint *endpoint)
 		(void)ipa_endpoint_program_suspend(endpoint, true);
 	}
 
-	/* Starting with IPA v4.0, endpoints are suspended by stopping the
-	 * underlying GSI channel rather than using endpoint suspend mode.
-	 */
-	stop_channel = endpoint->ipa->version >= IPA_VERSION_4_0;
-	ret = gsi_channel_suspend(gsi, endpoint->channel_id, stop_channel);
+	ret = gsi_channel_suspend(gsi, endpoint->channel_id);
 	if (ret)
 		dev_err(dev, "error %d suspending channel %u\n", ret,
 			endpoint->channel_id);
@@ -1612,7 +1607,6 @@ void ipa_endpoint_resume_one(struct ipa_endpoint *endpoint)
 {
 	struct device *dev = &endpoint->ipa->pdev->dev;
 	struct gsi *gsi = &endpoint->ipa->gsi;
-	bool start_channel;
 	int ret;
 
 	if (!(endpoint->ipa->enabled & BIT(endpoint->endpoint_id)))
@@ -1621,11 +1615,7 @@ void ipa_endpoint_resume_one(struct ipa_endpoint *endpoint)
 	if (!endpoint->toward_ipa)
 		(void)ipa_endpoint_program_suspend(endpoint, false);
 
-	/* Starting with IPA v4.0, the underlying GSI channel must be
-	 * restarted for resume.
-	 */
-	start_channel = endpoint->ipa->version >= IPA_VERSION_4_0;
-	ret = gsi_channel_resume(gsi, endpoint->channel_id, start_channel);
+	ret = gsi_channel_resume(gsi, endpoint->channel_id);
 	if (ret)
 		dev_err(dev, "error %d resuming channel %u\n", ret,
 			endpoint->channel_id);
-- 
2.27.0

[PATCH net-next 2/6] net: ipa: move version check for channel suspend/resume

[Alex Elder]

Change the Boolean flags passed to __gsi_channel_start() and
__gsi_channel_stop() so they represent whether the request is being
made to implement suspend (versus stop) or resume (versus start).

Then stop or start the channel for suspend/resume requests only if
the hardware version indicates it should be done.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index e143deddb7c09..5c5a2571d2faf 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -920,12 +920,13 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell)
 	/* All done! */
 }
 
-static int __gsi_channel_start(struct gsi_channel *channel, bool start)
+static int __gsi_channel_start(struct gsi_channel *channel, bool resume)
 {
 	struct gsi *gsi = channel->gsi;
 	int ret;
 
-	if (!start)
+	/* Prior to IPA v4.0 suspend/resume is not implemented by GSI */
+	if (resume && gsi->version < IPA_VERSION_4_0)
 		return 0;
 
 	mutex_lock(&gsi->mutex);
@@ -947,7 +948,7 @@ int gsi_channel_start(struct gsi *gsi, u32 channel_id)
 	napi_enable(&channel->napi);
 	gsi_irq_ieob_enable_one(gsi, channel->evt_ring_id);
 
-	ret = __gsi_channel_start(channel, true);
+	ret = __gsi_channel_start(channel, false);
 	if (ret) {
 		gsi_irq_ieob_disable_one(gsi, channel->evt_ring_id);
 		napi_disable(&channel->napi);
@@ -971,7 +972,7 @@ static int gsi_channel_stop_retry(struct gsi_channel *channel)
 	return ret;
 }
 
-static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
+static int __gsi_channel_stop(struct gsi_channel *channel, bool suspend)
 {
 	struct gsi *gsi = channel->gsi;
 	int ret;
@@ -979,7 +980,8 @@ static int __gsi_channel_stop(struct gsi_channel *channel, bool stop)
 	/* Wait for any underway transactions to complete before stopping. */
 	gsi_channel_trans_quiesce(channel);
 
-	if (!stop)
+	/* Prior to IPA v4.0 suspend/resume is not implemented by GSI */
+	if (suspend && gsi->version < IPA_VERSION_4_0)
 		return 0;
 
 	mutex_lock(&gsi->mutex);
@@ -997,7 +999,7 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 	int ret;
 
-	ret = __gsi_channel_stop(channel, true);
+	ret = __gsi_channel_stop(channel, false);
 	if (ret)
 		return ret;
 
@@ -1032,8 +1034,7 @@ int gsi_channel_suspend(struct gsi *gsi, u32 channel_id)
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 	int ret;
 
-	/* Prior to IPA v4.0 suspend/resume is not implemented by GSI */
-	ret = __gsi_channel_stop(channel, gsi->version >= IPA_VERSION_4_0);
+	ret = __gsi_channel_stop(channel, true);
 	if (ret)
 		return ret;
 
@@ -1048,8 +1049,7 @@ int gsi_channel_resume(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 
-	/* Prior to IPA v4.0 suspend/resume is not implemented by GSI */
-	return __gsi_channel_start(channel, gsi->version >= IPA_VERSION_4_0);
+	return __gsi_channel_start(channel, true);
 }
 
 /**
-- 
2.27.0

[PATCH net-next 3/6] net: ipa: move some GSI setup functions

[Alex Elder]

Move gsi_irq_setup() and gsi_ring_setup() so they're defined right
above gsi_setup() where they're called.  This is a trivial movement
of code to prepare for upcoming patches.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 5c5a2571d2faf..a5d23a2837cb6 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -198,77 +198,6 @@ static void gsi_irq_type_disable(struct gsi *gsi, enum gsi_irq_type_id type_id)
 	gsi_irq_type_update(gsi, gsi->type_enabled_bitmap & ~BIT(type_id));
 }
 
-/* Turn off all GSI interrupts initially; there is no gsi_irq_teardown() */
-static void gsi_irq_setup(struct gsi *gsi)
-{
-	/* Disable all interrupt types */
-	gsi_irq_type_update(gsi, 0);
-
-	/* Clear all type-specific interrupt masks */
-	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
-
-	/* The inter-EE interrupts are not supported for IPA v3.0-v3.1 */
-	if (gsi->version > IPA_VERSION_3_1) {
-		u32 offset;
-
-		/* These registers are in the non-adjusted address range */
-		offset = GSI_INTER_EE_SRC_CH_IRQ_MSK_OFFSET;
-		iowrite32(0, gsi->virt_raw + offset);
-		offset = GSI_INTER_EE_SRC_EV_CH_IRQ_MSK_OFFSET;
-		iowrite32(0, gsi->virt_raw + offset);
-	}
-
-	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
-}
-
-/* Get # supported channel and event rings; there is no gsi_ring_teardown() */
-static int gsi_ring_setup(struct gsi *gsi)
-{
-	struct device *dev = gsi->dev;
-	u32 count;
-	u32 val;
-
-	if (gsi->version < IPA_VERSION_3_5_1) {
-		/* No HW_PARAM_2 register prior to IPA v3.5.1, assume the max */
-		gsi->channel_count = GSI_CHANNEL_COUNT_MAX;
-		gsi->evt_ring_count = GSI_EVT_RING_COUNT_MAX;
-
-		return 0;
-	}
-
-	val = ioread32(gsi->virt + GSI_GSI_HW_PARAM_2_OFFSET);
-
-	count = u32_get_bits(val, NUM_CH_PER_EE_FMASK);
-	if (!count) {
-		dev_err(dev, "GSI reports zero channels supported\n");
-		return -EINVAL;
-	}
-	if (count > GSI_CHANNEL_COUNT_MAX) {
-		dev_warn(dev, "limiting to %u channels; hardware supports %u\n",
-			 GSI_CHANNEL_COUNT_MAX, count);
-		count = GSI_CHANNEL_COUNT_MAX;
-	}
-	gsi->channel_count = count;
-
-	count = u32_get_bits(val, NUM_EV_PER_EE_FMASK);
-	if (!count) {
-		dev_err(dev, "GSI reports zero event rings supported\n");
-		return -EINVAL;
-	}
-	if (count > GSI_EVT_RING_COUNT_MAX) {
-		dev_warn(dev,
-			 "limiting to %u event rings; hardware supports %u\n",
-			 GSI_EVT_RING_COUNT_MAX, count);
-		count = GSI_EVT_RING_COUNT_MAX;
-	}
-	gsi->evt_ring_count = count;
-
-	return 0;
-}
-
 /* Event ring commands are performed one at a time.  Their completion
  * is signaled by the event ring control GSI interrupt type, which is
  * only enabled when we issue an event ring command.  Only the event
@@ -1878,6 +1807,77 @@ static void gsi_channel_teardown(struct gsi *gsi)
 	gsi_irq_disable(gsi);
 }
 
+/* Turn off all GSI interrupts initially; there is no gsi_irq_teardown() */
+static void gsi_irq_setup(struct gsi *gsi)
+{
+	/* Disable all interrupt types */
+	gsi_irq_type_update(gsi, 0);
+
+	/* Clear all type-specific interrupt masks */
+	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
+	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
+	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
+	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
+
+	/* The inter-EE interrupts are not supported for IPA v3.0-v3.1 */
+	if (gsi->version > IPA_VERSION_3_1) {
+		u32 offset;
+
+		/* These registers are in the non-adjusted address range */
+		offset = GSI_INTER_EE_SRC_CH_IRQ_MSK_OFFSET;
+		iowrite32(0, gsi->virt_raw + offset);
+		offset = GSI_INTER_EE_SRC_EV_CH_IRQ_MSK_OFFSET;
+		iowrite32(0, gsi->virt_raw + offset);
+	}
+
+	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
+}
+
+/* Get # supported channel and event rings; there is no gsi_ring_teardown() */
+static int gsi_ring_setup(struct gsi *gsi)
+{
+	struct device *dev = gsi->dev;
+	u32 count;
+	u32 val;
+
+	if (gsi->version < IPA_VERSION_3_5_1) {
+		/* No HW_PARAM_2 register prior to IPA v3.5.1, assume the max */
+		gsi->channel_count = GSI_CHANNEL_COUNT_MAX;
+		gsi->evt_ring_count = GSI_EVT_RING_COUNT_MAX;
+
+		return 0;
+	}
+
+	val = ioread32(gsi->virt + GSI_GSI_HW_PARAM_2_OFFSET);
+
+	count = u32_get_bits(val, NUM_CH_PER_EE_FMASK);
+	if (!count) {
+		dev_err(dev, "GSI reports zero channels supported\n");
+		return -EINVAL;
+	}
+	if (count > GSI_CHANNEL_COUNT_MAX) {
+		dev_warn(dev, "limiting to %u channels; hardware supports %u\n",
+			 GSI_CHANNEL_COUNT_MAX, count);
+		count = GSI_CHANNEL_COUNT_MAX;
+	}
+	gsi->channel_count = count;
+
+	count = u32_get_bits(val, NUM_EV_PER_EE_FMASK);
+	if (!count) {
+		dev_err(dev, "GSI reports zero event rings supported\n");
+		return -EINVAL;
+	}
+	if (count > GSI_EVT_RING_COUNT_MAX) {
+		dev_warn(dev,
+			 "limiting to %u event rings; hardware supports %u\n",
+			 GSI_EVT_RING_COUNT_MAX, count);
+		count = GSI_EVT_RING_COUNT_MAX;
+	}
+	gsi->evt_ring_count = count;
+
+	return 0;
+}
+
 /* Setup function for GSI.  GSI firmware must be loaded and initialized */
 int gsi_setup(struct gsi *gsi)
 {
-- 
2.27.0

[PATCH net-next 4/6] net: ipa: have gsi_irq_setup() return an error code

[Alex Elder]

Change gsi_irq_setup() so it returns an error value, and introduce
gsi_irq_teardown() as its inverse.  Set the interrupt type (IRQ
rather than MSI) in gsi_irq_setup().

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index a5d23a2837cb6..be069d7c4feb9 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1807,9 +1807,12 @@ static void gsi_channel_teardown(struct gsi *gsi)
 	gsi_irq_disable(gsi);
 }
 
-/* Turn off all GSI interrupts initially; there is no gsi_irq_teardown() */
-static void gsi_irq_setup(struct gsi *gsi)
+/* Turn off all GSI interrupts initially */
+static int gsi_irq_setup(struct gsi *gsi)
 {
+	/* Writing 1 indicates IRQ interrupts; 0 would be MSI */
+	iowrite32(1, gsi->virt + GSI_CNTXT_INTSET_OFFSET);
+
 	/* Disable all interrupt types */
 	gsi_irq_type_update(gsi, 0);
 
@@ -1831,6 +1834,12 @@ static void gsi_irq_setup(struct gsi *gsi)
 	}
 
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
+
+	return 0;
+}
+
+static void gsi_irq_teardown(struct gsi *gsi)
+{
 }
 
 /* Get # supported channel and event rings; there is no gsi_ring_teardown() */
@@ -1891,25 +1900,34 @@ int gsi_setup(struct gsi *gsi)
 		return -EIO;
 	}
 
-	gsi_irq_setup(gsi);		/* No matching teardown required */
+	ret = gsi_irq_setup(gsi);
+	if (ret)
+		return ret;
 
 	ret = gsi_ring_setup(gsi);	/* No matching teardown required */
 	if (ret)
-		return ret;
+		goto err_irq_teardown;
 
 	/* Initialize the error log */
 	iowrite32(0, gsi->virt + GSI_ERROR_LOG_OFFSET);
 
-	/* Writing 1 indicates IRQ interrupts; 0 would be MSI */
-	iowrite32(1, gsi->virt + GSI_CNTXT_INTSET_OFFSET);
+	ret = gsi_channel_setup(gsi);
+	if (ret)
+		goto err_irq_teardown;
 
-	return gsi_channel_setup(gsi);
+	return 0;
+
+err_irq_teardown:
+	gsi_irq_teardown(gsi);
+
+	return ret;
 }
 
 /* Inverse of gsi_setup() */
 void gsi_teardown(struct gsi *gsi)
 {
 	gsi_channel_teardown(gsi);
+	gsi_irq_teardown(gsi);
 }
 
 /* Initialize a channel's event ring */
-- 
2.27.0

[PATCH net-next 5/6] net: ipa: move gsi_irq_init() code into setup

[Alex Elder]

The GSI IRQ handler could be triggered as soon as it is registered
with request_irq().  The handler function, gsi_isr(), touches
hardware, meaning the IPA clock must be operational.  The IPA clock
is not operating when the handler is registered (in gsi_irq_init()),
so this is a problem.

Move the call to request_irq() for the GSI interrupt handler into
gsi_irq_setup(), which is called when the IPA clock is known to be
operational (and furthermore, the GSI firmware will have been
loaded).  Request the IRQ at the end of that function, after all
interrupt types have been disabled and masked.

Move the matching free_irq() call into gsi_irq_teardown(), and get
rid of the now empty gsi_irq_exit(),

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index be069d7c4feb9..c555ccd778bb8 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1303,33 +1303,20 @@ static irqreturn_t gsi_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+/* Init function for GSI IRQ lookup; there is no gsi_irq_exit() */
 static int gsi_irq_init(struct gsi *gsi, struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
-	unsigned int irq;
 	int ret;
 
 	ret = platform_get_irq_byname(pdev, "gsi");
 	if (ret <= 0)
 		return ret ? : -EINVAL;
 
-	irq = ret;
-
-	ret = request_irq(irq, gsi_isr, 0, "gsi", gsi);
-	if (ret) {
-		dev_err(dev, "error %d requesting \"gsi\" IRQ\n", ret);
-		return ret;
-	}
-	gsi->irq = irq;
+	gsi->irq = ret;
 
 	return 0;
 }
 
-static void gsi_irq_exit(struct gsi *gsi)
-{
-	free_irq(gsi->irq, gsi);
-}
-
 /* Return the transaction associated with a transfer completion event */
 static struct gsi_trans *gsi_event_trans(struct gsi_channel *channel,
 					 struct gsi_event *event)
@@ -1810,6 +1797,8 @@ static void gsi_channel_teardown(struct gsi *gsi)
 /* Turn off all GSI interrupts initially */
 static int gsi_irq_setup(struct gsi *gsi)
 {
+	int ret;
+
 	/* Writing 1 indicates IRQ interrupts; 0 would be MSI */
 	iowrite32(1, gsi->virt + GSI_CNTXT_INTSET_OFFSET);
 
@@ -1835,11 +1824,16 @@ static int gsi_irq_setup(struct gsi *gsi)
 
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
 
-	return 0;
+	ret = request_irq(gsi->irq, gsi_isr, 0, "gsi", gsi);
+	if (ret)
+		dev_err(gsi->dev, "error %d requesting \"gsi\" IRQ\n", ret);
+
+	return ret;
 }
 
 static void gsi_irq_teardown(struct gsi *gsi)
 {
+	free_irq(gsi->irq, gsi);
 }
 
 /* Get # supported channel and event rings; there is no gsi_ring_teardown() */
@@ -2224,20 +2218,18 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
 
 	init_completion(&gsi->completion);
 
-	ret = gsi_irq_init(gsi, pdev);
+	ret = gsi_irq_init(gsi, pdev);	/* No matching exit required */
 	if (ret)
 		goto err_iounmap;
 
 	ret = gsi_channel_init(gsi, count, data);
 	if (ret)
-		goto err_irq_exit;
+		goto err_iounmap;
 
 	mutex_init(&gsi->mutex);
 
 	return 0;
 
-err_irq_exit:
-	gsi_irq_exit(gsi);
 err_iounmap:
 	iounmap(gsi->virt_raw);
 
@@ -2249,7 +2241,6 @@ void gsi_exit(struct gsi *gsi)
 {
 	mutex_destroy(&gsi->mutex);
 	gsi_channel_exit(gsi);
-	gsi_irq_exit(gsi);
 	iounmap(gsi->virt_raw);
 }
 
-- 
2.27.0

[PATCH net-next 6/6] net: ipa: disable GSI interrupts while suspended

[Alex Elder]

Introduce new functions gsi_suspend() and gsi_resume(), which will
disable the GSI interrupt handler after all endpoints are suspended
and re-enable it before endpoints are resumed.  This will ensure no
GSI interrupt handler will fire when the hardware is suspended.

Here's a little further explanation.  There are seven GSI interrupt
types, and most are disabled except when needed.
  - These two are not used (never enabled):
      GSI_INTER_EE_CH_CTRL
      GSI_INTER_EE_EV_CTRL
  - These two are only used to implement channel and event ring
    commands, and are only enabled while a command is underway:
      GSI_CH_CTRL
      GSI_EV_CTRL
  - The IEOB interrupt signals I/O completion.  It will not fire
    when a channel is stopped (or "suspended").
      GSI_IEOB
  - This interrupt is used to allocate or halt modem channels,
    and is only enabled while such a command is underway.
      GSI_GLOB_EE
    However it also is used to signal certain errors, and this could
    occur at any time.
  - The general interrupt signals general errors, and could occur at
    any time.
      GSI_GENERAL

The purpose for this change is to ensure no global or general
interrupts fire due to errors while the hardware is suspended.
We enable the clock on resume, and at that time we can "handle"
(at least report) these error conditions.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c      | 12 ++++++++++++
 drivers/net/ipa/gsi.h      | 12 ++++++++++++
 drivers/net/ipa/ipa_main.c |  5 ++++-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index c555ccd778bb8..a2fcdb1abdb96 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -981,6 +981,18 @@ int gsi_channel_resume(struct gsi *gsi, u32 channel_id)
 	return __gsi_channel_start(channel, true);
 }
 
+/* Prevent all GSI interrupts while suspended */
+void gsi_suspend(struct gsi *gsi)
+{
+	disable_irq(gsi->irq);
+}
+
+/* Allow all GSI interrupts again when resuming */
+void gsi_resume(struct gsi *gsi)
+{
+	enable_irq(gsi->irq);
+}
+
 /**
  * gsi_channel_tx_queued() - Report queued TX transfers for a channel
  * @channel:	Channel for which to report
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 97163b58b4ebc..88b80dc3db79f 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -232,6 +232,18 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id);
  */
 void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell);
 
+/**
+ * gsi_suspend() - Prepare the GSI subsystem for suspend
+ * @gsi:	GSI pointer
+ */
+void gsi_suspend(struct gsi *gsi);
+
+/**
+ * gsi_resume() - Resume the GSI subsystem following suspend
+ * @gsi:	GSI pointer
+ */
+void gsi_resume(struct gsi *gsi);
+
 /**
  * gsi_channel_suspend() - Suspend a GSI channel
  * @gsi:	GSI pointer
diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
index 2e728d4914c82..ae51109dea01b 100644
--- a/drivers/net/ipa/ipa_main.c
+++ b/drivers/net/ipa/ipa_main.c
@@ -892,6 +892,7 @@ static int ipa_suspend(struct device *dev)
 	if (ipa->setup_complete) {
 		__clear_bit(IPA_FLAG_RESUMED, ipa->flags);
 		ipa_endpoint_suspend(ipa);
+		gsi_suspend(&ipa->gsi);
 	}
 
 	ipa_clock_put(ipa);
@@ -919,8 +920,10 @@ static int ipa_resume(struct device *dev)
 	ipa_clock_get(ipa);
 
 	/* Endpoints aren't usable until setup is complete */
-	if (ipa->setup_complete)
+	if (ipa->setup_complete) {
+		gsi_resume(&ipa->gsi);
 		ipa_endpoint_resume(ipa);
+	}
 
 	return 0;
 }
-- 
2.27.0

Re: [PATCH net-next 0/6] net: ipa: prepare GSI interrupts for runtime PM

[patchwork-bot+netdevbpf]

Hello:

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

On Tue,  3 Aug 2021 09:00:57 -0500 you wrote:
> The last patch in this series arranges for GSI interrupts to be
> disabled when the IPA hardware is suspended.  This ensures the clock
> is always operational when a GSI interrupt fires.  Leading up to
> that are patches that rearrange the code a bit to allow this to
> be done.
> 
> The first two patches aren't *directly* related.  They remove some
> flag arguments to some GSI suspend/resume related functions, using
> the version field now present in the GSI structure.
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] net: ipa: use gsi->version for channel suspend/resume
    https://git.kernel.org/netdev/net-next/c/decfef0fa6b2
  - [net-next,2/6] net: ipa: move version check for channel suspend/resume
    https://git.kernel.org/netdev/net-next/c/4a4ba483e4a5
  - [net-next,3/6] net: ipa: move some GSI setup functions
    https://git.kernel.org/netdev/net-next/c/a7860a5f898c
  - [net-next,4/6] net: ipa: have gsi_irq_setup() return an error code
    https://git.kernel.org/netdev/net-next/c/1657d8a45823
  - [net-next,5/6] net: ipa: move gsi_irq_init() code into setup
    https://git.kernel.org/netdev/net-next/c/b176f95b5728
  - [net-next,6/6] net: ipa: disable GSI interrupts while suspended
    https://git.kernel.org/netdev/net-next/c/45a42a3c50b5

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