All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] CoreSight: Support static funnel with DT binding consolidation
@ 2019-04-06 11:21 ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

Since before there have no platforms use static funnel in mainline
kernel (though maybe some in-house SoC has used it but didn't upstream
for mainline kernel yet so we don't be aware for it), when enable
CoreSight DT binding for hikey960, we found the SoC uses the static
funnel in the link path and but it's not supported in CoreSight funnel
driver.

When support static funnel it's found the DT binding compatible string
doesn't follow the same fashion between replicator and funnel; so the
first two patches are firstly to refine the replicator DT binding and
correspondingly, change the replicator driver to support new DT binding
compatible.

Patch 0003 is to update DT documentation to support static funnel (we
call it as non-configurable funnel in documentation) and unify the
compatible string with the same fashion for replicator and funnel.

Patch 0004 is to support the static funnel in the CoreSight funnel
driver.

This patch set has been rebased on CoreSight next branch [1] with
latest commit d5d246a56af0 ("coresight: Merge the static and dynamic
replicator drivers") and tested on Hikey960/Hikey with perf commands:

  Hikey960:

  # perf record -e cs_etm/@ec033000.etr/ --per-thread ./main
  # perf report --tui

  Hikey:
  # perf record -e cs_etm/@f6404000.etr/ --per-thread uname
  # perf report --tui

P.s. in this version, Hikey960 CoreSight DT binding has not been
included and will be sent out separately.

[1] https://git.linaro.org/kernel/coresight.git/log/?h=next

== Changes for v4 ==
* Consolidate the DT binding compatible strings for replicator and
  funnel (Mathieu).
* Changed replicator driver to support new compatible string.


Leo Yan (4):
  dt-bindings: arm: coresight: Add new compatible for static replicator
  coresight: replicator: Add new device id for static replicator
  dt-bindings: arm: coresight: Unify funnel DT binding
  coresight: funnel: Support static funnel

 .../devicetree/bindings/arm/coresight.txt     |  60 +++++++++-
 .../hwtracing/coresight/coresight-funnel.c    | 112 +++++++++++++-----
 .../coresight/coresight-replicator.c          |   3 +-
 3 files changed, 141 insertions(+), 34 deletions(-)

-- 
2.17.1


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

* [PATCH v4 0/4] CoreSight: Support static funnel with DT binding consolidation
@ 2019-04-06 11:21 ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

Since before there have no platforms use static funnel in mainline
kernel (though maybe some in-house SoC has used it but didn't upstream
for mainline kernel yet so we don't be aware for it), when enable
CoreSight DT binding for hikey960, we found the SoC uses the static
funnel in the link path and but it's not supported in CoreSight funnel
driver.

When support static funnel it's found the DT binding compatible string
doesn't follow the same fashion between replicator and funnel; so the
first two patches are firstly to refine the replicator DT binding and
correspondingly, change the replicator driver to support new DT binding
compatible.

Patch 0003 is to update DT documentation to support static funnel (we
call it as non-configurable funnel in documentation) and unify the
compatible string with the same fashion for replicator and funnel.

Patch 0004 is to support the static funnel in the CoreSight funnel
driver.

This patch set has been rebased on CoreSight next branch [1] with
latest commit d5d246a56af0 ("coresight: Merge the static and dynamic
replicator drivers") and tested on Hikey960/Hikey with perf commands:

  Hikey960:

  # perf record -e cs_etm/@ec033000.etr/ --per-thread ./main
  # perf report --tui

  Hikey:
  # perf record -e cs_etm/@f6404000.etr/ --per-thread uname
  # perf report --tui

P.s. in this version, Hikey960 CoreSight DT binding has not been
included and will be sent out separately.

[1] https://git.linaro.org/kernel/coresight.git/log/?h=next

== Changes for v4 ==
* Consolidate the DT binding compatible strings for replicator and
  funnel (Mathieu).
* Changed replicator driver to support new compatible string.


Leo Yan (4):
  dt-bindings: arm: coresight: Add new compatible for static replicator
  coresight: replicator: Add new device id for static replicator
  dt-bindings: arm: coresight: Unify funnel DT binding
  coresight: funnel: Support static funnel

 .../devicetree/bindings/arm/coresight.txt     |  60 +++++++++-
 .../hwtracing/coresight/coresight-funnel.c    | 112 +++++++++++++-----
 .../coresight/coresight-replicator.c          |   3 +-
 3 files changed, 141 insertions(+), 34 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v4 1/4] dt-bindings: arm: coresight: Add new compatible for static replicator
  2019-04-06 11:21 ` Leo Yan
@ 2019-04-06 11:21   ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

CoreSight uses below bindings for replicator:

  Static replicator, aka. non-configurable replicator:
    "arm,coresight-replicator";

  Dynamic replicator, aka. configurable replicator:
    "arm,coresight-dynamic-replicator", "arm,primecell";

The compatible string "arm,coresight-replicator" is not an explicit
naming to express the replicator is 'static'.  To unify the naming
convention, this patch introduces a new compatible string
"arm,coresight-static-replicator" for the static replicator; the
compatible string "arm,coresight-replicator" is still kept thus we
can support the old DT binding, but tag it as obsolete and suggest
to use the new compatible string.

As result CoreSight replicator have below bindings:

  Static replicator:
    "arm,coresight-replicator"; (obsolete)
    "arm,coresight-static-replicator";

  Dynamic replicator:
    "arm,coresight-dynamic-replicator", "arm,primecell";

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index f8aff65ab921..f8f794869af2 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -69,7 +69,11 @@ its hardware characteristcs.
 
 	* compatible: Currently supported value is (note the absence of the
 	  AMBA markee):
-		- "arm,coresight-replicator"
+		- Coresight Non-configurable Replicator, the compatible string
+		  "arm,coresight-replicator" is obsolete, keep it to support
+		  the old DT bindings:
+			"arm,coresight-replicator";
+			"arm,coresight-static-replicator";
 
 	* port or ports: see "Graph bindings for Coresight" below.
 
@@ -169,7 +173,7 @@ Example:
 		/* non-configurable replicators don't show up on the
 		 * AMBA bus.  As such no need to add "arm,primecell".
 		 */
-		compatible = "arm,coresight-replicator";
+		compatible = "arm,coresight-static-replicator";
 
 		out-ports {
 			#address-cells = <1>;
-- 
2.17.1


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

* [PATCH v4 1/4] dt-bindings: arm: coresight: Add new compatible for static replicator
@ 2019-04-06 11:21   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

CoreSight uses below bindings for replicator:

  Static replicator, aka. non-configurable replicator:
    "arm,coresight-replicator";

  Dynamic replicator, aka. configurable replicator:
    "arm,coresight-dynamic-replicator", "arm,primecell";

The compatible string "arm,coresight-replicator" is not an explicit
naming to express the replicator is 'static'.  To unify the naming
convention, this patch introduces a new compatible string
"arm,coresight-static-replicator" for the static replicator; the
compatible string "arm,coresight-replicator" is still kept thus we
can support the old DT binding, but tag it as obsolete and suggest
to use the new compatible string.

As result CoreSight replicator have below bindings:

  Static replicator:
    "arm,coresight-replicator"; (obsolete)
    "arm,coresight-static-replicator";

  Dynamic replicator:
    "arm,coresight-dynamic-replicator", "arm,primecell";

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index f8aff65ab921..f8f794869af2 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -69,7 +69,11 @@ its hardware characteristcs.
 
 	* compatible: Currently supported value is (note the absence of the
 	  AMBA markee):
-		- "arm,coresight-replicator"
+		- Coresight Non-configurable Replicator, the compatible string
+		  "arm,coresight-replicator" is obsolete, keep it to support
+		  the old DT bindings:
+			"arm,coresight-replicator";
+			"arm,coresight-static-replicator";
 
 	* port or ports: see "Graph bindings for Coresight" below.
 
@@ -169,7 +173,7 @@ Example:
 		/* non-configurable replicators don't show up on the
 		 * AMBA bus.  As such no need to add "arm,primecell".
 		 */
-		compatible = "arm,coresight-replicator";
+		compatible = "arm,coresight-static-replicator";
 
 		out-ports {
 			#address-cells = <1>;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v4 2/4] coresight: replicator: Add new device id for static replicator
  2019-04-06 11:21 ` Leo Yan
@ 2019-04-06 11:21   ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

This patch adds a device id for the new static replicator compatible
string; it changes the driver name from "coresight-replicator" to
"coresight-static-replicator" as well.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/coresight-replicator.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 4e0da85efd2d..e6a03642bbdb 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -285,13 +285,14 @@ static const struct dev_pm_ops replicator_dev_pm_ops = {
 
 static const struct of_device_id static_replicator_match[] = {
 	{.compatible = "arm,coresight-replicator"},
+	{.compatible = "arm,coresight-static-replicator"},
 	{}
 };
 
 static struct platform_driver static_replicator_driver = {
 	.probe          = static_replicator_probe,
 	.driver         = {
-		.name   = "coresight-replicator",
+		.name   = "coresight-static-replicator",
 		.of_match_table = static_replicator_match,
 		.pm	= &replicator_dev_pm_ops,
 		.suppress_bind_attrs = true,
-- 
2.17.1


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

* [PATCH v4 2/4] coresight: replicator: Add new device id for static replicator
@ 2019-04-06 11:21   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

This patch adds a device id for the new static replicator compatible
string; it changes the driver name from "coresight-replicator" to
"coresight-static-replicator" as well.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/coresight-replicator.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 4e0da85efd2d..e6a03642bbdb 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -285,13 +285,14 @@ static const struct dev_pm_ops replicator_dev_pm_ops = {
 
 static const struct of_device_id static_replicator_match[] = {
 	{.compatible = "arm,coresight-replicator"},
+	{.compatible = "arm,coresight-static-replicator"},
 	{}
 };
 
 static struct platform_driver static_replicator_driver = {
 	.probe          = static_replicator_probe,
 	.driver         = {
-		.name   = "coresight-replicator",
+		.name   = "coresight-static-replicator",
 		.of_match_table = static_replicator_match,
 		.pm	= &replicator_dev_pm_ops,
 		.suppress_bind_attrs = true,
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
  2019-04-06 11:21 ` Leo Yan
@ 2019-04-06 11:21   ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

Following the same fashion with replicator DT binding, this patch is to
unify the DT binding for funnel to support static and dynamic modes;
finally we get the funnel DT binding as below:

Before patch:

  Static funnel, aka. non-configurable funnel:
    Not supported;

  Dynamic funnel, aka. configurable funnel:
    "arm,coresight-funnel", "arm,primecell";

After patch:

  Static funnel:
    "arm,coresight-static-funnel";

  Dynamic funnel:
    "arm,coresight-funnel", "arm,primecell"; (obsolete)
    "arm,coresight-dynamic-funnel", "arm,primecell";

At the end of this patch, it gives an example for static funnel DT
binding, and updates the dynamic funnel example.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Wanglai Shi <shiwanglai@hisilicon.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 .../devicetree/bindings/arm/coresight.txt     | 52 +++++++++++++++++--
 1 file changed, 48 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index f8f794869af2..f8ad11a17cd5 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -8,7 +8,8 @@ through the intermediate links connecting the source to the currently selected
 sink. Each CoreSight component device should use these properties to describe
 its hardware characteristcs.
 
-* Required properties for all components *except* non-configurable replicators:
+* Required properties for all components *except* non-configurable replicators
+  and non-configurable funnels:
 
 	* compatible: These have to be supplemented with "arm,primecell" as
 	  drivers are using the AMBA bus interface.  Possible values include:
@@ -24,8 +25,11 @@ its hardware characteristcs.
 		  discovered at boot time when the device is probed.
 			"arm,coresight-tmc", "arm,primecell";
 
-		- Trace Funnel:
+		- Trace Programmable Funnel, the compatible string
+		  "arm,coresight-funnel" is obsolete, keep it to support
+		  the old DT bindings:
 			"arm,coresight-funnel", "arm,primecell";
+			"arm,coresight-dynamic-funnel", "arm,primecell";
 
 		- Embedded Trace Macrocell (version 3.x) and
 					Program Flow Trace Macrocell:
@@ -65,7 +69,7 @@ its hardware characteristcs.
 	  "stm-stimulus-base", each corresponding to the areas defined in "reg".
 
 * Required properties for devices that don't show up on the AMBA bus, such as
-  non-configurable replicators:
+  non-configurable replicators and non-configurable funnels:
 
 	* compatible: Currently supported value is (note the absence of the
 	  AMBA markee):
@@ -75,6 +79,9 @@ its hardware characteristcs.
 			"arm,coresight-replicator";
 			"arm,coresight-static-replicator";
 
+		- Coresight Non-configurable Funnel:
+			"arm,coresight-static-funnel";
+
 	* port or ports: see "Graph bindings for Coresight" below.
 
 * Optional properties for ETM/PTMs:
@@ -204,8 +211,45 @@ Example:
 		};
 	};
 
+	funnel {
+		/*
+		 * non-configurable funnel don't show up on the AMBA
+		 * bus.  As such no need to add "arm,primecell".
+		 */
+		compatible = "arm,coresight-static-funnel";
+		clocks = <&crg_ctrl HI3660_PCLK>;
+		clock-names = "apb_pclk";
+
+		out-ports {
+			port {
+				combo_funnel_out: endpoint {
+					remote-endpoint = <&top_funnel_in>;
+				};
+			};
+		};
+
+		in-ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				combo_funnel_in0: endpoint {
+					remote-endpoint = <&cluster0_etf_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				combo_funnel_in1: endpoint {
+					remote-endpoint = <&cluster1_etf_out>;
+				};
+			};
+		};
+	};
+
 	funnel@20040000 {
-		compatible = "arm,coresight-funnel", "arm,primecell";
+		compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
 		reg = <0 0x20040000 0 0x1000>;
 
 		clocks = <&oscclk6a>;
-- 
2.17.1


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

* [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
@ 2019-04-06 11:21   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

Following the same fashion with replicator DT binding, this patch is to
unify the DT binding for funnel to support static and dynamic modes;
finally we get the funnel DT binding as below:

Before patch:

  Static funnel, aka. non-configurable funnel:
    Not supported;

  Dynamic funnel, aka. configurable funnel:
    "arm,coresight-funnel", "arm,primecell";

After patch:

  Static funnel:
    "arm,coresight-static-funnel";

  Dynamic funnel:
    "arm,coresight-funnel", "arm,primecell"; (obsolete)
    "arm,coresight-dynamic-funnel", "arm,primecell";

At the end of this patch, it gives an example for static funnel DT
binding, and updates the dynamic funnel example.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Wanglai Shi <shiwanglai@hisilicon.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 .../devicetree/bindings/arm/coresight.txt     | 52 +++++++++++++++++--
 1 file changed, 48 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index f8f794869af2..f8ad11a17cd5 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -8,7 +8,8 @@ through the intermediate links connecting the source to the currently selected
 sink. Each CoreSight component device should use these properties to describe
 its hardware characteristcs.
 
-* Required properties for all components *except* non-configurable replicators:
+* Required properties for all components *except* non-configurable replicators
+  and non-configurable funnels:
 
 	* compatible: These have to be supplemented with "arm,primecell" as
 	  drivers are using the AMBA bus interface.  Possible values include:
@@ -24,8 +25,11 @@ its hardware characteristcs.
 		  discovered at boot time when the device is probed.
 			"arm,coresight-tmc", "arm,primecell";
 
-		- Trace Funnel:
+		- Trace Programmable Funnel, the compatible string
+		  "arm,coresight-funnel" is obsolete, keep it to support
+		  the old DT bindings:
 			"arm,coresight-funnel", "arm,primecell";
+			"arm,coresight-dynamic-funnel", "arm,primecell";
 
 		- Embedded Trace Macrocell (version 3.x) and
 					Program Flow Trace Macrocell:
@@ -65,7 +69,7 @@ its hardware characteristcs.
 	  "stm-stimulus-base", each corresponding to the areas defined in "reg".
 
 * Required properties for devices that don't show up on the AMBA bus, such as
-  non-configurable replicators:
+  non-configurable replicators and non-configurable funnels:
 
 	* compatible: Currently supported value is (note the absence of the
 	  AMBA markee):
@@ -75,6 +79,9 @@ its hardware characteristcs.
 			"arm,coresight-replicator";
 			"arm,coresight-static-replicator";
 
+		- Coresight Non-configurable Funnel:
+			"arm,coresight-static-funnel";
+
 	* port or ports: see "Graph bindings for Coresight" below.
 
 * Optional properties for ETM/PTMs:
@@ -204,8 +211,45 @@ Example:
 		};
 	};
 
+	funnel {
+		/*
+		 * non-configurable funnel don't show up on the AMBA
+		 * bus.  As such no need to add "arm,primecell".
+		 */
+		compatible = "arm,coresight-static-funnel";
+		clocks = <&crg_ctrl HI3660_PCLK>;
+		clock-names = "apb_pclk";
+
+		out-ports {
+			port {
+				combo_funnel_out: endpoint {
+					remote-endpoint = <&top_funnel_in>;
+				};
+			};
+		};
+
+		in-ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				combo_funnel_in0: endpoint {
+					remote-endpoint = <&cluster0_etf_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+				combo_funnel_in1: endpoint {
+					remote-endpoint = <&cluster1_etf_out>;
+				};
+			};
+		};
+	};
+
 	funnel@20040000 {
-		compatible = "arm,coresight-funnel", "arm,primecell";
+		compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
 		reg = <0 0x20040000 0 0x1000>;
 
 		clocks = <&oscclk6a>;
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v4 4/4] coresight: funnel: Support static funnel
  2019-04-06 11:21 ` Leo Yan
@ 2019-04-06 11:21   ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

Since CoreSight hardware topology can use a 'hidden' funnel in the
trace data path, this kind funnel doesn't have register for accessing
and is used by default from hardware design perspective.  Below is an
example for related hardware topology:

  +------+  +------+
  | cpu0 |->| ETM  |-\
  +------+  +------+  \-> +--------+  +-----+
   ......                 | Funnel |->| ETF |-\    Hidden funnel
  +------+  +------+  /-> +--------+  +-----+  \        |
  | cpu3 |->| ETM  |-/                          \       V
  +------+  +------+                             \-> +--------+
                                                     | Funnel |-> ...
  +------+  +------+                             /-> +--------+
  | cpu4 |->| ETM  |-\                          /
  +------+  +------+  \-> +--------+  +-----+  /
   ......                 | Funnel |->| ETF |-/
  +------+  +------+  /-> +--------+  +-----+
  | cpu7 |->| ETM  |-/
  +------+  +------+

The CoreSight funnel driver only supports dynamic funnel with
registration register resource, thus it cannot support for the static
funnel case and it's impossible to create trace data path for this case.

This patch is to extend CoreSight funnel driver to support both for
static funnel and dynamic funnel.  For the dynamic funnel it reuses the
code existed in the driver, for static funnel the driver will support
device probe if without providing register resource and the driver skips
registers accessing when detect the register base is NULL.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Wanglai Shi <shiwanglai@hisilicon.com>
Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../hwtracing/coresight/coresight-funnel.c    | 112 +++++++++++++-----
 1 file changed, 85 insertions(+), 27 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index 927925151509..2030d2596d90 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -12,6 +12,7 @@
 #include <linux/err.h>
 #include <linux/fs.h>
 #include <linux/slab.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/coresight.h>
 #include <linux/amba/bus.h>
@@ -43,7 +44,7 @@ struct funnel_drvdata {
 	unsigned long		priority;
 };
 
-static int funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
+static int dynamic_funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
 {
 	u32 functl;
 	int rc = 0;
@@ -71,17 +72,19 @@ static int funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
 static int funnel_enable(struct coresight_device *csdev, int inport,
 			 int outport)
 {
-	int rc;
+	int rc = 0;
 	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	rc = funnel_enable_hw(drvdata, inport);
+	if (drvdata->base)
+		rc = dynamic_funnel_enable_hw(drvdata, inport);
 
 	if (!rc)
 		dev_dbg(drvdata->dev, "FUNNEL inport %d enabled\n", inport);
 	return rc;
 }
 
-static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport)
+static void dynamic_funnel_disable_hw(struct funnel_drvdata *drvdata,
+				      int inport)
 {
 	u32 functl;
 
@@ -103,7 +106,8 @@ static void funnel_disable(struct coresight_device *csdev, int inport,
 {
 	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	funnel_disable_hw(drvdata, inport);
+	if (drvdata->base)
+		dynamic_funnel_disable_hw(drvdata, inport);
 
 	dev_dbg(drvdata->dev, "FUNNEL inport %d disabled\n", inport);
 }
@@ -177,54 +181,67 @@ static struct attribute *coresight_funnel_attrs[] = {
 };
 ATTRIBUTE_GROUPS(coresight_funnel);
 
-static int funnel_probe(struct amba_device *adev, const struct amba_id *id)
+static int funnel_probe(struct device *dev, struct resource *res)
 {
 	int ret;
 	void __iomem *base;
-	struct device *dev = &adev->dev;
 	struct coresight_platform_data *pdata = NULL;
 	struct funnel_drvdata *drvdata;
-	struct resource *res = &adev->res;
 	struct coresight_desc desc = { 0 };
-	struct device_node *np = adev->dev.of_node;
+	struct device_node *np = dev->of_node;
 
 	if (np) {
 		pdata = of_get_coresight_platform_data(dev, np);
 		if (IS_ERR(pdata))
 			return PTR_ERR(pdata);
-		adev->dev.platform_data = pdata;
+		dev->platform_data = pdata;
 	}
 
 	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
 	if (!drvdata)
 		return -ENOMEM;
 
-	drvdata->dev = &adev->dev;
-	drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
+	drvdata->dev = dev;
+	drvdata->atclk = devm_clk_get(dev, "atclk"); /* optional */
 	if (!IS_ERR(drvdata->atclk)) {
 		ret = clk_prepare_enable(drvdata->atclk);
 		if (ret)
 			return ret;
 	}
-	dev_set_drvdata(dev, drvdata);
 
-	/* Validity for the resource is already checked by the AMBA core */
-	base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	/*
+	 * Map the device base for dynamic-funnel, which has been
+	 * validated by AMBA core.
+	 */
+	if (res) {
+		base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(base)) {
+			ret = PTR_ERR(base);
+			goto out_disable_clk;
+		}
+		drvdata->base = base;
+		desc.groups = coresight_funnel_groups;
+	}
 
-	drvdata->base = base;
-	pm_runtime_put(&adev->dev);
+	dev_set_drvdata(dev, drvdata);
 
 	desc.type = CORESIGHT_DEV_TYPE_LINK;
 	desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
 	desc.ops = &funnel_cs_ops;
 	desc.pdata = pdata;
 	desc.dev = dev;
-	desc.groups = coresight_funnel_groups;
 	drvdata->csdev = coresight_register(&desc);
+	if (IS_ERR(drvdata->csdev)) {
+		ret = PTR_ERR(drvdata->csdev);
+		goto out_disable_clk;
+	}
+
+	pm_runtime_put(dev);
 
-	return PTR_ERR_OR_ZERO(drvdata->csdev);
+out_disable_clk:
+	if (ret && !IS_ERR_OR_NULL(drvdata->atclk))
+		clk_disable_unprepare(drvdata->atclk);
+	return ret;
 }
 
 #ifdef CONFIG_PM
@@ -253,7 +270,48 @@ static const struct dev_pm_ops funnel_dev_pm_ops = {
 	SET_RUNTIME_PM_OPS(funnel_runtime_suspend, funnel_runtime_resume, NULL)
 };
 
-static const struct amba_id funnel_ids[] = {
+static int static_funnel_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	/* Static funnel do not have programming base */
+	ret = funnel_probe(&pdev->dev, NULL);
+
+	if (ret) {
+		pm_runtime_put_noidle(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+	}
+
+	return ret;
+}
+
+static const struct of_device_id static_funnel_match[] = {
+	{.compatible = "arm,coresight-static-funnel"},
+	{}
+};
+
+static struct platform_driver static_funnel_driver = {
+	.probe          = static_funnel_probe,
+	.driver         = {
+		.name   = "coresight-static-funnel",
+		.of_match_table = static_funnel_match,
+		.pm	= &funnel_dev_pm_ops,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(static_funnel_driver);
+
+static int dynamic_funnel_probe(struct amba_device *adev,
+				const struct amba_id *id)
+{
+	return funnel_probe(&adev->dev, &adev->res);
+}
+
+static const struct amba_id dynamic_funnel_ids[] = {
 	{
 		.id     = 0x000bb908,
 		.mask   = 0x000fffff,
@@ -266,14 +324,14 @@ static const struct amba_id funnel_ids[] = {
 	{ 0, 0},
 };
 
-static struct amba_driver funnel_driver = {
+static struct amba_driver dynamic_funnel_driver = {
 	.drv = {
-		.name	= "coresight-funnel",
+		.name	= "coresight-dynamic-funnel",
 		.owner	= THIS_MODULE,
 		.pm	= &funnel_dev_pm_ops,
 		.suppress_bind_attrs = true,
 	},
-	.probe		= funnel_probe,
-	.id_table	= funnel_ids,
+	.probe		= dynamic_funnel_probe,
+	.id_table	= dynamic_funnel_ids,
 };
-builtin_amba_driver(funnel_driver);
+builtin_amba_driver(dynamic_funnel_driver);
-- 
2.17.1


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

* [PATCH v4 4/4] coresight: funnel: Support static funnel
@ 2019-04-06 11:21   ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:21 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel
  Cc: Leo Yan

Since CoreSight hardware topology can use a 'hidden' funnel in the
trace data path, this kind funnel doesn't have register for accessing
and is used by default from hardware design perspective.  Below is an
example for related hardware topology:

  +------+  +------+
  | cpu0 |->| ETM  |-\
  +------+  +------+  \-> +--------+  +-----+
   ......                 | Funnel |->| ETF |-\    Hidden funnel
  +------+  +------+  /-> +--------+  +-----+  \        |
  | cpu3 |->| ETM  |-/                          \       V
  +------+  +------+                             \-> +--------+
                                                     | Funnel |-> ...
  +------+  +------+                             /-> +--------+
  | cpu4 |->| ETM  |-\                          /
  +------+  +------+  \-> +--------+  +-----+  /
   ......                 | Funnel |->| ETF |-/
  +------+  +------+  /-> +--------+  +-----+
  | cpu7 |->| ETM  |-/
  +------+  +------+

The CoreSight funnel driver only supports dynamic funnel with
registration register resource, thus it cannot support for the static
funnel case and it's impossible to create trace data path for this case.

This patch is to extend CoreSight funnel driver to support both for
static funnel and dynamic funnel.  For the dynamic funnel it reuses the
code existed in the driver, for static funnel the driver will support
device probe if without providing register resource and the driver skips
registers accessing when detect the register base is NULL.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Wanglai Shi <shiwanglai@hisilicon.com>
Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../hwtracing/coresight/coresight-funnel.c    | 112 +++++++++++++-----
 1 file changed, 85 insertions(+), 27 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index 927925151509..2030d2596d90 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -12,6 +12,7 @@
 #include <linux/err.h>
 #include <linux/fs.h>
 #include <linux/slab.h>
+#include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/coresight.h>
 #include <linux/amba/bus.h>
@@ -43,7 +44,7 @@ struct funnel_drvdata {
 	unsigned long		priority;
 };
 
-static int funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
+static int dynamic_funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
 {
 	u32 functl;
 	int rc = 0;
@@ -71,17 +72,19 @@ static int funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
 static int funnel_enable(struct coresight_device *csdev, int inport,
 			 int outport)
 {
-	int rc;
+	int rc = 0;
 	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	rc = funnel_enable_hw(drvdata, inport);
+	if (drvdata->base)
+		rc = dynamic_funnel_enable_hw(drvdata, inport);
 
 	if (!rc)
 		dev_dbg(drvdata->dev, "FUNNEL inport %d enabled\n", inport);
 	return rc;
 }
 
-static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport)
+static void dynamic_funnel_disable_hw(struct funnel_drvdata *drvdata,
+				      int inport)
 {
 	u32 functl;
 
@@ -103,7 +106,8 @@ static void funnel_disable(struct coresight_device *csdev, int inport,
 {
 	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	funnel_disable_hw(drvdata, inport);
+	if (drvdata->base)
+		dynamic_funnel_disable_hw(drvdata, inport);
 
 	dev_dbg(drvdata->dev, "FUNNEL inport %d disabled\n", inport);
 }
@@ -177,54 +181,67 @@ static struct attribute *coresight_funnel_attrs[] = {
 };
 ATTRIBUTE_GROUPS(coresight_funnel);
 
-static int funnel_probe(struct amba_device *adev, const struct amba_id *id)
+static int funnel_probe(struct device *dev, struct resource *res)
 {
 	int ret;
 	void __iomem *base;
-	struct device *dev = &adev->dev;
 	struct coresight_platform_data *pdata = NULL;
 	struct funnel_drvdata *drvdata;
-	struct resource *res = &adev->res;
 	struct coresight_desc desc = { 0 };
-	struct device_node *np = adev->dev.of_node;
+	struct device_node *np = dev->of_node;
 
 	if (np) {
 		pdata = of_get_coresight_platform_data(dev, np);
 		if (IS_ERR(pdata))
 			return PTR_ERR(pdata);
-		adev->dev.platform_data = pdata;
+		dev->platform_data = pdata;
 	}
 
 	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
 	if (!drvdata)
 		return -ENOMEM;
 
-	drvdata->dev = &adev->dev;
-	drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
+	drvdata->dev = dev;
+	drvdata->atclk = devm_clk_get(dev, "atclk"); /* optional */
 	if (!IS_ERR(drvdata->atclk)) {
 		ret = clk_prepare_enable(drvdata->atclk);
 		if (ret)
 			return ret;
 	}
-	dev_set_drvdata(dev, drvdata);
 
-	/* Validity for the resource is already checked by the AMBA core */
-	base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	/*
+	 * Map the device base for dynamic-funnel, which has been
+	 * validated by AMBA core.
+	 */
+	if (res) {
+		base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(base)) {
+			ret = PTR_ERR(base);
+			goto out_disable_clk;
+		}
+		drvdata->base = base;
+		desc.groups = coresight_funnel_groups;
+	}
 
-	drvdata->base = base;
-	pm_runtime_put(&adev->dev);
+	dev_set_drvdata(dev, drvdata);
 
 	desc.type = CORESIGHT_DEV_TYPE_LINK;
 	desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
 	desc.ops = &funnel_cs_ops;
 	desc.pdata = pdata;
 	desc.dev = dev;
-	desc.groups = coresight_funnel_groups;
 	drvdata->csdev = coresight_register(&desc);
+	if (IS_ERR(drvdata->csdev)) {
+		ret = PTR_ERR(drvdata->csdev);
+		goto out_disable_clk;
+	}
+
+	pm_runtime_put(dev);
 
-	return PTR_ERR_OR_ZERO(drvdata->csdev);
+out_disable_clk:
+	if (ret && !IS_ERR_OR_NULL(drvdata->atclk))
+		clk_disable_unprepare(drvdata->atclk);
+	return ret;
 }
 
 #ifdef CONFIG_PM
@@ -253,7 +270,48 @@ static const struct dev_pm_ops funnel_dev_pm_ops = {
 	SET_RUNTIME_PM_OPS(funnel_runtime_suspend, funnel_runtime_resume, NULL)
 };
 
-static const struct amba_id funnel_ids[] = {
+static int static_funnel_probe(struct platform_device *pdev)
+{
+	int ret;
+
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	/* Static funnel do not have programming base */
+	ret = funnel_probe(&pdev->dev, NULL);
+
+	if (ret) {
+		pm_runtime_put_noidle(&pdev->dev);
+		pm_runtime_disable(&pdev->dev);
+	}
+
+	return ret;
+}
+
+static const struct of_device_id static_funnel_match[] = {
+	{.compatible = "arm,coresight-static-funnel"},
+	{}
+};
+
+static struct platform_driver static_funnel_driver = {
+	.probe          = static_funnel_probe,
+	.driver         = {
+		.name   = "coresight-static-funnel",
+		.of_match_table = static_funnel_match,
+		.pm	= &funnel_dev_pm_ops,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(static_funnel_driver);
+
+static int dynamic_funnel_probe(struct amba_device *adev,
+				const struct amba_id *id)
+{
+	return funnel_probe(&adev->dev, &adev->res);
+}
+
+static const struct amba_id dynamic_funnel_ids[] = {
 	{
 		.id     = 0x000bb908,
 		.mask   = 0x000fffff,
@@ -266,14 +324,14 @@ static const struct amba_id funnel_ids[] = {
 	{ 0, 0},
 };
 
-static struct amba_driver funnel_driver = {
+static struct amba_driver dynamic_funnel_driver = {
 	.drv = {
-		.name	= "coresight-funnel",
+		.name	= "coresight-dynamic-funnel",
 		.owner	= THIS_MODULE,
 		.pm	= &funnel_dev_pm_ops,
 		.suppress_bind_attrs = true,
 	},
-	.probe		= funnel_probe,
-	.id_table	= funnel_ids,
+	.probe		= dynamic_funnel_probe,
+	.id_table	= dynamic_funnel_ids,
 };
-builtin_amba_driver(funnel_driver);
+builtin_amba_driver(dynamic_funnel_driver);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
  2019-04-06 11:21   ` Leo Yan
@ 2019-04-06 11:29     ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:29 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel

Hi Rob, Suzuki,

On Sat, Apr 06, 2019 at 07:21:44PM +0800, Leo Yan wrote:
> Following the same fashion with replicator DT binding, this patch is to
> unify the DT binding for funnel to support static and dynamic modes;
> finally we get the funnel DT binding as below:
> 
> Before patch:
> 
>   Static funnel, aka. non-configurable funnel:
>     Not supported;
> 
>   Dynamic funnel, aka. configurable funnel:
>     "arm,coresight-funnel", "arm,primecell";
> 
> After patch:
> 
>   Static funnel:
>     "arm,coresight-static-funnel";
> 
>   Dynamic funnel:
>     "arm,coresight-funnel", "arm,primecell"; (obsolete)
>     "arm,coresight-dynamic-funnel", "arm,primecell";
> 
> At the end of this patch, it gives an example for static funnel DT
> binding, and updates the dynamic funnel example.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Wanglai Shi <shiwanglai@hisilicon.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>

Though you gave the reviewing tag for patch v3, but in v4 I added a new
compatible string "arm,coresight-dynamic-funnel" and mark
"arm,coresight-funnel" as obsolete; and also changed the commit log.

For this reason I didn't add your tags in this patch, so please review
again.  Thanks!

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

* Re: [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
@ 2019-04-06 11:29     ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-06 11:29 UTC (permalink / raw)
  To: Mathieu Poirier, Suzuki K Poulose, Rob Herring, Mark Rutland,
	Alexander Shishkin, Wanglai Shi, linux-arm-kernel, devicetree,
	linux-kernel

Hi Rob, Suzuki,

On Sat, Apr 06, 2019 at 07:21:44PM +0800, Leo Yan wrote:
> Following the same fashion with replicator DT binding, this patch is to
> unify the DT binding for funnel to support static and dynamic modes;
> finally we get the funnel DT binding as below:
> 
> Before patch:
> 
>   Static funnel, aka. non-configurable funnel:
>     Not supported;
> 
>   Dynamic funnel, aka. configurable funnel:
>     "arm,coresight-funnel", "arm,primecell";
> 
> After patch:
> 
>   Static funnel:
>     "arm,coresight-static-funnel";
> 
>   Dynamic funnel:
>     "arm,coresight-funnel", "arm,primecell"; (obsolete)
>     "arm,coresight-dynamic-funnel", "arm,primecell";
> 
> At the end of this patch, it gives an example for static funnel DT
> binding, and updates the dynamic funnel example.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Wanglai Shi <shiwanglai@hisilicon.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>

Though you gave the reviewing tag for patch v3, but in v4 I added a new
compatible string "arm,coresight-dynamic-funnel" and mark
"arm,coresight-funnel" as obsolete; and also changed the commit log.

For this reason I didn't add your tags in this patch, so please review
again.  Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v4 1/4] dt-bindings: arm: coresight: Add new compatible for static replicator
  2019-04-06 11:21   ` Leo Yan
@ 2019-04-08 10:44     ` Suzuki K Poulose
  -1 siblings, 0 replies; 23+ messages in thread
From: Suzuki K Poulose @ 2019-04-08 10:44 UTC (permalink / raw)
  To: leo.yan, mathieu.poirier, robh+dt, mark.rutland,
	alexander.shishkin, shiwanglai, linux-arm-kernel, devicetree,
	linux-kernel

On 04/06/2019 12:21 PM, Leo Yan wrote:
> CoreSight uses below bindings for replicator:
> 
>    Static replicator, aka. non-configurable replicator:
>      "arm,coresight-replicator";
> 
>    Dynamic replicator, aka. configurable replicator:
>      "arm,coresight-dynamic-replicator", "arm,primecell";
> 
> The compatible string "arm,coresight-replicator" is not an explicit
> naming to express the replicator is 'static'.  To unify the naming
> convention, this patch introduces a new compatible string
> "arm,coresight-static-replicator" for the static replicator; the
> compatible string "arm,coresight-replicator" is still kept thus we
> can support the old DT binding, but tag it as obsolete and suggest
> to use the new compatible string.
> 
> As result CoreSight replicator have below bindings:
> 
>    Static replicator:
>      "arm,coresight-replicator"; (obsolete)
>      "arm,coresight-static-replicator";
> 
>    Dynamic replicator:
>      "arm,coresight-dynamic-replicator", "arm,primecell";
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>   Documentation/devicetree/bindings/arm/coresight.txt | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index f8aff65ab921..f8f794869af2 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -69,7 +69,11 @@ its hardware characteristcs.
>   
>   	* compatible: Currently supported value is (note the absence of the
>   	  AMBA markee):
> -		- "arm,coresight-replicator"
> +		- Coresight Non-configurable Replicator, the compatible string
> +		  "arm,coresight-replicator" is obsolete, keep it to support

To make it more obvious : s/obsolete/OBSOLETE/

> +		  the old DT bindings:
> +			"arm,coresight-replicator";
> +			"arm,coresight-static-replicator";

nit: I would re-order the compatible strings to show the precedence of
choice.

e.g,
	- CoreSight non-configurable Replicator
		"arm,coresight-static-replicator";
		"arm,coresight-replicator"; (OBSOLETE. For
			backward compatibility. Will be removed )


Also, please could we put a warning in the driver when we encounter it
and request to update it ?

Cheers
Suzuki

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

* Re: [PATCH v4 1/4] dt-bindings: arm: coresight: Add new compatible for static replicator
@ 2019-04-08 10:44     ` Suzuki K Poulose
  0 siblings, 0 replies; 23+ messages in thread
From: Suzuki K Poulose @ 2019-04-08 10:44 UTC (permalink / raw)
  To: leo.yan, mathieu.poirier, robh+dt, mark.rutland,
	alexander.shishkin, shiwanglai, linux-arm-kernel, devicetree,
	linux-kernel

On 04/06/2019 12:21 PM, Leo Yan wrote:
> CoreSight uses below bindings for replicator:
> 
>    Static replicator, aka. non-configurable replicator:
>      "arm,coresight-replicator";
> 
>    Dynamic replicator, aka. configurable replicator:
>      "arm,coresight-dynamic-replicator", "arm,primecell";
> 
> The compatible string "arm,coresight-replicator" is not an explicit
> naming to express the replicator is 'static'.  To unify the naming
> convention, this patch introduces a new compatible string
> "arm,coresight-static-replicator" for the static replicator; the
> compatible string "arm,coresight-replicator" is still kept thus we
> can support the old DT binding, but tag it as obsolete and suggest
> to use the new compatible string.
> 
> As result CoreSight replicator have below bindings:
> 
>    Static replicator:
>      "arm,coresight-replicator"; (obsolete)
>      "arm,coresight-static-replicator";
> 
>    Dynamic replicator:
>      "arm,coresight-dynamic-replicator", "arm,primecell";
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>   Documentation/devicetree/bindings/arm/coresight.txt | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index f8aff65ab921..f8f794869af2 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -69,7 +69,11 @@ its hardware characteristcs.
>   
>   	* compatible: Currently supported value is (note the absence of the
>   	  AMBA markee):
> -		- "arm,coresight-replicator"
> +		- Coresight Non-configurable Replicator, the compatible string
> +		  "arm,coresight-replicator" is obsolete, keep it to support

To make it more obvious : s/obsolete/OBSOLETE/

> +		  the old DT bindings:
> +			"arm,coresight-replicator";
> +			"arm,coresight-static-replicator";

nit: I would re-order the compatible strings to show the precedence of
choice.

e.g,
	- CoreSight non-configurable Replicator
		"arm,coresight-static-replicator";
		"arm,coresight-replicator"; (OBSOLETE. For
			backward compatibility. Will be removed )


Also, please could we put a warning in the driver when we encounter it
and request to update it ?

Cheers
Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v4 2/4] coresight: replicator: Add new device id for static replicator
  2019-04-06 11:21   ` Leo Yan
@ 2019-04-08 10:46     ` Suzuki K Poulose
  -1 siblings, 0 replies; 23+ messages in thread
From: Suzuki K Poulose @ 2019-04-08 10:46 UTC (permalink / raw)
  To: leo.yan, mathieu.poirier, robh+dt, mark.rutland,
	alexander.shishkin, shiwanglai, linux-arm-kernel, devicetree,
	linux-kernel

On 04/06/2019 12:21 PM, Leo Yan wrote:
> This patch adds a device id for the new static replicator compatible
> string; it changes the driver name from "coresight-replicator" to
> "coresight-static-replicator" as well.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>   drivers/hwtracing/coresight/coresight-replicator.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 4e0da85efd2d..e6a03642bbdb 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -285,13 +285,14 @@ static const struct dev_pm_ops replicator_dev_pm_ops = {
>   
>   static const struct of_device_id static_replicator_match[] = {
>   	{.compatible = "arm,coresight-replicator"},
> +	{.compatible = "arm,coresight-static-replicator"},
>   	{}

As mentioned in the previous patch, please could you add a warning if
you encounter the old binding, mentioning that it is OBSOLETE.
pr_warn_once() is sufficient.

Otherwise, looks good.

Suzuki

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

* Re: [PATCH v4 2/4] coresight: replicator: Add new device id for static replicator
@ 2019-04-08 10:46     ` Suzuki K Poulose
  0 siblings, 0 replies; 23+ messages in thread
From: Suzuki K Poulose @ 2019-04-08 10:46 UTC (permalink / raw)
  To: leo.yan, mathieu.poirier, robh+dt, mark.rutland,
	alexander.shishkin, shiwanglai, linux-arm-kernel, devicetree,
	linux-kernel

On 04/06/2019 12:21 PM, Leo Yan wrote:
> This patch adds a device id for the new static replicator compatible
> string; it changes the driver name from "coresight-replicator" to
> "coresight-static-replicator" as well.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>   drivers/hwtracing/coresight/coresight-replicator.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 4e0da85efd2d..e6a03642bbdb 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -285,13 +285,14 @@ static const struct dev_pm_ops replicator_dev_pm_ops = {
>   
>   static const struct of_device_id static_replicator_match[] = {
>   	{.compatible = "arm,coresight-replicator"},
> +	{.compatible = "arm,coresight-static-replicator"},
>   	{}

As mentioned in the previous patch, please could you add a warning if
you encounter the old binding, mentioning that it is OBSOLETE.
pr_warn_once() is sufficient.

Otherwise, looks good.

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
  2019-04-06 11:21   ` Leo Yan
@ 2019-04-08 10:51     ` Suzuki K Poulose
  -1 siblings, 0 replies; 23+ messages in thread
From: Suzuki K Poulose @ 2019-04-08 10:51 UTC (permalink / raw)
  To: leo.yan, mathieu.poirier, robh+dt, mark.rutland,
	alexander.shishkin, shiwanglai, linux-arm-kernel, devicetree,
	linux-kernel

On 04/06/2019 12:21 PM, Leo Yan wrote:
> Following the same fashion with replicator DT binding, this patch is to
> unify the DT binding for funnel to support static and dynamic modes;
> finally we get the funnel DT binding as below:
> 
> Before patch:
> 
>    Static funnel, aka. non-configurable funnel:
>      Not supported;
> 
>    Dynamic funnel, aka. configurable funnel:
>      "arm,coresight-funnel", "arm,primecell";
> 
> After patch:
> 
>    Static funnel:
>      "arm,coresight-static-funnel";
> 
>    Dynamic funnel:
>      "arm,coresight-funnel", "arm,primecell"; (obsolete)
>      "arm,coresight-dynamic-funnel", "arm,primecell";
> 
> At the end of this patch, it gives an example for static funnel DT
> binding, and updates the dynamic funnel example.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Wanglai Shi <shiwanglai@hisilicon.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>   .../devicetree/bindings/arm/coresight.txt     | 52 +++++++++++++++++--
>   1 file changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index f8f794869af2..f8ad11a17cd5 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -8,7 +8,8 @@ through the intermediate links connecting the source to the currently selected
>   sink. Each CoreSight component device should use these properties to describe
>   its hardware characteristcs.
>   
> -* Required properties for all components *except* non-configurable replicators:
> +* Required properties for all components *except* non-configurable replicators
> +  and non-configurable funnels:
>   
>   	* compatible: These have to be supplemented with "arm,primecell" as
>   	  drivers are using the AMBA bus interface.  Possible values include:
> @@ -24,8 +25,11 @@ its hardware characteristcs.
>   		  discovered at boot time when the device is probed.
>   			"arm,coresight-tmc", "arm,primecell";
>   
> -		- Trace Funnel:
> +		- Trace Programmable Funnel, the compatible string
> +		  "arm,coresight-funnel" is obsolete, keep it to support
> +		  the old DT bindings:
>   			"arm,coresight-funnel", "arm,primecell";
> +			"arm,coresight-dynamic-funnel", "arm,primecell";

Same comments as the first patch here.


> +	funnel {
> +		/*
> +		 * non-configurable funnel don't show up on the AMBA
> +		 * bus.  As such no need to add "arm,primecell".
> +		 */
> +		compatible = "arm,coresight-static-funnel";
> +		clocks = <&crg_ctrl HI3660_PCLK>;
> +		clock-names = "apb_pclk";
> +
> +		out-ports {
> +			port {
> +				combo_funnel_out: endpoint {
> +					remote-endpoint = <&top_funnel_in>;
> +				};
> +			};
> +		};
> +
> +		in-ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				combo_funnel_in0: endpoint {
> +					remote-endpoint = <&cluster0_etf_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				combo_funnel_in1: endpoint {
> +					remote-endpoint = <&cluster1_etf_out>;
> +				};
> +			};
> +		};
> +	};
> +
>   	funnel@20040000 {

Should we rename this to say dynamic_funnel@2004000 { ?

> -		compatible = "arm,coresight-funnel", "arm,primecell";
> +		compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
>   		reg = <0 0x20040000 0 0x1000>;


Rest looks fine to me.

Suzuki

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

* Re: [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
@ 2019-04-08 10:51     ` Suzuki K Poulose
  0 siblings, 0 replies; 23+ messages in thread
From: Suzuki K Poulose @ 2019-04-08 10:51 UTC (permalink / raw)
  To: leo.yan, mathieu.poirier, robh+dt, mark.rutland,
	alexander.shishkin, shiwanglai, linux-arm-kernel, devicetree,
	linux-kernel

On 04/06/2019 12:21 PM, Leo Yan wrote:
> Following the same fashion with replicator DT binding, this patch is to
> unify the DT binding for funnel to support static and dynamic modes;
> finally we get the funnel DT binding as below:
> 
> Before patch:
> 
>    Static funnel, aka. non-configurable funnel:
>      Not supported;
> 
>    Dynamic funnel, aka. configurable funnel:
>      "arm,coresight-funnel", "arm,primecell";
> 
> After patch:
> 
>    Static funnel:
>      "arm,coresight-static-funnel";
> 
>    Dynamic funnel:
>      "arm,coresight-funnel", "arm,primecell"; (obsolete)
>      "arm,coresight-dynamic-funnel", "arm,primecell";
> 
> At the end of this patch, it gives an example for static funnel DT
> binding, and updates the dynamic funnel example.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Wanglai Shi <shiwanglai@hisilicon.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>   .../devicetree/bindings/arm/coresight.txt     | 52 +++++++++++++++++--
>   1 file changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index f8f794869af2..f8ad11a17cd5 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -8,7 +8,8 @@ through the intermediate links connecting the source to the currently selected
>   sink. Each CoreSight component device should use these properties to describe
>   its hardware characteristcs.
>   
> -* Required properties for all components *except* non-configurable replicators:
> +* Required properties for all components *except* non-configurable replicators
> +  and non-configurable funnels:
>   
>   	* compatible: These have to be supplemented with "arm,primecell" as
>   	  drivers are using the AMBA bus interface.  Possible values include:
> @@ -24,8 +25,11 @@ its hardware characteristcs.
>   		  discovered at boot time when the device is probed.
>   			"arm,coresight-tmc", "arm,primecell";
>   
> -		- Trace Funnel:
> +		- Trace Programmable Funnel, the compatible string
> +		  "arm,coresight-funnel" is obsolete, keep it to support
> +		  the old DT bindings:
>   			"arm,coresight-funnel", "arm,primecell";
> +			"arm,coresight-dynamic-funnel", "arm,primecell";

Same comments as the first patch here.


> +	funnel {
> +		/*
> +		 * non-configurable funnel don't show up on the AMBA
> +		 * bus.  As such no need to add "arm,primecell".
> +		 */
> +		compatible = "arm,coresight-static-funnel";
> +		clocks = <&crg_ctrl HI3660_PCLK>;
> +		clock-names = "apb_pclk";
> +
> +		out-ports {
> +			port {
> +				combo_funnel_out: endpoint {
> +					remote-endpoint = <&top_funnel_in>;
> +				};
> +			};
> +		};
> +
> +		in-ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				combo_funnel_in0: endpoint {
> +					remote-endpoint = <&cluster0_etf_out>;
> +				};
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				combo_funnel_in1: endpoint {
> +					remote-endpoint = <&cluster1_etf_out>;
> +				};
> +			};
> +		};
> +	};
> +
>   	funnel@20040000 {

Should we rename this to say dynamic_funnel@2004000 { ?

> -		compatible = "arm,coresight-funnel", "arm,primecell";
> +		compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
>   		reg = <0 0x20040000 0 0x1000>;


Rest looks fine to me.

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v4 1/4] dt-bindings: arm: coresight: Add new compatible for static replicator
  2019-04-08 10:44     ` Suzuki K Poulose
@ 2019-04-08 14:01       ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-08 14:01 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mathieu.poirier, robh+dt, mark.rutland, alexander.shishkin,
	shiwanglai, linux-arm-kernel, devicetree, linux-kernel

Hi Suzuki,

On Mon, Apr 08, 2019 at 11:44:56AM +0100, Suzuki K Poulose wrote:
> On 04/06/2019 12:21 PM, Leo Yan wrote:
> > CoreSight uses below bindings for replicator:
> > 
> >    Static replicator, aka. non-configurable replicator:
> >      "arm,coresight-replicator";
> > 
> >    Dynamic replicator, aka. configurable replicator:
> >      "arm,coresight-dynamic-replicator", "arm,primecell";
> > 
> > The compatible string "arm,coresight-replicator" is not an explicit
> > naming to express the replicator is 'static'.  To unify the naming
> > convention, this patch introduces a new compatible string
> > "arm,coresight-static-replicator" for the static replicator; the
> > compatible string "arm,coresight-replicator" is still kept thus we
> > can support the old DT binding, but tag it as obsolete and suggest
> > to use the new compatible string.
> > 
> > As result CoreSight replicator have below bindings:
> > 
> >    Static replicator:
> >      "arm,coresight-replicator"; (obsolete)
> >      "arm,coresight-static-replicator";
> > 
> >    Dynamic replicator:
> >      "arm,coresight-dynamic-replicator", "arm,primecell";
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >   Documentation/devicetree/bindings/arm/coresight.txt | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> > index f8aff65ab921..f8f794869af2 100644
> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> > @@ -69,7 +69,11 @@ its hardware characteristcs.
> >   	* compatible: Currently supported value is (note the absence of the
> >   	  AMBA markee):
> > -		- "arm,coresight-replicator"
> > +		- Coresight Non-configurable Replicator, the compatible string
> > +		  "arm,coresight-replicator" is obsolete, keep it to support
> 
> To make it more obvious : s/obsolete/OBSOLETE/
> 
> > +		  the old DT bindings:
> > +			"arm,coresight-replicator";
> > +			"arm,coresight-static-replicator";
> 
> nit: I would re-order the compatible strings to show the precedence of
> choice.
> 
> e.g,
> 	- CoreSight non-configurable Replicator
> 		"arm,coresight-static-replicator";
> 		"arm,coresight-replicator"; (OBSOLETE. For
> 			backward compatibility. Will be removed )

The suggestion looks good to me since it's neat.  Will do it.

> Also, please could we put a warning in the driver when we encounter it
> and request to update it ?

Will do.

Thanks for reviewing!
Leo Yan

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

* Re: [PATCH v4 1/4] dt-bindings: arm: coresight: Add new compatible for static replicator
@ 2019-04-08 14:01       ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-08 14:01 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mark.rutland, devicetree, shiwanglai, mathieu.poirier,
	alexander.shishkin, linux-kernel, robh+dt, linux-arm-kernel

Hi Suzuki,

On Mon, Apr 08, 2019 at 11:44:56AM +0100, Suzuki K Poulose wrote:
> On 04/06/2019 12:21 PM, Leo Yan wrote:
> > CoreSight uses below bindings for replicator:
> > 
> >    Static replicator, aka. non-configurable replicator:
> >      "arm,coresight-replicator";
> > 
> >    Dynamic replicator, aka. configurable replicator:
> >      "arm,coresight-dynamic-replicator", "arm,primecell";
> > 
> > The compatible string "arm,coresight-replicator" is not an explicit
> > naming to express the replicator is 'static'.  To unify the naming
> > convention, this patch introduces a new compatible string
> > "arm,coresight-static-replicator" for the static replicator; the
> > compatible string "arm,coresight-replicator" is still kept thus we
> > can support the old DT binding, but tag it as obsolete and suggest
> > to use the new compatible string.
> > 
> > As result CoreSight replicator have below bindings:
> > 
> >    Static replicator:
> >      "arm,coresight-replicator"; (obsolete)
> >      "arm,coresight-static-replicator";
> > 
> >    Dynamic replicator:
> >      "arm,coresight-dynamic-replicator", "arm,primecell";
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >   Documentation/devicetree/bindings/arm/coresight.txt | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> > index f8aff65ab921..f8f794869af2 100644
> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> > @@ -69,7 +69,11 @@ its hardware characteristcs.
> >   	* compatible: Currently supported value is (note the absence of the
> >   	  AMBA markee):
> > -		- "arm,coresight-replicator"
> > +		- Coresight Non-configurable Replicator, the compatible string
> > +		  "arm,coresight-replicator" is obsolete, keep it to support
> 
> To make it more obvious : s/obsolete/OBSOLETE/
> 
> > +		  the old DT bindings:
> > +			"arm,coresight-replicator";
> > +			"arm,coresight-static-replicator";
> 
> nit: I would re-order the compatible strings to show the precedence of
> choice.
> 
> e.g,
> 	- CoreSight non-configurable Replicator
> 		"arm,coresight-static-replicator";
> 		"arm,coresight-replicator"; (OBSOLETE. For
> 			backward compatibility. Will be removed )

The suggestion looks good to me since it's neat.  Will do it.

> Also, please could we put a warning in the driver when we encounter it
> and request to update it ?

Will do.

Thanks for reviewing!
Leo Yan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
  2019-04-08 10:51     ` Suzuki K Poulose
  (?)
@ 2019-04-08 14:07       ` Leo Yan
  -1 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-08 14:07 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mathieu.poirier, robh+dt, mark.rutland, alexander.shishkin,
	shiwanglai, linux-arm-kernel, devicetree, linux-kernel

On Mon, Apr 08, 2019 at 11:51:16AM +0100, Suzuki K Poulose wrote:
> On 04/06/2019 12:21 PM, Leo Yan wrote:
> > Following the same fashion with replicator DT binding, this patch is to
> > unify the DT binding for funnel to support static and dynamic modes;
> > finally we get the funnel DT binding as below:
> > 
> > Before patch:
> > 
> >    Static funnel, aka. non-configurable funnel:
> >      Not supported;
> > 
> >    Dynamic funnel, aka. configurable funnel:
> >      "arm,coresight-funnel", "arm,primecell";
> > 
> > After patch:
> > 
> >    Static funnel:
> >      "arm,coresight-static-funnel";
> > 
> >    Dynamic funnel:
> >      "arm,coresight-funnel", "arm,primecell"; (obsolete)
> >      "arm,coresight-dynamic-funnel", "arm,primecell";
> > 
> > At the end of this patch, it gives an example for static funnel DT
> > binding, and updates the dynamic funnel example.
> > 
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Wanglai Shi <shiwanglai@hisilicon.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >   .../devicetree/bindings/arm/coresight.txt     | 52 +++++++++++++++++--
> >   1 file changed, 48 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> > index f8f794869af2..f8ad11a17cd5 100644
> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> > @@ -8,7 +8,8 @@ through the intermediate links connecting the source to the currently selected
> >   sink. Each CoreSight component device should use these properties to describe
> >   its hardware characteristcs.
> > -* Required properties for all components *except* non-configurable replicators:
> > +* Required properties for all components *except* non-configurable replicators
> > +  and non-configurable funnels:
> >   	* compatible: These have to be supplemented with "arm,primecell" as
> >   	  drivers are using the AMBA bus interface.  Possible values include:
> > @@ -24,8 +25,11 @@ its hardware characteristcs.
> >   		  discovered at boot time when the device is probed.
> >   			"arm,coresight-tmc", "arm,primecell";
> > -		- Trace Funnel:
> > +		- Trace Programmable Funnel, the compatible string
> > +		  "arm,coresight-funnel" is obsolete, keep it to support
> > +		  the old DT bindings:
> >   			"arm,coresight-funnel", "arm,primecell";
> > +			"arm,coresight-dynamic-funnel", "arm,primecell";
> 
> Same comments as the first patch here.

Will do it.

> > +	funnel {
> > +		/*
> > +		 * non-configurable funnel don't show up on the AMBA
> > +		 * bus.  As such no need to add "arm,primecell".
> > +		 */
> > +		compatible = "arm,coresight-static-funnel";
> > +		clocks = <&crg_ctrl HI3660_PCLK>;
> > +		clock-names = "apb_pclk";
> > +
> > +		out-ports {
> > +			port {
> > +				combo_funnel_out: endpoint {
> > +					remote-endpoint = <&top_funnel_in>;
> > +				};
> > +			};
> > +		};
> > +
> > +		in-ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +				combo_funnel_in0: endpoint {
> > +					remote-endpoint = <&cluster0_etf_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				combo_funnel_in1: endpoint {
> > +					remote-endpoint = <&cluster1_etf_out>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> >   	funnel@20040000 {
> 
> Should we rename this to say dynamic_funnel@2004000 { ?

I read ePAPR and it suggests "The name of a node should be somewhat
generic, reflecting the function of the device and not its precise
programming model".

So seems to me it's good to keep using generic naming 'funnel'.  If I
misunderstand anything, just let me know.

Will spin patch set for following other suggestions.

Thanks,
Leo Yan

> 
> > -		compatible = "arm,coresight-funnel", "arm,primecell";
> > +		compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
> >   		reg = <0 0x20040000 0 0x1000>;
> 
> 
> Rest looks fine to me.
> 
> Suzuki

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

* Re: [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
@ 2019-04-08 14:07       ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-08 14:07 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mark.rutland, devicetree, shiwanglai, mathieu.poirier,
	alexander.shishkin, linux-kernel, robh+dt, linux-arm-kernel

On Mon, Apr 08, 2019 at 11:51:16AM +0100, Suzuki K Poulose wrote:
> On 04/06/2019 12:21 PM, Leo Yan wrote:
> > Following the same fashion with replicator DT binding, this patch is to
> > unify the DT binding for funnel to support static and dynamic modes;
> > finally we get the funnel DT binding as below:
> > 
> > Before patch:
> > 
> >    Static funnel, aka. non-configurable funnel:
> >      Not supported;
> > 
> >    Dynamic funnel, aka. configurable funnel:
> >      "arm,coresight-funnel", "arm,primecell";
> > 
> > After patch:
> > 
> >    Static funnel:
> >      "arm,coresight-static-funnel";
> > 
> >    Dynamic funnel:
> >      "arm,coresight-funnel", "arm,primecell"; (obsolete)
> >      "arm,coresight-dynamic-funnel", "arm,primecell";
> > 
> > At the end of this patch, it gives an example for static funnel DT
> > binding, and updates the dynamic funnel example.
> > 
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Wanglai Shi <shiwanglai@hisilicon.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >   .../devicetree/bindings/arm/coresight.txt     | 52 +++++++++++++++++--
> >   1 file changed, 48 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> > index f8f794869af2..f8ad11a17cd5 100644
> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> > @@ -8,7 +8,8 @@ through the intermediate links connecting the source to the currently selected
> >   sink. Each CoreSight component device should use these properties to describe
> >   its hardware characteristcs.
> > -* Required properties for all components *except* non-configurable replicators:
> > +* Required properties for all components *except* non-configurable replicators
> > +  and non-configurable funnels:
> >   	* compatible: These have to be supplemented with "arm,primecell" as
> >   	  drivers are using the AMBA bus interface.  Possible values include:
> > @@ -24,8 +25,11 @@ its hardware characteristcs.
> >   		  discovered at boot time when the device is probed.
> >   			"arm,coresight-tmc", "arm,primecell";
> > -		- Trace Funnel:
> > +		- Trace Programmable Funnel, the compatible string
> > +		  "arm,coresight-funnel" is obsolete, keep it to support
> > +		  the old DT bindings:
> >   			"arm,coresight-funnel", "arm,primecell";
> > +			"arm,coresight-dynamic-funnel", "arm,primecell";
> 
> Same comments as the first patch here.

Will do it.

> > +	funnel {
> > +		/*
> > +		 * non-configurable funnel don't show up on the AMBA
> > +		 * bus.  As such no need to add "arm,primecell".
> > +		 */
> > +		compatible = "arm,coresight-static-funnel";
> > +		clocks = <&crg_ctrl HI3660_PCLK>;
> > +		clock-names = "apb_pclk";
> > +
> > +		out-ports {
> > +			port {
> > +				combo_funnel_out: endpoint {
> > +					remote-endpoint = <&top_funnel_in>;
> > +				};
> > +			};
> > +		};
> > +
> > +		in-ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +				combo_funnel_in0: endpoint {
> > +					remote-endpoint = <&cluster0_etf_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				combo_funnel_in1: endpoint {
> > +					remote-endpoint = <&cluster1_etf_out>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> >   	funnel@20040000 {
> 
> Should we rename this to say dynamic_funnel@2004000 { ?

I read ePAPR and it suggests "The name of a node should be somewhat
generic, reflecting the function of the device and not its precise
programming model".

So seems to me it's good to keep using generic naming 'funnel'.  If I
misunderstand anything, just let me know.

Will spin patch set for following other suggestions.

Thanks,
Leo Yan

> 
> > -		compatible = "arm,coresight-funnel", "arm,primecell";
> > +		compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
> >   		reg = <0 0x20040000 0 0x1000>;
> 
> 
> Rest looks fine to me.
> 
> Suzuki

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

* Re: [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding
@ 2019-04-08 14:07       ` Leo Yan
  0 siblings, 0 replies; 23+ messages in thread
From: Leo Yan @ 2019-04-08 14:07 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: mark.rutland, devicetree, shiwanglai, mathieu.poirier,
	alexander.shishkin, linux-kernel, robh+dt, linux-arm-kernel

On Mon, Apr 08, 2019 at 11:51:16AM +0100, Suzuki K Poulose wrote:
> On 04/06/2019 12:21 PM, Leo Yan wrote:
> > Following the same fashion with replicator DT binding, this patch is to
> > unify the DT binding for funnel to support static and dynamic modes;
> > finally we get the funnel DT binding as below:
> > 
> > Before patch:
> > 
> >    Static funnel, aka. non-configurable funnel:
> >      Not supported;
> > 
> >    Dynamic funnel, aka. configurable funnel:
> >      "arm,coresight-funnel", "arm,primecell";
> > 
> > After patch:
> > 
> >    Static funnel:
> >      "arm,coresight-static-funnel";
> > 
> >    Dynamic funnel:
> >      "arm,coresight-funnel", "arm,primecell"; (obsolete)
> >      "arm,coresight-dynamic-funnel", "arm,primecell";
> > 
> > At the end of this patch, it gives an example for static funnel DT
> > binding, and updates the dynamic funnel example.
> > 
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Wanglai Shi <shiwanglai@hisilicon.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >   .../devicetree/bindings/arm/coresight.txt     | 52 +++++++++++++++++--
> >   1 file changed, 48 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> > index f8f794869af2..f8ad11a17cd5 100644
> > --- a/Documentation/devicetree/bindings/arm/coresight.txt
> > +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> > @@ -8,7 +8,8 @@ through the intermediate links connecting the source to the currently selected
> >   sink. Each CoreSight component device should use these properties to describe
> >   its hardware characteristcs.
> > -* Required properties for all components *except* non-configurable replicators:
> > +* Required properties for all components *except* non-configurable replicators
> > +  and non-configurable funnels:
> >   	* compatible: These have to be supplemented with "arm,primecell" as
> >   	  drivers are using the AMBA bus interface.  Possible values include:
> > @@ -24,8 +25,11 @@ its hardware characteristcs.
> >   		  discovered at boot time when the device is probed.
> >   			"arm,coresight-tmc", "arm,primecell";
> > -		- Trace Funnel:
> > +		- Trace Programmable Funnel, the compatible string
> > +		  "arm,coresight-funnel" is obsolete, keep it to support
> > +		  the old DT bindings:
> >   			"arm,coresight-funnel", "arm,primecell";
> > +			"arm,coresight-dynamic-funnel", "arm,primecell";
> 
> Same comments as the first patch here.

Will do it.

> > +	funnel {
> > +		/*
> > +		 * non-configurable funnel don't show up on the AMBA
> > +		 * bus.  As such no need to add "arm,primecell".
> > +		 */
> > +		compatible = "arm,coresight-static-funnel";
> > +		clocks = <&crg_ctrl HI3660_PCLK>;
> > +		clock-names = "apb_pclk";
> > +
> > +		out-ports {
> > +			port {
> > +				combo_funnel_out: endpoint {
> > +					remote-endpoint = <&top_funnel_in>;
> > +				};
> > +			};
> > +		};
> > +
> > +		in-ports {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			port@0 {
> > +				reg = <0>;
> > +				combo_funnel_in0: endpoint {
> > +					remote-endpoint = <&cluster0_etf_out>;
> > +				};
> > +			};
> > +
> > +			port@1 {
> > +				reg = <1>;
> > +				combo_funnel_in1: endpoint {
> > +					remote-endpoint = <&cluster1_etf_out>;
> > +				};
> > +			};
> > +		};
> > +	};
> > +
> >   	funnel@20040000 {
> 
> Should we rename this to say dynamic_funnel@2004000 { ?

I read ePAPR and it suggests "The name of a node should be somewhat
generic, reflecting the function of the device and not its precise
programming model".

So seems to me it's good to keep using generic naming 'funnel'.  If I
misunderstand anything, just let me know.

Will spin patch set for following other suggestions.

Thanks,
Leo Yan

> 
> > -		compatible = "arm,coresight-funnel", "arm,primecell";
> > +		compatible = "arm,coresight-dynamic-funnel", "arm,primecell";
> >   		reg = <0 0x20040000 0 0x1000>;
> 
> 
> Rest looks fine to me.
> 
> Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2019-04-08 14:07 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-06 11:21 [PATCH v4 0/4] CoreSight: Support static funnel with DT binding consolidation Leo Yan
2019-04-06 11:21 ` Leo Yan
2019-04-06 11:21 ` [PATCH v4 1/4] dt-bindings: arm: coresight: Add new compatible for static replicator Leo Yan
2019-04-06 11:21   ` Leo Yan
2019-04-08 10:44   ` Suzuki K Poulose
2019-04-08 10:44     ` Suzuki K Poulose
2019-04-08 14:01     ` Leo Yan
2019-04-08 14:01       ` Leo Yan
2019-04-06 11:21 ` [PATCH v4 2/4] coresight: replicator: Add new device id " Leo Yan
2019-04-06 11:21   ` Leo Yan
2019-04-08 10:46   ` Suzuki K Poulose
2019-04-08 10:46     ` Suzuki K Poulose
2019-04-06 11:21 ` [PATCH v4 3/4] dt-bindings: arm: coresight: Unify funnel DT binding Leo Yan
2019-04-06 11:21   ` Leo Yan
2019-04-06 11:29   ` Leo Yan
2019-04-06 11:29     ` Leo Yan
2019-04-08 10:51   ` Suzuki K Poulose
2019-04-08 10:51     ` Suzuki K Poulose
2019-04-08 14:07     ` Leo Yan
2019-04-08 14:07       ` Leo Yan
2019-04-08 14:07       ` Leo Yan
2019-04-06 11:21 ` [PATCH v4 4/4] coresight: funnel: Support static funnel Leo Yan
2019-04-06 11:21   ` Leo Yan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.