[PATCH net-next 00/13] net: ipa: constrain GSI interrupts

[PATCH net-next 00/13] net: ipa: constrain GSI interrupts

[Alex Elder]

The goal of this series is to more tightly control when GSI
interrupts are enabled.  This is a long-ish series, so I'll
describe it in parts.

The first patch is actually unrelated...  I forgot to include
it in my previous series (which exposed the GSI layer to the
IPA version).  It is a trivial comments-only update patch.

The second patch defers registering the GSI interrupt handler
until *after* all of the resources that handler touches have
been initialized.  In practice, we don't see this interrupt
that early, but this precludes an obvious problem.

The next two patches are simple changes.  The first just
trivially renames a field.  The second switches from using
constant mask values to using an enumerated type of bit
positions to represent each GSI interrupt type.

The rest implement the "real work."  First, all interrupts
are disabled at initialization time.  Next, we keep track of
a bitmask of enabled GSI interrupt types, updating it each
time we enable or disable one of them.  From there we have
a set of patches that one-by-one enable each interrupt type
only during the period it is required.  This includes allowing
a channel to generate IEOB interrupts only when it has been
enabled.  And finally, the last patch simplifies some code
now that all GSI interrupt types are handled uniformly.

					-Alex

Alex Elder (13):
  net: ipa: refer to IPA versions, not GSI

  net: ipa: request GSI IRQ later

  net: ipa: rename gsi->event_enable_bitmap
  net: ipa: define GSI interrupt types with an enum

  net: ipa: disable all GSI interrupt types initially
  net: ipa: cache last-saved GSI IRQ enabled type
  net: ipa: only enable GSI channel control IRQs when needed
  net: ipa: only enable GSI event control IRQs when needed
  net: ipa: only enable generic command completion IRQ when needed
  net: ipa: only enable GSI IEOB IRQs when needed
  net: ipa: explicitly disallow inter-EE interrupts
  net: ipa: only enable GSI general IRQs when needed
  net: ipa: pass a value to gsi_irq_type_update()

 drivers/net/ipa/gsi.c     | 257 +++++++++++++++++++++++++++-----------
 drivers/net/ipa/gsi.h     |   7 +-
 drivers/net/ipa/gsi_reg.h |  31 +++--
 3 files changed, 205 insertions(+), 90 deletions(-)

-- 
2.20.1

[PATCH net-next 01/13] net: ipa: refer to IPA versions, not GSI

[Alex Elder]

The GSI code is now exposed to IPA version numbers, and we handle
version-specific behavior based on the IPA version.

Modify some comments that talk about GSI versions so they reference
IPA versions instead.  Correct version number errors in a couple of
these comments.

The (comment) mapping between IPA and GSI versions in the definition
of the ipa_version enumerated type remains.

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

diff --git a/drivers/net/ipa/gsi_reg.h b/drivers/net/ipa/gsi_reg.h
index 8e0e9350c3831..9668797aa58ef 100644
--- a/drivers/net/ipa/gsi_reg.h
+++ b/drivers/net/ipa/gsi_reg.h
@@ -66,7 +66,7 @@
 #define CHTYPE_DIR_FMASK		GENMASK(3, 3)
 #define EE_FMASK			GENMASK(7, 4)
 #define CHID_FMASK			GENMASK(12, 8)
-/* The next field is present for GSI v2.0 and above */
+/* The next field is present for IPA v4.5 and above */
 #define CHTYPE_PROTOCOL_MSB_FMASK	GENMASK(13, 13)
 #define ERINDEX_FMASK			GENMASK(18, 14)
 #define CHSTATE_FMASK			GENMASK(23, 20)
@@ -95,7 +95,7 @@
 #define WRR_WEIGHT_FMASK		GENMASK(3, 0)
 #define MAX_PREFETCH_FMASK		GENMASK(8, 8)
 #define USE_DB_ENG_FMASK		GENMASK(9, 9)
-/* The next field is present for GSI v2.0 and above */
+/* The next field is only present for IPA v4.0, v4.1, and v4.2 */
 #define USE_ESCAPE_BUF_ONLY_FMASK	GENMASK(10, 10)
 
 #define GSI_CH_C_SCRATCH_0_OFFSET(ch) \
@@ -238,19 +238,19 @@
 #define IRAM_SIZE_FMASK			GENMASK(2, 0)
 #define IRAM_SIZE_ONE_KB_FVAL			0
 #define IRAM_SIZE_TWO_KB_FVAL			1
-/* The next two values are available for GSI v2.0 and above */
+/* The next two values are available for IPA v4.0 and above */
 #define IRAM_SIZE_TWO_N_HALF_KB_FVAL		2
 #define IRAM_SIZE_THREE_KB_FVAL			3
 #define NUM_CH_PER_EE_FMASK		GENMASK(7, 3)
 #define NUM_EV_PER_EE_FMASK		GENMASK(12, 8)
 #define GSI_CH_PEND_TRANSLATE_FMASK	GENMASK(13, 13)
 #define GSI_CH_FULL_LOGIC_FMASK		GENMASK(14, 14)
-/* Fields below are present for GSI v2.0 and above */
+/* Fields below are present for IPA v4.0 and above */
 #define GSI_USE_SDMA_FMASK		GENMASK(15, 15)
 #define GSI_SDMA_N_INT_FMASK		GENMASK(18, 16)
 #define GSI_SDMA_MAX_BURST_FMASK	GENMASK(26, 19)
 #define GSI_SDMA_N_IOVEC_FMASK		GENMASK(29, 27)
-/* Fields below are present for GSI v2.2 and above */
+/* Fields below are present for IPA v4.2 and above */
 #define GSI_USE_RD_WR_ENG_FMASK		GENMASK(30, 30)
 #define GSI_USE_INTER_EE_FMASK		GENMASK(31, 31)
 
-- 
2.20.1

[PATCH net-next 02/13] net: ipa: request GSI IRQ later

[Alex Elder]

Introduce gsi_irq_init() and gsi_irq_exit(), to encapsulate looking
up the GSI IRQ and registering its handler.  Call gsi_irq_init() a
little later in gsi_init(), and initialize the completion earlier.
The IRQ handler accesses both the GSI virtual memory pointer and the
completion, and this way these things will have been initialized
before the gsi_irq() can ever be called.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 12a2001ee1e9c..299791f9b94d0 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1170,6 +1170,34 @@ static irqreturn_t gsi_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+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) {
+		dev_err(dev, "DT error %d getting \"gsi\" IRQ property\n", ret);
+		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;
+
+	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)
@@ -1962,7 +1990,6 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	resource_size_t size;
-	unsigned int irq;
 	int ret;
 
 	gsi_validate_build();
@@ -1976,55 +2003,43 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
 	 */
 	init_dummy_netdev(&gsi->dummy_dev);
 
-	ret = platform_get_irq_byname(pdev, "gsi");
-	if (ret <= 0) {
-		dev_err(dev, "DT error %d getting \"gsi\" IRQ property\n", ret);
-		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;
-
 	/* Get GSI memory range and map it */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gsi");
 	if (!res) {
 		dev_err(dev, "DT error getting \"gsi\" memory property\n");
-		ret = -ENODEV;
-		goto err_free_irq;
+		return -ENODEV;
 	}
 
 	size = resource_size(res);
 	if (res->start > U32_MAX || size > U32_MAX - res->start) {
 		dev_err(dev, "DT memory resource \"gsi\" out of range\n");
-		ret = -EINVAL;
-		goto err_free_irq;
+		return -EINVAL;
 	}
 
 	gsi->virt = ioremap(res->start, size);
 	if (!gsi->virt) {
 		dev_err(dev, "unable to remap \"gsi\" memory\n");
-		ret = -ENOMEM;
-		goto err_free_irq;
+		return -ENOMEM;
 	}
 
-	ret = gsi_channel_init(gsi, count, data);
+	init_completion(&gsi->completion);
+
+	ret = gsi_irq_init(gsi, pdev);
 	if (ret)
 		goto err_iounmap;
 
+	ret = gsi_channel_init(gsi, count, data);
+	if (ret)
+		goto err_irq_exit;
+
 	mutex_init(&gsi->mutex);
-	init_completion(&gsi->completion);
 
 	return 0;
 
+err_irq_exit:
+	gsi_irq_exit(gsi);
 err_iounmap:
 	iounmap(gsi->virt);
-err_free_irq:
-	free_irq(gsi->irq, gsi);
 
 	return ret;
 }
@@ -2034,7 +2049,7 @@ void gsi_exit(struct gsi *gsi)
 {
 	mutex_destroy(&gsi->mutex);
 	gsi_channel_exit(gsi);
-	free_irq(gsi->irq, gsi);
+	gsi_irq_exit(gsi);
 	iounmap(gsi->virt);
 }
 
-- 
2.20.1

[PATCH net-next 03/13] net: ipa: rename gsi->event_enable_bitmap

[Alex Elder]

Rename the "event_enable_bitmap" field of the GSI structure to be
"ieob_enabled_bitmap".  An upcoming patch will cache the last value
stored for another interrupt mask and this is a more direct naming
convention to follow.

Add a few comments to explain the bitmap fields in the GSI structure.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 299791f9b94d0..ea1126a827a1c 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -234,8 +234,8 @@ static void gsi_irq_ieob_enable(struct gsi *gsi, u32 evt_ring_id)
 {
 	u32 val;
 
-	gsi->event_enable_bitmap |= BIT(evt_ring_id);
-	val = gsi->event_enable_bitmap;
+	gsi->ieob_enabled_bitmap |= BIT(evt_ring_id);
+	val = gsi->ieob_enabled_bitmap;
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 }
 
@@ -243,8 +243,8 @@ static void gsi_irq_ieob_disable(struct gsi *gsi, u32 evt_ring_id)
 {
 	u32 val;
 
-	gsi->event_enable_bitmap &= ~BIT(evt_ring_id);
-	val = gsi->event_enable_bitmap;
+	gsi->ieob_enabled_bitmap &= ~BIT(evt_ring_id);
+	val = gsi->ieob_enabled_bitmap;
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 }
 
@@ -1774,7 +1774,7 @@ static void gsi_evt_ring_init(struct gsi *gsi)
 	u32 evt_ring_id = 0;
 
 	gsi->event_bitmap = gsi_event_bitmap_init(GSI_EVT_RING_COUNT_MAX);
-	gsi->event_enable_bitmap = 0;
+	gsi->ieob_enabled_bitmap = 0;
 	do
 		init_completion(&gsi->evt_ring[evt_ring_id].completion);
 	while (++evt_ring_id < GSI_EVT_RING_COUNT_MAX);
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 59ace83d404c4..fa7e2d35c19cb 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -156,9 +156,9 @@ struct gsi {
 	u32 evt_ring_count;
 	struct gsi_channel channel[GSI_CHANNEL_COUNT_MAX];
 	struct gsi_evt_ring evt_ring[GSI_EVT_RING_COUNT_MAX];
-	u32 event_bitmap;
-	u32 event_enable_bitmap;
-	u32 modem_channel_bitmap;
+	u32 event_bitmap;		/* allocated event rings */
+	u32 modem_channel_bitmap;	/* modem channels to allocate */
+	u32 ieob_enabled_bitmap;	/* IEOB IRQ enabled (event rings) */
 	struct completion completion;	/* for global EE commands */
 	struct mutex mutex;		/* protects commands, programming */
 };
-- 
2.20.1

[PATCH net-next 04/13] net: ipa: define GSI interrupt types with an enum

[Alex Elder]

Define the GSI interrupt types with an enumerated type whose values
are the bit positions representing each interrupt type.  Include a
short comment describing how each interrupt type is used.

Build up the enabled interrupt mask explicitly in gsi_irq_enable(),
and get rid of the definition of GSI_CNTXT_TYPE_IRQ_MSK_ALL.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c     | 21 ++++++++++++---------
 drivers/net/ipa/gsi_reg.h | 19 ++++++++++---------
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index ea1126a827a1c..da5204268df29 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -253,10 +253,12 @@ static void gsi_irq_enable(struct gsi *gsi)
 {
 	u32 val;
 
-	/* We don't use inter-EE channel or event interrupts */
-	val = GSI_CNTXT_TYPE_IRQ_MSK_ALL;
-	val &= ~INTER_EE_CH_CTRL_FMASK;
-	val &= ~INTER_EE_EV_CTRL_FMASK;
+	val = BIT(GSI_CH_CTRL);
+	val |= BIT(GSI_EV_CTRL);
+	val |= BIT(GSI_GLOB_EE);
+	val |= BIT(GSI_IEOB);
+	/* We don't use inter-EE channel or event control interrupts */
+	val |= BIT(GSI_GENERAL);
 	iowrite32(val, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
 
 	val = GENMASK(gsi->channel_count - 1, 0);
@@ -1130,6 +1132,7 @@ static irqreturn_t gsi_isr(int irq, void *dev_id)
 	u32 intr_mask;
 	u32 cnt = 0;
 
+	/* enum gsi_irq_type_id defines GSI interrupt types */
 	while ((intr_mask = ioread32(gsi->virt + GSI_CNTXT_TYPE_IRQ_OFFSET))) {
 		/* intr_mask contains bitmask of pending GSI interrupts */
 		do {
@@ -1138,19 +1141,19 @@ static irqreturn_t gsi_isr(int irq, void *dev_id)
 			intr_mask ^= gsi_intr;
 
 			switch (gsi_intr) {
-			case CH_CTRL_FMASK:
+			case BIT(GSI_CH_CTRL):
 				gsi_isr_chan_ctrl(gsi);
 				break;
-			case EV_CTRL_FMASK:
+			case BIT(GSI_EV_CTRL):
 				gsi_isr_evt_ctrl(gsi);
 				break;
-			case GLOB_EE_FMASK:
+			case BIT(GSI_GLOB_EE):
 				gsi_isr_glob_ee(gsi);
 				break;
-			case IEOB_FMASK:
+			case BIT(GSI_IEOB):
 				gsi_isr_ieob(gsi);
 				break;
-			case GENERAL_FMASK:
+			case BIT(GSI_GENERAL):
 				gsi_isr_general(gsi);
 				break;
 			default:
diff --git a/drivers/net/ipa/gsi_reg.h b/drivers/net/ipa/gsi_reg.h
index 9668797aa58ef..1dd81cf0b46a8 100644
--- a/drivers/net/ipa/gsi_reg.h
+++ b/drivers/net/ipa/gsi_reg.h
@@ -262,15 +262,16 @@
 			GSI_EE_N_CNTXT_TYPE_IRQ_MSK_OFFSET(GSI_EE_AP)
 #define GSI_EE_N_CNTXT_TYPE_IRQ_MSK_OFFSET(ee) \
 			(0x0001f088 + 0x4000 * (ee))
-/* The masks below are used for the TYPE_IRQ and TYPE_IRQ_MASK registers */
-#define CH_CTRL_FMASK			GENMASK(0, 0)
-#define EV_CTRL_FMASK			GENMASK(1, 1)
-#define GLOB_EE_FMASK			GENMASK(2, 2)
-#define IEOB_FMASK			GENMASK(3, 3)
-#define INTER_EE_CH_CTRL_FMASK		GENMASK(4, 4)
-#define INTER_EE_EV_CTRL_FMASK		GENMASK(5, 5)
-#define GENERAL_FMASK			GENMASK(6, 6)
-#define GSI_CNTXT_TYPE_IRQ_MSK_ALL	GENMASK(6, 0)
+/* Values here are bit positions in the TYPE_IRQ and TYPE_IRQ_MSK registers */
+enum gsi_irq_type_id {
+	GSI_CH_CTRL		= 0,	/* channel allocation, etc.  */
+	GSI_EV_CTRL		= 1,	/* event ring allocation, etc. */
+	GSI_GLOB_EE		= 2,	/* global/general event */
+	GSI_IEOB		= 3,	/* TRE completion */
+	GSI_INTER_EE_CH_CTRL	= 4,	/* remote-issued stop/reset (unused) */
+	GSI_INTER_EE_EV_CTRL	= 5,	/* remote-issued event reset (unused) */
+	GSI_GENERAL		= 6,	/* general-purpose event */
+};
 
 #define GSI_CNTXT_SRC_CH_IRQ_OFFSET \
 			GSI_EE_N_CNTXT_SRC_CH_IRQ_OFFSET(GSI_EE_AP)
-- 
2.20.1

[PATCH net-next 05/13] net: ipa: disable all GSI interrupt types initially

[Alex Elder]

Introduce gsi_irq_setup() and gsi_irq_teardown() to disable all
GSI interrupts when first setting up GSI hardware, and to clean
things up when we're done.

Re-enable all GSI interrupt types in gsi_irq_enable(), but do
so only after each of the type-specific interrupt masks has
been configured.  Similarly, disable all interrupt types in
gsi_irq_disable()--first--before zeroing out the type-specific
masks.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index da5204268df29..669d7496f8bdb 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -230,6 +230,18 @@ static u32 gsi_channel_id(struct gsi_channel *channel)
 	return channel - &channel->gsi->channel[0];
 }
 
+/* Turn off all GSI interrupts initially */
+static void gsi_irq_setup(struct gsi *gsi)
+{
+	iowrite32(0, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+}
+
+/* Turn off all GSI interrupts when we're all done */
+static void gsi_irq_teardown(struct gsi *gsi)
+{
+	iowrite32(0, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+}
+
 static void gsi_irq_ieob_enable(struct gsi *gsi, u32 evt_ring_id)
 {
 	u32 val;
@@ -253,14 +265,6 @@ static void gsi_irq_enable(struct gsi *gsi)
 {
 	u32 val;
 
-	val = BIT(GSI_CH_CTRL);
-	val |= BIT(GSI_EV_CTRL);
-	val |= BIT(GSI_GLOB_EE);
-	val |= BIT(GSI_IEOB);
-	/* We don't use inter-EE channel or event control interrupts */
-	val |= BIT(GSI_GENERAL);
-	iowrite32(val, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
-
 	val = GENMASK(gsi->channel_count - 1, 0);
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
 
@@ -276,17 +280,27 @@ static void gsi_irq_enable(struct gsi *gsi)
 	/* Never enable GSI_BREAK_POINT */
 	val = GSI_CNTXT_GSI_IRQ_ALL & ~BREAK_POINT_FMASK;
 	iowrite32(val, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
+
+	/* Finally enable the interrupt types we use */
+	val = BIT(GSI_CH_CTRL);
+	val |= BIT(GSI_EV_CTRL);
+	val |= BIT(GSI_GLOB_EE);
+	val |= BIT(GSI_IEOB);
+	/* We don't use inter-EE channel or event interrupts */
+	val |= BIT(GSI_GENERAL);
+	iowrite32(val, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
 }
 
 /* Disable all GSI_interrupt types */
 static void gsi_irq_disable(struct gsi *gsi)
 {
+	iowrite32(0, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
 }
 
 /* Return the virtual address associated with a ring index */
@@ -1683,6 +1697,7 @@ int gsi_setup(struct gsi *gsi)
 {
 	struct device *dev = gsi->dev;
 	u32 val;
+	int ret;
 
 	/* Here is where we first touch the GSI hardware */
 	val = ioread32(gsi->virt + GSI_GSI_STATUS_OFFSET);
@@ -1691,6 +1706,8 @@ int gsi_setup(struct gsi *gsi)
 		return -EIO;
 	}
 
+	gsi_irq_setup(gsi);
+
 	val = ioread32(gsi->virt + GSI_GSI_HW_PARAM_2_OFFSET);
 
 	gsi->channel_count = u32_get_bits(val, NUM_CH_PER_EE_FMASK);
@@ -1723,13 +1740,18 @@ int gsi_setup(struct gsi *gsi)
 	/* Writing 1 indicates IRQ interrupts; 0 would be MSI */
 	iowrite32(1, gsi->virt + GSI_CNTXT_INTSET_OFFSET);
 
-	return gsi_channel_setup(gsi);
+	ret = gsi_channel_setup(gsi);
+	if (ret)
+		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.20.1

[PATCH net-next 06/13] net: ipa: cache last-saved GSI IRQ enabled type

[Alex Elder]

Keep track of the set of GSI interrupt types that are currently
enabled by recording the mask value to write (or last written) to
the TYPE_IRQ_MSK register.

Create a new helper function gsi_irq_type_update() to handle
actually writing the register.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c | 35 +++++++++++++++++++++++------------
 drivers/net/ipa/gsi.h |  1 +
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 669d7496f8bdb..f76b5a1e1f8d5 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -230,16 +230,25 @@ static u32 gsi_channel_id(struct gsi_channel *channel)
 	return channel - &channel->gsi->channel[0];
 }
 
+/* Update the GSI IRQ type register with the cached value */
+static void gsi_irq_type_update(struct gsi *gsi)
+{
+	iowrite32(gsi->type_enabled_bitmap,
+		  gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+}
+
 /* Turn off all GSI interrupts initially */
 static void gsi_irq_setup(struct gsi *gsi)
 {
-	iowrite32(0, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+	gsi->type_enabled_bitmap = 0;
+	gsi_irq_type_update(gsi);
 }
 
 /* Turn off all GSI interrupts when we're all done */
 static void gsi_irq_teardown(struct gsi *gsi)
 {
-	iowrite32(0, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+	gsi->type_enabled_bitmap = 0;
+	gsi_irq_type_update(gsi);
 }
 
 static void gsi_irq_ieob_enable(struct gsi *gsi, u32 evt_ring_id)
@@ -267,34 +276,36 @@ static void gsi_irq_enable(struct gsi *gsi)
 
 	val = GENMASK(gsi->channel_count - 1, 0);
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
+	gsi->type_enabled_bitmap |= BIT(GSI_CH_CTRL);
 
 	val = GENMASK(gsi->evt_ring_count - 1, 0);
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
+	gsi->type_enabled_bitmap |= BIT(GSI_EV_CTRL);
 
 	/* Each IEOB interrupt is enabled (later) as needed by channels */
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
+	gsi->type_enabled_bitmap |= BIT(GSI_IEOB);
 
 	val = GSI_CNTXT_GLOB_IRQ_ALL;
 	iowrite32(val, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
+	gsi->type_enabled_bitmap |= BIT(GSI_GLOB_EE);
+
+	/* We don't use inter-EE channel or event interrupts */
 
 	/* Never enable GSI_BREAK_POINT */
 	val = GSI_CNTXT_GSI_IRQ_ALL & ~BREAK_POINT_FMASK;
 	iowrite32(val, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
+	gsi->type_enabled_bitmap |= BIT(GSI_GENERAL);
 
-	/* Finally enable the interrupt types we use */
-	val = BIT(GSI_CH_CTRL);
-	val |= BIT(GSI_EV_CTRL);
-	val |= BIT(GSI_GLOB_EE);
-	val |= BIT(GSI_IEOB);
-	/* We don't use inter-EE channel or event interrupts */
-	val |= BIT(GSI_GENERAL);
-	iowrite32(val, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+	/* Finally update the interrupt types we want enabled */
+	gsi_irq_type_update(gsi);
 }
 
-/* Disable all GSI_interrupt types */
+/* Disable all GSI interrupt types */
 static void gsi_irq_disable(struct gsi *gsi)
 {
-	iowrite32(0, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+	gsi->type_enabled_bitmap = 0;
+	gsi_irq_type_update(gsi);
 
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index fa7e2d35c19cb..758125737c8e9 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -158,6 +158,7 @@ struct gsi {
 	struct gsi_evt_ring evt_ring[GSI_EVT_RING_COUNT_MAX];
 	u32 event_bitmap;		/* allocated event rings */
 	u32 modem_channel_bitmap;	/* modem channels to allocate */
+	u32 type_enabled_bitmap;	/* GSI IRQ types enabled */
 	u32 ieob_enabled_bitmap;	/* IEOB IRQ enabled (event rings) */
 	struct completion completion;	/* for global EE commands */
 	struct mutex mutex;		/* protects commands, programming */
-- 
2.20.1

[PATCH net-next 07/13] net: ipa: only enable GSI channel control IRQs when needed

[Alex Elder]

A GSI channel causes a channel control interrupt to fire whenever
its state changes (between NOT_ALLOCATED, ALLOCATED, STARTED, etc.).
We do not support inter-EE channel commands (initiated by other EEs),
so no channel should ever change state except when we request it to.

Currently, we permit *all* channels to generate channel control
interrupts--even those that are never used.  And we enable channel
control interrupts essentially at all times, from setup to teardown.

Instead, disable all channel control interrupts initially in
gsi_irq_setup(), and only enable the channel control interrupt
type for the duration of a channel command.  When doing so, only
allow the channel being operated upon to cause the interrupt to
fire.

Because a channel's interrupt is now enabled only when needed (one
channel at a time), there is no longer any need to zero the channel
mask in gsi_irq_disable().

Add new gsi_irq_type_enable() and gsi_irq_type_disable() as helper
functions to control whether a given GSI interrupt type is enabled.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index f76b5a1e1f8d5..4fc72dfe1e9b0 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -237,11 +237,25 @@ static void gsi_irq_type_update(struct gsi *gsi)
 		  gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
 }
 
+static void gsi_irq_type_enable(struct gsi *gsi, enum gsi_irq_type_id type_id)
+{
+	gsi->type_enabled_bitmap |= BIT(type_id);
+	gsi_irq_type_update(gsi);
+}
+
+static void gsi_irq_type_disable(struct gsi *gsi, enum gsi_irq_type_id type_id)
+{
+	gsi->type_enabled_bitmap &= ~BIT(type_id);
+	gsi_irq_type_update(gsi);
+}
+
 /* Turn off all GSI interrupts initially */
 static void gsi_irq_setup(struct gsi *gsi)
 {
 	gsi->type_enabled_bitmap = 0;
 	gsi_irq_type_update(gsi);
+
+	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
 }
 
 /* Turn off all GSI interrupts when we're all done */
@@ -274,10 +288,6 @@ static void gsi_irq_enable(struct gsi *gsi)
 {
 	u32 val;
 
-	val = GENMASK(gsi->channel_count - 1, 0);
-	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
-	gsi->type_enabled_bitmap |= BIT(GSI_CH_CTRL);
-
 	val = GENMASK(gsi->evt_ring_count - 1, 0);
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
 	gsi->type_enabled_bitmap |= BIT(GSI_EV_CTRL);
@@ -311,7 +321,6 @@ static void gsi_irq_disable(struct gsi *gsi)
 	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
 }
 
 /* Return the virtual address associated with a ring index */
@@ -461,13 +470,29 @@ gsi_channel_command(struct gsi_channel *channel, enum gsi_ch_cmd_opcode opcode)
 	u32 channel_id = gsi_channel_id(channel);
 	struct gsi *gsi = channel->gsi;
 	struct device *dev = gsi->dev;
+	bool success;
 	u32 val;
 
+	/* We only perform one channel command at a time, and channel
+	 * control interrupts should only occur when such a command is
+	 * issued here.  So we only permit *this* channel to trigger
+	 * an interrupt and only enable the channel control IRQ type
+	 * when we expect it to occur.
+	 */
+	val = BIT(channel_id);
+	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
+	gsi_irq_type_enable(gsi, GSI_CH_CTRL);
+
 	val = u32_encode_bits(channel_id, CH_CHID_FMASK);
 	val |= u32_encode_bits(opcode, CH_OPCODE_FMASK);
+	success = gsi_command(gsi, GSI_CH_CMD_OFFSET, val, completion);
 
-	if (gsi_command(gsi, GSI_CH_CMD_OFFSET, val, completion))
-		return 0;	/* Success! */
+	/* Disable the interrupt again */
+	gsi_irq_type_disable(gsi, GSI_CH_CTRL);
+	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
+
+	if (success)
+		return 0;
 
 	dev_err(dev, "GSI command %u for channel %u timed out, state %u\n",
 		opcode, channel_id, gsi_channel_state(channel));
-- 
2.20.1

[PATCH net-next 08/13] net: ipa: only enable GSI event control IRQs when needed

[Alex Elder]

A GSI event ring causes an event control interrupt to fire whenever
its state changes (between NOT_ALLOCATED and ALLOCATED).  No event
ring should ever change state except when we request it to.

Currently, we permit *all* events rings to generate event control
interrupts--even those that are never used.  And we enable event
control interrupts essentially at all times, from setup to teardown.

Instead, only enable the event control interrupt type for the
duration of an event ring command, and when doing so, only allow
the event ring being operated upon to cause the interrupt to fire.
Disallow all event rings from issuing the event control interrupt
in gsi_irq_setup().

Because an event ring's interrupt is only enabled when needed,
there is no longer any need to zero the event channel mask in
gsi_irq_disable().

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 4fc72dfe1e9b0..2c01a04e07b70 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -256,6 +256,7 @@ static void gsi_irq_setup(struct gsi *gsi)
 	gsi_irq_type_update(gsi);
 
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_CH_IRQ_MSK_OFFSET);
+	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
 }
 
 /* Turn off all GSI interrupts when we're all done */
@@ -288,10 +289,6 @@ static void gsi_irq_enable(struct gsi *gsi)
 {
 	u32 val;
 
-	val = GENMASK(gsi->evt_ring_count - 1, 0);
-	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
-	gsi->type_enabled_bitmap |= BIT(GSI_EV_CTRL);
-
 	/* Each IEOB interrupt is enabled (later) as needed by channels */
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 	gsi->type_enabled_bitmap |= BIT(GSI_IEOB);
@@ -320,7 +317,6 @@ static void gsi_irq_disable(struct gsi *gsi)
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
 }
 
 /* Return the virtual address associated with a ring index */
@@ -374,13 +370,30 @@ static int evt_ring_command(struct gsi *gsi, u32 evt_ring_id,
 	struct gsi_evt_ring *evt_ring = &gsi->evt_ring[evt_ring_id];
 	struct completion *completion = &evt_ring->completion;
 	struct device *dev = gsi->dev;
+	bool success;
 	u32 val;
 
+	/* We only perform one event ring command at a time, and event
+	 * control interrupts should only occur when such a command
+	 * is issued here.  Only permit *this* event ring to trigger
+	 * an interrupt, and only enable the event control IRQ type
+	 * when we expect it to occur.
+	 */
+	val = BIT(evt_ring_id);
+	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
+	gsi_irq_type_enable(gsi, GSI_EV_CTRL);
+
 	val = u32_encode_bits(evt_ring_id, EV_CHID_FMASK);
 	val |= u32_encode_bits(opcode, EV_OPCODE_FMASK);
 
-	if (gsi_command(gsi, GSI_EV_CH_CMD_OFFSET, val, completion))
-		return 0;	/* Success! */
+	success = gsi_command(gsi, GSI_EV_CH_CMD_OFFSET, val, completion);
+
+	/* Disable the interrupt again */
+	gsi_irq_type_disable(gsi, GSI_EV_CTRL);
+	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_EV_CH_IRQ_MSK_OFFSET);
+
+	if (success)
+		return 0;
 
 	dev_err(dev, "GSI command %u for event ring %u timed out, state %u\n",
 		opcode, evt_ring_id, evt_ring->state);
-- 
2.20.1

[PATCH net-next 09/13] net: ipa: only enable generic command completion IRQ when needed

[Alex Elder]

The completion of a generic EE GSI command is signaled by a global
interrupt of type GP_INT1.  The only other used type for a global
interrupt is a hardware error report.

First, disallow all global interrupt types in gsi_irq_setup().  We
want to know about hardware errors, so re-enable the interrupt type
in gsi_irq_enable(), to allow hardware errors to be reported.
Disable that interrupt type again in gsi_irq_disable().

We only issue generic EE commands one at a time, and there's no
reason to keep the completion interrupt enabled when no generic
EE command is pending.  We furthermore have no need to enable the
GP_INT2 or GP_INT3 interrupt types (which aren't used).

The change in gsi_irq_enable() makes GSI_CNTXT_GLOB_IRQ_ALL unused,
so get rid of it.  Have gsi_generic_command() enable the GP_INT1
interrupt type (in addition to the ERROR_INT type) only while a
generic command is pending.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c     | 35 +++++++++++++++++++++++++++--------
 drivers/net/ipa/gsi_reg.h |  1 -
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 2c01a04e07b70..4ab1d89f642ea 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -257,6 +257,7 @@ static void gsi_irq_setup(struct gsi *gsi)
 
 	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);
 }
 
 /* Turn off all GSI interrupts when we're all done */
@@ -289,14 +290,16 @@ static void gsi_irq_enable(struct gsi *gsi)
 {
 	u32 val;
 
+	/* Global interrupts include hardware error reports.  Enable
+	 * that so we can at least report the error should it occur.
+	 */
+	iowrite32(ERROR_INT_FMASK, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
+	gsi->type_enabled_bitmap |= BIT(GSI_GLOB_EE);
+
 	/* Each IEOB interrupt is enabled (later) as needed by channels */
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 	gsi->type_enabled_bitmap |= BIT(GSI_IEOB);
 
-	val = GSI_CNTXT_GLOB_IRQ_ALL;
-	iowrite32(val, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
-	gsi->type_enabled_bitmap |= BIT(GSI_GLOB_EE);
-
 	/* We don't use inter-EE channel or event interrupts */
 
 	/* Never enable GSI_BREAK_POINT */
@@ -315,8 +318,8 @@ static void gsi_irq_disable(struct gsi *gsi)
 	gsi_irq_type_update(gsi);
 
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
+	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 }
 
 /* Return the virtual address associated with a ring index */
@@ -1101,8 +1104,8 @@ static void gsi_isr_glob_err(struct gsi *gsi)
 	iowrite32(~0, gsi->virt + GSI_ERROR_LOG_CLR_OFFSET);
 
 	ee = u32_get_bits(val, ERR_EE_FMASK);
-	which = u32_get_bits(val, ERR_VIRT_IDX_FMASK);
 	type = u32_get_bits(val, ERR_TYPE_FMASK);
+	which = u32_get_bits(val, ERR_VIRT_IDX_FMASK);
 	code = u32_get_bits(val, ERR_CODE_FMASK);
 
 	if (type == GSI_ERR_TYPE_CHAN)
@@ -1606,8 +1609,19 @@ static int gsi_generic_command(struct gsi *gsi, u32 channel_id,
 			       enum gsi_generic_cmd_opcode opcode)
 {
 	struct completion *completion = &gsi->completion;
+	bool success;
 	u32 val;
 
+	/* The error global interrupt type is always enabled (until we
+	 * teardown), so we won't change that.  A generic EE command
+	 * completes with a GSI global interrupt of type GP_INT1.  We
+	 * only perform one generic command at a time (to allocate or
+	 * halt a modem channel) and only from this function.  So we
+	 * enable the GP_INT1 IRQ type here while we're expecting it.
+	 */
+	val = ERROR_INT_FMASK | GP_INT1_FMASK;
+	iowrite32(val, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
+
 	/* First zero the result code field */
 	val = ioread32(gsi->virt + GSI_CNTXT_SCRATCH_0_OFFSET);
 	val &= ~GENERIC_EE_RESULT_FMASK;
@@ -1618,8 +1632,13 @@ static int gsi_generic_command(struct gsi *gsi, u32 channel_id,
 	val |= u32_encode_bits(channel_id, GENERIC_CHID_FMASK);
 	val |= u32_encode_bits(GSI_EE_MODEM, GENERIC_EE_FMASK);
 
-	if (gsi_command(gsi, GSI_GENERIC_CMD_OFFSET, val, completion))
-		return 0;	/* Success! */
+	success = gsi_command(gsi, GSI_GENERIC_CMD_OFFSET, val, completion);
+
+	/* Disable the GP_INT1 IRQ type again */
+	iowrite32(ERROR_INT_FMASK, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
+
+	if (success)
+		return 0;
 
 	dev_err(gsi->dev, "GSI generic command %u to channel %u timed out\n",
 		opcode, channel_id);
diff --git a/drivers/net/ipa/gsi_reg.h b/drivers/net/ipa/gsi_reg.h
index 1dd81cf0b46a8..ae00aff1cfa50 100644
--- a/drivers/net/ipa/gsi_reg.h
+++ b/drivers/net/ipa/gsi_reg.h
@@ -335,7 +335,6 @@ enum gsi_irq_type_id {
 #define GP_INT1_FMASK			GENMASK(1, 1)
 #define GP_INT2_FMASK			GENMASK(2, 2)
 #define GP_INT3_FMASK			GENMASK(3, 3)
-#define GSI_CNTXT_GLOB_IRQ_ALL		GENMASK(3, 0)
 
 #define GSI_CNTXT_GSI_IRQ_STTS_OFFSET \
 			GSI_EE_N_CNTXT_GSI_IRQ_STTS_OFFSET(GSI_EE_AP)
-- 
2.20.1

[PATCH net-next 10/13] net: ipa: only enable GSI IEOB IRQs when needed

[Alex Elder]

A GSI channel must be started in order to use it to perform a
transfer data (or command) transaction.  And the only time we'll see
an IEOB interrupt is if we send a transaction to a started channel.
Therefore we do not need to have the IEOB interrupt type enabled
until at least one channel has been started.  And once the last
started channel has been stopped, we can disable the IEOB interrupt
type again.

We already enable the IEOB interrupt for a particular channel only
when it is started.  Extend that by having the IEOB interrupt *type*
be enabled only when at least one channel is in STARTED state.

Disallow all channels from triggering the IEOB interrupt in
gsi_irq_setup().  We only enable an channel's interrupt when
needed, so there is no longer any need to zero the channel mask
in gsi_irq_disable().

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 4ab1d89f642ea..aae8ea852349d 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -258,6 +258,7 @@ static void gsi_irq_setup(struct gsi *gsi)
 	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);
 }
 
 /* Turn off all GSI interrupts when we're all done */
@@ -269,11 +270,16 @@ static void gsi_irq_teardown(struct gsi *gsi)
 
 static void gsi_irq_ieob_enable(struct gsi *gsi, u32 evt_ring_id)
 {
+	bool enable_ieob = !gsi->ieob_enabled_bitmap;
 	u32 val;
 
 	gsi->ieob_enabled_bitmap |= BIT(evt_ring_id);
 	val = gsi->ieob_enabled_bitmap;
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
+
+	/* Enable the interrupt type if this is the first channel enabled */
+	if (enable_ieob)
+		gsi_irq_type_enable(gsi, GSI_IEOB);
 }
 
 static void gsi_irq_ieob_disable(struct gsi *gsi, u32 evt_ring_id)
@@ -281,6 +287,11 @@ static void gsi_irq_ieob_disable(struct gsi *gsi, u32 evt_ring_id)
 	u32 val;
 
 	gsi->ieob_enabled_bitmap &= ~BIT(evt_ring_id);
+
+	/* Disable the interrupt type if this was the last enabled channel */
+	if (!gsi->ieob_enabled_bitmap)
+		gsi_irq_type_disable(gsi, GSI_IEOB);
+
 	val = gsi->ieob_enabled_bitmap;
 	iowrite32(val, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 }
@@ -296,10 +307,6 @@ static void gsi_irq_enable(struct gsi *gsi)
 	iowrite32(ERROR_INT_FMASK, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 	gsi->type_enabled_bitmap |= BIT(GSI_GLOB_EE);
 
-	/* Each IEOB interrupt is enabled (later) as needed by channels */
-	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
-	gsi->type_enabled_bitmap |= BIT(GSI_IEOB);
-
 	/* We don't use inter-EE channel or event interrupts */
 
 	/* Never enable GSI_BREAK_POINT */
@@ -318,7 +325,6 @@ static void gsi_irq_disable(struct gsi *gsi)
 	gsi_irq_type_update(gsi);
 
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
-	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 }
 
-- 
2.20.1

[PATCH net-next 11/13] net: ipa: explicitly disallow inter-EE interrupts

[Alex Elder]

It is possible for other execution environments (EEs, like the modem)
to request changes to local (AP) channel or event ring state.  We do
not support this feature.

In gsi_irq_setup(), explicitly zero the mask that defines which
channels are permitted to generate inter-EE channel state change
interrupts.  Do the same for the event ring mask.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index aae8ea852349d..5e10e5c1713b1 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -259,6 +259,8 @@ static void gsi_irq_setup(struct gsi *gsi)
 	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);
+	iowrite32(0, gsi->virt + GSI_INTER_EE_SRC_CH_IRQ_OFFSET);
+	iowrite32(0, gsi->virt + GSI_INTER_EE_SRC_EV_CH_IRQ_OFFSET);
 }
 
 /* Turn off all GSI interrupts when we're all done */
@@ -307,8 +309,6 @@ static void gsi_irq_enable(struct gsi *gsi)
 	iowrite32(ERROR_INT_FMASK, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 	gsi->type_enabled_bitmap |= BIT(GSI_GLOB_EE);
 
-	/* We don't use inter-EE channel or event interrupts */
-
 	/* Never enable GSI_BREAK_POINT */
 	val = GSI_CNTXT_GSI_IRQ_ALL & ~BREAK_POINT_FMASK;
 	iowrite32(val, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
-- 
2.20.1

[PATCH net-next 12/13] net: ipa: only enable GSI general IRQs when needed

[Alex Elder]

Most GSI general errors are unrecoverable without a full reset.
Despite that, we want to receive these errors so we can at least
report what happened before whatever undefined behavior ensues.

Explicitly disable all such interrupts in gsi_irq_setup(), then
enable those we want in gsi_irq_enable().  List the interrupt types
we are interested in (everything but breakpoint) explicitly rather
than using GSI_CNTXT_GSI_IRQ_ALL, and remove that symbol's
definition.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/gsi.c     | 14 ++++++++++----
 drivers/net/ipa/gsi_reg.h |  1 -
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 5e10e5c1713b1..aa3983649bc30 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -261,6 +261,7 @@ static void gsi_irq_setup(struct gsi *gsi)
 	iowrite32(0, gsi->virt + GSI_CNTXT_SRC_IEOB_IRQ_MSK_OFFSET);
 	iowrite32(0, gsi->virt + GSI_INTER_EE_SRC_CH_IRQ_OFFSET);
 	iowrite32(0, gsi->virt + GSI_INTER_EE_SRC_EV_CH_IRQ_OFFSET);
+	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
 }
 
 /* Turn off all GSI interrupts when we're all done */
@@ -309,8 +310,14 @@ static void gsi_irq_enable(struct gsi *gsi)
 	iowrite32(ERROR_INT_FMASK, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 	gsi->type_enabled_bitmap |= BIT(GSI_GLOB_EE);
 
-	/* Never enable GSI_BREAK_POINT */
-	val = GSI_CNTXT_GSI_IRQ_ALL & ~BREAK_POINT_FMASK;
+	/* General GSI interrupts are reported to all EEs; if they occur
+	 * they are unrecoverable (without reset).  A breakpoint interrupt
+	 * also exists, but we don't support that.  We want to be notified
+	 * of errors so we can report them, even if they can't be handled.
+	 */
+	val = BUS_ERROR_FMASK;
+	val |= CMD_FIFO_OVRFLOW_FMASK;
+	val |= MCS_STACK_OVRFLOW_FMASK;
 	iowrite32(val, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
 	gsi->type_enabled_bitmap |= BIT(GSI_GENERAL);
 
@@ -1186,8 +1193,7 @@ static void gsi_isr_general(struct gsi *gsi)
 	val = ioread32(gsi->virt + GSI_CNTXT_GSI_IRQ_STTS_OFFSET);
 	iowrite32(val, gsi->virt + GSI_CNTXT_GSI_IRQ_CLR_OFFSET);
 
-	if (val)
-		dev_err(dev, "unexpected general interrupt 0x%08x\n", val);
+	dev_err(dev, "unexpected general interrupt 0x%08x\n", val);
 }
 
 /**
diff --git a/drivers/net/ipa/gsi_reg.h b/drivers/net/ipa/gsi_reg.h
index ae00aff1cfa50..c50464984c6e3 100644
--- a/drivers/net/ipa/gsi_reg.h
+++ b/drivers/net/ipa/gsi_reg.h
@@ -353,7 +353,6 @@ enum gsi_irq_type_id {
 #define BUS_ERROR_FMASK			GENMASK(1, 1)
 #define CMD_FIFO_OVRFLOW_FMASK		GENMASK(2, 2)
 #define MCS_STACK_OVRFLOW_FMASK		GENMASK(3, 3)
-#define GSI_CNTXT_GSI_IRQ_ALL		GENMASK(3, 0)
 
 #define GSI_CNTXT_INTSET_OFFSET \
 			GSI_EE_N_CNTXT_INTSET_OFFSET(GSI_EE_AP)
-- 
2.20.1

[PATCH net-next 13/13] net: ipa: pass a value to gsi_irq_type_update()

[Alex Elder]

Now that all of the GSI interrupts are handled uniformly,
change gsi_irq_type_update() so it takes a value.  Have the
function assign that value to the cached mask of enabled GSI
IRQ types before writing it to hardware.

Note that gsi_irq_teardown() will only be called after
gsi_irq_disable(), so it's not necessary for the former
to disable all IRQ types.  Get rid of that.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index aa3983649bc30..961a11d4fb270 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -231,30 +231,29 @@ static u32 gsi_channel_id(struct gsi_channel *channel)
 }
 
 /* Update the GSI IRQ type register with the cached value */
-static void gsi_irq_type_update(struct gsi *gsi)
+static void gsi_irq_type_update(struct gsi *gsi, u32 val)
 {
-	iowrite32(gsi->type_enabled_bitmap,
-		  gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
+	gsi->type_enabled_bitmap = val;
+	iowrite32(val, gsi->virt + GSI_CNTXT_TYPE_IRQ_MSK_OFFSET);
 }
 
 static void gsi_irq_type_enable(struct gsi *gsi, enum gsi_irq_type_id type_id)
 {
-	gsi->type_enabled_bitmap |= BIT(type_id);
-	gsi_irq_type_update(gsi);
+	gsi_irq_type_update(gsi, gsi->type_enabled_bitmap | BIT(type_id));
 }
 
 static void gsi_irq_type_disable(struct gsi *gsi, enum gsi_irq_type_id type_id)
 {
-	gsi->type_enabled_bitmap &= ~BIT(type_id);
-	gsi_irq_type_update(gsi);
+	gsi_irq_type_update(gsi, gsi->type_enabled_bitmap & ~BIT(type_id));
 }
 
 /* Turn off all GSI interrupts initially */
 static void gsi_irq_setup(struct gsi *gsi)
 {
-	gsi->type_enabled_bitmap = 0;
-	gsi_irq_type_update(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);
@@ -267,8 +266,7 @@ static void gsi_irq_setup(struct gsi *gsi)
 /* Turn off all GSI interrupts when we're all done */
 static void gsi_irq_teardown(struct gsi *gsi)
 {
-	gsi->type_enabled_bitmap = 0;
-	gsi_irq_type_update(gsi);
+	/* Nothing to do */
 }
 
 static void gsi_irq_ieob_enable(struct gsi *gsi, u32 evt_ring_id)
@@ -308,7 +306,7 @@ static void gsi_irq_enable(struct gsi *gsi)
 	 * that so we can at least report the error should it occur.
 	 */
 	iowrite32(ERROR_INT_FMASK, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
-	gsi->type_enabled_bitmap |= BIT(GSI_GLOB_EE);
+	gsi_irq_type_update(gsi, gsi->type_enabled_bitmap | BIT(GSI_GLOB_EE));
 
 	/* General GSI interrupts are reported to all EEs; if they occur
 	 * they are unrecoverable (without reset).  A breakpoint interrupt
@@ -319,18 +317,15 @@ static void gsi_irq_enable(struct gsi *gsi)
 	val |= CMD_FIFO_OVRFLOW_FMASK;
 	val |= MCS_STACK_OVRFLOW_FMASK;
 	iowrite32(val, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
-	gsi->type_enabled_bitmap |= BIT(GSI_GENERAL);
-
-	/* Finally update the interrupt types we want enabled */
-	gsi_irq_type_update(gsi);
+	gsi_irq_type_update(gsi, gsi->type_enabled_bitmap | BIT(GSI_GENERAL));
 }
 
 /* Disable all GSI interrupt types */
 static void gsi_irq_disable(struct gsi *gsi)
 {
-	gsi->type_enabled_bitmap = 0;
-	gsi_irq_type_update(gsi);
+	gsi_irq_type_update(gsi, 0);
 
+	/* Clear the type-specific interrupt masks set by gsi_irq_enable() */
 	iowrite32(0, gsi->virt + GSI_CNTXT_GSI_IRQ_EN_OFFSET);
 	iowrite32(0, gsi->virt + GSI_CNTXT_GLOB_IRQ_EN_OFFSET);
 }
-- 
2.20.1

Re: [PATCH net-next 00/13] net: ipa: constrain GSI interrupts

[patchwork-bot+netdevbpf]

Hello:

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

On Thu,  5 Nov 2020 12:13:54 -0600 you wrote:
> The goal of this series is to more tightly control when GSI
> interrupts are enabled.  This is a long-ish series, so I'll
> describe it in parts.
> 
> The first patch is actually unrelated...  I forgot to include
> it in my previous series (which exposed the GSI layer to the
> IPA version).  It is a trivial comments-only update patch.
> 
> [...]

Here is the summary with links:
  - [net-next,01/13] net: ipa: refer to IPA versions, not GSI
    https://git.kernel.org/netdev/net-next/c/4a04d65c964e
  - [net-next,02/13] net: ipa: request GSI IRQ later
    https://git.kernel.org/netdev/net-next/c/0b8d67610845
  - [net-next,03/13] net: ipa: rename gsi->event_enable_bitmap
    https://git.kernel.org/netdev/net-next/c/a054539db196
  - [net-next,04/13] net: ipa: define GSI interrupt types with an enum
    https://git.kernel.org/netdev/net-next/c/f9b28804ab50
  - [net-next,05/13] net: ipa: disable all GSI interrupt types initially
    https://git.kernel.org/netdev/net-next/c/97eb94c8c790
  - [net-next,06/13] net: ipa: cache last-saved GSI IRQ enabled type
    https://git.kernel.org/netdev/net-next/c/3ca97ffd984c
  - [net-next,07/13] net: ipa: only enable GSI channel control IRQs when needed
    https://git.kernel.org/netdev/net-next/c/b054d4f9eb4b
  - [net-next,08/13] net: ipa: only enable GSI event control IRQs when needed
    https://git.kernel.org/netdev/net-next/c/b4175f8731f7
  - [net-next,09/13] net: ipa: only enable generic command completion IRQ when needed
    https://git.kernel.org/netdev/net-next/c/d6c9e3f506ae
  - [net-next,10/13] net: ipa: only enable GSI IEOB IRQs when needed
    https://git.kernel.org/netdev/net-next/c/06c8632833c2
  - [net-next,11/13] net: ipa: explicitly disallow inter-EE interrupts
    https://git.kernel.org/netdev/net-next/c/46f748ccaf01
  - [net-next,12/13] net: ipa: only enable GSI general IRQs when needed
    https://git.kernel.org/netdev/net-next/c/352f26a886d8
  - [net-next,13/13] net: ipa: pass a value to gsi_irq_type_update()
    https://git.kernel.org/netdev/net-next/c/8194be79fbbc

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