Message-Id: <20190925054133.206992-1-swboyd@chromium.org>
Date:   Tue, 24 Sep 2019 22:41:33 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Georgi Djakov <georgi.djakov@...aro.org>
Cc:     linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Maxime Ripard <mripard@...nel.org>, linux-pm@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Evan Green <evgreen@...omium.org>,
        David Dai <daidavid1@...eaurora.org>
Subject: [RFC PATCH] interconnect: Replace of_icc_get() with icc_get() and reduce DT binding

I don't see any users of icc_get() in the kernel today, and adding them
doesn't make sense. That's because adding calls to that function in a
driver will make the driver SoC specific given that the arguments are
some sort of source and destination numbers that would typically be
listed in DT or come from platform data so they can match a global
numberspace of interconnect numbers. It would be better to follow the
approach of other kernel frameworks where the API is the same no matter
how the platform is described (i.e. platform data, DT, ACPI, etc.) and
swizzle the result in the framework to match whatever the device is by
checking for a DT node pointer or a fwnode pointer, etc. Therefore,
install icc_get() as the defacto API and make drivers use that instead
of of_icc_get() which implies the driver is DT specific when it doesn't
need to be.

The DT binding could also be simplified somewhat. Currently a path needs
to be specified in DT for each and every use case that is possible for a
device to want. Typically the path is to memory, which looks to be
reserved for in the binding with the "dma-mem" named path, but sometimes
the path is from a device to the CPU or more generically from a device
to another device which could be a CPU, cache, DMA master, or another
device if some sort of DMA to DMA scenario is happening. Let's remove
the pair part of the binding so that we just list out a device's
possible endpoints on the bus or busses that it's connected to.

If the kernel wants to figure out what the path is to memory or the CPU
or a cache or something else it should be able to do that by finding the
node for the "destination" endpoint, extracting that node's
"interconnects" property, and deriving the path in software. For
example, we shouldn't need to write out each use case path by path in DT
for each endpoint node that wants to set a bandwidth to memory. We
should just be able to indicate what endpoint(s) a device sits on based
on the interconnect provider in the system and then walk the various
interconnects to find the path from that source endpoint to the
destination endpoint.

Obviously this patch doesn't compile but I'm sending it out to start
this discussion so we don't get stuck on the binding or the kernel APIs
for a long time. It looks like we should be OK in terms of backwards
compatibility because we can just ignore the second element in an old
binding, but maybe we'll want to describe paths in different directions
(e.g. the path from the CPU to the SD controller may be different than
the path the SD controller takes to the CPU) and that may require
extending interconnect-names to indicate what direction/sort of path it
is. I'm basically thinking about master vs. slave ports in AXI land.

Cc: Maxime Ripard <mripard@...nel.org>
Cc: <linux-pm@...r.kernel.org>
Cc: Rob Herring <robh+dt@...nel.org>
Cc: <devicetree@...r.kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@...aro.org>
Cc: Evan Green <evgreen@...omium.org>
Cc: David Dai <daidavid1@...eaurora.org>
Signed-off-by: Stephen Boyd <swboyd@...omium.org>
---
 .../bindings/interconnect/interconnect.txt    | 19 ++++---------------
 include/linux/interconnect.h                  | 13 ++-----------
 2 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt
index 6f5d23a605b7..f8979186b8a7 100644
--- a/Documentation/devicetree/bindings/interconnect/interconnect.txt
+++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt
@@ -11,7 +11,7 @@ The interconnect provider binding is intended to represent the interconnect
 controllers in the system. Each provider registers a set of interconnect
 nodes, which expose the interconnect related capabilities of the interconnect
 to consumer drivers. These capabilities can be throughput, latency, priority
-etc. The consumer drivers set constraints on interconnect path (or endpoints)
+etc. The consumer drivers set constraints on interconnect paths (or endpoints)
 depending on the use case. Interconnect providers can also be interconnect
 consumers, such as in the case where two network-on-chip fabrics interface
 directly.
@@ -42,23 +42,12 @@ multiple paths from different providers depending on use case and the
 components it has to interact with.
 
 Required properties:
-interconnects : Pairs of phandles and interconnect provider specifier to denote
-	        the edge source and destination ports of the interconnect path.
-
-Optional properties:
-interconnect-names : List of interconnect path name strings sorted in the same
-		     order as the interconnects property. Consumers drivers will use
-		     interconnect-names to match interconnect paths with interconnect
-		     specifier pairs.
-
-                     Reserved interconnect names:
-			 * dma-mem: Path from the device to the main memory of
-			            the system
+interconnects : phandle and interconnect provider specifier to denote
+	        the edge source for this node.
 
 Example:
 
 	sdhci@...4000 {
 		...
-		interconnects = <&pnoc MASTER_SDCC_1 &bimc SLAVE_EBI_CH0>;
-		interconnect-names = "sdhc-mem";
+		interconnects = <&pnoc MASTER_SDCC_1>;
 	};
diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
index d70a914cba11..e1ae704f5ab1 100644
--- a/include/linux/interconnect.h
+++ b/include/linux/interconnect.h
@@ -25,23 +25,14 @@ struct device;
 
 #if IS_ENABLED(CONFIG_INTERCONNECT)
 
-struct icc_path *icc_get(struct device *dev, const int src_id,
-			 const int dst_id);
-struct icc_path *of_icc_get(struct device *dev, const char *name);
+struct icc_path *icc_get(struct device *dev, const char *name);
 void icc_put(struct icc_path *path);
 int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
 void icc_set_tag(struct icc_path *path, u32 tag);
 
 #else
 
-static inline struct icc_path *icc_get(struct device *dev, const int src_id,
-				       const int dst_id)
-{
-	return NULL;
-}
-
-static inline struct icc_path *of_icc_get(struct device *dev,
-					  const char *name)
+static inline struct icc_path *icc_get(struct device *dev, const char *name)
 {
 	return NULL;
 }

base-commit: b5b3bd898ba99fb0fb6aed3b23ec6353a1724d6f
-- 
Sent by a computer through tubes

Powered by blists - more mailing lists