Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHv2 0/4] Add support for replicators which loses context on clock removal
@ 2020-05-19 13:35 Sai Prakash Ranjan
  2020-05-19 13:36 ` [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table Sai Prakash Ranjan
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-19 13:35 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach
  Cc: Stephen Boyd, linux-kernel, linux-arm-msm, coresight, 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.

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(-)

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table
  2020-05-19 13:35 [PATCHv2 0/4] Add support for replicators which loses context on clock removal Sai Prakash Ranjan
@ 2020-05-19 13:36 ` Sai Prakash Ranjan
  2020-05-21 16:14   ` Mike Leach
  2020-05-19 13:36 ` [PATCHv2 2/4] coresight: catu: " Sai Prakash Ranjan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-19 13:36 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach
  Cc: Stephen Boyd, linux-kernel, linux-arm-msm, coresight, Sai Prakash Ranjan

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

Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-replicator.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index e7dc1c31d20d..c619b456f55a 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -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 = {
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCHv2 2/4] coresight: catu: Use CS_AMBA_ID macro for id table
  2020-05-19 13:35 [PATCHv2 0/4] Add support for replicators which loses context on clock removal Sai Prakash Ranjan
  2020-05-19 13:36 ` [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table Sai Prakash Ranjan
@ 2020-05-19 13:36 ` Sai Prakash Ranjan
  2020-05-21 16:15   ` Mike Leach
  2020-05-19 13:36 ` [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost Sai Prakash Ranjan
  2020-05-19 13:36 ` [PATCHv2 4/4] dt-bindings: arm: coresight: Add optional property to replicators Sai Prakash Ranjan
  3 siblings, 1 reply; 12+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-19 13:36 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach
  Cc: Stephen Boyd, linux-kernel, linux-arm-msm, coresight, Sai Prakash Ranjan

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

Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/hwtracing/coresight/coresight-catu.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index 16ebf38a9f66..1801804a7762 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -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),
 	{},
 };
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost
  2020-05-19 13:35 [PATCHv2 0/4] Add support for replicators which loses context on clock removal Sai Prakash Ranjan
  2020-05-19 13:36 ` [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table Sai Prakash Ranjan
  2020-05-19 13:36 ` [PATCHv2 2/4] coresight: catu: " Sai Prakash Ranjan
@ 2020-05-19 13:36 ` Sai Prakash Ranjan
  2020-05-21 16:08   ` Mike Leach
  2020-05-22 17:40   ` Mathieu Poirier
  2020-05-19 13:36 ` [PATCHv2 4/4] dt-bindings: arm: coresight: Add optional property to replicators Sai Prakash Ranjan
  3 siblings, 2 replies; 12+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-19 13:36 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach
  Cc: Stephen Boyd, linux-kernel, linux-arm-msm, coresight, 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 <mike.leach@linaro.org>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---

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(-)

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index c619b456f55a..ba66160c8140 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -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);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCHv2 4/4] dt-bindings: arm: coresight: Add optional property to replicators
  2020-05-19 13:35 [PATCHv2 0/4] Add support for replicators which loses context on clock removal Sai Prakash Ranjan
                   ` (2 preceding siblings ...)
  2020-05-19 13:36 ` [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost Sai Prakash Ranjan
@ 2020-05-19 13:36 ` Sai Prakash Ranjan
  2020-05-21 16:11   ` Mike Leach
  3 siblings, 1 reply; 12+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-19 13:36 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Mike Leach
  Cc: Stephen Boyd, linux-kernel, linux-arm-msm, coresight, 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 <saiprakash.ranjan@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index 846f6daae71b..b598a5f0037d 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -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
 -------------------------------
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost
  2020-05-19 13:36 ` [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost Sai Prakash Ranjan
@ 2020-05-21 16:08   ` Mike Leach
  2020-05-21 17:03     ` Sai Prakash Ranjan
  2020-05-22 17:40   ` Mathieu Poirier
  1 sibling, 1 reply; 12+ messages in thread
From: Mike Leach @ 2020-05-21 16:08 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Mathieu Poirier, Suzuki K Poulose, Stephen Boyd,
	Linux Kernel Mailing List, linux-arm-msm, Coresight ML

Hi Sai,

On Tue, 19 May 2020 at 14:36, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> 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 <mike.leach@linaro.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>
> 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(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index c619b456f55a..ba66160c8140 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -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);
> --
> 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 <mike.leach@linaro.org>


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 4/4] dt-bindings: arm: coresight: Add optional property to replicators
  2020-05-19 13:36 ` [PATCHv2 4/4] dt-bindings: arm: coresight: Add optional property to replicators Sai Prakash Ranjan
@ 2020-05-21 16:11   ` Mike Leach
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Leach @ 2020-05-21 16:11 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Mathieu Poirier, Suzuki K Poulose, linux-arm-msm, Coresight ML,
	Linux Kernel Mailing List, Stephen Boyd

On Tue, 19 May 2020 at 14:37, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> 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 <saiprakash.ranjan@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/coresight.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index 846f6daae71b..b598a5f0037d 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -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
>  -------------------------------
>

Reviewed-by Mike Leach <mike.leach@linaro.org>

> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
> _______________________________________________
> CoreSight mailing list
> CoreSight@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/coresight



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table
  2020-05-19 13:36 ` [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table Sai Prakash Ranjan
@ 2020-05-21 16:14   ` Mike Leach
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Leach @ 2020-05-21 16:14 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Mathieu Poirier, Suzuki K Poulose, linux-arm-msm, Coresight ML,
	Linux Kernel Mailing List, Stephen Boyd

On Tue, 19 May 2020 at 14:36, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Use CS_AMBA_ID macro for dynamic replicator AMBA id table
> instead of open coding.
>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  drivers/hwtracing/coresight/coresight-replicator.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index e7dc1c31d20d..c619b456f55a 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -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 */
> +       {},
>  };
>

Reviewed by: Mike Leach <mike.leach@linaro.org>

>  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
> CoreSight@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/coresight



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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 2/4] coresight: catu: Use CS_AMBA_ID macro for id table
  2020-05-19 13:36 ` [PATCHv2 2/4] coresight: catu: " Sai Prakash Ranjan
@ 2020-05-21 16:15   ` Mike Leach
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Leach @ 2020-05-21 16:15 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Mathieu Poirier, Suzuki K Poulose, Stephen Boyd,
	Linux Kernel Mailing List, linux-arm-msm, Coresight ML

On Tue, 19 May 2020 at 14:36, Sai Prakash Ranjan
<saiprakash.ranjan@codeaurora.org> wrote:
>
> Use CS_AMBA_ID macro for coresight catu AMBA id table
> instead of open coding.
>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
>  drivers/hwtracing/coresight/coresight-catu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index 16ebf38a9f66..1801804a7762 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -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),
>         {},
>  };
>

Reviewed by: Mike Leach <mike.leach@linaro.org>

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


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost
  2020-05-21 16:08   ` Mike Leach
@ 2020-05-21 17:03     ` Sai Prakash Ranjan
  0 siblings, 0 replies; 12+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-21 17:03 UTC (permalink / raw)
  To: Mike Leach
  Cc: Mathieu Poirier, Suzuki K Poulose, Stephen Boyd,
	Linux Kernel Mailing List, linux-arm-msm, Coresight ML

Hi Mike,

On 2020-05-21 21:38, Mike Leach wrote:
> Hi Sai,
> 
> On Tue, 19 May 2020 at 14:36, Sai Prakash Ranjan
> <saiprakash.ranjan@codeaurora.org> wrote:
>> 
>> 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 <mike.leach@linaro.org>
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>> 
>> 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.
>> 

[...]

> 
> Suggested-by is just fine - I didn't actually try out my suggested
> code after all!
> 
> Reviewed-by Mike Leach <mike.leach@linaro.org>

Sure, thanks for the review.

Thanks,
Sai

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost
  2020-05-19 13:36 ` [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost Sai Prakash Ranjan
  2020-05-21 16:08   ` Mike Leach
@ 2020-05-22 17:40   ` Mathieu Poirier
  2020-05-22 17:56     ` Sai Prakash Ranjan
  1 sibling, 1 reply; 12+ messages in thread
From: Mathieu Poirier @ 2020-05-22 17:40 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Suzuki K Poulose, Mike Leach, Stephen Boyd, linux-kernel,
	linux-arm-msm, coresight

Hi Sai,

On Tue, May 19, 2020 at 07:06:02PM +0530, Sai Prakash Ranjan wrote:
> 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 <mike.leach@linaro.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
> 
> 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.

> 
> ---
>  .../coresight/coresight-replicator.c          | 53 +++++++++++++------
>  1 file changed, 36 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index c619b456f55a..ba66160c8140 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -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.

>  };
>  
>  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;
> +

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

>  	dev_set_drvdata(dev, drvdata);
>  
>  	pdata = coresight_get_platform_data(dev);
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost
  2020-05-22 17:40   ` Mathieu Poirier
@ 2020-05-22 17:56     ` Sai Prakash Ranjan
  0 siblings, 0 replies; 12+ messages in thread
From: Sai Prakash Ranjan @ 2020-05-22 17:56 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Suzuki K Poulose, Mike Leach, Stephen Boyd, linux-kernel,
	linux-arm-msm, coresight

Hi Mathieu,

On 2020-05-22 23:10, Mathieu Poirier wrote:
> Hi Sai,
> 
> On Tue, May 19, 2020 at 07:06:02PM +0530, Sai Prakash Ranjan wrote:
>> 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 <mike.leach@linaro.org>
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> ---
>> 
>> 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.

>> 
>> ---
>>  .../coresight/coresight-replicator.c          | 53 
>> +++++++++++++------
>>  1 file changed, 36 insertions(+), 17 deletions(-)
>> 
>> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c 
>> b/drivers/hwtracing/coresight/coresight-replicator.c
>> index c619b456f55a..ba66160c8140 100644
>> --- a/drivers/hwtracing/coresight/coresight-replicator.c
>> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
>> @@ -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.

>>  };
>> 
>>  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;
>> +
> 
> The header <linux/property.h> needs to be added for function
> fwnode_property_present().
> 

Sure will add in next version.

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

> 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
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 13:35 [PATCHv2 0/4] Add support for replicators which loses context on clock removal Sai Prakash Ranjan
2020-05-19 13:36 ` [PATCHv2 1/4] coresight: replicator: Use CS_AMBA_ID macro for id table Sai Prakash Ranjan
2020-05-21 16:14   ` Mike Leach
2020-05-19 13:36 ` [PATCHv2 2/4] coresight: catu: " Sai Prakash Ranjan
2020-05-21 16:15   ` Mike Leach
2020-05-19 13:36 ` [PATCHv2 3/4] coresight: replicator: Reset replicator if context is lost Sai Prakash Ranjan
2020-05-21 16:08   ` Mike Leach
2020-05-21 17:03     ` Sai Prakash Ranjan
2020-05-22 17:40   ` Mathieu Poirier
2020-05-22 17:56     ` Sai Prakash Ranjan
2020-05-19 13:36 ` [PATCHv2 4/4] dt-bindings: arm: coresight: Add optional property to replicators Sai Prakash Ranjan
2020-05-21 16:11   ` Mike Leach

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git