[PATCH v2 0/2] interconnect: qcom: msm8939: Coalesce snoc and snoc_mm

[PATCH v2 0/2] interconnect: qcom: msm8939: Coalesce snoc and snoc_mm

[Bryan O'Donoghue]

V2:
Made snoc-mm into a child of snoc - Dmitry
Removes redundant msm8939_snoc_mm_regmap_config - bod

V1:
Booting msm8939 on tip-of-tree I encountered the following error.

[    1.212340] qnoc-msm8939 580000.interconnect_mm: can't request region for resource [mem 0x00580000-0x0059407f]
[    1.212391] qnoc-msm8939 580000.interconnect_mm: Cannot ioremap interconnect bus resource
[    1.221524] qnoc-msm8939: probe of 580000.interconnect_mm failed with error -16

Initially I thought this was a bug with the interconnect driver but,
examining it a bit more I realized the DTS I was working with based on
downstream, declares snoc and snoc_mm as existing at the same address
range.

When we were developing the DTS for 8939 we weren't using the common rpm
interconnect driver so we never saw the ioremap collision.

Taking a hard look at the qcom documentation as well as the downstream code
we see that yes downstream declares snoc and snoc_mm separately but, also
at the same overlapping address.

The qcom documentation for performance points for msm8936/msm8939 snoc,
deliniates snoc_mm as simply two new performance-points i.e. a faster GPLL0
vote associated with new multi-media devices attached to the snoc.

In other words the snoc had two new RPM vote indices added to it, to
represent the higher performance clocks, should one of the multi-media IP
blocks call for it.

We can fix the ioremap collision and still represent the two higher
performance point clock votes by coalsecing snoc and snoc_mm into snoc. The
DTS clock references will take care of the appropriate votes.

Bryan O'Donoghue (2):
  dt-bindings: interconnect: Convert snoc-mm to a sub-node of snoc
  interconnect: qcom: msm8939: Remove snoc_mm specific regmap

 .../bindings/interconnect/qcom,rpm.yaml       | 52 ++++++++++++++++++-
 drivers/interconnect/qcom/msm8939.c           | 10 +---
 2 files changed, 51 insertions(+), 11 deletions(-)

-- 
2.33.0

[PATCH v2 1/2] dt-bindings: interconnect: Convert snoc-mm to a sub-node of snoc

[Bryan O'Donoghue]

snoc and snoc-mm in downstream use the same address space a mistake which
we have carried over into upstream. In silicon terms snoc-mm has been
tacked on to the original snoc which came from msm8916.

Convert to a better description where snoc-mm is a sub-node of snoc, thus
avoiding a mmio remap collision when using the RPM interconnect driver.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../bindings/interconnect/qcom,rpm.yaml       | 33 +++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml b/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml
index e4c3c2818119e..89853b4825133 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,rpm.yaml
@@ -26,7 +26,6 @@ properties:
       - qcom,msm8939-bimc
       - qcom,msm8939-pcnoc
       - qcom,msm8939-snoc
-      - qcom,msm8939-snoc-mm
       - qcom,msm8996-a0noc
       - qcom,msm8996-a1noc
       - qcom,msm8996-a2noc
@@ -80,7 +79,6 @@ allOf:
               - qcom,msm8939-bimc
               - qcom,msm8939-pcnoc
               - qcom,msm8939-snoc
-              - qcom,msm8939-snoc-mm
               - qcom,msm8996-a1noc
               - qcom,msm8996-a2noc
               - qcom,msm8996-bimc
@@ -107,6 +105,37 @@ allOf:
               - description: Bus Clock
               - description: Bus A Clock
 
+        # Child node's properties
+        patternProperties:
+          '^interconnect-[a-z0-9]+$':
+            type: object
+            description:
+              snoc-mm is a child of snoc, sharing snoc's register address space.
+
+            properties:
+              compatible:
+                enum:
+                  - qcom,msm8939-snoc-mm
+
+              '#interconnect-cells':
+                const: 1
+
+              clock-names:
+                items:
+                  - const: bus
+                  - const: bus_a
+
+              clocks:
+                items:
+                  - description: Bus Clock
+                  - description: Bus A Clock
+
+            required:
+              - compatible
+              - '#interconnect-cells'
+              - clock-names
+              - clocks
+
   - if:
       properties:
         compatible:
-- 
2.33.0

Re: [PATCH v2 1/2] dt-bindings: interconnect: Convert snoc-mm to a sub-node of snoc

[Rob Herring]

On Sat, 29 Jan 2022 03:27:34 +0000, Bryan O'Donoghue wrote:
> snoc and snoc-mm in downstream use the same address space a mistake which
> we have carried over into upstream. In silicon terms snoc-mm has been
> tacked on to the original snoc which came from msm8916.
> 
> Convert to a better description where snoc-mm is a sub-node of snoc, thus
> avoiding a mmio remap collision when using the RPM interconnect driver.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../bindings/interconnect/qcom,rpm.yaml       | 33 +++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v2 2/2] interconnect: qcom: msm8939: Remove snoc_mm specific regmap

[Bryan O'Donoghue]

Booting tip-of-tree on msm8939 we find the following error caused by our
modelling of snoc and snoc_mm as peer devices with the same address space.

[    1.212340] qnoc-msm8939 580000.interconnect_mm: can't request region for resource [mem 0x00580000-0x0059407f]
[    1.212391] qnoc-msm8939 580000.interconnect_mm: Cannot ioremap interconnect bus resource
[    1.221524] qnoc-msm8939: probe of 580000.interconnect_mm failed with error -16

Declaring snoc_mm as a child device of snoc mitigates the fault and is
consistent with a similar extant implementation in qcm2290.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/interconnect/qcom/msm8939.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c
index d188f3636e4c3..f9c2d7d3100dd 100644
--- a/drivers/interconnect/qcom/msm8939.c
+++ b/drivers/interconnect/qcom/msm8939.c
@@ -1301,19 +1301,11 @@ static struct qcom_icc_node *msm8939_snoc_mm_nodes[] = {
 	[SNOC_MM_INT_2] = &mm_int_2,
 };
 
-static const struct regmap_config msm8939_snoc_mm_regmap_config = {
-	.reg_bits	= 32,
-	.reg_stride	= 4,
-	.val_bits	= 32,
-	.max_register	= 0x14080,
-	.fast_io	= true,
-};
-
 static struct qcom_icc_desc msm8939_snoc_mm = {
 	.type = QCOM_ICC_NOC,
 	.nodes = msm8939_snoc_mm_nodes,
 	.num_nodes = ARRAY_SIZE(msm8939_snoc_mm_nodes),
-	.regmap_cfg = &msm8939_snoc_mm_regmap_config,
+	.regmap_cfg = &msm8939_snoc_regmap_config,
 	.qos_offset = 0x7000,
 };
 
-- 
2.33.0