[PATCH net-next 0/6] net: ipa: tell GSI the IPA version

[PATCH net-next 0/6] net: ipa: tell GSI the IPA version

[Alex Elder]

The GSI code that supports IPA avoids having knowledge about the
IPA layer it serves.  One result of this is that Boolean flags are
used during GSI initialization to convey that certain hardware
version-dependent special behaviors should be used.

A given version of IPA hardware uses a fixed/well-defined version
of GSI, so the IPA version really implies the GSI version.

If given only the IPA version, the GSI code supporting IPA can
use it to implement certain special behaviors required for IPA
*or* GSI.  This avoids the need to pass and maintain numerous
Boolean flags.

Note:  the last patch in this series depends on this patch posted
for review earlier today:
  https://lore.kernel.org/netdev/20201102173435.5987-1-elder@linaro.org

					-Alex

Alex Elder (6):
  net: ipa: expose IPA version to the GSI layer
  net: ipa: record IPA version in GSI structure
  net: ipa: use version in gsi_channel_init()
  net: ipa: use version in gsi_channel_reset()
  net: ipa: use version in gsi_channel_program()
  net: ipa: eliminate legacy arguments

 drivers/net/ipa/gsi.c          | 52 ++++++++++++++++++----------------
 drivers/net/ipa/gsi.h          | 24 +++++++++-------
 drivers/net/ipa/ipa_endpoint.c | 16 ++++-------
 drivers/net/ipa/ipa_main.c     | 14 ++-------
 4 files changed, 51 insertions(+), 55 deletions(-)

-- 
2.20.1

[PATCH net-next 1/6] net: ipa: expose IPA version to the GSI layer

[Alex Elder]

Although GSI is integral to IPA, it is a separate hardware component
and the IPA code supporting it has been structured to avoid explicit
dependence on IPA details.  An example of this is that gsi_init() is
passed a number of Boolean flags to indicate special behaviors,
whose values are dependent on the IPA hardware version.  Looking
ahead, newer hardware versions would require even more such special
behaviors.

For any given version of IPA hardware (like 3.5.1 or 4.2), the GSI
hardware version is fixed (in this case, 1.3 and 2.2, respectively).
So the IPA version *implies* the GSI version, and the IPA version
can be used as effectively the equivalent of the GSI hardware version.

Rather than proliferating new special behavior flags, just provide
the IPA version to the GSI layer when it is initialized.  The GSI
code can then use that directly to determine whether special
behaviors are required.  The IPA version enumerated type is already
isolated to its own header file, so the exposure of this IPA detail
is very limited.

For now, just change gsi_init() to pass the version rather than the
Boolean flags, and set the flag values internal to that function.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 6bfac1efe037c..1e19160281dd3 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -21,6 +21,7 @@
 #include "gsi_trans.h"
 #include "ipa_gsi.h"
 #include "ipa_data.h"
+#include "ipa_version.h"
 
 /**
  * DOC: The IPA Generic Software Interface
@@ -1952,18 +1953,25 @@ static void gsi_channel_exit(struct gsi *gsi)
 }
 
 /* Init function for GSI.  GSI hardware does not need to be "ready" */
-int gsi_init(struct gsi *gsi, struct platform_device *pdev, bool prefetch,
-	     u32 count, const struct ipa_gsi_endpoint_data *data,
-	     bool modem_alloc)
+int gsi_init(struct gsi *gsi, struct platform_device *pdev,
+	     enum ipa_version version, u32 count,
+	     const struct ipa_gsi_endpoint_data *data)
 {
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	resource_size_t size;
 	unsigned int irq;
+	bool modem_alloc;
+	bool prefetch;
 	int ret;
 
 	gsi_validate_build();
 
+	/* IPA v4.0+ (GSI v2.0+) uses prefetch for the command channel */
+	prefetch = version != IPA_VERSION_3_5_1;
+	/* IPA v4.2 requires the AP to allocate channels for the modem */
+	modem_alloc = version == IPA_VERSION_4_2;
+
 	gsi->dev = dev;
 
 	/* The GSI layer performs NAPI on all endpoints.  NAPI requires a
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 3f9f29d531c43..2dd8ee78aa8c7 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -20,6 +20,8 @@
 /* Maximum TLV FIFO size for a channel; 64 here is arbitrary (and high) */
 #define GSI_TLV_MAX		64
 
+enum ipa_version;
+
 struct device;
 struct scatterlist;
 struct platform_device;
@@ -236,15 +238,18 @@ int gsi_channel_resume(struct gsi *gsi, u32 channel_id, bool start);
  * gsi_init() - Initialize the GSI subsystem
  * @gsi:	Address of GSI structure embedded in an IPA structure
  * @pdev:	IPA platform device
+ * @version:	IPA hardware version (implies GSI version)
+ * @count:	Number of entries in the configuration data array
+ * @data:	Endpoint and channel configuration data
  *
  * Return:	0 if successful, or a negative error code
  *
  * Early stage initialization of the GSI subsystem, performing tasks
  * that can be done before the GSI hardware is ready to use.
  */
-int gsi_init(struct gsi *gsi, struct platform_device *pdev, bool prefetch,
-	     u32 count, const struct ipa_gsi_endpoint_data *data,
-	     bool modem_alloc);
+int gsi_init(struct gsi *gsi, struct platform_device *pdev,
+	     enum ipa_version version, u32 count,
+	     const struct ipa_gsi_endpoint_data *data);
 
 /**
  * gsi_exit() - Exit the GSI subsystem
diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
index f4dd14d9550fe..0d3d1a5cf07c1 100644
--- a/drivers/net/ipa/ipa_main.c
+++ b/drivers/net/ipa/ipa_main.c
@@ -723,10 +723,8 @@ static int ipa_probe(struct platform_device *pdev)
 	const struct ipa_data *data;
 	struct ipa_clock *clock;
 	struct rproc *rproc;
-	bool modem_alloc;
 	bool modem_init;
 	struct ipa *ipa;
-	bool prefetch;
 	phandle ph;
 	int ret;
 
@@ -788,13 +786,8 @@ static int ipa_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_reg_exit;
 
-	/* GSI v2.0+ (IPA v4.0+) uses prefetch for the command channel */
-	prefetch = ipa->version != IPA_VERSION_3_5_1;
-	/* IPA v4.2 requires the AP to allocate channels for the modem */
-	modem_alloc = ipa->version == IPA_VERSION_4_2;
-
-	ret = gsi_init(&ipa->gsi, pdev, prefetch, data->endpoint_count,
-		       data->endpoint_data, modem_alloc);
+	ret = gsi_init(&ipa->gsi, pdev, ipa->version, data->endpoint_count,
+		       data->endpoint_data);
 	if (ret)
 		goto err_mem_exit;
 
-- 
2.20.1

[PATCH net-next 2/6] net: ipa: record IPA version in GSI structure

[Alex Elder]

Record the IPA version passed to gsi_init() in the GSI structure.
This allows that value to be used directly where needed, rather than
passing and storing certain flag arguments through the code.

In particular, for all but one supported version of IPA, the command
channel is programmed to only use an "escape buffer".  By storing
the IPA version, we can do a simple version check in one location,
and avoid storing a flag field in every channel (and passing a flag
along while initializing channels to set that field properly).

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 1e19160281dd3..178d6ec2699eb 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -747,7 +747,8 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell)
 	if (doorbell)
 		val |= USE_DB_ENG_FMASK;
 
-	if (!channel->use_prefetch)
+	/* Starting with IPA v4.0 the command channel uses the escape buffer */
+	if (gsi->version != IPA_VERSION_3_5_1 && channel->command)
 		val |= USE_ESCAPE_BUF_ONLY_FMASK;
 
 	iowrite32(val, gsi->virt + GSI_CH_C_QOS_OFFSET(channel_id));
@@ -1815,7 +1816,7 @@ static bool gsi_channel_data_valid(struct gsi *gsi,
 /* Init function for a single channel */
 static int gsi_channel_init_one(struct gsi *gsi,
 				const struct ipa_gsi_endpoint_data *data,
-				bool command, bool prefetch)
+				bool command)
 {
 	struct gsi_channel *channel;
 	u32 tre_count;
@@ -1839,7 +1840,6 @@ static int gsi_channel_init_one(struct gsi *gsi,
 	channel->gsi = gsi;
 	channel->toward_ipa = data->toward_ipa;
 	channel->command = command;
-	channel->use_prefetch = command && prefetch;
 	channel->tlv_count = data->channel.tlv_count;
 	channel->tre_count = tre_count;
 	channel->event_count = data->channel.event_count;
@@ -1893,7 +1893,7 @@ static void gsi_channel_exit_one(struct gsi_channel *channel)
 }
 
 /* Init function for channels */
-static int gsi_channel_init(struct gsi *gsi, bool prefetch, u32 count,
+static int gsi_channel_init(struct gsi *gsi, u32 count,
 			    const struct ipa_gsi_endpoint_data *data,
 			    bool modem_alloc)
 {
@@ -1917,7 +1917,7 @@ static int gsi_channel_init(struct gsi *gsi, bool prefetch, u32 count,
 			continue;
 		}
 
-		ret = gsi_channel_init_one(gsi, &data[i], command, prefetch);
+		ret = gsi_channel_init_one(gsi, &data[i], command);
 		if (ret)
 			goto err_unwind;
 	}
@@ -1962,17 +1962,15 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
 	resource_size_t size;
 	unsigned int irq;
 	bool modem_alloc;
-	bool prefetch;
 	int ret;
 
 	gsi_validate_build();
 
-	/* IPA v4.0+ (GSI v2.0+) uses prefetch for the command channel */
-	prefetch = version != IPA_VERSION_3_5_1;
 	/* IPA v4.2 requires the AP to allocate channels for the modem */
 	modem_alloc = version == IPA_VERSION_4_2;
 
 	gsi->dev = dev;
+	gsi->version = version;
 
 	/* The GSI layer performs NAPI on all endpoints.  NAPI requires a
 	 * network device structure, but the GSI layer does not have one,
@@ -2016,7 +2014,7 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
 		goto err_free_irq;
 	}
 
-	ret = gsi_channel_init(gsi, prefetch, count, data, modem_alloc);
+	ret = gsi_channel_init(gsi, count, data, modem_alloc);
 	if (ret)
 		goto err_iounmap;
 
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 2dd8ee78aa8c7..cf117b52496c1 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -13,6 +13,8 @@
 #include <linux/platform_device.h>
 #include <linux/netdevice.h>
 
+#include "ipa_version.h"
+
 /* Maximum number of channels and event rings supported by the driver */
 #define GSI_CHANNEL_COUNT_MAX	17
 #define GSI_EVT_RING_COUNT_MAX	13
@@ -20,8 +22,6 @@
 /* Maximum TLV FIFO size for a channel; 64 here is arbitrary (and high) */
 #define GSI_TLV_MAX		64
 
-enum ipa_version;
-
 struct device;
 struct scatterlist;
 struct platform_device;
@@ -109,7 +109,6 @@ struct gsi_channel {
 	struct gsi *gsi;
 	bool toward_ipa;
 	bool command;			/* AP command TX channel or not */
-	bool use_prefetch;		/* use prefetch (else escape buf) */
 
 	u8 tlv_count;			/* # entries in TLV FIFO */
 	u16 tre_count;
@@ -149,6 +148,7 @@ struct gsi_evt_ring {
 
 struct gsi {
 	struct device *dev;		/* Same as IPA device */
+	enum ipa_version version;
 	struct net_device dummy_dev;	/* needed for NAPI */
 	void __iomem *virt;
 	u32 irq;
-- 
2.20.1

[PATCH net-next 3/6] net: ipa: use version in gsi_channel_init()

[Alex Elder]

A quirk of IPA v4.2 requires the AP to allocate the GSI channels
that are owned by the modem.

Rather than pass a flag argument to gsi_channel_init(), use the
IPA version directly in that function to determine whether modem
channels need to be allocated.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 178d6ec2699eb..eae8ed83c1004 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1894,12 +1894,15 @@ static void gsi_channel_exit_one(struct gsi_channel *channel)
 
 /* Init function for channels */
 static int gsi_channel_init(struct gsi *gsi, u32 count,
-			    const struct ipa_gsi_endpoint_data *data,
-			    bool modem_alloc)
+			    const struct ipa_gsi_endpoint_data *data)
 {
+	bool modem_alloc;
 	int ret = 0;
 	u32 i;
 
+	/* IPA v4.2 requires the AP to allocate channels for the modem */
+	modem_alloc = gsi->version == IPA_VERSION_4_2;
+
 	gsi_evt_ring_init(gsi);
 
 	/* The endpoint data array is indexed by endpoint name */
@@ -1961,14 +1964,10 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
 	struct resource *res;
 	resource_size_t size;
 	unsigned int irq;
-	bool modem_alloc;
 	int ret;
 
 	gsi_validate_build();
 
-	/* IPA v4.2 requires the AP to allocate channels for the modem */
-	modem_alloc = version == IPA_VERSION_4_2;
-
 	gsi->dev = dev;
 	gsi->version = version;
 
@@ -2014,7 +2013,7 @@ int gsi_init(struct gsi *gsi, struct platform_device *pdev,
 		goto err_free_irq;
 	}
 
-	ret = gsi_channel_init(gsi, count, data, modem_alloc);
+	ret = gsi_channel_init(gsi, count, data);
 	if (ret)
 		goto err_iounmap;
 
-- 
2.20.1

[PATCH net-next 4/6] net: ipa: use version in gsi_channel_reset()

[Alex Elder]

A quirk of IPA v3.5.1 requires a channel reset on an RX channel to
be performed twice.  Use the IPA version in gsi_channel_reset()
rather than the passed-in legacy flag to determine that.

This is actually a bug fix, because this double reset is supposed
to occur independent of whether we're enabling the doorbell engine.
Now they will be independent.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index eae8ed83c1004..729ef712a10fd 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -840,7 +840,7 @@ void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool legacy)
 
 	gsi_channel_reset_command(channel);
 	/* Due to a hardware quirk we may need to reset RX channels twice. */
-	if (legacy && !channel->toward_ipa)
+	if (gsi->version == IPA_VERSION_3_5_1 && !channel->toward_ipa)
 		gsi_channel_reset_command(channel);
 
 	gsi_channel_program(channel, legacy);
-- 
2.20.1

[PATCH net-next 5/6] net: ipa: use version in gsi_channel_program()

[Alex Elder]

Use the IPA version in gsi_channel_program() to determine whether
we should enable the GSI doorbell engine when requested.  This way,
callers only say whether or not it should be enabled if needed,
regardless of hardware version.

Rename the "legacy" argument to gsi_channel_reset(), and have
it indicate whether the doorbell engine should be enabled when
reprogramming following the reset.

Change all callers of gsi_channel_reset() to indicate whether to
enable the doorbell engine after reset, independent of hardware
version.

Rework a little logic in ipa_endpoint_reset() to get rid of the
"legacy" variable previously passed to gsi_channel_reset().

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index 729ef712a10fd..f22b5d2efaf9d 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -743,8 +743,8 @@ static void gsi_channel_program(struct gsi_channel *channel, bool doorbell)
 
 	/* Max prefetch is 1 segment (do not set MAX_PREFETCH_FMASK) */
 
-	/* Enable the doorbell engine if requested */
-	if (doorbell)
+	/* We enable the doorbell engine for IPA v3.5.1 */
+	if (gsi->version == IPA_VERSION_3_5_1 && doorbell)
 		val |= USE_DB_ENG_FMASK;
 
 	/* Starting with IPA v4.0 the command channel uses the escape buffer */
@@ -831,8 +831,8 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id)
 	return ret;
 }
 
-/* Reset and reconfigure a channel (possibly leaving doorbell disabled) */
-void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool legacy)
+/* Reset and reconfigure a channel, (possibly) enabling the doorbell engine */
+void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool doorbell)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 
@@ -843,7 +843,7 @@ void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool legacy)
 	if (gsi->version == IPA_VERSION_3_5_1 && !channel->toward_ipa)
 		gsi_channel_reset_command(channel);
 
-	gsi_channel_program(channel, legacy);
+	gsi_channel_program(channel, doorbell);
 	gsi_channel_trans_cancel_pending(channel);
 
 	mutex_unlock(&gsi->mutex);
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index cf117b52496c1..36f876fb8f5ae 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -221,15 +221,15 @@ int gsi_channel_stop(struct gsi *gsi, u32 channel_id);
  * gsi_channel_reset() - Reset an allocated GSI channel
  * @gsi:	GSI pointer
  * @channel_id:	Channel to be reset
- * @legacy:	Legacy behavior
+ * @doorbell:	Whether to (possibly) enable the doorbell engine
  *
- * Reset a channel and reconfigure it.  The @legacy flag indicates
- * that some steps should be done differently for legacy hardware.
+ * Reset a channel and reconfigure it.  The @doorbell flag indicates
+ * that the doorbell engine should be enabled if needed.
  *
  * GSI hardware relinquishes ownership of all pending receive buffer
  * transactions and they will complete with their cancelled flag set.
  */
-void gsi_channel_reset(struct gsi *gsi, u32 channel_id, bool legacy);
+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);
diff --git a/drivers/net/ipa/ipa_endpoint.c b/drivers/net/ipa/ipa_endpoint.c
index 6f79028910e3c..548121b1531b7 100644
--- a/drivers/net/ipa/ipa_endpoint.c
+++ b/drivers/net/ipa/ipa_endpoint.c
@@ -1217,7 +1217,6 @@ static int ipa_endpoint_reset_rx_aggr(struct ipa_endpoint *endpoint)
 	struct gsi *gsi = &ipa->gsi;
 	bool suspended = false;
 	dma_addr_t addr;
-	bool legacy;
 	u32 retries;
 	u32 len = 1;
 	void *virt;
@@ -1279,8 +1278,7 @@ static int ipa_endpoint_reset_rx_aggr(struct ipa_endpoint *endpoint)
 	 * complete the channel reset sequence.  Finish by suspending the
 	 * channel again (if necessary).
 	 */
-	legacy = ipa->version == IPA_VERSION_3_5_1;
-	gsi_channel_reset(gsi, endpoint->channel_id, legacy);
+	gsi_channel_reset(gsi, endpoint->channel_id, true);
 
 	msleep(1);
 
@@ -1303,21 +1301,19 @@ static void ipa_endpoint_reset(struct ipa_endpoint *endpoint)
 	u32 channel_id = endpoint->channel_id;
 	struct ipa *ipa = endpoint->ipa;
 	bool special;
-	bool legacy;
 	int ret = 0;
 
 	/* On IPA v3.5.1, if an RX endpoint is reset while aggregation
 	 * is active, we need to handle things specially to recover.
 	 * All other cases just need to reset the underlying GSI channel.
-	 *
-	 * IPA v3.5.1 enables the doorbell engine.  Newer versions do not.
 	 */
-	legacy = ipa->version == IPA_VERSION_3_5_1;
-	special = !endpoint->toward_ipa && endpoint->data->aggregation;
-	if (legacy && special && ipa_endpoint_aggr_active(endpoint))
+	special = ipa->version == IPA_VERSION_3_5_1 &&
+			!endpoint->toward_ipa &&
+			endpoint->data->aggregation;
+	if (special && ipa_endpoint_aggr_active(endpoint))
 		ret = ipa_endpoint_reset_rx_aggr(endpoint);
 	else
-		gsi_channel_reset(&ipa->gsi, channel_id, legacy);
+		gsi_channel_reset(&ipa->gsi, channel_id, true);
 
 	if (ret)
 		dev_err(&ipa->pdev->dev,
-- 
2.20.1

[PATCH net-next 6/6] net: ipa: eliminate legacy arguments

[Alex Elder]

We enable a channel doorbell engine only for IPA v3.5.1, and that is
now handled directly by gsi_channel_program().

When initially setting up a channel, we want that doorbell engine
enabled, and we can request that independent of the IPA version.

Doing that makes the "legacy" argument to gsi_channel_setup_one()
unnecessary.  And with that gone we can get rid of the "legacy"
argument to gsi_channel_setup(), and gsi_setup() as well.

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

diff --git a/drivers/net/ipa/gsi.c b/drivers/net/ipa/gsi.c
index f22b5d2efaf9d..12a2001ee1e9c 100644
--- a/drivers/net/ipa/gsi.c
+++ b/drivers/net/ipa/gsi.c
@@ -1454,8 +1454,7 @@ static void gsi_evt_ring_teardown(struct gsi *gsi)
 }
 
 /* Setup function for a single channel */
-static int gsi_channel_setup_one(struct gsi *gsi, u32 channel_id,
-				 bool legacy)
+static int gsi_channel_setup_one(struct gsi *gsi, u32 channel_id)
 {
 	struct gsi_channel *channel = &gsi->channel[channel_id];
 	u32 evt_ring_id = channel->evt_ring_id;
@@ -1474,7 +1473,7 @@ static int gsi_channel_setup_one(struct gsi *gsi, u32 channel_id,
 	if (ret)
 		goto err_evt_ring_de_alloc;
 
-	gsi_channel_program(channel, legacy);
+	gsi_channel_program(channel, true);
 
 	if (channel->toward_ipa)
 		netif_tx_napi_add(&gsi->dummy_dev, &channel->napi,
@@ -1551,7 +1550,7 @@ static void gsi_modem_channel_halt(struct gsi *gsi, u32 channel_id)
 }
 
 /* Setup function for channels */
-static int gsi_channel_setup(struct gsi *gsi, bool legacy)
+static int gsi_channel_setup(struct gsi *gsi)
 {
 	u32 channel_id = 0;
 	u32 mask;
@@ -1563,7 +1562,7 @@ static int gsi_channel_setup(struct gsi *gsi, bool legacy)
 	mutex_lock(&gsi->mutex);
 
 	do {
-		ret = gsi_channel_setup_one(gsi, channel_id, legacy);
+		ret = gsi_channel_setup_one(gsi, channel_id);
 		if (ret)
 			goto err_unwind;
 	} while (++channel_id < gsi->channel_count);
@@ -1649,7 +1648,7 @@ static void gsi_channel_teardown(struct gsi *gsi)
 }
 
 /* Setup function for GSI.  GSI firmware must be loaded and initialized */
-int gsi_setup(struct gsi *gsi, bool legacy)
+int gsi_setup(struct gsi *gsi)
 {
 	struct device *dev = gsi->dev;
 	u32 val;
@@ -1693,7 +1692,7 @@ int gsi_setup(struct gsi *gsi, bool legacy)
 	/* Writing 1 indicates IRQ interrupts; 0 would be MSI */
 	iowrite32(1, gsi->virt + GSI_CNTXT_INTSET_OFFSET);
 
-	return gsi_channel_setup(gsi, legacy);
+	return gsi_channel_setup(gsi);
 }
 
 /* Inverse of gsi_setup() */
diff --git a/drivers/net/ipa/gsi.h b/drivers/net/ipa/gsi.h
index 36f876fb8f5ae..59ace83d404c4 100644
--- a/drivers/net/ipa/gsi.h
+++ b/drivers/net/ipa/gsi.h
@@ -166,14 +166,13 @@ struct gsi {
 /**
  * gsi_setup() - Set up the GSI subsystem
  * @gsi:	Address of GSI structure embedded in an IPA structure
- * @legacy:	Set up for legacy hardware
  *
  * Return:	0 if successful, or a negative error code
  *
  * Performs initialization that must wait until the GSI hardware is
  * ready (including firmware loaded).
  */
-int gsi_setup(struct gsi *gsi, bool legacy);
+int gsi_setup(struct gsi *gsi);
 
 /**
  * gsi_teardown() - Tear down GSI subsystem
diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
index 0d3d1a5cf07c1..a580cab794b1c 100644
--- a/drivers/net/ipa/ipa_main.c
+++ b/drivers/net/ipa/ipa_main.c
@@ -111,8 +111,7 @@ int ipa_setup(struct ipa *ipa)
 	struct device *dev = &ipa->pdev->dev;
 	int ret;
 
-	/* Setup for IPA v3.5.1 has some slight differences */
-	ret = gsi_setup(&ipa->gsi, ipa->version == IPA_VERSION_3_5_1);
+	ret = gsi_setup(&ipa->gsi);
 	if (ret)
 		return ret;
 
-- 
2.20.1

Re: [PATCH net-next 0/6] net: ipa: tell GSI the IPA version

[Jakub Kicinski]

On Mon,  2 Nov 2020 11:53:54 -0600 Alex Elder wrote:
> The GSI code that supports IPA avoids having knowledge about the
> IPA layer it serves.  One result of this is that Boolean flags are
> used during GSI initialization to convey that certain hardware
> version-dependent special behaviors should be used.
> 
> A given version of IPA hardware uses a fixed/well-defined version
> of GSI, so the IPA version really implies the GSI version.
> 
> If given only the IPA version, the GSI code supporting IPA can
> use it to implement certain special behaviors required for IPA
> *or* GSI.  This avoids the need to pass and maintain numerous
> Boolean flags.

Applied, thanks.

> Note:  the last patch in this series depends on this patch posted
> for review earlier today:
>   https://lore.kernel.org/netdev/20201102173435.5987-1-elder@linaro.org

But in the future please don't post dependent changes like this. The
build bots cannot figure this out and give up on checking your series.

Re: [PATCH net-next 0/6] net: ipa: tell GSI the IPA version

[Alex Elder]

On 11/4/20 6:31 PM, Jakub Kicinski wrote:
>> Note:  the last patch in this series depends on this patch posted
>> for review earlier today:
>>    https://lore.kernel.org/netdev/20201102173435.5987-1-elder@linaro.org
> But in the future please don't post dependent changes like this. The
> build bots cannot figure this out and give up on checking your series.

Sorry about that, I didn't realize this would be a problem.
I will avoid this in the future.

I will be sending a steady stream of patches in the coming
weeks.  If you have any other comments or suggestions that
would make that easier to manage, please let me know.

Thank you.

					-Alex