Skip to contentSkip to conversation

Add support for replicators which loses context on clock removal

PATCH In review Sai Prakash Ranjan 3 participants 8 messages May 22, 2020 – May 28, 2020

Subsystems (4)
Device tree bindings Arm Qualcomm mailing list Arm/CoreSight framework and drivers Hardware tracing facilities
lore
Share & tools
lore

Sai Prakash Ranjan

author
#0 lore raw

v3 cover Add support for replicators which loses context on clock removal

This series is mainly to add support for replicators which lose context on removing AMBA clock like on SC7180 SoC where replicator in AOSS domain loses context.

v2 - https://lore.kernel.org/patchwork/cover/1244340/
More discussion is found here - https://lore.kernel.org/patchwork/patch/1231182/

Changes since v2:
* Added DT maintainers which I missed in v2
* Added proper kernel-doc and header as per Mathieu

Sai Prakash Ranjan (2):
coresight: replicator: Reset replicator if context is lost
dt-bindings: arm: coresight: Add optional property to replicators

 .../devicetree/bindings/arm/coresight.txt     |  6 ++
 .../coresight/coresight-replicator.c          | 55 +++++++++++++------
 2 files changed, 44 insertions(+), 17 deletions(-)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#1 lore raw

PATCH v3 1/2 coresight: replicator: Reset replicator if context is lost

On some QCOM SoCs, replicators in Always-On domain loses its context as soon as the clock is disabled. Currently as a part of pm_runtime workqueue, clock is disabled after the replicator is initialized by amba_pm_runtime_suspend assuming that context is not lost which is not true for replicators with such limitations. So add a new property "qcom,replicator-loses-context" to identify such replicators and reset them.

Suggested-by Mike Leach <[email protected]>
Reviewed-by Mike Leach <[email protected]>
Signed-off-by Sai Prakash Ranjan <[email protected]>
---
 .../coresight/coresight-replicator.c          | 55 +++++++++++++------
 1 file changed, 38 insertions(+), 17 deletions(-)
drivers/hwtracing/coresight/coresight-replicator.c +38 -17
index e7dc1c31d20d..9d3a8f915784 100644
@@ -14,6 +14,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/coresight.h>

@@ -32,12 +33,14 @@ DEFINE_CORESIGHT_DEVLIST(replicator_devs, "replicator");
  * @atclk:	optional clock for the core parts of the replicator.
  * @csdev:	component vitals needed by the framework
  * @spinlock:	serialize enable/disable operations.
+ * @check_idfilter_val: check if the context is lost upon clock removal.
  */
 struct replicator_drvdata {
 	void __iomem		*base;
 	struct clk		*atclk;
 	struct coresight_device	*csdev;
 	spinlock_t		spinlock;
+	bool			check_idfilter_val;
 };
 
 static void dynamic_replicator_reset(struct replicator_drvdata *drvdata)

@@ -66,29 +69,43 @@ static int dynamic_replicator_enable(struct replicator_drvdata *drvdata,
 				     int inport, int outport)
 {
 	int rc = 0;
-	u32 reg;
-
-	switch (outport) {
-	case 0:
-		reg = REPLICATOR_IDFILTER0;
-		break;
-	case 1:
-		reg = REPLICATOR_IDFILTER1;
-		break;
-	default:
-		WARN_ON(1);
-		return -EINVAL;
-	}
+	u32 id0val, id1val;
 
 	CS_UNLOCK(drvdata->base);
 
-	if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0xff) &&
-	    (readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff))
+	id0val = readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0);
+	id1val = readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1);
+
+	/*
+	 * Some replicator designs lose context when AMBA clocks are removed,
+	 * so have a check for this.
+	 */
+	if (drvdata->check_idfilter_val && id0val == 0x0 && id1val == 0x0)
+		id0val = id1val = 0xff;
+
+	if (id0val == 0xff && id1val == 0xff)
 		rc = coresight_claim_device_unlocked(drvdata->base);
 
+	if (!rc) {
+		switch (outport) {
+		case 0:
+			id0val = 0x0;
+			break;
+		case 1:
+			id1val = 0x0;
+			break;
+		default:
+			WARN_ON(1);
+			rc = -EINVAL;
+		}
+	}
+
 	/* Ensure that the outport is enabled. */
-	if (!rc)
-		writel_relaxed(0x00, drvdata->base + reg);
+	if (!rc) {
+		writel_relaxed(id0val, drvdata->base + REPLICATOR_IDFILTER0);
+		writel_relaxed(id1val, drvdata->base + REPLICATOR_IDFILTER1);
+	}
+
 	CS_LOCK(drvdata->base);
 
 	return rc;

@@ -239,6 +256,10 @@ static int replicator_probe(struct device *dev, struct resource *res)
 		desc.groups = replicator_groups;
 	}
 
+	if (fwnode_property_present(dev_fwnode(dev),
+				    "qcom,replicator-loses-context"))
+		drvdata->check_idfilter_val = true;
+
 	dev_set_drvdata(dev, drvdata);
 
 	pdata = coresight_get_platform_data(dev);
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#2 lore raw

PATCH v3 2/2 dt-bindings: arm: coresight: Add optional property to replicators

In reply to Sai Prakash Ranjan

Add an optional boolean property "qcom,replicator-loses-context" to identify replicators which loses context when AMBA clocks are removed in certain configurable replicator designs.

Reviewed-by Mike Leach <[email protected]>
Signed-off-by Sai Prakash Ranjan <[email protected]>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/arm/coresight.txt +6 -0
index 846f6daae71b..b598a5f0037d 100644
@@ -121,6 +121,12 @@ its hardware characteristcs.
 	* interrupts : Exactly one SPI may be listed for reporting the address
 	  error
 
+* Optional property for configurable replicators:
+
+	* qcom,replicator-loses-context: boolean. Indicates that the replicator
+	  will lose register context when AMBA clock is removed which is observed
+	  in some replicator designs.
+
 Graph bindings for Coresight
 -------------------------------
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Reviewed-by on v3 2/2
#3 lore raw

Re: [PATCHv3 2/2] dt-bindings: arm: coresight: Add optional property to replicators

Quoted 16 lines · Sai Prakash Ranjan

Add an optional boolean property "qcom,replicator-loses-context" to identify replicators which loses context when AMBA clocks are removed in certain configurable replicator designs.

Reviewed-by Mike Leach <[email protected]>
Signed-off-by Sai Prakash Ranjan <[email protected]>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 6 ++++++
 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/arm/coresight.txt +0 -0
index 846f6daae71b..b598a5f0037d 100644

Documentation/devicetree/bindings/arm/coresight.txt its hardware characteristcs. · line 128

Quoted 8 lines
Documentation/devicetree/bindings/arm/coresight.txt +5 -0
@@ -121,6 +121,12 @@ its hardware characteristcs.
        * interrupts : Exactly one SPI may be listed for reporting the address
          error
 
+* Optional property for configurable replicators:
+
+       * qcom,replicator-loses-context: boolean. Indicates that the replicator
+         will lose register context when AMBA clock is removed which is observed
+         in some replicator designs.
Reviewed-by Mathieu Poirier <[email protected]>
Quoted 8 lines
Documentation/devicetree/bindings/arm/coresight.txt +1 -1
+
 Graph bindings for Coresight
 -------------------------------
 
--

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

#4 lore raw
In reply to Sai Prakash Ranjan

Hi Sai,

Quoted 4 lines · Sai Prakash Ranjan

This series is mainly to add support for replicators which lose context on removing AMBA clock like on SC7180 SoC where replicator in AOSS domain loses context.

I am good with this set but need a reviewed-by on the DT binding before I can add it to my tree. The same goes for your other set[1].

Thanks,
Mathieu

[1]. "coresight: etm4x: Add support to skip trace unit power up"

Quoted 19 lines

v2 - https://lore.kernel.org/patchwork/cover/1244340/
More discussion is found here - https://lore.kernel.org/patchwork/patch/1231182/

Changes since v2:
* Added DT maintainers which I missed in v2
* Added proper kernel-doc and header as per Mathieu

Sai Prakash Ranjan (2):
coresight: replicator: Reset replicator if context is lost
dt-bindings: arm: coresight: Add optional property to replicators

 .../devicetree/bindings/arm/coresight.txt     |  6 ++
 .../coresight/coresight-replicator.c          | 55 +++++++++++++------
 2 files changed, 44 insertions(+), 17 deletions(-)
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
author
#5 lore raw

Hi Mathieu,

Quoted 11 lines · Mathieu Poirier

Hi Sai,

Quoted 4 lines · Sai Prakash Ranjan

This series is mainly to add support for replicators which lose context on removing AMBA clock like on SC7180 SoC where replicator in AOSS domain loses context.

I am good with this set but need a reviewed-by on the DT binding before I can add it to my tree. The same goes for your other set[1].

Sure, let's wait for an ack from Rob and thanks for the reviews.

Thanks,
Sai

Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
maintainer Acked-by
#6 lore raw
In reply to Mathieu Poirier
Quoted 10 lines · Mathieu Poirier

Hi Sai,

Quoted 4 lines · Sai Prakash Ranjan

This series is mainly to add support for replicators which lose context on removing AMBA clock like on SC7180 SoC where replicator in AOSS domain loses context.

I am good with this set but need a reviewed-by on the DT binding before I can add it to my tree. The same goes for your other set[1].

Then why is it already in linux-next?

In any case,

Acked-by Rob Herring <[email protected]>

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#7 lore raw
Quoted 9 lines · Rob Herring
Quoted 8 lines · Mathieu Poirier

Hi Sai,

Quoted 4 lines · Sai Prakash Ranjan

This series is mainly to add support for replicators which lose context on removing AMBA clock like on SC7180 SoC where replicator in AOSS domain loses context.

Quoted 4 lines
Quoted 2 lines

I am good with this set but need a reviewed-by on the DT binding before I can add it to my tree. The same goes for your other set[1].

Then why is it already in linux-next?

I wanted the patches to get exposure to linux-next before sending them on to Greg for inclusion in the next merge window. Rest assured that I would not have moved forward without your consent.

Quoted 4 lines

In any case,

Acked-by Rob Herring <[email protected]>

Thank you for that.

Mathieu