[PATCH net-next 0/7] net: ipa: interconnect improvements

[PATCH net-next 0/7] net: ipa: interconnect improvements

[Alex Elder]

The main outcome of this series is to allow the number of
interconnects used by the IPA to differ from the three that
are implemented now.  With this series in place, any number
of interconnects can now be used, all specified in the
configuration data for a specific platform.

A few minor interconnect-related cleanups are implemented as well.

					-Alex

Alex Elder (7):
  net: ipa: rename interconnect settings
  net: ipa: don't return an error from ipa_interconnect_disable()
  net: ipa: introduce an IPA interconnect structure
  net: ipa: store average and peak interconnect bandwidth
  net: ipa: add interconnect name to configuration data
  net: ipa: clean up interconnect initialization
  net: ipa: allow arbitrary number of interconnects

 drivers/net/ipa/ipa_clock.c       | 192 ++++++++++++++++--------------
 drivers/net/ipa/ipa_data-sc7180.c |  38 +++---
 drivers/net/ipa/ipa_data-sdm845.c |  38 +++---
 drivers/net/ipa/ipa_data.h        |  26 ++--
 4 files changed, 157 insertions(+), 137 deletions(-)

-- 
2.20.1

[PATCH net-next 1/7] net: ipa: rename interconnect settings

[Alex Elder]

Use "bandwidth" rather than "rate" in describing the average and
peak values to use for IPA interconnects.  They should have been
named that way to begin with.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_clock.c       | 20 ++++++++++----------
 drivers/net/ipa/ipa_data-sc7180.c | 16 ++++++++--------
 drivers/net/ipa/ipa_data-sdm845.c | 16 ++++++++--------
 drivers/net/ipa/ipa_data.h        | 10 +++++-----
 4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
index 135c393437f12..459c357e09678 100644
--- a/drivers/net/ipa/ipa_clock.c
+++ b/drivers/net/ipa/ipa_clock.c
@@ -109,20 +109,20 @@ static int ipa_interconnect_enable(struct ipa *ipa)
 	int ret;
 
 	data = &clock->interconnect_data[IPA_INTERCONNECT_MEMORY];
-	ret = icc_set_bw(clock->memory_path, data->average_rate,
-			 data->peak_rate);
+	ret = icc_set_bw(clock->memory_path, data->average_bandwidth,
+			 data->peak_bandwidth);
 	if (ret)
 		return ret;
 
 	data = &clock->interconnect_data[IPA_INTERCONNECT_IMEM];
-	ret = icc_set_bw(clock->imem_path, data->average_rate,
-			 data->peak_rate);
+	ret = icc_set_bw(clock->imem_path, data->average_bandwidth,
+			 data->peak_bandwidth);
 	if (ret)
 		goto err_memory_path_disable;
 
 	data = &clock->interconnect_data[IPA_INTERCONNECT_CONFIG];
-	ret = icc_set_bw(clock->config_path, data->average_rate,
-			 data->peak_rate);
+	ret = icc_set_bw(clock->config_path, data->average_bandwidth,
+			 data->peak_bandwidth);
 	if (ret)
 		goto err_imem_path_disable;
 
@@ -159,12 +159,12 @@ static int ipa_interconnect_disable(struct ipa *ipa)
 
 err_imem_path_reenable:
 	data = &clock->interconnect_data[IPA_INTERCONNECT_IMEM];
-	(void)icc_set_bw(clock->imem_path, data->average_rate,
-			 data->peak_rate);
+	(void)icc_set_bw(clock->imem_path, data->average_bandwidth,
+			 data->peak_bandwidth);
 err_memory_path_reenable:
 	data = &clock->interconnect_data[IPA_INTERCONNECT_MEMORY];
-	(void)icc_set_bw(clock->memory_path, data->average_rate,
-			 data->peak_rate);
+	(void)icc_set_bw(clock->memory_path, data->average_bandwidth,
+			 data->peak_bandwidth);
 
 	return ret;
 }
diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c
index 5cc0ed77edb9c..491572c0a34dc 100644
--- a/drivers/net/ipa/ipa_data-sc7180.c
+++ b/drivers/net/ipa/ipa_data-sc7180.c
@@ -311,20 +311,20 @@ static struct ipa_mem_data ipa_mem_data = {
 
 static struct ipa_clock_data ipa_clock_data = {
 	.core_clock_rate	= 100 * 1000 * 1000,	/* Hz */
-	/* Interconnect rates are in 1000 byte/second units */
+	/* Interconnect bandwidths are in 1000 byte/second units */
 	.interconnect = {
 		[IPA_INTERCONNECT_MEMORY] = {
-			.peak_rate	= 465000,	/* 465 MBps */
-			.average_rate	= 80000,	/* 80 MBps */
+			.peak_bandwidth		= 465000,	/* 465 MBps */
+			.average_bandwidth	= 80000,	/* 80 MBps */
 		},
-		/* Average rate is unused for the next two interconnects */
+		/* Average bandwidth unused for the next two interconnects */
 		[IPA_INTERCONNECT_IMEM] = {
-			.peak_rate	= 68570,	/* 68.570 MBps */
-			.average_rate	= 0,		/* unused */
+			.peak_bandwidth		= 68570,	/* 68.57 MBps */
+			.average_bandwidth	= 0,		/* unused */
 		},
 		[IPA_INTERCONNECT_CONFIG] = {
-			.peak_rate	= 30000,	/* 30 MBps */
-			.average_rate	= 0,		/* unused */
+			.peak_bandwidth		= 30000,	/* 30 MBps */
+			.average_bandwidth	= 0,		/* unused */
 		},
 	},
 };
diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c
index f8fee8d3ca42a..c62b86171b929 100644
--- a/drivers/net/ipa/ipa_data-sdm845.c
+++ b/drivers/net/ipa/ipa_data-sdm845.c
@@ -331,20 +331,20 @@ static struct ipa_mem_data ipa_mem_data = {
 
 static struct ipa_clock_data ipa_clock_data = {
 	.core_clock_rate	= 75 * 1000 * 1000,	/* Hz */
-	/* Interconnect rates are in 1000 byte/second units */
+	/* Interconnect bandwidths are in 1000 byte/second units */
 	.interconnect = {
 		[IPA_INTERCONNECT_MEMORY] = {
-			.peak_rate	= 600000,	/* 600 MBps */
-			.average_rate	= 80000,	/* 80 MBps */
+			.peak_bandwidth		= 600000,	/* 600 MBps */
+			.average_bandwidth	= 80000,	/* 80 MBps */
 		},
-		/* Average rate is unused for the next two interconnects */
+		/* Average bandwidth unused for the next two interconnects */
 		[IPA_INTERCONNECT_IMEM] = {
-			.peak_rate	= 350000,	/* 350 MBps */
-			.average_rate	= 0,		/* unused */
+			.peak_bandwidth		= 350000,	/* 350 MBps */
+			.average_bandwidth	= 0,		/* unused */
 		},
 		[IPA_INTERCONNECT_CONFIG] = {
-			.peak_rate	= 40000,	/* 40 MBps */
-			.average_rate	= 0,		/* unused */
+			.peak_bandwidth		= 40000,	/* 40 MBps */
+			.average_bandwidth	= 0,		/* unused */
 		},
 	},
 };
diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
index 0ed5ffe2b8da0..96a9771a6cc05 100644
--- a/drivers/net/ipa/ipa_data.h
+++ b/drivers/net/ipa/ipa_data.h
@@ -267,13 +267,13 @@ enum ipa_interconnect_id {
 };
 
 /**
- * struct ipa_interconnect_data - description of IPA interconnect rates
- * @peak_rate:		Peak interconnect bandwidth (in 1000 byte/sec units)
- * @average_rate:	Average interconnect bandwidth (in 1000 byte/sec units)
+ * struct ipa_interconnect_data - description of IPA interconnect bandwidths
+ * @peak_bandwidth:	Peak interconnect bandwidth (in 1000 byte/sec units)
+ * @average_bandwidth:	Average interconnect bandwidth (in 1000 byte/sec units)
  */
 struct ipa_interconnect_data {
-	u32 peak_rate;
-	u32 average_rate;
+	u32 peak_bandwidth;
+	u32 average_bandwidth;
 };
 
 /**
-- 
2.20.1

[PATCH net-next 2/7] net: ipa: don't return an error from ipa_interconnect_disable()

[Alex Elder]

If disabling interconnects fails there's not a lot we can do.  The
only two callers of ipa_interconnect_disable() ignore the return
value, so just give the function a void return type.

Print an error message if disabling any of the interconnects is not
successful.  Return (and print) only the first error seen.

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

diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
index 459c357e09678..baedb481fe824 100644
--- a/drivers/net/ipa/ipa_clock.c
+++ b/drivers/net/ipa/ipa_clock.c
@@ -137,36 +137,27 @@ static int ipa_interconnect_enable(struct ipa *ipa)
 }
 
 /* To disable an interconnect, we just its bandwidth to 0 */
-static int ipa_interconnect_disable(struct ipa *ipa)
+static void ipa_interconnect_disable(struct ipa *ipa)
 {
-	const struct ipa_interconnect_data *data;
 	struct ipa_clock *clock = ipa->clock;
+	int result = 0;
 	int ret;
 
 	ret = icc_set_bw(clock->memory_path, 0, 0);
 	if (ret)
-		return ret;
+		result = ret;
 
 	ret = icc_set_bw(clock->imem_path, 0, 0);
-	if (ret)
-		goto err_memory_path_reenable;
+	if (ret && !result)
+		result = ret;
 
 	ret = icc_set_bw(clock->config_path, 0, 0);
-	if (ret)
-		goto err_imem_path_reenable;
+	if (ret && !result)
+		result = ret;
 
-	return 0;
-
-err_imem_path_reenable:
-	data = &clock->interconnect_data[IPA_INTERCONNECT_IMEM];
-	(void)icc_set_bw(clock->imem_path, data->average_bandwidth,
-			 data->peak_bandwidth);
-err_memory_path_reenable:
-	data = &clock->interconnect_data[IPA_INTERCONNECT_MEMORY];
-	(void)icc_set_bw(clock->memory_path, data->average_bandwidth,
-			 data->peak_bandwidth);
-
-	return ret;
+	if (result)
+		dev_err(&ipa->pdev->dev,
+			"error %d disabling IPA interconnects\n", ret);
 }
 
 /* Turn on IPA clocks, including interconnects */
@@ -189,7 +180,7 @@ static int ipa_clock_enable(struct ipa *ipa)
 static void ipa_clock_disable(struct ipa *ipa)
 {
 	clk_disable_unprepare(ipa->clock->core);
-	(void)ipa_interconnect_disable(ipa);
+	ipa_interconnect_disable(ipa);
 }
 
 /* Get an IPA clock reference, but only if the reference count is
-- 
2.20.1

[PATCH net-next 3/7] net: ipa: introduce an IPA interconnect structure

[Alex Elder]

Rather than having separate pointers for the memory, imem, and
config interconnect paths, maintain an array of ipa_interconnect
structures each of which contains a pointer to a path.

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

diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
index baedb481fe824..2bf5af6823d8c 100644
--- a/drivers/net/ipa/ipa_clock.c
+++ b/drivers/net/ipa/ipa_clock.c
@@ -30,23 +30,27 @@
  * An IPA clock reference must be held for any access to IPA hardware.
  */
 
+/**
+ * struct ipa_interconnect - IPA interconnect information
+ * @path:		Interconnect path
+ */
+struct ipa_interconnect {
+	struct icc_path *path;
+};
+
 /**
  * struct ipa_clock - IPA clocking information
  * @count:		Clocking reference count
  * @mutex:		Protects clock enable/disable
  * @core:		IPA core clock
- * @memory_path:	Memory interconnect
- * @imem_path:		Internal memory interconnect
- * @config_path:	Configuration space interconnect
+ * @interconnect:	Interconnect array
  * @interconnect_data:	Interconnect configuration data
  */
 struct ipa_clock {
 	refcount_t count;
 	struct mutex mutex; /* protects clock enable/disable */
 	struct clk *core;
-	struct icc_path *memory_path;
-	struct icc_path *imem_path;
-	struct icc_path *config_path;
+	struct ipa_interconnect *interconnect[IPA_INTERCONNECT_COUNT];
 	const struct ipa_interconnect_data *interconnect_data;
 };
 
@@ -71,24 +75,24 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev)
 	path = ipa_interconnect_init_one(dev, "memory");
 	if (IS_ERR(path))
 		goto err_return;
-	clock->memory_path = path;
+	clock->interconnect[IPA_INTERCONNECT_MEMORY]->path = path;
 
 	path = ipa_interconnect_init_one(dev, "imem");
 	if (IS_ERR(path))
 		goto err_memory_path_put;
-	clock->imem_path = path;
+	clock->interconnect[IPA_INTERCONNECT_IMEM]->path = path;
 
 	path = ipa_interconnect_init_one(dev, "config");
 	if (IS_ERR(path))
 		goto err_imem_path_put;
-	clock->config_path = path;
+	clock->interconnect[IPA_INTERCONNECT_CONFIG]->path = path;
 
 	return 0;
 
 err_imem_path_put:
-	icc_put(clock->imem_path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM]->path);
 err_memory_path_put:
-	icc_put(clock->memory_path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path);
 err_return:
 	return PTR_ERR(path);
 }
@@ -96,9 +100,9 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev)
 /* Inverse of ipa_interconnect_init() */
 static void ipa_interconnect_exit(struct ipa_clock *clock)
 {
-	icc_put(clock->config_path);
-	icc_put(clock->imem_path);
-	icc_put(clock->memory_path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_CONFIG]->path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM]->path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path);
 }
 
 /* Currently we only use one bandwidth level, so just "enable" interconnects */
@@ -109,29 +113,31 @@ static int ipa_interconnect_enable(struct ipa *ipa)
 	int ret;
 
 	data = &clock->interconnect_data[IPA_INTERCONNECT_MEMORY];
-	ret = icc_set_bw(clock->memory_path, data->average_bandwidth,
-			 data->peak_bandwidth);
+	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path,
+			 data->average_bandwidth, data->peak_bandwidth);
 	if (ret)
 		return ret;
 
 	data = &clock->interconnect_data[IPA_INTERCONNECT_IMEM];
-	ret = icc_set_bw(clock->imem_path, data->average_bandwidth,
-			 data->peak_bandwidth);
+	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_IMEM]->path,
+			 data->average_bandwidth, data->peak_bandwidth);
 	if (ret)
 		goto err_memory_path_disable;
 
 	data = &clock->interconnect_data[IPA_INTERCONNECT_CONFIG];
-	ret = icc_set_bw(clock->config_path, data->average_bandwidth,
-			 data->peak_bandwidth);
+	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_CONFIG]->path,
+			 data->average_bandwidth, data->peak_bandwidth);
 	if (ret)
 		goto err_imem_path_disable;
 
 	return 0;
 
 err_imem_path_disable:
-	(void)icc_set_bw(clock->imem_path, 0, 0);
+	(void)icc_set_bw(clock->interconnect[IPA_INTERCONNECT_IMEM]->path,
+			 0, 0);
 err_memory_path_disable:
-	(void)icc_set_bw(clock->memory_path, 0, 0);
+	(void)icc_set_bw(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path,
+			 0, 0);
 
 	return ret;
 }
@@ -143,15 +149,18 @@ static void ipa_interconnect_disable(struct ipa *ipa)
 	int result = 0;
 	int ret;
 
-	ret = icc_set_bw(clock->memory_path, 0, 0);
+	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path,
+			 0, 0);
 	if (ret)
 		result = ret;
 
-	ret = icc_set_bw(clock->imem_path, 0, 0);
+	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_IMEM]->path,
+			 0, 0);
 	if (ret && !result)
 		result = ret;
 
-	ret = icc_set_bw(clock->config_path, 0, 0);
+	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_CONFIG]->path,
+			 0, 0);
 	if (ret && !result)
 		result = ret;
 
-- 
2.20.1

[PATCH net-next 4/7] net: ipa: store average and peak interconnect bandwidth

[Alex Elder]

Add fields in the ipa_interconnect structure to hold the average and
peak bandwidth values for the interconnect.  Pass the configuring
data for interconnects to ipa_interconnect_init() so these values
can be recorded, and use them when enabling the interconnects.

There's no longer any need to keep a copy of the interconnect data
after initialization.

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

diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
index 2bf5af6823d8c..537c72b5267f6 100644
--- a/drivers/net/ipa/ipa_clock.c
+++ b/drivers/net/ipa/ipa_clock.c
@@ -33,9 +33,13 @@
 /**
  * struct ipa_interconnect - IPA interconnect information
  * @path:		Interconnect path
+ * @average_bandwidth:	Average interconnect bandwidth (KB/second)
+ * @peak_bandwidth:	Peak interconnect bandwidth (KB/second)
  */
 struct ipa_interconnect {
 	struct icc_path *path;
+	u32 average_bandwidth;
+	u32 peak_bandwidth;
 };
 
 /**
@@ -44,14 +48,12 @@ struct ipa_interconnect {
  * @mutex:		Protects clock enable/disable
  * @core:		IPA core clock
  * @interconnect:	Interconnect array
- * @interconnect_data:	Interconnect configuration data
  */
 struct ipa_clock {
 	refcount_t count;
 	struct mutex mutex; /* protects clock enable/disable */
 	struct clk *core;
-	struct ipa_interconnect *interconnect[IPA_INTERCONNECT_COUNT];
-	const struct ipa_interconnect_data *interconnect_data;
+	struct ipa_interconnect interconnect[IPA_INTERCONNECT_COUNT];
 };
 
 static struct icc_path *
@@ -61,38 +63,52 @@ ipa_interconnect_init_one(struct device *dev, const char *name)
 
 	path = of_icc_get(dev, name);
 	if (IS_ERR(path))
-		dev_err(dev, "error %ld getting %s interconnect\n",
-			PTR_ERR(path), name);
+		dev_err(dev, "error %d getting %s interconnect\n",
+			(int)PTR_ERR(path), name);
 
 	return path;
 }
 
 /* Initialize interconnects required for IPA operation */
-static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev)
+static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev,
+				 const struct ipa_interconnect_data *data)
 {
+	struct ipa_interconnect *interconnect;
 	struct icc_path *path;
 
 	path = ipa_interconnect_init_one(dev, "memory");
 	if (IS_ERR(path))
 		goto err_return;
-	clock->interconnect[IPA_INTERCONNECT_MEMORY]->path = path;
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
+	interconnect->path = path;
+	interconnect->average_bandwidth = data->average_bandwidth;
+	interconnect->peak_bandwidth = data->peak_bandwidth;
+	data++;
 
 	path = ipa_interconnect_init_one(dev, "imem");
 	if (IS_ERR(path))
 		goto err_memory_path_put;
-	clock->interconnect[IPA_INTERCONNECT_IMEM]->path = path;
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	interconnect->path = path;
+	interconnect->average_bandwidth = data->average_bandwidth;
+	interconnect->peak_bandwidth = data->peak_bandwidth;
+	data++;
 
 	path = ipa_interconnect_init_one(dev, "config");
 	if (IS_ERR(path))
 		goto err_imem_path_put;
-	clock->interconnect[IPA_INTERCONNECT_CONFIG]->path = path;
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_CONFIG];
+	interconnect->path = path;
+	interconnect->average_bandwidth = data->average_bandwidth;
+	interconnect->peak_bandwidth = data->peak_bandwidth;
+	data++;
 
 	return 0;
 
 err_imem_path_put:
-	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM]->path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM].path);
 err_memory_path_put:
-	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY].path);
 err_return:
 	return PTR_ERR(path);
 }
@@ -100,44 +116,44 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev)
 /* Inverse of ipa_interconnect_init() */
 static void ipa_interconnect_exit(struct ipa_clock *clock)
 {
-	icc_put(clock->interconnect[IPA_INTERCONNECT_CONFIG]->path);
-	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM]->path);
-	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_CONFIG].path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM].path);
+	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY].path);
 }
 
 /* Currently we only use one bandwidth level, so just "enable" interconnects */
 static int ipa_interconnect_enable(struct ipa *ipa)
 {
-	const struct ipa_interconnect_data *data;
+	struct ipa_interconnect *interconnect;
 	struct ipa_clock *clock = ipa->clock;
 	int ret;
 
-	data = &clock->interconnect_data[IPA_INTERCONNECT_MEMORY];
-	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path,
-			 data->average_bandwidth, data->peak_bandwidth);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
+	ret = icc_set_bw(interconnect->path, interconnect->average_bandwidth,
+			 interconnect->peak_bandwidth);
 	if (ret)
 		return ret;
 
-	data = &clock->interconnect_data[IPA_INTERCONNECT_IMEM];
-	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_IMEM]->path,
-			 data->average_bandwidth, data->peak_bandwidth);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	ret = icc_set_bw(interconnect->path, interconnect->average_bandwidth,
+			 interconnect->peak_bandwidth);
 	if (ret)
 		goto err_memory_path_disable;
 
-	data = &clock->interconnect_data[IPA_INTERCONNECT_CONFIG];
-	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_CONFIG]->path,
-			 data->average_bandwidth, data->peak_bandwidth);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_CONFIG];
+	ret = icc_set_bw(interconnect->path, interconnect->average_bandwidth,
+			 interconnect->peak_bandwidth);
 	if (ret)
 		goto err_imem_path_disable;
 
 	return 0;
 
 err_imem_path_disable:
-	(void)icc_set_bw(clock->interconnect[IPA_INTERCONNECT_IMEM]->path,
-			 0, 0);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	(void)icc_set_bw(interconnect->path, 0, 0);
 err_memory_path_disable:
-	(void)icc_set_bw(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path,
-			 0, 0);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
+	(void)icc_set_bw(interconnect->path, 0, 0);
 
 	return ret;
 }
@@ -145,22 +161,23 @@ static int ipa_interconnect_enable(struct ipa *ipa)
 /* To disable an interconnect, we just its bandwidth to 0 */
 static void ipa_interconnect_disable(struct ipa *ipa)
 {
+	struct ipa_interconnect *interconnect;
 	struct ipa_clock *clock = ipa->clock;
 	int result = 0;
 	int ret;
 
-	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_MEMORY]->path,
-			 0, 0);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
+	ret = icc_set_bw(interconnect->path, 0, 0);
 	if (ret)
 		result = ret;
 
-	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_IMEM]->path,
-			 0, 0);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	ret = icc_set_bw(interconnect->path, 0, 0);
 	if (ret && !result)
 		result = ret;
 
-	ret = icc_set_bw(clock->interconnect[IPA_INTERCONNECT_CONFIG]->path,
-			 0, 0);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	ret = icc_set_bw(interconnect->path, 0, 0);
 	if (ret && !result)
 		result = ret;
 
@@ -286,9 +303,8 @@ ipa_clock_init(struct device *dev, const struct ipa_clock_data *data)
 		goto err_clk_put;
 	}
 	clock->core = clk;
-	clock->interconnect_data = data->interconnect;
 
-	ret = ipa_interconnect_init(clock, dev);
+	ret = ipa_interconnect_init(clock, dev, data->interconnect);
 	if (ret)
 		goto err_kfree;
 
-- 
2.20.1

[PATCH net-next 5/7] net: ipa: add interconnect name to configuration data

[Alex Elder]

Add the name to the configuration data for each interconnect.  Use
this information rather than a constant string during initialization.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_clock.c       | 6 +++---
 drivers/net/ipa/ipa_data-sc7180.c | 3 +++
 drivers/net/ipa/ipa_data-sdm845.c | 3 +++
 drivers/net/ipa/ipa_data.h        | 2 ++
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
index 537c72b5267f6..07069dbc6d033 100644
--- a/drivers/net/ipa/ipa_clock.c
+++ b/drivers/net/ipa/ipa_clock.c
@@ -76,7 +76,7 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev,
 	struct ipa_interconnect *interconnect;
 	struct icc_path *path;
 
-	path = ipa_interconnect_init_one(dev, "memory");
+	path = ipa_interconnect_init_one(dev, data->name);
 	if (IS_ERR(path))
 		goto err_return;
 	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
@@ -85,7 +85,7 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev,
 	interconnect->peak_bandwidth = data->peak_bandwidth;
 	data++;
 
-	path = ipa_interconnect_init_one(dev, "imem");
+	path = ipa_interconnect_init_one(dev, data->name);
 	if (IS_ERR(path))
 		goto err_memory_path_put;
 	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
@@ -94,7 +94,7 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev,
 	interconnect->peak_bandwidth = data->peak_bandwidth;
 	data++;
 
-	path = ipa_interconnect_init_one(dev, "config");
+	path = ipa_interconnect_init_one(dev, data->name);
 	if (IS_ERR(path))
 		goto err_imem_path_put;
 	interconnect = &clock->interconnect[IPA_INTERCONNECT_CONFIG];
diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c
index 491572c0a34dc..1936ecb4c1104 100644
--- a/drivers/net/ipa/ipa_data-sc7180.c
+++ b/drivers/net/ipa/ipa_data-sc7180.c
@@ -314,15 +314,18 @@ static struct ipa_clock_data ipa_clock_data = {
 	/* Interconnect bandwidths are in 1000 byte/second units */
 	.interconnect = {
 		[IPA_INTERCONNECT_MEMORY] = {
+			.name			= "memory",
 			.peak_bandwidth		= 465000,	/* 465 MBps */
 			.average_bandwidth	= 80000,	/* 80 MBps */
 		},
 		/* Average bandwidth unused for the next two interconnects */
 		[IPA_INTERCONNECT_IMEM] = {
+			.name			= "imem",
 			.peak_bandwidth		= 68570,	/* 68.57 MBps */
 			.average_bandwidth	= 0,		/* unused */
 		},
 		[IPA_INTERCONNECT_CONFIG] = {
+			.name			= "config",
 			.peak_bandwidth		= 30000,	/* 30 MBps */
 			.average_bandwidth	= 0,		/* unused */
 		},
diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c
index c62b86171b929..3b556b5a63406 100644
--- a/drivers/net/ipa/ipa_data-sdm845.c
+++ b/drivers/net/ipa/ipa_data-sdm845.c
@@ -334,15 +334,18 @@ static struct ipa_clock_data ipa_clock_data = {
 	/* Interconnect bandwidths are in 1000 byte/second units */
 	.interconnect = {
 		[IPA_INTERCONNECT_MEMORY] = {
+			.name			= "memory",
 			.peak_bandwidth		= 600000,	/* 600 MBps */
 			.average_bandwidth	= 80000,	/* 80 MBps */
 		},
 		/* Average bandwidth unused for the next two interconnects */
 		[IPA_INTERCONNECT_IMEM] = {
+			.name			= "imem",
 			.peak_bandwidth		= 350000,	/* 350 MBps */
 			.average_bandwidth	= 0,		/* unused */
 		},
 		[IPA_INTERCONNECT_CONFIG] = {
+			.name			= "config",
 			.peak_bandwidth		= 40000,	/* 40 MBps */
 			.average_bandwidth	= 0,		/* unused */
 		},
diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
index 96a9771a6cc05..d8ea6266dc6a1 100644
--- a/drivers/net/ipa/ipa_data.h
+++ b/drivers/net/ipa/ipa_data.h
@@ -268,10 +268,12 @@ enum ipa_interconnect_id {
 
 /**
  * struct ipa_interconnect_data - description of IPA interconnect bandwidths
+ * @name:		Interconnect name (matches interconnect-name in DT)
  * @peak_bandwidth:	Peak interconnect bandwidth (in 1000 byte/sec units)
  * @average_bandwidth:	Average interconnect bandwidth (in 1000 byte/sec units)
  */
 struct ipa_interconnect_data {
+	const char *name;
 	u32 peak_bandwidth;
 	u32 average_bandwidth;
 };
-- 
2.20.1

[PATCH net-next 6/7] net: ipa: clean up interconnect initialization

[Alex Elder]

Pass an the address of an IPA interconnect structure and its
configuration data to ipa_interconnect_init_one() and have that
function initialize all the structure's fields.  Change the function
to simply return an error code.

Introduce ipa_interconnect_exit_one() to encapsulate the cleanup of
an IPA interconnect structure.

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

diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
index 07069dbc6d033..fbe42106fc2a8 100644
--- a/drivers/net/ipa/ipa_clock.c
+++ b/drivers/net/ipa/ipa_clock.c
@@ -56,17 +56,33 @@ struct ipa_clock {
 	struct ipa_interconnect interconnect[IPA_INTERCONNECT_COUNT];
 };
 
-static struct icc_path *
-ipa_interconnect_init_one(struct device *dev, const char *name)
+static int ipa_interconnect_init_one(struct device *dev,
+				     struct ipa_interconnect *interconnect,
+				     const struct ipa_interconnect_data *data)
 {
 	struct icc_path *path;
 
-	path = of_icc_get(dev, name);
-	if (IS_ERR(path))
-		dev_err(dev, "error %d getting %s interconnect\n",
-			(int)PTR_ERR(path), name);
+	path = of_icc_get(dev, data->name);
+	if (IS_ERR(path)) {
+		int ret = PTR_ERR(path);
 
-	return path;
+		dev_err(dev, "error %d getting %s interconnect\n", ret,
+			data->name);
+
+		return ret;
+	}
+
+	interconnect->path = path;
+	interconnect->average_bandwidth = data->average_bandwidth;
+	interconnect->peak_bandwidth = data->peak_bandwidth;
+
+	return 0;
+}
+
+static void ipa_interconnect_exit_one(struct ipa_interconnect *interconnect)
+{
+	icc_put(interconnect->path);
+	memset(interconnect, 0, sizeof(*interconnect));
 }
 
 /* Initialize interconnects required for IPA operation */
@@ -74,51 +90,46 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev,
 				 const struct ipa_interconnect_data *data)
 {
 	struct ipa_interconnect *interconnect;
-	struct icc_path *path;
+	int ret;
 
-	path = ipa_interconnect_init_one(dev, data->name);
-	if (IS_ERR(path))
-		goto err_return;
 	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
-	interconnect->path = path;
-	interconnect->average_bandwidth = data->average_bandwidth;
-	interconnect->peak_bandwidth = data->peak_bandwidth;
-	data++;
+	ret = ipa_interconnect_init_one(dev, interconnect, data++);
+	if (ret)
+		return ret;
 
-	path = ipa_interconnect_init_one(dev, data->name);
-	if (IS_ERR(path))
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	ret = ipa_interconnect_init_one(dev, interconnect, data++);
+	if (ret)
 		goto err_memory_path_put;
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	interconnect->path = path;
-	interconnect->average_bandwidth = data->average_bandwidth;
-	interconnect->peak_bandwidth = data->peak_bandwidth;
-	data++;
 
-	path = ipa_interconnect_init_one(dev, data->name);
-	if (IS_ERR(path))
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	ret = ipa_interconnect_init_one(dev, interconnect, data++);
+	if (ret)
 		goto err_imem_path_put;
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_CONFIG];
-	interconnect->path = path;
-	interconnect->average_bandwidth = data->average_bandwidth;
-	interconnect->peak_bandwidth = data->peak_bandwidth;
-	data++;
 
 	return 0;
 
 err_imem_path_put:
-	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM].path);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	ipa_interconnect_exit_one(interconnect);
 err_memory_path_put:
-	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY].path);
-err_return:
-	return PTR_ERR(path);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
+	ipa_interconnect_exit_one(interconnect);
+
+	return ret;
 }
 
 /* Inverse of ipa_interconnect_init() */
 static void ipa_interconnect_exit(struct ipa_clock *clock)
 {
-	icc_put(clock->interconnect[IPA_INTERCONNECT_CONFIG].path);
-	icc_put(clock->interconnect[IPA_INTERCONNECT_IMEM].path);
-	icc_put(clock->interconnect[IPA_INTERCONNECT_MEMORY].path);
+	struct ipa_interconnect *interconnect;
+
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_CONFIG];
+	ipa_interconnect_exit_one(interconnect);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
+	ipa_interconnect_exit_one(interconnect);
+	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
+	ipa_interconnect_exit_one(interconnect);
 }
 
 /* Currently we only use one bandwidth level, so just "enable" interconnects */
-- 
2.20.1

[PATCH net-next 7/7] net: ipa: allow arbitrary number of interconnects

[Alex Elder]

Currently we assume that the IPA hardware has exactly three
interconnects.  But that won't be guaranteed for all platforms,
so allow any number of interconnects to be specified in the
configuration data.

For each platform, define an array of interconnect data entries
(still associated with the IPA clock structure), and record the
number of entries initialized in that array.

Loop over all entries in this array when initializing, enabling,
disabling, or tearing down the set of interconnects.

With this change we no longer need the ipa_interconnect_id
enumerated type, so get rid of it.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/net/ipa/ipa_clock.c       | 113 +++++++++++++-----------------
 drivers/net/ipa/ipa_data-sc7180.c |  41 ++++++-----
 drivers/net/ipa/ipa_data-sdm845.c |  41 ++++++-----
 drivers/net/ipa/ipa_data.h        |  14 ++--
 4 files changed, 97 insertions(+), 112 deletions(-)

diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
index fbe42106fc2a8..354675a643db5 100644
--- a/drivers/net/ipa/ipa_clock.c
+++ b/drivers/net/ipa/ipa_clock.c
@@ -47,13 +47,15 @@ struct ipa_interconnect {
  * @count:		Clocking reference count
  * @mutex:		Protects clock enable/disable
  * @core:		IPA core clock
+ * @interconnect_count:	Number of elements in interconnect[]
  * @interconnect:	Interconnect array
  */
 struct ipa_clock {
 	refcount_t count;
 	struct mutex mutex; /* protects clock enable/disable */
 	struct clk *core;
-	struct ipa_interconnect interconnect[IPA_INTERCONNECT_COUNT];
+	u32 interconnect_count;
+	struct ipa_interconnect *interconnect;
 };
 
 static int ipa_interconnect_init_one(struct device *dev,
@@ -90,31 +92,29 @@ static int ipa_interconnect_init(struct ipa_clock *clock, struct device *dev,
 				 const struct ipa_interconnect_data *data)
 {
 	struct ipa_interconnect *interconnect;
+	u32 count;
 	int ret;
 
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
-	ret = ipa_interconnect_init_one(dev, interconnect, data++);
-	if (ret)
-		return ret;
+	count = clock->interconnect_count;
+	interconnect = kcalloc(count, sizeof(*interconnect), GFP_KERNEL);
+	if (!interconnect)
+		return -ENOMEM;
+	clock->interconnect = interconnect;
 
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	ret = ipa_interconnect_init_one(dev, interconnect, data++);
-	if (ret)
-		goto err_memory_path_put;
-
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	ret = ipa_interconnect_init_one(dev, interconnect, data++);
-	if (ret)
-		goto err_imem_path_put;
+	while (count--) {
+		ret = ipa_interconnect_init_one(dev, interconnect, data++);
+		if (ret)
+			goto out_unwind;
+		interconnect++;
+	}
 
 	return 0;
 
-err_imem_path_put:
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	ipa_interconnect_exit_one(interconnect);
-err_memory_path_put:
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
-	ipa_interconnect_exit_one(interconnect);
+out_unwind:
+	while (interconnect-- > clock->interconnect)
+		ipa_interconnect_exit_one(interconnect);
+	kfree(clock->interconnect);
+	clock->interconnect = NULL;
 
 	return ret;
 }
@@ -124,12 +124,11 @@ static void ipa_interconnect_exit(struct ipa_clock *clock)
 {
 	struct ipa_interconnect *interconnect;
 
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_CONFIG];
-	ipa_interconnect_exit_one(interconnect);
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	ipa_interconnect_exit_one(interconnect);
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
-	ipa_interconnect_exit_one(interconnect);
+	interconnect = clock->interconnect + clock->interconnect_count;
+	while (interconnect-- > clock->interconnect)
+		ipa_interconnect_exit_one(interconnect);
+	kfree(clock->interconnect);
+	clock->interconnect = NULL;
 }
 
 /* Currently we only use one bandwidth level, so just "enable" interconnects */
@@ -138,33 +137,23 @@ static int ipa_interconnect_enable(struct ipa *ipa)
 	struct ipa_interconnect *interconnect;
 	struct ipa_clock *clock = ipa->clock;
 	int ret;
+	u32 i;
 
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
-	ret = icc_set_bw(interconnect->path, interconnect->average_bandwidth,
-			 interconnect->peak_bandwidth);
-	if (ret)
-		return ret;
-
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	ret = icc_set_bw(interconnect->path, interconnect->average_bandwidth,
-			 interconnect->peak_bandwidth);
-	if (ret)
-		goto err_memory_path_disable;
-
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_CONFIG];
-	ret = icc_set_bw(interconnect->path, interconnect->average_bandwidth,
-			 interconnect->peak_bandwidth);
-	if (ret)
-		goto err_imem_path_disable;
+	interconnect = clock->interconnect;
+	for (i = 0; i < clock->interconnect_count; i++) {
+		ret = icc_set_bw(interconnect->path,
+				 interconnect->average_bandwidth,
+				 interconnect->peak_bandwidth);
+		if (ret)
+			goto out_unwind;
+		interconnect++;
+	}
 
 	return 0;
 
-err_imem_path_disable:
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	(void)icc_set_bw(interconnect->path, 0, 0);
-err_memory_path_disable:
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
-	(void)icc_set_bw(interconnect->path, 0, 0);
+out_unwind:
+	while (interconnect-- > clock->interconnect)
+		(void)icc_set_bw(interconnect->path, 0, 0);
 
 	return ret;
 }
@@ -175,22 +164,17 @@ static void ipa_interconnect_disable(struct ipa *ipa)
 	struct ipa_interconnect *interconnect;
 	struct ipa_clock *clock = ipa->clock;
 	int result = 0;
+	u32 count;
 	int ret;
 
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_MEMORY];
-	ret = icc_set_bw(interconnect->path, 0, 0);
-	if (ret)
-		result = ret;
-
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	ret = icc_set_bw(interconnect->path, 0, 0);
-	if (ret && !result)
-		result = ret;
-
-	interconnect = &clock->interconnect[IPA_INTERCONNECT_IMEM];
-	ret = icc_set_bw(interconnect->path, 0, 0);
-	if (ret && !result)
-		result = ret;
+	count = clock->interconnect_count;
+	interconnect = clock->interconnect + count;
+	while (count--) {
+		interconnect--;
+		ret = icc_set_bw(interconnect->path, 0, 0);
+		if (ret && !result)
+			result = ret;
+	}
 
 	if (result)
 		dev_err(&ipa->pdev->dev,
@@ -314,8 +298,9 @@ ipa_clock_init(struct device *dev, const struct ipa_clock_data *data)
 		goto err_clk_put;
 	}
 	clock->core = clk;
+	clock->interconnect_count = data->interconnect_count;
 
-	ret = ipa_interconnect_init(clock, dev, data->interconnect);
+	ret = ipa_interconnect_init(clock, dev, data->interconnect_data);
 	if (ret)
 		goto err_kfree;
 
diff --git a/drivers/net/ipa/ipa_data-sc7180.c b/drivers/net/ipa/ipa_data-sc7180.c
index 1936ecb4c1104..997b51ceb7d76 100644
--- a/drivers/net/ipa/ipa_data-sc7180.c
+++ b/drivers/net/ipa/ipa_data-sc7180.c
@@ -309,27 +309,30 @@ static struct ipa_mem_data ipa_mem_data = {
 	.smem_size	= 0x00002000,
 };
 
+/* Interconnect bandwidths are in 1000 byte/second units */
+static struct ipa_interconnect_data ipa_interconnect_data[] = {
+	{
+		.name			= "memory",
+		.peak_bandwidth		= 465000,	/* 465 MBps */
+		.average_bandwidth	= 80000,	/* 80 MBps */
+	},
+	/* Average bandwidth is unused for the next two interconnects */
+	{
+		.name			= "imem",
+		.peak_bandwidth		= 68570,	/* 68.570 MBps */
+		.average_bandwidth	= 0,		/* unused */
+	},
+	{
+		.name			= "config",
+		.peak_bandwidth		= 30000,	/* 30 MBps */
+		.average_bandwidth	= 0,		/* unused */
+	},
+};
+
 static struct ipa_clock_data ipa_clock_data = {
 	.core_clock_rate	= 100 * 1000 * 1000,	/* Hz */
-	/* Interconnect bandwidths are in 1000 byte/second units */
-	.interconnect = {
-		[IPA_INTERCONNECT_MEMORY] = {
-			.name			= "memory",
-			.peak_bandwidth		= 465000,	/* 465 MBps */
-			.average_bandwidth	= 80000,	/* 80 MBps */
-		},
-		/* Average bandwidth unused for the next two interconnects */
-		[IPA_INTERCONNECT_IMEM] = {
-			.name			= "imem",
-			.peak_bandwidth		= 68570,	/* 68.57 MBps */
-			.average_bandwidth	= 0,		/* unused */
-		},
-		[IPA_INTERCONNECT_CONFIG] = {
-			.name			= "config",
-			.peak_bandwidth		= 30000,	/* 30 MBps */
-			.average_bandwidth	= 0,		/* unused */
-		},
-	},
+	.interconnect_count	= ARRAY_SIZE(ipa_interconnect_data),
+	.interconnect_data	= ipa_interconnect_data,
 };
 
 /* Configuration data for the SC7180 SoC. */
diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c
index 3b556b5a63406..88c9c3562ab79 100644
--- a/drivers/net/ipa/ipa_data-sdm845.c
+++ b/drivers/net/ipa/ipa_data-sdm845.c
@@ -329,27 +329,30 @@ static struct ipa_mem_data ipa_mem_data = {
 	.smem_size	= 0x00002000,
 };
 
+/* Interconnect bandwidths are in 1000 byte/second units */
+static struct ipa_interconnect_data ipa_interconnect_data[] = {
+	{
+		.name			= "memory",
+		.peak_bandwidth		= 600000,	/* 600 MBps */
+		.average_bandwidth	= 80000,	/* 80 MBps */
+	},
+	/* Average bandwidth is unused for the next two interconnects */
+	{
+		.name			= "imem",
+		.peak_bandwidth		= 350000,	/* 350 MBps */
+		.average_bandwidth	= 0,		/* unused */
+	},
+	{
+		.name			= "config",
+		.peak_bandwidth		= 40000,	/* 40 MBps */
+		.average_bandwidth	= 0,		/* unused */
+	},
+};
+
 static struct ipa_clock_data ipa_clock_data = {
 	.core_clock_rate	= 75 * 1000 * 1000,	/* Hz */
-	/* Interconnect bandwidths are in 1000 byte/second units */
-	.interconnect = {
-		[IPA_INTERCONNECT_MEMORY] = {
-			.name			= "memory",
-			.peak_bandwidth		= 600000,	/* 600 MBps */
-			.average_bandwidth	= 80000,	/* 80 MBps */
-		},
-		/* Average bandwidth unused for the next two interconnects */
-		[IPA_INTERCONNECT_IMEM] = {
-			.name			= "imem",
-			.peak_bandwidth		= 350000,	/* 350 MBps */
-			.average_bandwidth	= 0,		/* unused */
-		},
-		[IPA_INTERCONNECT_CONFIG] = {
-			.name			= "config",
-			.peak_bandwidth		= 40000,	/* 40 MBps */
-			.average_bandwidth	= 0,		/* unused */
-		},
-	},
+	.interconnect_count	= ARRAY_SIZE(ipa_interconnect_data),
+	.interconnect_data	= ipa_interconnect_data,
 };
 
 /* Configuration data for the SDM845 SoC. */
diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
index d8ea6266dc6a1..b476fc373f7fe 100644
--- a/drivers/net/ipa/ipa_data.h
+++ b/drivers/net/ipa/ipa_data.h
@@ -258,14 +258,6 @@ struct ipa_mem_data {
 	u32 smem_size;
 };
 
-/** enum ipa_interconnect_id - IPA interconnect identifier */
-enum ipa_interconnect_id {
-	IPA_INTERCONNECT_MEMORY,
-	IPA_INTERCONNECT_IMEM,
-	IPA_INTERCONNECT_CONFIG,
-	IPA_INTERCONNECT_COUNT,		/* Last; not an interconnect */
-};
-
 /**
  * struct ipa_interconnect_data - description of IPA interconnect bandwidths
  * @name:		Interconnect name (matches interconnect-name in DT)
@@ -281,11 +273,13 @@ struct ipa_interconnect_data {
 /**
  * struct ipa_clock_data - description of IPA clock and interconnect rates
  * @core_clock_rate:	Core clock rate (Hz)
- * @interconnect:	Array of interconnect bandwidth parameters
+ * @interconnect_count:	Number of entries in the interconnect_data array
+ * @interconnect_data:	IPA interconnect configuration data
  */
 struct ipa_clock_data {
 	u32 core_clock_rate;
-	struct ipa_interconnect_data interconnect[IPA_INTERCONNECT_COUNT];
+	u32 interconnect_count;		/* # entries in interconnect_data[] */
+	const struct ipa_interconnect_data *interconnect_data;
 };
 
 /**
-- 
2.20.1

Re: [PATCH net-next 7/7] net: ipa: allow arbitrary number of interconnects

[Jakub Kicinski]

On Fri, 15 Jan 2021 06:50:50 -0600 Alex Elder wrote:
> Currently we assume that the IPA hardware has exactly three
> interconnects.  But that won't be guaranteed for all platforms,
> so allow any number of interconnects to be specified in the
> configuration data.
> 
> For each platform, define an array of interconnect data entries
> (still associated with the IPA clock structure), and record the
> number of entries initialized in that array.
> 
> Loop over all entries in this array when initializing, enabling,
> disabling, or tearing down the set of interconnects.
> 
> With this change we no longer need the ipa_interconnect_id
> enumerated type, so get rid of it.

Okay, all the platforms supported as of the end of the series 
still have 3 interconnects, or there is no upstream user of 
this functionality, if you will. What's the story?

Re: [PATCH net-next 7/7] net: ipa: allow arbitrary number of interconnects

[Alex Elder]

On 1/16/21 9:12 PM, Jakub Kicinski wrote:
> On Fri, 15 Jan 2021 06:50:50 -0600 Alex Elder wrote:
>> Currently we assume that the IPA hardware has exactly three
>> interconnects.  But that won't be guaranteed for all platforms,
>> so allow any number of interconnects to be specified in the
>> configuration data.
>>
>> For each platform, define an array of interconnect data entries
>> (still associated with the IPA clock structure), and record the
>> number of entries initialized in that array.
>>
>> Loop over all entries in this array when initializing, enabling,
>> disabling, or tearing down the set of interconnects.
>>
>> With this change we no longer need the ipa_interconnect_id
>> enumerated type, so get rid of it.
> 
> Okay, all the platforms supported as of the end of the series
> still have 3 interconnects, or there is no upstream user of
> this functionality, if you will. What's the story?

The short answer is that there is another version of IPA that
has four interconnects instead of three.  (A DTS file for it is
in "sdxprairie.dtsi" in the Qualcomm "downstream" code.)  I hope
to have that version supported this year, but it's not my top
priority right now.  Generalizing the interconnect definitions
as done in this series improves the driver, but you're right, it
is technically not required at this time.

And some more background:
The upstream IPA driver is derived from the Qualcomm downstream
code published at codeaurora.org.  The downstream driver is huge
(it's well over 100,000 lines of code) and it supports lots of
IPA versions and some features that are not present upstream.

In order to have any hope of getting upstream support for the
IPA hardware, the downstream driver functionality was reduced,
removing support for filtering, routing, and NAT.  I spent many
months refactoring and reworking that code to make it more
"upstreamable," and eventually posted the result for review.

Now that there is an upstream driver, a long term goal is to
add back functionality that got removed, matching the most
important features and hardware support found in the downstream
code.  So in cases like this, even though this feature is not
yet required, adding it now lays groundwork to make later work
easier.

Everything I do with the upstream IPA driver is aimed at in-tree
support for additional IPA features and hardware versions.  So
even if an improvement isn't required *now*, there is at least
a general plan to add support "soon" for something that will
need it.

Beyond even that, there are some things I intend to do that
will improve the driver, even if they aren't technically
required near-term.  For example, I'd like to dynamically
allocate endpoints based on what's needed, rather than
having the driver support any number of them up to a maximum
fixed at build time.

Probably a longer response than you needed, but I thought it
would help to provide more background.  Besides, you *did* ask
for "the story..."

					-Alex

Re: [PATCH net-next 7/7] net: ipa: allow arbitrary number of interconnects

[Jakub Kicinski]

On Sun, 17 Jan 2021 10:03:41 -0600 Alex Elder wrote:
> On 1/16/21 9:12 PM, Jakub Kicinski wrote:
> > On Fri, 15 Jan 2021 06:50:50 -0600 Alex Elder wrote:  
> >> Currently we assume that the IPA hardware has exactly three
> >> interconnects.  But that won't be guaranteed for all platforms,
> >> so allow any number of interconnects to be specified in the
> >> configuration data.
> >>
> >> For each platform, define an array of interconnect data entries
> >> (still associated with the IPA clock structure), and record the
> >> number of entries initialized in that array.
> >>
> >> Loop over all entries in this array when initializing, enabling,
> >> disabling, or tearing down the set of interconnects.
> >>
> >> With this change we no longer need the ipa_interconnect_id
> >> enumerated type, so get rid of it.  
> > 
> > Okay, all the platforms supported as of the end of the series
> > still have 3 interconnects, or there is no upstream user of
> > this functionality, if you will. What's the story?  
> 
> The short answer is that there is another version of IPA that
> has four interconnects instead of three.  (A DTS file for it is
> in "sdxprairie.dtsi" in the Qualcomm "downstream" code.)  I hope
> to have that version supported this year, but it's not my top
> priority right now.  Generalizing the interconnect definitions
> as done in this series improves the driver, but you're right, it
> is technically not required at this time.
> 
> And some more background:
> The upstream IPA driver is derived from the Qualcomm downstream
> code published at codeaurora.org.  The downstream driver is huge
> (it's well over 100,000 lines of code) and it supports lots of
> IPA versions and some features that are not present upstream.
> 
> In order to have any hope of getting upstream support for the
> IPA hardware, the downstream driver functionality was reduced,
> removing support for filtering, routing, and NAT.  I spent many
> months refactoring and reworking that code to make it more
> "upstreamable," and eventually posted the result for review.
> 
> Now that there is an upstream driver, a long term goal is to
> add back functionality that got removed, matching the most
> important features and hardware support found in the downstream
> code.  So in cases like this, even though this feature is not
> yet required, adding it now lays groundwork to make later work
> easier.
> 
> Everything I do with the upstream IPA driver is aimed at in-tree
> support for additional IPA features and hardware versions.  So
> even if an improvement isn't required *now*, there is at least
> a general plan to add support "soon" for something that will
> need it.
> 
> Beyond even that, there are some things I intend to do that
> will improve the driver, even if they aren't technically
> required near-term.  For example, I'd like to dynamically
> allocate endpoints based on what's needed, rather than
> having the driver support any number of them up to a maximum
> fixed at build time.
> 
> Probably a longer response than you needed, but I thought it
> would help to provide more background.  Besides, you *did* ask
> for "the story..."

Thanks, I think I get it now.

But it does sound a little too much like aligning with the vendor
driver for the sake of aligning with the vendor driver. This makes 
the review for someone not familiar with the vendor driver hard, 
and raises questions like "is this really needed upstream or just
downstream / out-of-tree". Please try to reorient the work towards
implementing particular pieces of functionality upstream start to end. 

Applied, with a warning :)

Re: [PATCH net-next 7/7] net: ipa: allow arbitrary number of interconnects

[Alex Elder]

On 1/18/21 1:58 PM, Jakub Kicinski wrote:
> But it does sound a little too much like aligning with the vendor
> driver for the sake of aligning with the vendor driver. This makes 
> the review for someone not familiar with the vendor driver hard, 
> and raises questions like "is this really needed upstream or just
> downstream / out-of-tree". Please try to reorient the work towards
> implementing particular pieces of functionality upstream start to end. 

I have lots of this kind of preparatory work "ready to go" and I'm
eager to get it upstream.  But I'll try to be more patient and wait
to send things out until something justifies the need, rather than
just doing it in support of a longer-term goal.

I would not consider what I'm doing "aligning with the vendor
driver," but maybe I misunderstand your meaning.  The upstream
driver is very different from Qualcomm's driver.  I'm basically
writing all new code now, using the Qualcomm code for reference.

Thank you very much for applying the patches.

					-Alex