Skip to contentSkip to conversation

Add support for replicators which loses context on clock removal

PATCH Superseded Sai Prakash Ranjan 3 participants 12 messages May 19, 2020 – May 22, 2020

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

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Sai Prakash Ranjan

author
#0 lore raw

v2 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.

Patch 1 and 2 are minor cleanups to use macros. Patch 3 and 4 adds support for replicators which loses context on removing AMBA clock.

Previous version is here - https://lore.kernel.org/patchwork/patch/1239923/
More discussion is found here - https://lore.kernel.org/patchwork/patch/1231182/
There were no patch 1 and patch 2 in v1.

Sai Prakash Ranjan (4):
coresight: replicator: Use CS_AMBA_ID macro for id table
coresight: catu: Use CS_AMBA_ID macro for id table
coresight: replicator: Reset replicator if context is lost
dt-bindings: arm: coresight: Add optional property to replicators

 .../devicetree/bindings/arm/coresight.txt     |  6 ++
 drivers/hwtracing/coresight/coresight-catu.c  |  5 +-
 .../coresight/coresight-replicator.c          | 66 +++++++++++--------
 3 files changed, 46 insertions(+), 31 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 v2 1/4 coresight: replicator: Use CS_AMBA_ID macro for id table

Use CS_AMBA_ID macro for dynamic replicator AMBA id table instead of open coding.

Signed-off-by Sai Prakash Ranjan <[email protected]>
---
 drivers/hwtracing/coresight/coresight-replicator.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/hwtracing/coresight/coresight-replicator.c +3 -10
index e7dc1c31d20d..c619b456f55a 100644
@@ -348,16 +348,9 @@ static int dynamic_replicator_probe(struct amba_device *adev,
 }
 
 static const struct amba_id dynamic_replicator_ids[] = {
-	{
-		.id     = 0x000bb909,
-		.mask   = 0x000fffff,
-	},
-	{
-		/* Coresight SoC-600 */
-		.id     = 0x000bb9ec,
-		.mask   = 0x000fffff,
-	},
-	{ 0, 0 },
+	CS_AMBA_ID(0x000bb909),
+	CS_AMBA_ID(0x000bb9ec),		/* Coresight SoC-600 */
+	{},
 };
 
 static struct amba_driver dynamic_replicator_driver = {
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#2 lore raw

Re: [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table

Quoted 22 lines · Sai Prakash Ranjan

Use CS_AMBA_ID macro for dynamic replicator AMBA id table instead of open coding.

Signed-off-by Sai Prakash Ranjan <[email protected]>
---
 drivers/hwtracing/coresight/coresight-replicator.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/hwtracing/coresight/coresight-replicator.c +0 -5
index e7dc1c31d20d..c619b456f55a 100644
@@ -348,16 +348,9 @@ static int dynamic_replicator_probe(struct amba_device *adev,
 }
 
 static const struct amba_id dynamic_replicator_ids[] = {
-       {
-               .id     = 0x000bb909,
-               .mask   = 0x000fffff,
-       },
-       {

drivers/hwtracing/coresight/coresight-replicator.c static int dynamic_replicator_probe(struct amba_device *adev, · line 355

Quoted 10 lines
drivers/hwtracing/coresight/coresight-replicator.c +3 -5
-               /* Coresight SoC-600 */
-               .id     = 0x000bb9ec,
-               .mask   = 0x000fffff,
-       },
-       { 0, 0 },
+       CS_AMBA_ID(0x000bb909),
+       CS_AMBA_ID(0x000bb9ec),         /* Coresight SoC-600 */
+       {},
 };

Reviewed by: Mike Leach <[email protected]>

Quoted 9 lines
drivers/hwtracing/coresight/coresight-replicator.c +0 -1
 static struct amba_driver dynamic_replicator_driver = {
--

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

_______________________________________________
CoreSight mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/coresight

Signature · 3 lines
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
author
#3 lore raw

PATCH v2 2/4 coresight: catu: Use CS_AMBA_ID macro for id table

In reply to Sai Prakash Ranjan

Use CS_AMBA_ID macro for coresight catu AMBA id table instead of open coding.

Signed-off-by Sai Prakash Ranjan <[email protected]>
---
 drivers/hwtracing/coresight/coresight-catu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/hwtracing/coresight/coresight-catu.c +1 -4
index 16ebf38a9f66..1801804a7762 100644
@@ -568,10 +568,7 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id)
 }
 
 static struct amba_id catu_ids[] = {
-	{
-		.id	= 0x000bb9ee,
-		.mask	= 0x000fffff,
-	},
+	CS_AMBA_ID(0x000bb9ee),
 	{},
 };
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
#4 lore raw

Re: [PATCHv2 2/4] coresight: catu: Use CS_AMBA_ID macro for id table

Quoted 15 lines · Sai Prakash Ranjan

Use CS_AMBA_ID macro for coresight catu AMBA id table instead of open coding.

Signed-off-by Sai Prakash Ranjan <[email protected]>
---
 drivers/hwtracing/coresight/coresight-catu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/hwtracing/coresight/coresight-catu.c +0 -0
index 16ebf38a9f66..1801804a7762 100644
@@ -568,10 +568,7 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id)
 }

drivers/hwtracing/coresight/coresight-catu.c static int catu_probe(struct amba_device *adev, const struct amba_id *id) · line 574

Quoted 10 lines
drivers/hwtracing/coresight/coresight-catu.c +1 -4
 
 static struct amba_id catu_ids[] = {
-       {
-               .id     = 0x000bb9ee,
-               .mask   = 0x000fffff,
-       },
+       CS_AMBA_ID(0x000bb9ee),
        {},
 };

Reviewed by: Mike Leach <[email protected]>

Quoted 4 lines

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

Signature · 3 lines
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
author
#5 lore raw

PATCH v2 3/4 coresight: replicator: Reset replicator if context is lost

In reply to Sai Prakash Ranjan

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]>
Signed-off-by Sai Prakash Ranjan <[email protected]>

---

Added Mike's suggested by for parts other than the DT property. Perhaps I should add Co-developed-by Mike since the full skeletal was given by Mike. I can add that if required on the next version.

---
 .../coresight/coresight-replicator.c          | 53 +++++++++++++------
 1 file changed, 36 insertions(+), 17 deletions(-)
drivers/hwtracing/coresight/coresight-replicator.c +36 -17
index c619b456f55a..ba66160c8140 100644
@@ -38,6 +38,7 @@ struct replicator_drvdata {
 	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 +67,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 +254,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
#6 lore raw

Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost

Hi Sai,

Quoted 106 lines · Sai Prakash Ranjan

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]>
Signed-off-by Sai Prakash Ranjan <[email protected]>

---

Added Mike's suggested by for parts other than the DT property. Perhaps I should add Co-developed-by Mike since the full skeletal was given by Mike. I can add that if required on the next version.

---
 .../coresight/coresight-replicator.c          | 53 +++++++++++++------
 1 file changed, 36 insertions(+), 17 deletions(-)
drivers/hwtracing/coresight/coresight-replicator.c +36 -18
index c619b456f55a..ba66160c8140 100644
@@ -38,6 +38,7 @@ struct replicator_drvdata {
        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 +67,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 +254,10 @@ static int replicator_probe(struct device *dev, struct resource *res)
                desc.groups = replicator_groups;On HolidayOn Holiday
        }
 
+       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);
--
Quoted 3 lines

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

Suggested-by is just fine - I didn't actually try out my suggested code after all!

Reviewed-by Mike Leach <[email protected]>

Signature · 3 lines
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
author
#7 lore raw

Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost

Hi Mike,

Quoted 21 lines · Mike Leach

Hi Sai,

Quoted 17 lines · Sai Prakash Ranjan

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]>
Signed-off-by Sai Prakash Ranjan <[email protected]>

---

Added Mike's suggested by for parts other than the DT property. Perhaps I should add Co-developed-by Mike since the full skeletal was given by Mike. I can add that if required on the next version.

[...]

Quoted 5 lines

Suggested-by is just fine - I didn't actually try out my suggested code after all!

Reviewed-by Mike Leach <[email protected]>

Sure, thanks for the review.

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
#8 lore raw

Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost

In reply to Sai Prakash Ranjan

Hi Sai,

Quoted 8 lines · Sai Prakash Ranjan

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.

Quoted 7 lines
Suggested-by Mike Leach <[email protected]>
Signed-off-by Sai Prakash Ranjan <[email protected]>

---

Added Mike's suggested by for parts other than the DT property. Perhaps I should add Co-developed-by Mike since the full skeletal was given by Mike. I can add that if required on the next version.

I will let Mike decide what he wants to do - I'm fine either way.

Quoted 10 lines
---
 .../coresight/coresight-replicator.c          | 53 +++++++++++++------
 1 file changed, 36 insertions(+), 17 deletions(-)
drivers/hwtracing/coresight/coresight-replicator.c +0 -0
index c619b456f55a..ba66160c8140 100644

drivers/hwtracing/coresight/coresight-replicator.c struct replicator_drvdata { · line 41

Quoted 4 lines
drivers/hwtracing/coresight/coresight-replicator.c +1 -0
@@ -38,6 +38,7 @@ struct replicator_drvdata {
 	struct clk		*atclk;
 	struct coresight_device	*csdev;
 	spinlock_t		spinlock;
+	bool			check_idfilter_val;

Please add documentation for the new field, the same way other fields are documented.

Quoted 65 lines
drivers/hwtracing/coresight/coresight-replicator.c +31 -17
 };
 
 static void dynamic_replicator_reset(struct replicator_drvdata *drvdata)

@@ -66,29 +67,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;

drivers/hwtracing/coresight/coresight-replicator.c static int replicator_probe(struct device *dev, struct resource *res) · line 260

Quoted 7 lines
drivers/hwtracing/coresight/coresight-replicator.c +4 -0
@@ -239,6 +254,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;
+

The header <linux/property.h> needs to be added for function fwnode_property_present().

What is the clock situation with other QC components like funnels? Have they also been designed the same way? If so the binding should probably be "qcom,component-loses-context", otherwise what you have suggested will work just fine. My goal here is to avoid having "qcom,replicator-loses-context" and "qcom,funnel-loses-context".

Lastly, I have applied patch 1 and 2 of this set to my tree so no need to resend them again with the next revision.

Thanks,
Mathieu

Quoted 7 lines
drivers/hwtracing/coresight/coresight-replicator.c +0 -0
 	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
#9 lore raw

Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost

Hi Mathieu,

Quoted 15 lines · Mathieu Poirier

Hi Sai,

Quoted 12 lines · Sai Prakash Ranjan

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]>
Signed-off-by Sai Prakash Ranjan <[email protected]>

---

Quoted 6 lines
Quoted 3 lines

Added Mike's suggested by for parts other than the DT property. Perhaps I should add Co-developed-by Mike since the full skeletal was given by Mike. I can add that if required on the next version.

I will let Mike decide what he wants to do - I'm fine either way.

Mike was ok with suggested-by, so I will go with that.

Quoted 6 lines
Quoted 6 lines
---
 .../coresight/coresight-replicator.c          | 53 
+++++++++++++------
 1 file changed, 36 insertions(+), 17 deletions(-)
Quoted 13 lines

drivers/hwtracing/coresight/coresight-replicator.c struct replicator_drvdata { · line 41

Quoted 9 lines
drivers/hwtracing/coresight/coresight-replicator.c +1 -0
index c619b456f55a..ba66160c8140 100644
@@ -38,6 +38,7 @@ struct replicator_drvdata {
 	struct clk		*atclk;
 	struct coresight_device	*csdev;
 	spinlock_t		spinlock;
+	bool			check_idfilter_val;

Please add documentation for the new field, the same way other fields are documented.

Sure will add that.

Quoted 72 lines
Quoted 72 lines
drivers/hwtracing/coresight/coresight-replicator.c +0 -0
 };
 
 static void dynamic_replicator_reset(struct replicator_drvdata

*drvdata)

drivers/hwtracing/coresight/coresight-replicator.c +31 -17
@@ -66,29 +67,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);
drivers/hwtracing/coresight/coresight-replicator.c +0 -0
 	return rc;

@@ -239,6 +254,10 @@ static int replicator_probe(struct device *dev, 
 struct resource *res)
 		desc.groups = replicator_groups;
 	}
Quoted 8 lines

drivers/hwtracing/coresight/coresight-replicator.c static int replicator_probe(struct device *dev, struct resource *res) · line 260

Quoted 4 lines
drivers/hwtracing/coresight/coresight-replicator.c +4 -0
+	if (fwnode_property_present(dev_fwnode(dev),
+				    "qcom,replicator-loses-context"))
+		drvdata->check_idfilter_val = true;
+

The header <linux/property.h> needs to be added for function fwnode_property_present().

Sure will add in next version.

Quoted 6 lines

What is the clock situation with other QC components like funnels? Have they also been designed the same way? If so the binding should probably be "qcom,component-loses-context", otherwise what you have suggested will work just fine. My goal here is to avoid having "qcom,replicator-loses-context" and "qcom,funnel-loses-context".

Yes I understand it is quite ugly, but AFAIK we do not have any SoCs already released and coming in near future with such limitations in funnels or other components. So I will stick to the replicator specific property.

Quoted 3 lines

Lastly, I have applied patch 1 and 2 of this set to my tree so no need to resend them again with the next revision.

Thanks for reviewing these patches.

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
author
#10 lore raw

PATCH v2 4/4 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.

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
#11 lore raw

Re: [PATCHv2 4/4] dt-bindings: arm: coresight: Add optional property to replicators

Quoted 17 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.

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
@@ -121,6 +121,12 @@ its hardware characteristcs.
        * interrupts : Exactly one SPI may be listed for reporting the address
          error

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

Quoted 10 lines
Documentation/devicetree/bindings/arm/coresight.txt +6 -0
 
+* 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
 -------------------------------

Reviewed-by Mike Leach <[email protected]>

Quoted 8 lines

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

_______________________________________________
CoreSight mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/coresight

--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK