Coresight uses DT graph bindings to describe the connections of the components. However we have some undocumented usage of the bindings to describe some of the properties of the connections.
The coresight driver needs to know the hardware ports invovled in the connection and the direction of data flow to effectively manage the trace sessions. So far we have relied on the "port" address (as described by the generic graph bindings) to represent the hardware port of the component for a connection.
The hardware uses separate numbering scheme for input and output ports, which implies, we could have two different (input and output) ports with the same port number. This could create problems in the graph bindings where the label of the port wouldn't match the address.
e.g, with the existing bindings we get :
port@0{ // Output port 0 reg = <0>; ... };
port@1{ reg = <0>; // Input port 0 endpoint { slave-mode; ... }; };
With the new enforcement in the DT rules, mismatches in label and address are not allowed (as see in the case for port@1). So, we need a new mechanism to describe the hardware port number reliably.
Also, we relied on an undocumented "slave-mode" property (see the above example) to indicate if the port is an input port. Let us formalise and switch to a new property to describe the direction of data flow.
There were three options considered for the hardware port number scheme:
1) Use natural ordering in the DT to infer the hardware port number. i.e, Mandate that the all ports are listed in the DT and in the ascending order for each class (input and output respectively). Pros : - We don't need new properties and if the existing DTS list them in order (which most of them do), they work out of the box. Cons : - We must list all the ports even if the system cannot/shouldn't use it. - It is prone to human errors (if the order is not kept).
2) Use an explicit property to list both the direction and the hw port number and direction. Define "coresight,hwid" as 2 member array of u32, where the members are port number and the direction respectively. e.g
port@0{ reg = <0>; endpoint { coresight,hwid = <0 1>; // Port # 0, Output } };
port@1{ reg = <1>; endpoint { coresight,hwid = <0 0>; // Port # 0, Input }; };
Pros: - The bindings are formal but not so reader friendly and could potentially lead to human errors. Cons: - Backward compatiblity is lost. 3) Use explicit properties (implemented in the series) for the hardware port id and direction. We define a new property "coresight,hwid" for each endpoint in coresight devices to specify the hardware port number explicitly. Also use a separate property "direction" to specify the direction of the data flow.
e.g,
port@0{ reg = <0>; endpoint { direction = <1>; // Output coresight,hwid = <0>; // Port # 0 } };
port@1{ reg = <1>; endpoint { direction = <0>; // Input coresight,hwid = <0>; // Port # 0 }; };
Pros: - The bindings are formal and reader friendly, and less prone to errors. Cons: - Backward compatibility is lost.
After a round of discussions [1], the following option (4) is adopted :
4) Group ports based on the directions under a dedicated node. This has been checked with the upstream DTC tool to resolve the "address mismatch" issue.
e.g,
out-ports { // Output ports for this component
port@0 { // Outport 0 reg = 0; endpoint { ... }; };
port@1 { // Outport 1 reg = 1; endpoint { ... }; };
};
in-ports { // Input ports for this component port@0 { // Inport 0 reg = 0; endpoint { ... }; };
port@1 { // Inport 1 reg = 1; endpoint { ... }; };
};
This series implements Option (4) listed above and falls back to the old bindings if the new bindings are not available. This allows the systems with old bindings work with the new driver. The driver now issues a warning (once) when it encounters the old bindings. The series contains DT update for Juno platform. The remaining in-kernel sources could be updated once we are fine with the proposal.
It also cleans up the platform parsing code to reduce the memory usage by reusing the platform description.
Applies on coresight/next
Changes since V1: - Implement the proposal by Rob. - Drop the DTS updates for all platforms except Juno - Drop the incorrect fix in coresight_register. Instead document the code to prevent people trying to un-fix it again. - Add a patch to drop remote device references in DT graph parsing - Split of_node refcount fixing patch, fix a typo in the comment. - Add Reviewed-by tags from Mathieu. - Drop patches picked up for 4.18-rc series
Changes since RFC: - Fixed style issues - Fix an existing memory leak coresight_register (Found in code update) - Fix missing of_node_put() in the existing driver (Reported-by Mathieu) - Update the existing dts in kernel tree.
Suzuki K Poulose (10): coresight: Document error handling in coresight_register coresight: platform: Refactor graph endpoint parsing coresight: platform: Fix refcounting for graph nodes coresight: platform: Fix leaking device reference coresight: Fix remote endpoint parsing coresight: Add helper to check if the endpoint is input coresight: platform: Cleanup coresight connection handling coresight: dts: Document usage of graph bindings coresight: Cleanup coresight DT bindings dts: juno: Update coresight bindings for hw port
.../devicetree/bindings/arm/coresight.txt | 95 ++++---- arch/arm64/boot/dts/arm/juno-base.dtsi | 161 ++++++------- arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 52 ++-- arch/arm64/boot/dts/arm/juno.dts | 13 +- drivers/hwtracing/coresight/coresight.c | 35 +-- drivers/hwtracing/coresight/of_coresight.c | 262 ++++++++++++++------- include/linux/coresight.h | 9 +- 7 files changed, 353 insertions(+), 274 deletions(-)
commit 6403587a930c ("coresight: use put_device() instead of kfree()") fixes the double freeing of resources and ensures that the device refcount is dropped properly. Add a comment to explain this to help the readers and prevent people trying to "unfix" it again.
While at it, rename the labels for better readability.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Arvind Yadav arvind.yadav.cs@gmail.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Change since v1: - Document the error handling than trying to "unfix" the problem. --- drivers/hwtracing/coresight/coresight.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 3e07fd3..9fd0c38 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -1006,7 +1006,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev = kzalloc(sizeof(*csdev), GFP_KERNEL); if (!csdev) { ret = -ENOMEM; - goto err_kzalloc_csdev; + goto err_out; }
if (desc->type == CORESIGHT_DEV_TYPE_LINK || @@ -1022,7 +1022,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) refcnts = kcalloc(nr_refcnts, sizeof(*refcnts), GFP_KERNEL); if (!refcnts) { ret = -ENOMEM; - goto err_kzalloc_refcnts; + goto err_free_csdev; }
csdev->refcnt = refcnts; @@ -1035,7 +1035,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); if (!conns) { ret = -ENOMEM; - goto err_kzalloc_conns; + goto err_free_refcnts; }
for (i = 0; i < csdev->nr_outport; i++) { @@ -1062,7 +1062,11 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) ret = device_register(&csdev->dev); if (ret) { put_device(&csdev->dev); - goto err_kzalloc_csdev; + /* + * All resources are free'd explicitly via + * coresight_device_release(), triggered from put_device(). + */ + goto err_out; }
mutex_lock(&coresight_mutex); @@ -1074,11 +1078,11 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
return csdev;
-err_kzalloc_conns: +err_free_refcnts: kfree(refcnts); -err_kzalloc_refcnts: +err_free_csdev: kfree(csdev); -err_kzalloc_csdev: +err_out: return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(coresight_register);
Refactor the of graph endpoint parsing code, to make the error handling easier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v1: - Splitted from the of_node refcounting fix, part1 --- drivers/hwtracing/coresight/of_coresight.c | 129 +++++++++++++++++------------ 1 file changed, 75 insertions(+), 54 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 6880bee..68faaf8 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -114,17 +114,69 @@ int of_coresight_get_cpu(const struct device_node *node) } EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+/* + * of_coresight_parse_endpoint : Parse the given output endpoint @ep + * and fill the connection information in @pdata[*@i]. + * + * Parses the local port, remote device name and the remote port. Also + * updates *@i to point to the next index, when an entry is added. + * + * Returns : + * 0 - If the parsing completed without any fatal errors. + * -Errno - Fatal error, abort the scanning. + */ +static int of_coresight_parse_endpoint(struct device *dev, + struct device_node *ep, + struct coresight_platform_data *pdata, + int *i) +{ + int ret = 0; + struct of_endpoint endpoint, rendpoint; + struct device_node *rparent = NULL; + struct device_node *rport = NULL; + struct device *rdev = NULL; + + do { + /* Parse the local port details */ + if (of_graph_parse_endpoint(ep, &endpoint)) + break; + /* + * Get a handle on the remote port and parent + * attached to it. + */ + rparent = of_graph_get_remote_port_parent(ep); + if (!rparent) + break; + rport = of_graph_get_remote_port(ep); + if (!rport) + break; + if (of_graph_parse_endpoint(rport, &rendpoint)) + break; + + /* If the remote device is not available, defer probing */ + rdev = of_coresight_get_endpoint_device(rparent); + if (!rdev) { + ret = -EPROBE_DEFER; + break; + } + + pdata->outports[*i] = endpoint.port; + pdata->child_names[*i] = dev_name(rdev); + pdata->child_ports[*i] = rendpoint.id; + /* Move the index */ + (*i)++; + } while (0); + + return ret; +} + struct coresight_platform_data * of_get_coresight_platform_data(struct device *dev, const struct device_node *node) { int i = 0, ret = 0; struct coresight_platform_data *pdata; - struct of_endpoint endpoint, rendpoint; - struct device *rdev; struct device_node *ep = NULL; - struct device_node *rparent = NULL; - struct device_node *rport = NULL;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) @@ -132,64 +184,33 @@ of_get_coresight_platform_data(struct device *dev,
/* Use device name as sysfs handle */ pdata->name = dev_name(dev); + pdata->cpu = of_coresight_get_cpu(node);
/* Get the number of input and output port for this component */ of_coresight_get_ports(node, &pdata->nr_inport, &pdata->nr_outport);
- if (pdata->nr_outport) { - ret = of_coresight_alloc_memory(dev, pdata); + /* If there are no output connections, we are done */ + if (!pdata->nr_outport) + return pdata; + + ret = of_coresight_alloc_memory(dev, pdata); + if (ret) + return ERR_PTR(ret); + + /* Iterate through each port to discover topology */ + while ((ep = of_graph_get_next_endpoint(node, ep))) { + /* + * No need to deal with input ports, processing for as + * processing for output ports will deal with them. + */ + if (of_find_property(ep, "slave-mode", NULL)) + continue; + + ret = of_coresight_parse_endpoint(dev, ep, pdata, &i); if (ret) return ERR_PTR(ret); - - /* Iterate through each port to discover topology */ - do { - /* Get a handle on a port */ - ep = of_graph_get_next_endpoint(node, ep); - if (!ep) - break; - - /* - * No need to deal with input ports, processing for as - * processing for output ports will deal with them. - */ - if (of_find_property(ep, "slave-mode", NULL)) - continue; - - /* Get a handle on the local endpoint */ - ret = of_graph_parse_endpoint(ep, &endpoint); - - if (ret) - continue; - - /* The local out port number */ - pdata->outports[i] = endpoint.port; - - /* - * Get a handle on the remote port and parent - * attached to it. - */ - rparent = of_graph_get_remote_port_parent(ep); - rport = of_graph_get_remote_port(ep); - - if (!rparent || !rport) - continue; - - if (of_graph_parse_endpoint(rport, &rendpoint)) - continue; - - rdev = of_coresight_get_endpoint_device(rparent); - if (!rdev) - return ERR_PTR(-EPROBE_DEFER); - - pdata->child_names[i] = dev_name(rdev); - pdata->child_ports[i] = rendpoint.id; - - i++; - } while (ep); }
- pdata->cpu = of_coresight_get_cpu(node); - return pdata; } EXPORT_SYMBOL_GPL(of_get_coresight_platform_data);
Good afternoon,
On Thu, Jul 19, 2018 at 11:55:06AM +0100, Suzuki K Poulose wrote:
Refactor the of graph endpoint parsing code, to make the error handling easier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v1:
- Splitted from the of_node refcounting fix, part1
drivers/hwtracing/coresight/of_coresight.c | 129 +++++++++++++++++------------ 1 file changed, 75 insertions(+), 54 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 6880bee..68faaf8 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -114,17 +114,69 @@ int of_coresight_get_cpu(const struct device_node *node) } EXPORT_SYMBOL_GPL(of_coresight_get_cpu); +/*
- of_coresight_parse_endpoint : Parse the given output endpoint @ep
- and fill the connection information in @pdata[*@i].
- Parses the local port, remote device name and the remote port. Also
- updates *@i to point to the next index, when an entry is added.
- Returns :
0 - If the parsing completed without any fatal errors.
- -Errno - Fatal error, abort the scanning.
- */
+static int of_coresight_parse_endpoint(struct device *dev,
struct device_node *ep,
struct coresight_platform_data *pdata,
int *i)
+{
- int ret = 0;
- struct of_endpoint endpoint, rendpoint;
- struct device_node *rparent = NULL;
- struct device_node *rport = NULL;
- struct device *rdev = NULL;
- do {
/* Parse the local port details */
if (of_graph_parse_endpoint(ep, &endpoint))
break;
/*
* Get a handle on the remote port and parent
* attached to it.
*/
rparent = of_graph_get_remote_port_parent(ep);
if (!rparent)
break;
rport = of_graph_get_remote_port(ep);
if (!rport)
break;
if (of_graph_parse_endpoint(rport, &rendpoint))
break;
/* If the remote device is not available, defer probing */
rdev = of_coresight_get_endpoint_device(rparent);
if (!rdev) {
ret = -EPROBE_DEFER;
break;
}
pdata->outports[*i] = endpoint.port;
pdata->child_names[*i] = dev_name(rdev);
pdata->child_ports[*i] = rendpoint.id;
/* Move the index */
(*i)++;
Not a big fan, makes the code needlessly complex. Incrementation of the index can be done in the while loop of of_get_coresight_platform_data(). See below.
- } while (0);
- return ret;
+}
struct coresight_platform_data * of_get_coresight_platform_data(struct device *dev, const struct device_node *node) { int i = 0, ret = 0; struct coresight_platform_data *pdata;
- struct of_endpoint endpoint, rendpoint;
- struct device *rdev; struct device_node *ep = NULL;
- struct device_node *rparent = NULL;
- struct device_node *rport = NULL;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) @@ -132,64 +184,33 @@ of_get_coresight_platform_data(struct device *dev, /* Use device name as sysfs handle */ pdata->name = dev_name(dev);
- pdata->cpu = of_coresight_get_cpu(node);
/* Get the number of input and output port for this component */ of_coresight_get_ports(node, &pdata->nr_inport, &pdata->nr_outport);
- if (pdata->nr_outport) {
ret = of_coresight_alloc_memory(dev, pdata);
- /* If there are no output connections, we are done */
- if (!pdata->nr_outport)
return pdata;
- ret = of_coresight_alloc_memory(dev, pdata);
- if (ret)
return ERR_PTR(ret);
- /* Iterate through each port to discover topology */
- while ((ep = of_graph_get_next_endpoint(node, ep))) {
/*
* No need to deal with input ports, processing for as
* processing for output ports will deal with them.
*/
This comment has been broken for a while... It should read:
/* * No need to deal with input ports, as processing * for output ports will deal with them. */
if (of_find_property(ep, "slave-mode", NULL))
continue;
if (ret) return ERR_PTR(ret);ret = of_coresight_parse_endpoint(dev, ep, pdata, &i);
i++;
/* Iterate through each port to discover topology */
do {
/* Get a handle on a port */
ep = of_graph_get_next_endpoint(node, ep);
if (!ep)
break;
/*
* No need to deal with input ports, processing for as
* processing for output ports will deal with them.
*/
if (of_find_property(ep, "slave-mode", NULL))
continue;
/* Get a handle on the local endpoint */
ret = of_graph_parse_endpoint(ep, &endpoint);
if (ret)
continue;
/* The local out port number */
pdata->outports[i] = endpoint.port;
/*
* Get a handle on the remote port and parent
* attached to it.
*/
rparent = of_graph_get_remote_port_parent(ep);
rport = of_graph_get_remote_port(ep);
if (!rparent || !rport)
continue;
if (of_graph_parse_endpoint(rport, &rendpoint))
continue;
rdev = of_coresight_get_endpoint_device(rparent);
if (!rdev)
return ERR_PTR(-EPROBE_DEFER);
pdata->child_names[i] = dev_name(rdev);
pdata->child_ports[i] = rendpoint.id;
i++;
}} while (ep);
- pdata->cpu = of_coresight_get_cpu(node);
- return pdata;
} EXPORT_SYMBOL_GPL(of_get_coresight_platform_data); -- 2.7.4
On Tue, 24 Jul 2018 at 15:30, Mathieu Poirier mathieu.poirier@linaro.org wrote:
Good afternoon,
On Thu, Jul 19, 2018 at 11:55:06AM +0100, Suzuki K Poulose wrote:
Refactor the of graph endpoint parsing code, to make the error handling easier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v1:
- Splitted from the of_node refcounting fix, part1
drivers/hwtracing/coresight/of_coresight.c | 129 +++++++++++++++++------------ 1 file changed, 75 insertions(+), 54 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 6880bee..68faaf8 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -114,17 +114,69 @@ int of_coresight_get_cpu(const struct device_node *node) } EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+/*
- of_coresight_parse_endpoint : Parse the given output endpoint @ep
- and fill the connection information in @pdata[*@i].
- Parses the local port, remote device name and the remote port. Also
- updates *@i to point to the next index, when an entry is added.
- Returns :
- 0 - If the parsing completed without any fatal errors.
- -Errno - Fatal error, abort the scanning.
- */
+static int of_coresight_parse_endpoint(struct device *dev,
struct device_node *ep,
struct coresight_platform_data *pdata,
int *i)
+{
int ret = 0;
struct of_endpoint endpoint, rendpoint;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
struct device *rdev = NULL;
do {
/* Parse the local port details */
if (of_graph_parse_endpoint(ep, &endpoint))
break;
/*
* Get a handle on the remote port and parent
* attached to it.
*/
rparent = of_graph_get_remote_port_parent(ep);
if (!rparent)
break;
rport = of_graph_get_remote_port(ep);
if (!rport)
break;
if (of_graph_parse_endpoint(rport, &rendpoint))
break;
/* If the remote device is not available, defer probing */
rdev = of_coresight_get_endpoint_device(rparent);
if (!rdev) {
ret = -EPROBE_DEFER;
break;
}
pdata->outports[*i] = endpoint.port;
pdata->child_names[*i] = dev_name(rdev);
pdata->child_ports[*i] = rendpoint.id;
/* Move the index */
(*i)++;
Not a big fan, makes the code needlessly complex. Incrementation of the index can be done in the while loop of of_get_coresight_platform_data(). See below.
Void that - it properly deals with the first of_parse_endpoint().
} while (0);
return ret;
+}
struct coresight_platform_data * of_get_coresight_platform_data(struct device *dev, const struct device_node *node) { int i = 0, ret = 0; struct coresight_platform_data *pdata;
struct of_endpoint endpoint, rendpoint;
struct device *rdev; struct device_node *ep = NULL;
struct device_node *rparent = NULL;
struct device_node *rport = NULL; pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); if (!pdata)
@@ -132,64 +184,33 @@ of_get_coresight_platform_data(struct device *dev,
/* Use device name as sysfs handle */ pdata->name = dev_name(dev);
pdata->cpu = of_coresight_get_cpu(node); /* Get the number of input and output port for this component */ of_coresight_get_ports(node, &pdata->nr_inport, &pdata->nr_outport);
if (pdata->nr_outport) {
ret = of_coresight_alloc_memory(dev, pdata);
/* If there are no output connections, we are done */
if (!pdata->nr_outport)
return pdata;
ret = of_coresight_alloc_memory(dev, pdata);
if (ret)
return ERR_PTR(ret);
/* Iterate through each port to discover topology */
while ((ep = of_graph_get_next_endpoint(node, ep))) {
/*
* No need to deal with input ports, processing for as
* processing for output ports will deal with them.
*/
This comment has been broken for a while... It should read:
/* * No need to deal with input ports, as processing * for output ports will deal with them. */
if (of_find_property(ep, "slave-mode", NULL))
continue;
ret = of_coresight_parse_endpoint(dev, ep, pdata, &i); if (ret) return ERR_PTR(ret);
i++;
/* Iterate through each port to discover topology */
do {
/* Get a handle on a port */
ep = of_graph_get_next_endpoint(node, ep);
if (!ep)
break;
/*
* No need to deal with input ports, processing for as
* processing for output ports will deal with them.
*/
if (of_find_property(ep, "slave-mode", NULL))
continue;
/* Get a handle on the local endpoint */
ret = of_graph_parse_endpoint(ep, &endpoint);
if (ret)
continue;
/* The local out port number */
pdata->outports[i] = endpoint.port;
/*
* Get a handle on the remote port and parent
* attached to it.
*/
rparent = of_graph_get_remote_port_parent(ep);
rport = of_graph_get_remote_port(ep);
if (!rparent || !rport)
continue;
if (of_graph_parse_endpoint(rport, &rendpoint))
continue;
rdev = of_coresight_get_endpoint_device(rparent);
if (!rdev)
return ERR_PTR(-EPROBE_DEFER);
pdata->child_names[i] = dev_name(rdev);
pdata->child_ports[i] = rendpoint.id;
i++;
} while (ep); }
pdata->cpu = of_coresight_get_cpu(node);
return pdata;
} EXPORT_SYMBOL_GPL(of_get_coresight_platform_data); -- 2.7.4
On 07/24/2018 10:34 PM, Mathieu Poirier wrote:
On Tue, 24 Jul 2018 at 15:30, Mathieu Poirier mathieu.poirier@linaro.org wrote:
Good afternoon,
On Thu, Jul 19, 2018 at 11:55:06AM +0100, Suzuki K Poulose wrote:
Refactor the of graph endpoint parsing code, to make the error handling easier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v1:
- Splitted from the of_node refcounting fix, part1
drivers/hwtracing/coresight/of_coresight.c | 129 +++++++++++++++++------------ 1 file changed, 75 insertions(+), 54 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 6880bee..68faaf8 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -114,17 +114,69 @@ int of_coresight_get_cpu(const struct device_node *node) } EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+/*
- of_coresight_parse_endpoint : Parse the given output endpoint @ep
- and fill the connection information in @pdata[*@i].
- Parses the local port, remote device name and the remote port. Also
- updates *@i to point to the next index, when an entry is added.
- Returns :
- 0 - If the parsing completed without any fatal errors.
- -Errno - Fatal error, abort the scanning.
- */
+static int of_coresight_parse_endpoint(struct device *dev,
struct device_node *ep,
struct coresight_platform_data *pdata,
int *i)
+{
int ret = 0;
struct of_endpoint endpoint, rendpoint;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
struct device *rdev = NULL;
do {
/* Parse the local port details */
if (of_graph_parse_endpoint(ep, &endpoint))
break;
/*
* Get a handle on the remote port and parent
* attached to it.
*/
rparent = of_graph_get_remote_port_parent(ep);
if (!rparent)
break;
rport = of_graph_get_remote_port(ep);
if (!rport)
break;
if (of_graph_parse_endpoint(rport, &rendpoint))
break;
/* If the remote device is not available, defer probing */
rdev = of_coresight_get_endpoint_device(rparent);
if (!rdev) {
ret = -EPROBE_DEFER;
break;
}
pdata->outports[*i] = endpoint.port;
pdata->child_names[*i] = dev_name(rdev);
pdata->child_ports[*i] = rendpoint.id;
/* Move the index */
(*i)++;
Not a big fan, makes the code needlessly complex. Incrementation of the index can be done in the while loop of of_get_coresight_platform_data(). See below.
Me neither..
Void that - it properly deals with the first of_parse_endpoint().
Or may be we could do something with the return value :
1 - Successfully parsed and populated a connection entry 0 - No errors in parsing < 0 - Errors in parsing
@@ -132,64 +184,33 @@ of_get_coresight_platform_data(struct device *dev,
while ((ep = of_graph_get_next_endpoint(node, ep))) {
/*
* No need to deal with input ports, processing for as
* processing for output ports will deal with them.
*/
This comment has been broken for a while... It should read:
/* * No need to deal with input ports, as processing * for output ports will deal with them. */
Sure, will fix it.
Suzuki
On Wed, 25 Jul 2018 at 03:01, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 07/24/2018 10:34 PM, Mathieu Poirier wrote:
On Tue, 24 Jul 2018 at 15:30, Mathieu Poirier mathieu.poirier@linaro.org wrote:
Good afternoon,
On Thu, Jul 19, 2018 at 11:55:06AM +0100, Suzuki K Poulose wrote:
Refactor the of graph endpoint parsing code, to make the error handling easier.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Changes since v1:
- Splitted from the of_node refcounting fix, part1
drivers/hwtracing/coresight/of_coresight.c | 129 +++++++++++++++++------------ 1 file changed, 75 insertions(+), 54 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 6880bee..68faaf8 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -114,17 +114,69 @@ int of_coresight_get_cpu(const struct device_node *node) } EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
+/*
- of_coresight_parse_endpoint : Parse the given output endpoint @ep
- and fill the connection information in @pdata[*@i].
- Parses the local port, remote device name and the remote port. Also
- updates *@i to point to the next index, when an entry is added.
- Returns :
- 0 - If the parsing completed without any fatal errors.
- -Errno - Fatal error, abort the scanning.
- */
+static int of_coresight_parse_endpoint(struct device *dev,
struct device_node *ep,
struct coresight_platform_data *pdata,
int *i)
+{
int ret = 0;
struct of_endpoint endpoint, rendpoint;
struct device_node *rparent = NULL;
struct device_node *rport = NULL;
struct device *rdev = NULL;
do {
/* Parse the local port details */
if (of_graph_parse_endpoint(ep, &endpoint))
break;
/*
* Get a handle on the remote port and parent
* attached to it.
*/
rparent = of_graph_get_remote_port_parent(ep);
if (!rparent)
break;
rport = of_graph_get_remote_port(ep);
if (!rport)
break;
if (of_graph_parse_endpoint(rport, &rendpoint))
break;
/* If the remote device is not available, defer probing */
rdev = of_coresight_get_endpoint_device(rparent);
if (!rdev) {
ret = -EPROBE_DEFER;
break;
}
pdata->outports[*i] = endpoint.port;
pdata->child_names[*i] = dev_name(rdev);
pdata->child_ports[*i] = rendpoint.id;
/* Move the index */
(*i)++;
Not a big fan, makes the code needlessly complex. Incrementation of the index can be done in the while loop of of_get_coresight_platform_data(). See below.
Me neither..
Void that - it properly deals with the first of_parse_endpoint().
Or may be we could do something with the return value :
1 - Successfully parsed and populated a connection entry 0 - No errors in parsing < 0 - Errors in parsing
Right, that is another solution. I usually try to avoid those but I think it is better than what we currently have.
@@ -132,64 +184,33 @@ of_get_coresight_platform_data(struct device *dev,
while ((ep = of_graph_get_next_endpoint(node, ep))) {
/*
* No need to deal with input ports, processing for as
* processing for output ports will deal with them.
*/
This comment has been broken for a while... It should read:
/* * No need to deal with input ports, as processing * for output ports will deal with them. */
Sure, will fix it.
Thanks, Mathieu
Suzuki
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
The coresight driver doesn't drop the references on the remote endpoint/port nodes. Add the missing of_node_put() calls.
Reported-by: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v1: - Splitted from of_node recounting fix, part 2 --- drivers/hwtracing/coresight/of_coresight.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 68faaf8..6c2f95a 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -167,6 +167,11 @@ static int of_coresight_parse_endpoint(struct device *dev, (*i)++; } while (0);
+ if (rparent) + of_node_put(rparent); + if (rport) + of_node_put(rport); + return ret; }
We don't drop the reference on the remote device while parsing the connection, held by bus_find_device(). Fix this by duplicating the device name and dropping the reference.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Kim Phillips kim.phillips@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- - New in v2 --- drivers/hwtracing/coresight/of_coresight.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 6c2f95a..c5f664c 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -161,7 +161,9 @@ static int of_coresight_parse_endpoint(struct device *dev, }
pdata->outports[*i] = endpoint.port; - pdata->child_names[*i] = dev_name(rdev); + pdata->child_names[*i] = devm_kstrdup(dev, + dev_name(rdev), + GFP_KERNEL); pdata->child_ports[*i] = rendpoint.id; /* Move the index */ (*i)++; @@ -171,6 +173,8 @@ static int of_coresight_parse_endpoint(struct device *dev, of_node_put(rparent); if (rport) of_node_put(rport); + if (rdev) + put_device(rdev);
return ret; }
When parsing the remote endpoint of an output port, we do : rport = of_graph_get_remote_port(ep); rparent = of_graph_get_remote_port_parent(ep);
and then parse the "remote_port" as if it was the remote endpoint, which is wrong. The code worked fine because we used endpoint number as the port number. Let us fix it and optimise a bit as:
remote_ep = of_graph_get_remote_endpoint(ep); if (remote_ep) remote_parent = of_graph_get_port_parent(remote_ep);
and then, parse the remote_ep for the port/endpoint details.
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/of_coresight.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index c5f664c..16a77a7 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -133,7 +133,7 @@ static int of_coresight_parse_endpoint(struct device *dev, int ret = 0; struct of_endpoint endpoint, rendpoint; struct device_node *rparent = NULL; - struct device_node *rport = NULL; + struct device_node *rep = NULL; struct device *rdev = NULL;
do { @@ -141,16 +141,16 @@ static int of_coresight_parse_endpoint(struct device *dev, if (of_graph_parse_endpoint(ep, &endpoint)) break; /* - * Get a handle on the remote port and parent - * attached to it. + * Get a handle on the remote endpoint and the device it is + * attached to. */ - rparent = of_graph_get_remote_port_parent(ep); + rep = of_graph_get_remote_endpoint(ep); + if (!rep) + break; + rparent = of_graph_get_port_parent(rep); if (!rparent) break; - rport = of_graph_get_remote_port(ep); - if (!rport) - break; - if (of_graph_parse_endpoint(rport, &rendpoint)) + if (of_graph_parse_endpoint(rep, &rendpoint)) break;
/* If the remote device is not available, defer probing */ @@ -164,15 +164,15 @@ static int of_coresight_parse_endpoint(struct device *dev, pdata->child_names[*i] = devm_kstrdup(dev, dev_name(rdev), GFP_KERNEL); - pdata->child_ports[*i] = rendpoint.id; + pdata->child_ports[*i] = rendpoint.port; /* Move the index */ (*i)++; } while (0);
if (rparent) of_node_put(rparent); - if (rport) - of_node_put(rport); + if (rep) + of_node_put(rep); if (rdev) put_device(rdev);
Add a helper to check if the given endpoint is input.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/of_coresight.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index 16a77a7..b6a170f 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -45,6 +45,11 @@ of_coresight_get_endpoint_device(struct device_node *endpoint) endpoint, of_dev_node_match); }
+static inline bool of_coresight_ep_is_input(struct device_node *ep) +{ + return of_property_read_bool(ep, "slave-mode"); +} + static void of_coresight_get_ports(const struct device_node *node, int *nr_inport, int *nr_outport) { @@ -56,7 +61,7 @@ static void of_coresight_get_ports(const struct device_node *node, if (!ep) break;
- if (of_property_read_bool(ep, "slave-mode")) + if (of_coresight_ep_is_input(ep)) in++; else out++; @@ -212,7 +217,7 @@ of_get_coresight_platform_data(struct device *dev, * No need to deal with input ports, processing for as * processing for output ports will deal with them. */ - if (of_find_property(ep, "slave-mode", NULL)) + if (of_coresight_ep_is_input(ep)) continue;
ret = of_coresight_parse_endpoint(dev, ep, pdata, &i);
The platform code parses the component connections and populates a platform-description of the output connections in arrays of fields (which is never freed). This is later copied in the coresight_register to a newly allocated area, represented by coresight_connection(s).
This patch cleans up the code dealing with connections by making use of the "coresight_connection" structure right at the platform code and lets the generic driver simply re-use information provided by the platform.
Thus making it reader friendly as well as avoiding the wastage of unused memory.
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight.c | 21 +---------- drivers/hwtracing/coresight/of_coresight.c | 58 ++++++++++++------------------ include/linux/coresight.h | 9 ++--- 3 files changed, 25 insertions(+), 63 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 9fd0c38..5e8880c 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -995,13 +995,11 @@ postcore_initcall(coresight_init);
struct coresight_device *coresight_register(struct coresight_desc *desc) { - int i; int ret; int link_subtype; int nr_refcnts = 1; atomic_t *refcnts = NULL; struct coresight_device *csdev; - struct coresight_connection *conns = NULL;
csdev = kzalloc(sizeof(*csdev), GFP_KERNEL); if (!csdev) { @@ -1030,22 +1028,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) csdev->nr_inport = desc->pdata->nr_inport; csdev->nr_outport = desc->pdata->nr_outport;
- /* Initialise connections if there is at least one outport */ - if (csdev->nr_outport) { - conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); - if (!conns) { - ret = -ENOMEM; - goto err_free_refcnts; - } - - for (i = 0; i < csdev->nr_outport; i++) { - conns[i].outport = desc->pdata->outports[i]; - conns[i].child_name = desc->pdata->child_names[i]; - conns[i].child_port = desc->pdata->child_ports[i]; - } - } - - csdev->conns = conns; + csdev->conns = desc->pdata->conns;
csdev->type = desc->type; csdev->subtype = desc->subtype; @@ -1078,8 +1061,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
return csdev;
-err_free_refcnts: - kfree(refcnts); err_free_csdev: kfree(csdev); err_out: diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index b6a170f..f9e2169 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -75,29 +75,13 @@ static void of_coresight_get_ports(const struct device_node *node, static int of_coresight_alloc_memory(struct device *dev, struct coresight_platform_data *pdata) { - /* List of output port on this component */ - pdata->outports = devm_kcalloc(dev, - pdata->nr_outport, - sizeof(*pdata->outports), - GFP_KERNEL); - if (!pdata->outports) - return -ENOMEM; - - /* Children connected to this component via @outports */ - pdata->child_names = devm_kcalloc(dev, - pdata->nr_outport, - sizeof(*pdata->child_names), - GFP_KERNEL); - if (!pdata->child_names) - return -ENOMEM; - - /* Port number on the child this component is connected to */ - pdata->child_ports = devm_kcalloc(dev, - pdata->nr_outport, - sizeof(*pdata->child_ports), - GFP_KERNEL); - if (!pdata->child_ports) - return -ENOMEM; + if (pdata->nr_outport) { + pdata->conns = devm_kzalloc(dev, pdata->nr_outport * + sizeof(*pdata->conns), + GFP_KERNEL); + if (!pdata->conns) + return -ENOMEM; + }
return 0; } @@ -121,10 +105,10 @@ EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
/* * of_coresight_parse_endpoint : Parse the given output endpoint @ep - * and fill the connection information in @pdata[*@i]. + * and fill the connection information in *@pconn. * * Parses the local port, remote device name and the remote port. Also - * updates *@i to point to the next index, when an entry is added. + * updates *@pconn to point to the next record, when an entry is added. * * Returns : * 0 - If the parsing completed without any fatal errors. @@ -132,14 +116,14 @@ EXPORT_SYMBOL_GPL(of_coresight_get_cpu); */ static int of_coresight_parse_endpoint(struct device *dev, struct device_node *ep, - struct coresight_platform_data *pdata, - int *i) + struct coresight_connection **pconn) { int ret = 0; struct of_endpoint endpoint, rendpoint; struct device_node *rparent = NULL; struct device_node *rep = NULL; struct device *rdev = NULL; + struct coresight_connection *conn = *pconn;
do { /* Parse the local port details */ @@ -165,13 +149,13 @@ static int of_coresight_parse_endpoint(struct device *dev, break; }
- pdata->outports[*i] = endpoint.port; - pdata->child_names[*i] = devm_kstrdup(dev, - dev_name(rdev), - GFP_KERNEL); - pdata->child_ports[*i] = rendpoint.port; - /* Move the index */ - (*i)++; + conn->outport = endpoint.port; + conn->child_name = devm_kstrdup(dev, + dev_name(rdev), + GFP_KERNEL); + conn->child_port = rendpoint.port; + /* Move the connection record */ + (*pconn)++; } while (0);
if (rparent) @@ -188,8 +172,9 @@ struct coresight_platform_data * of_get_coresight_platform_data(struct device *dev, const struct device_node *node) { - int i = 0, ret = 0; + int ret = 0; struct coresight_platform_data *pdata; + struct coresight_connection *conn; struct device_node *ep = NULL;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); @@ -211,6 +196,7 @@ of_get_coresight_platform_data(struct device *dev, if (ret) return ERR_PTR(ret);
+ conn = pdata->conns; /* Iterate through each port to discover topology */ while ((ep = of_graph_get_next_endpoint(node, ep))) { /* @@ -220,7 +206,7 @@ of_get_coresight_platform_data(struct device *dev, if (of_coresight_ep_is_input(ep)) continue;
- ret = of_coresight_parse_endpoint(dev, ep, pdata, &i); + ret = of_coresight_parse_endpoint(dev, ep, &conn); if (ret) return ERR_PTR(ret); } diff --git a/include/linux/coresight.h b/include/linux/coresight.h index d828a6e..41e1f43 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -94,20 +94,15 @@ union coresight_dev_subtype { * @cpu: the CPU a source belongs to. Only applicable for ETM/PTMs. * @name: name of the component as shown under sysfs. * @nr_inport: number of input ports for this component. - * @outports: list of remote endpoint port number. - * @child_names:name of all child components connected to this device. - * @child_ports:child component port number the current component is - connected to. * @nr_outport: number of output ports for this component. + * @conns: Array of nr_outport connections from this component */ struct coresight_platform_data { int cpu; const char *name; int nr_inport; - int *outports; - const char **child_names; - int *child_ports; int nr_outport; + struct coresight_connection *conns; };
/**
Before we update the bindings, document the current graph bindings and usage of additional properties.
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- .../devicetree/bindings/arm/coresight.txt | 30 +++++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 5d1ad09..8e21512 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -54,9 +54,7 @@ its hardware characteristcs. clocks the core of that coresight component. The latter clock is optional.
- * port or ports: The representation of the component's port - layout using the generic DT graph presentation found in - "bindings/graph.txt". + * port or ports: see "Graph bindings for Coresight" below.
* Additional required properties for System Trace Macrocells (STM): * reg: along with the physical base address and length of the register @@ -73,7 +71,7 @@ its hardware characteristcs. AMBA markee): - "arm,coresight-replicator"
- * port or ports: same as above. + * port or ports: see "Graph bindings for Coresight" below.
* Optional properties for ETM/PTMs:
@@ -96,6 +94,30 @@ its hardware characteristcs. * interrupts : Exactly one SPI may be listed for reporting the address error
+Graph bindings for Coresight +------------------------------- + +Coresight components are interconnected to create a data path for the flow of +trace data generated from the "sources" to their collection points "sink". +Each coresight component must describe the "input" and "output" connections. +The connections must be described via generic DT graph bindings as described +by the "bindings/graph.txt", where each "port" along with an "endpoint" +component represents a hardware port and the connection. + +Since it is possible to have multiple connections for any coresight component +with a specific direction of data flow, each connection must define the +following properties to uniquely identify the connection details. + + * Direction of the data flow w.r.t the component : + Each input port must have the following property defined at the "endpoint" + for the port. + "slave-mode" + + * Hardware Port number at the component: + - The hardware port number is assumed to be the address of the "port" + component. + + Example:
1. Sinks
The coresight drivers relied on default bindings for graph in DT, while reusing the "reg" field of the "ports" to indicate the actual hardware port number for the connections. This can cause duplicate ports with same addresses, but different direction. However, with the rules getting stricter w.r.t to the address mismatch with the label, it is no longer possible to use the port address field for the hardware port number.
This patch introduces new DT binding rules for coresight components, based on the same generic DT graph bindings, but avoiding the address duplication.
- All output ports must be specified under a child node with name "out-ports". - All input ports must be specified under a childe node with name "in-ports". - Port address should match the hardware port number.
The support for legacy bindings is retained, with a warning.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Rob Herring robh@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- .../devicetree/bindings/arm/coresight.txt | 91 ++++++++++---------- drivers/hwtracing/coresight/of_coresight.c | 97 +++++++++++++++++++--- 2 files changed, 129 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 8e21512..f39d2c6 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -104,19 +104,9 @@ The connections must be described via generic DT graph bindings as described by the "bindings/graph.txt", where each "port" along with an "endpoint" component represents a hardware port and the connection.
-Since it is possible to have multiple connections for any coresight component -with a specific direction of data flow, each connection must define the -following properties to uniquely identify the connection details. - - * Direction of the data flow w.r.t the component : - Each input port must have the following property defined at the "endpoint" - for the port. - "slave-mode" - - * Hardware Port number at the component: - - The hardware port number is assumed to be the address of the "port" - component. - + * All output ports must be listed inside a child node named "out-ports" + * All input ports must be listed inside a child node named "in-ports". + * Port address must match the hardware port number.
Example:
@@ -127,10 +117,11 @@ Example:
clocks = <&oscclk6a>; clock-names = "apb_pclk"; - port { - etb_in_port: endpoint@0 { - slave-mode; - remote-endpoint = <&replicator_out_port0>; + in-ports { + port { + etb_in_port: endpoint@0 { + remote-endpoint = <&replicator_out_port0>; + }; }; }; }; @@ -141,10 +132,11 @@ Example:
clocks = <&oscclk6a>; clock-names = "apb_pclk"; - port { - tpiu_in_port: endpoint@0 { - slave-mode; - remote-endpoint = <&replicator_out_port1>; + out-ports { + port { + tpiu_in_port: endpoint@0 { + remote-endpoint = <&replicator_out_port1>; + }; }; }; }; @@ -185,7 +177,7 @@ Example: */ compatible = "arm,coresight-replicator";
- ports { + out-ports { #address-cells = <1>; #size-cells = <0>;
@@ -203,12 +195,11 @@ Example: remote-endpoint = <&tpiu_in_port>; }; }; + };
- /* replicator input port */ - port@2 { - reg = <0>; + in-ports { + port { replicator_in_port0: endpoint { - slave-mode; remote-endpoint = <&funnel_out_port0>; }; }; @@ -221,40 +212,36 @@ Example:
clocks = <&oscclk6a>; clock-names = "apb_pclk"; - ports { - #address-cells = <1>; - #size-cells = <0>; - - /* funnel output port */ - port@0 { - reg = <0>; + out-ports { + port { funnel_out_port0: endpoint { remote-endpoint = <&replicator_in_port0>; }; }; + };
- /* funnel input ports */ - port@1 { + in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { reg = <0>; funnel_in_port0: endpoint { - slave-mode; remote-endpoint = <&ptm0_out_port>; }; };
- port@2 { + port@1 { reg = <1>; funnel_in_port1: endpoint { - slave-mode; remote-endpoint = <&ptm1_out_port>; }; };
- port@3 { + port@2 { reg = <2>; funnel_in_port2: endpoint { - slave-mode; remote-endpoint = <&etm0_out_port>; }; }; @@ -270,9 +257,11 @@ Example: cpu = <&cpu0>; clocks = <&oscclk6a>; clock-names = "apb_pclk"; - port { - ptm0_out_port: endpoint { - remote-endpoint = <&funnel_in_port0>; + out-ports { + port { + ptm0_out_port: endpoint { + remote-endpoint = <&funnel_in_port0>; + }; }; }; }; @@ -284,9 +273,11 @@ Example: cpu = <&cpu1>; clocks = <&oscclk6a>; clock-names = "apb_pclk"; - port { - ptm1_out_port: endpoint { - remote-endpoint = <&funnel_in_port1>; + out-ports { + port { + ptm1_out_port: endpoint { + remote-endpoint = <&funnel_in_port1>; + }; }; }; }; @@ -300,9 +291,11 @@ Example:
clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; - port { - stm_out_port: endpoint { - remote-endpoint = <&main_funnel_in_port2>; + out-ports { + port { + stm_out_port: endpoint { + remote-endpoint = <&main_funnel_in_port2>; + }; }; }; }; diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index f9e2169..2178734 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -45,13 +45,13 @@ of_coresight_get_endpoint_device(struct device_node *endpoint) endpoint, of_dev_node_match); }
-static inline bool of_coresight_ep_is_input(struct device_node *ep) +static inline bool of_coresight_legacy_ep_is_input(struct device_node *ep) { return of_property_read_bool(ep, "slave-mode"); }
-static void of_coresight_get_ports(const struct device_node *node, - int *nr_inport, int *nr_outport) +static void of_coresight_get_ports_legacy(const struct device_node *node, + int *nr_inport, int *nr_outport) { struct device_node *ep = NULL; int in = 0, out = 0; @@ -61,7 +61,7 @@ static void of_coresight_get_ports(const struct device_node *node, if (!ep) break;
- if (of_coresight_ep_is_input(ep)) + if (of_coresight_legacy_ep_is_input(ep)) in++; else out++; @@ -72,6 +72,67 @@ static void of_coresight_get_ports(const struct device_node *node, *nr_outport = out; }
+static struct device_node *of_coresight_get_port_parent(struct device_node *ep) +{ + struct device_node *parent = of_graph_get_port_parent(ep); + + /* + * Skip one-level up to the real device node, if we + * are using the new bindings. + */ + if (!of_node_cmp(parent->name, "in-ports") || + !of_node_cmp(parent->name, "out-ports")) + parent = of_get_next_parent(parent); + + return parent; +} + +static inline struct device_node * +of_coresight_get_input_ports_node(const struct device_node *node) +{ + return of_get_child_by_name(node, "in-ports"); +} + +static inline struct device_node * +of_coresight_get_output_ports_node(const struct device_node *node) +{ + return of_get_child_by_name(node, "out-ports"); +} + +static inline int +of_coresight_count_ports(struct device_node *port_parent) +{ + int i = 0; + struct device_node *ep = NULL; + + while ((ep = of_graph_get_next_endpoint(port_parent, ep))) + i++; + return i; +} + +static void of_coresight_get_ports(const struct device_node *node, + int *nr_inport, int *nr_outport) +{ + struct device_node *input_ports = NULL, *output_ports = NULL; + + input_ports = of_coresight_get_input_ports_node(node); + output_ports = of_coresight_get_output_ports_node(node); + + if (input_ports || output_ports) { + if (input_ports) { + *nr_inport = of_coresight_count_ports(input_ports); + of_node_put(input_ports); + } + if (output_ports) { + *nr_outport = of_coresight_count_ports(output_ports); + of_node_put(output_ports); + } + } else { + /* Fall back to legacy DT bindings parsing */ + of_coresight_get_ports_legacy(node, nr_inport, nr_outport); + } +} + static int of_coresight_alloc_memory(struct device *dev, struct coresight_platform_data *pdata) { @@ -136,7 +197,7 @@ static int of_coresight_parse_endpoint(struct device *dev, rep = of_graph_get_remote_endpoint(ep); if (!rep) break; - rparent = of_graph_get_port_parent(rep); + rparent = of_coresight_get_port_parent(rep); if (!rparent) break; if (of_graph_parse_endpoint(rep, &rendpoint)) @@ -176,6 +237,8 @@ of_get_coresight_platform_data(struct device *dev, struct coresight_platform_data *pdata; struct coresight_connection *conn; struct device_node *ep = NULL; + const struct device_node *parent = NULL; + bool legacy_binding = false;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) @@ -196,14 +259,28 @@ of_get_coresight_platform_data(struct device *dev, if (ret) return ERR_PTR(ret);
+ parent = of_coresight_get_output_ports_node(node); + /* + * If the DT uses obsoleted bindings, the ports are listed + * under the device and we need to filter out the input + * ports. + */ + if (!parent) { + legacy_binding = true; + parent = node; + dev_warn_once(dev, "Uses obsolete Coresight DT bindings\n"); + } + conn = pdata->conns; - /* Iterate through each port to discover topology */ - while ((ep = of_graph_get_next_endpoint(node, ep))) { + + /* Iterate through each output port to discover topology */ + while ((ep = of_graph_get_next_endpoint(parent, ep))) { /* - * No need to deal with input ports, processing for as - * processing for output ports will deal with them. + * Legacy binding mixes input/output ports under the + * same parent. So, skip the input ports if we are dealing + * with legacy binding. */ - if (of_coresight_ep_is_input(ep)) + if (legacy_binding && of_coresight_legacy_ep_is_input(ep)) continue;
ret = of_coresight_parse_endpoint(dev, ep, &conn);
On Thu, Jul 19, 2018 at 11:55:13AM +0100, Suzuki K Poulose wrote:
The coresight drivers relied on default bindings for graph in DT, while reusing the "reg" field of the "ports" to indicate the actual hardware port number for the connections. This can cause duplicate ports with same addresses, but different direction. However, with the rules getting stricter w.r.t to the address mismatch with the label, it is no longer possible to use the port address field for the hardware port number.
This patch introduces new DT binding rules for coresight components, based on the same generic DT graph bindings, but avoiding the address duplication.
- All output ports must be specified under a child node with name "out-ports".
- All input ports must be specified under a childe node with name "in-ports".
- Port address should match the hardware port number.
The support for legacy bindings is retained, with a warning.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Rob Herring robh@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
.../devicetree/bindings/arm/coresight.txt | 91 ++++++++++---------- drivers/hwtracing/coresight/of_coresight.c | 97 +++++++++++++++++++--- 2 files changed, 129 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 8e21512..f39d2c6 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -104,19 +104,9 @@ The connections must be described via generic DT graph bindings as described by the "bindings/graph.txt", where each "port" along with an "endpoint" component represents a hardware port and the connection. -Since it is possible to have multiple connections for any coresight component -with a specific direction of data flow, each connection must define the -following properties to uniquely identify the connection details.
- Direction of the data flow w.r.t the component :
- Each input port must have the following property defined at the "endpoint"
- for the port.
- "slave-mode"
- Hardware Port number at the component:
- The hardware port number is assumed to be the address of the "port"
component.
Why do you add this in the previous patch and then remove it here?
- All output ports must be listed inside a child node named "out-ports"
- All input ports must be listed inside a child node named "in-ports".
- Port address must match the hardware port number.
On 07/25/2018 05:09 PM, Rob Herring wrote:
On Thu, Jul 19, 2018 at 11:55:13AM +0100, Suzuki K Poulose wrote:
The coresight drivers relied on default bindings for graph in DT, while reusing the "reg" field of the "ports" to indicate the actual hardware port number for the connections. This can cause duplicate ports with same addresses, but different direction. However, with the rules getting stricter w.r.t to the address mismatch with the label, it is no longer possible to use the port address field for the hardware port number.
This patch introduces new DT binding rules for coresight components, based on the same generic DT graph bindings, but avoiding the address duplication.
- All output ports must be specified under a child node with name "out-ports".
- All input ports must be specified under a childe node with name "in-ports".
- Port address should match the hardware port number.
The support for legacy bindings is retained, with a warning.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Rob Herring robh@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
.../devicetree/bindings/arm/coresight.txt | 91 ++++++++++---------- drivers/hwtracing/coresight/of_coresight.c | 97 +++++++++++++++++++--- 2 files changed, 129 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 8e21512..f39d2c6 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -104,19 +104,9 @@ The connections must be described via generic DT graph bindings as described by the "bindings/graph.txt", where each "port" along with an "endpoint" component represents a hardware port and the connection. -Since it is possible to have multiple connections for any coresight component -with a specific direction of data flow, each connection must define the -following properties to uniquely identify the connection details.
- Direction of the data flow w.r.t the component :
- Each input port must have the following property defined at the "endpoint"
- for the port.
- "slave-mode"
- Hardware Port number at the component:
- The hardware port number is assumed to be the address of the "port"
component.
Why do you add this in the previous patch and then remove it here?
The only use case I can think of it is for someone to look back on the legacy bindings, which were never documented. I could skip the parts that are being removed.
Suzuki
On Wed, Jul 25, 2018 at 10:13 AM Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 07/25/2018 05:09 PM, Rob Herring wrote:
On Thu, Jul 19, 2018 at 11:55:13AM +0100, Suzuki K Poulose wrote:
The coresight drivers relied on default bindings for graph in DT, while reusing the "reg" field of the "ports" to indicate the actual hardware port number for the connections. This can cause duplicate ports with same addresses, but different direction. However, with the rules getting stricter w.r.t to the address mismatch with the label, it is no longer possible to use the port address field for the hardware port number.
This patch introduces new DT binding rules for coresight components, based on the same generic DT graph bindings, but avoiding the address duplication.
- All output ports must be specified under a child node with name "out-ports".
- All input ports must be specified under a childe node with name "in-ports".
- Port address should match the hardware port number.
The support for legacy bindings is retained, with a warning.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Rob Herring robh@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
.../devicetree/bindings/arm/coresight.txt | 91 ++++++++++---------- drivers/hwtracing/coresight/of_coresight.c | 97 +++++++++++++++++++--- 2 files changed, 129 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 8e21512..f39d2c6 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -104,19 +104,9 @@ The connections must be described via generic DT graph bindings as described by the "bindings/graph.txt", where each "port" along with an "endpoint" component represents a hardware port and the connection.
-Since it is possible to have multiple connections for any coresight component -with a specific direction of data flow, each connection must define the -following properties to uniquely identify the connection details.
- Direction of the data flow w.r.t the component :
- Each input port must have the following property defined at the "endpoint"
- for the port.
- "slave-mode"
- Hardware Port number at the component:
- The hardware port number is assumed to be the address of the "port"
component.
Why do you add this in the previous patch and then remove it here?
The only use case I can think of it is for someone to look back on the legacy bindings, which were never documented. I could skip the parts that are being removed.
Yeah, I'd just remove it. Finding the text that exists for 1 commit is a little hard. Now that we've discussed it here, googling for it will be easier.
Rob
Hello,
On Thu, Jul 19, 2018 at 11:55:13AM +0100, Suzuki K Poulose wrote:
The coresight drivers relied on default bindings for graph in DT, while reusing the "reg" field of the "ports" to indicate the actual hardware port number for the connections. This can cause duplicate ports with same addresses, but different direction. However, with the rules getting stricter w.r.t to the
It's only a matter of time before someone calls you out on the "w.r.t". Better to simply spell it out.
address mismatch with the label, it is no longer possible to use the port address field for the hardware port number.
This patch introduces new DT binding rules for coresight components, based on the same generic DT graph bindings, but avoiding the address duplication.
- All output ports must be specified under a child node with name "out-ports".
- All input ports must be specified under a childe node with name "in-ports".
- Port address should match the hardware port number.
The support for legacy bindings is retained, with a warning.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Rob Herring robh@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
.../devicetree/bindings/arm/coresight.txt | 91 ++++++++++---------- drivers/hwtracing/coresight/of_coresight.c | 97 +++++++++++++++++++--- 2 files changed, 129 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 8e21512..f39d2c6 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -104,19 +104,9 @@ The connections must be described via generic DT graph bindings as described by the "bindings/graph.txt", where each "port" along with an "endpoint" component represents a hardware port and the connection. -Since it is possible to have multiple connections for any coresight component -with a specific direction of data flow, each connection must define the -following properties to uniquely identify the connection details.
- Direction of the data flow w.r.t the component :
Same here.
With those changes:
Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org
- Each input port must have the following property defined at the "endpoint"
- for the port.
- "slave-mode"
- Hardware Port number at the component:
- The hardware port number is assumed to be the address of the "port"
component.
- All output ports must be listed inside a child node named "out-ports"
- All input ports must be listed inside a child node named "in-ports".
- Port address must match the hardware port number.
Example: @@ -127,10 +117,11 @@ Example: clocks = <&oscclk6a>; clock-names = "apb_pclk";
port {
etb_in_port: endpoint@0 {
slave-mode;
remote-endpoint = <&replicator_out_port0>;
in-ports {
port {
etb_in_port: endpoint@0 {
remote-endpoint = <&replicator_out_port0>;
}; };}; };
@@ -141,10 +132,11 @@ Example: clocks = <&oscclk6a>; clock-names = "apb_pclk";
port {
tpiu_in_port: endpoint@0 {
slave-mode;
remote-endpoint = <&replicator_out_port1>;
out-ports {
port {
tpiu_in_port: endpoint@0 {
remote-endpoint = <&replicator_out_port1>;
}; };}; };
@@ -185,7 +177,7 @@ Example: */ compatible = "arm,coresight-replicator";
ports {
out-ports { #address-cells = <1>; #size-cells = <0>;
@@ -203,12 +195,11 @@ Example: remote-endpoint = <&tpiu_in_port>; }; };
};
/* replicator input port */
port@2 {
reg = <0>;
in-ports {
port { replicator_in_port0: endpoint {
slave-mode; remote-endpoint = <&funnel_out_port0>; }; };
@@ -221,40 +212,36 @@ Example: clocks = <&oscclk6a>; clock-names = "apb_pclk";
ports {
#address-cells = <1>;
#size-cells = <0>;
/* funnel output port */
port@0 {
reg = <0>;
out-ports {
port { funnel_out_port0: endpoint { remote-endpoint = <&replicator_in_port0>; }; };
};
/* funnel input ports */
port@1 {
in-ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 { reg = <0>; funnel_in_port0: endpoint {
slave-mode; remote-endpoint = <&ptm0_out_port>; }; };
port@2 {
port@1 { reg = <1>; funnel_in_port1: endpoint {
slave-mode; remote-endpoint = <&ptm1_out_port>; }; };
port@3 {
port@2 { reg = <2>; funnel_in_port2: endpoint {
slave-mode; remote-endpoint = <&etm0_out_port>; }; };
@@ -270,9 +257,11 @@ Example: cpu = <&cpu0>; clocks = <&oscclk6a>; clock-names = "apb_pclk";
port {
ptm0_out_port: endpoint {
remote-endpoint = <&funnel_in_port0>;
out-ports {
port {
ptm0_out_port: endpoint {
remote-endpoint = <&funnel_in_port0>;
}; };}; };
@@ -284,9 +273,11 @@ Example: cpu = <&cpu1>; clocks = <&oscclk6a>; clock-names = "apb_pclk";
port {
ptm1_out_port: endpoint {
remote-endpoint = <&funnel_in_port1>;
out-ports {
port {
ptm1_out_port: endpoint {
remote-endpoint = <&funnel_in_port1>;
}; };}; };
@@ -300,9 +291,11 @@ Example: clocks = <&soc_smc50mhz>; clock-names = "apb_pclk";
port {
stm_out_port: endpoint {
remote-endpoint = <&main_funnel_in_port2>;
out-ports {
port {
stm_out_port: endpoint {
remote-endpoint = <&main_funnel_in_port2>;
}; };}; };
diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c index f9e2169..2178734 100644 --- a/drivers/hwtracing/coresight/of_coresight.c +++ b/drivers/hwtracing/coresight/of_coresight.c @@ -45,13 +45,13 @@ of_coresight_get_endpoint_device(struct device_node *endpoint) endpoint, of_dev_node_match); } -static inline bool of_coresight_ep_is_input(struct device_node *ep) +static inline bool of_coresight_legacy_ep_is_input(struct device_node *ep) { return of_property_read_bool(ep, "slave-mode"); } -static void of_coresight_get_ports(const struct device_node *node,
int *nr_inport, int *nr_outport)
+static void of_coresight_get_ports_legacy(const struct device_node *node,
int *nr_inport, int *nr_outport)
{ struct device_node *ep = NULL; int in = 0, out = 0; @@ -61,7 +61,7 @@ static void of_coresight_get_ports(const struct device_node *node, if (!ep) break;
if (of_coresight_ep_is_input(ep))
else out++;if (of_coresight_legacy_ep_is_input(ep)) in++;
@@ -72,6 +72,67 @@ static void of_coresight_get_ports(const struct device_node *node, *nr_outport = out; } +static struct device_node *of_coresight_get_port_parent(struct device_node *ep) +{
- struct device_node *parent = of_graph_get_port_parent(ep);
- /*
* Skip one-level up to the real device node, if we
* are using the new bindings.
*/
- if (!of_node_cmp(parent->name, "in-ports") ||
!of_node_cmp(parent->name, "out-ports"))
parent = of_get_next_parent(parent);
- return parent;
+}
+static inline struct device_node * +of_coresight_get_input_ports_node(const struct device_node *node) +{
- return of_get_child_by_name(node, "in-ports");
+}
+static inline struct device_node * +of_coresight_get_output_ports_node(const struct device_node *node) +{
- return of_get_child_by_name(node, "out-ports");
+}
+static inline int +of_coresight_count_ports(struct device_node *port_parent) +{
- int i = 0;
- struct device_node *ep = NULL;
- while ((ep = of_graph_get_next_endpoint(port_parent, ep)))
i++;
- return i;
+}
+static void of_coresight_get_ports(const struct device_node *node,
int *nr_inport, int *nr_outport)
+{
- struct device_node *input_ports = NULL, *output_ports = NULL;
- input_ports = of_coresight_get_input_ports_node(node);
- output_ports = of_coresight_get_output_ports_node(node);
- if (input_ports || output_ports) {
if (input_ports) {
*nr_inport = of_coresight_count_ports(input_ports);
of_node_put(input_ports);
}
if (output_ports) {
*nr_outport = of_coresight_count_ports(output_ports);
of_node_put(output_ports);
}
- } else {
/* Fall back to legacy DT bindings parsing */
of_coresight_get_ports_legacy(node, nr_inport, nr_outport);
- }
+}
static int of_coresight_alloc_memory(struct device *dev, struct coresight_platform_data *pdata) { @@ -136,7 +197,7 @@ static int of_coresight_parse_endpoint(struct device *dev, rep = of_graph_get_remote_endpoint(ep); if (!rep) break;
rparent = of_graph_get_port_parent(rep);
if (!rparent) break; if (of_graph_parse_endpoint(rep, &rendpoint))rparent = of_coresight_get_port_parent(rep);
@@ -176,6 +237,8 @@ of_get_coresight_platform_data(struct device *dev, struct coresight_platform_data *pdata; struct coresight_connection *conn; struct device_node *ep = NULL;
- const struct device_node *parent = NULL;
- bool legacy_binding = false;
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) @@ -196,14 +259,28 @@ of_get_coresight_platform_data(struct device *dev, if (ret) return ERR_PTR(ret);
- parent = of_coresight_get_output_ports_node(node);
- /*
* If the DT uses obsoleted bindings, the ports are listed
* under the device and we need to filter out the input
* ports.
*/
- if (!parent) {
legacy_binding = true;
parent = node;
dev_warn_once(dev, "Uses obsolete Coresight DT bindings\n");
- }
- conn = pdata->conns;
- /* Iterate through each port to discover topology */
- while ((ep = of_graph_get_next_endpoint(node, ep))) {
- /* Iterate through each output port to discover topology */
- while ((ep = of_graph_get_next_endpoint(parent, ep))) { /*
* No need to deal with input ports, processing for as
* processing for output ports will deal with them.
* Legacy binding mixes input/output ports under the
* same parent. So, skip the input ports if we are dealing
*/* with legacy binding.
if (of_coresight_ep_is_input(ep))
if (legacy_binding && of_coresight_legacy_ep_is_input(ep)) continue;
ret = of_coresight_parse_endpoint(dev, ep, &conn); -- 2.7.4
Switch to updated coresight bindings for hw ports.
Cc: Sudeep Holla sudeep.holla@arm.com Cc: Liviu Dudau liviu.dudau@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since V2: - Update to new format. --- arch/arm64/boot/dts/arm/juno-base.dtsi | 161 +++++++++++++++--------------- arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 52 ++++------ arch/arm64/boot/dts/arm/juno.dts | 13 +-- 3 files changed, 105 insertions(+), 121 deletions(-)
diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi index ce56a4a..a418193 100644 --- a/arch/arm64/boot/dts/arm/juno-base.dtsi +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi @@ -115,22 +115,17 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>;
- /* input port */ - port@0 { - reg = <0>; + in-ports { + port { etf0_in_port: endpoint { - slave-mode; remote-endpoint = <&main_funnel_out_port>; }; }; + };
- /* output port */ - port@1 { - reg = <0>; + out-ports { + port { etf0_out_port: endpoint { }; }; @@ -144,10 +139,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - tpiu_in_port: endpoint { - slave-mode; - remote-endpoint = <&replicator_out_port0>; + in-ports { + port { + tpiu_in_port: endpoint { + remote-endpoint = <&replicator_out_port0>; + }; }; }; }; @@ -160,31 +156,29 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>;
- /* output port */ - port@0 { - reg = <0>; + out-ports { + port { main_funnel_out_port: endpoint { remote-endpoint = <&etf0_in_port>; }; }; + };
- /* input ports */ - port@1 { + main_funnel_in_ports: in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { reg = <0>; main_funnel_in_port0: endpoint { - slave-mode; remote-endpoint = <&cluster0_funnel_out_port>; }; };
- port@2 { + port@1 { reg = <1>; main_funnel_in_port1: endpoint { - slave-mode; remote-endpoint = <&cluster1_funnel_out_port>; }; }; @@ -199,10 +193,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - etr_in_port: endpoint { - slave-mode; - remote-endpoint = <&replicator_out_port1>; + in-ports { + port { + etr_in_port: endpoint { + remote-endpoint = <&replicator_out_port1>; + }; }; }; }; @@ -216,8 +211,10 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - stm_out_port: endpoint { + out-ports { + port { + stm_out_port: endpoint { + }; }; }; }; @@ -238,9 +235,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster0_etm0_out_port: endpoint { - remote-endpoint = <&cluster0_funnel_in_port0>; + out-ports { + port { + cluster0_etm0_out_port: endpoint { + remote-endpoint = <&cluster0_funnel_in_port0>; + }; }; }; }; @@ -252,29 +251,28 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; + out-ports { + port { cluster0_funnel_out_port: endpoint { remote-endpoint = <&main_funnel_in_port0>; }; }; + };
- port@1 { + in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { reg = <0>; cluster0_funnel_in_port0: endpoint { - slave-mode; remote-endpoint = <&cluster0_etm0_out_port>; }; };
- port@2 { + port@1 { reg = <1>; cluster0_funnel_in_port1: endpoint { - slave-mode; remote-endpoint = <&cluster0_etm1_out_port>; }; }; @@ -297,9 +295,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster0_etm1_out_port: endpoint { - remote-endpoint = <&cluster0_funnel_in_port1>; + out-ports { + port { + cluster0_etm1_out_port: endpoint { + remote-endpoint = <&cluster0_funnel_in_port1>; + }; }; }; }; @@ -320,9 +320,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm0_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port0>; + out-ports { + port { + cluster1_etm0_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port0>; + }; }; }; }; @@ -334,43 +336,40 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; + out-ports { + port { cluster1_funnel_out_port: endpoint { remote-endpoint = <&main_funnel_in_port1>; }; }; + };
- port@1 { + in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { reg = <0>; cluster1_funnel_in_port0: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm0_out_port>; }; };
- port@2 { + port@1 { reg = <1>; cluster1_funnel_in_port1: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm1_out_port>; }; }; - port@3 { + port@2 { reg = <2>; cluster1_funnel_in_port2: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm2_out_port>; }; }; - port@4 { + port@3 { reg = <3>; cluster1_funnel_in_port3: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm3_out_port>; }; }; @@ -393,9 +392,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm1_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port1>; + out-ports { + port { + cluster1_etm1_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port1>; + }; }; }; }; @@ -416,9 +417,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm2_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port2>; + out-ports { + port { + cluster1_etm2_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port2>; + }; }; }; }; @@ -439,9 +442,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm3_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port3>; + out-ports { + port { + cluster1_etm3_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port3>; + }; }; }; }; @@ -454,7 +459,7 @@ clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>;
- ports { + out-ports { #address-cells = <1>; #size-cells = <0>;
@@ -472,12 +477,10 @@ remote-endpoint = <&etr_in_port>; }; }; - - /* replicator input port */ - port@2 { - reg = <0>; + }; + in-ports { + port { replicator_in_port0: endpoint { - slave-mode; }; }; }; diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi index 0c43fb3..cf28515 100644 --- a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi +++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi @@ -7,23 +7,16 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - /* output port */ - port@0 { - reg = <0>; + out-ports { + port { csys1_funnel_out_port: endpoint { remote-endpoint = <&etf1_in_port>; }; }; - - /* input port */ - port@1 { - reg = <0>; + }; + in-ports { + port { csys1_funnel_in_port0: endpoint { - slave-mode; }; };
@@ -37,22 +30,15 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - /* input port */ - port@0 { - reg = <0>; + in-ports { + port { etf1_in_port: endpoint { - slave-mode; remote-endpoint = <&csys1_funnel_out_port>; }; }; - - /* output port */ - port@1 { - reg = <0>; + }; + out-ports { + port { etf1_out_port: endpoint { remote-endpoint = <&csys2_funnel_in_port1>; }; @@ -67,20 +53,18 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - /* output port */ - port@0 { - reg = <0>; + out-ports { + port { csys2_funnel_out_port: endpoint { remote-endpoint = <&replicator_in_port0>; }; }; + };
- /* input ports */ - port@1 { + in-ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { reg = <0>; csys2_funnel_in_port0: endpoint { slave-mode; @@ -88,7 +72,7 @@ }; };
- port@2 { + port@1 { reg = <1>; csys2_funnel_in_port1: endpoint { slave-mode; diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts index 2b2bf39..eb55c2f 100644 --- a/arch/arm64/boot/dts/arm/juno.dts +++ b/arch/arm64/boot/dts/arm/juno.dts @@ -257,14 +257,11 @@ remote-endpoint = <&main_funnel_in_port2>; };
-&main_funnel { - ports { - port@3 { - reg = <2>; - main_funnel_in_port2: endpoint { - slave-mode; - remote-endpoint = <&stm_out_port>; - }; +&main_funnel_in_ports { + port@2 { + reg = <2>; + main_funnel_in_port2: endpoint { + remote-endpoint = <&stm_out_port>; }; }; };
On 19/07/18 11:55, Suzuki K Poulose wrote:
Switch to updated coresight bindings for hw ports.
Cc: Sudeep Holla sudeep.holla@arm.com Cc: Liviu Dudau liviu.dudau@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com
Please ignore this patch, this is indeed a duplicate of the real patch 10/10 in the series.
Suzuki
Switch to updated coresight bindings for Juno platforms.
Cc: Sudeep Holla sudeep.holla@arm.com Cc: Liviu Dudau liviu.dudau@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since V2: - Update to new format. --- arch/arm64/boot/dts/arm/juno-base.dtsi | 161 +++++++++++++++--------------- arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 52 ++++------ arch/arm64/boot/dts/arm/juno.dts | 13 +-- 3 files changed, 105 insertions(+), 121 deletions(-)
diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi index ce56a4a..a418193 100644 --- a/arch/arm64/boot/dts/arm/juno-base.dtsi +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi @@ -115,22 +115,17 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>;
- /* input port */ - port@0 { - reg = <0>; + in-ports { + port { etf0_in_port: endpoint { - slave-mode; remote-endpoint = <&main_funnel_out_port>; }; }; + };
- /* output port */ - port@1 { - reg = <0>; + out-ports { + port { etf0_out_port: endpoint { }; }; @@ -144,10 +139,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - tpiu_in_port: endpoint { - slave-mode; - remote-endpoint = <&replicator_out_port0>; + in-ports { + port { + tpiu_in_port: endpoint { + remote-endpoint = <&replicator_out_port0>; + }; }; }; }; @@ -160,31 +156,29 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>;
- /* output port */ - port@0 { - reg = <0>; + out-ports { + port { main_funnel_out_port: endpoint { remote-endpoint = <&etf0_in_port>; }; }; + };
- /* input ports */ - port@1 { + main_funnel_in_ports: in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { reg = <0>; main_funnel_in_port0: endpoint { - slave-mode; remote-endpoint = <&cluster0_funnel_out_port>; }; };
- port@2 { + port@1 { reg = <1>; main_funnel_in_port1: endpoint { - slave-mode; remote-endpoint = <&cluster1_funnel_out_port>; }; }; @@ -199,10 +193,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - etr_in_port: endpoint { - slave-mode; - remote-endpoint = <&replicator_out_port1>; + in-ports { + port { + etr_in_port: endpoint { + remote-endpoint = <&replicator_out_port1>; + }; }; }; }; @@ -216,8 +211,10 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - stm_out_port: endpoint { + out-ports { + port { + stm_out_port: endpoint { + }; }; }; }; @@ -238,9 +235,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster0_etm0_out_port: endpoint { - remote-endpoint = <&cluster0_funnel_in_port0>; + out-ports { + port { + cluster0_etm0_out_port: endpoint { + remote-endpoint = <&cluster0_funnel_in_port0>; + }; }; }; }; @@ -252,29 +251,28 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; + out-ports { + port { cluster0_funnel_out_port: endpoint { remote-endpoint = <&main_funnel_in_port0>; }; }; + };
- port@1 { + in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { reg = <0>; cluster0_funnel_in_port0: endpoint { - slave-mode; remote-endpoint = <&cluster0_etm0_out_port>; }; };
- port@2 { + port@1 { reg = <1>; cluster0_funnel_in_port1: endpoint { - slave-mode; remote-endpoint = <&cluster0_etm1_out_port>; }; }; @@ -297,9 +295,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster0_etm1_out_port: endpoint { - remote-endpoint = <&cluster0_funnel_in_port1>; + out-ports { + port { + cluster0_etm1_out_port: endpoint { + remote-endpoint = <&cluster0_funnel_in_port1>; + }; }; }; }; @@ -320,9 +320,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm0_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port0>; + out-ports { + port { + cluster1_etm0_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port0>; + }; }; }; }; @@ -334,43 +336,40 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; + out-ports { + port { cluster1_funnel_out_port: endpoint { remote-endpoint = <&main_funnel_in_port1>; }; }; + };
- port@1 { + in-ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { reg = <0>; cluster1_funnel_in_port0: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm0_out_port>; }; };
- port@2 { + port@1 { reg = <1>; cluster1_funnel_in_port1: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm1_out_port>; }; }; - port@3 { + port@2 { reg = <2>; cluster1_funnel_in_port2: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm2_out_port>; }; }; - port@4 { + port@3 { reg = <3>; cluster1_funnel_in_port3: endpoint { - slave-mode; remote-endpoint = <&cluster1_etm3_out_port>; }; }; @@ -393,9 +392,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm1_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port1>; + out-ports { + port { + cluster1_etm1_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port1>; + }; }; }; }; @@ -416,9 +417,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm2_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port2>; + out-ports { + port { + cluster1_etm2_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port2>; + }; }; }; }; @@ -439,9 +442,11 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - port { - cluster1_etm3_out_port: endpoint { - remote-endpoint = <&cluster1_funnel_in_port3>; + out-ports { + port { + cluster1_etm3_out_port: endpoint { + remote-endpoint = <&cluster1_funnel_in_port3>; + }; }; }; }; @@ -454,7 +459,7 @@ clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>;
- ports { + out-ports { #address-cells = <1>; #size-cells = <0>;
@@ -472,12 +477,10 @@ remote-endpoint = <&etr_in_port>; }; }; - - /* replicator input port */ - port@2 { - reg = <0>; + }; + in-ports { + port { replicator_in_port0: endpoint { - slave-mode; }; }; }; diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi index 0c43fb3..cf28515 100644 --- a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi +++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi @@ -7,23 +7,16 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - /* output port */ - port@0 { - reg = <0>; + out-ports { + port { csys1_funnel_out_port: endpoint { remote-endpoint = <&etf1_in_port>; }; }; - - /* input port */ - port@1 { - reg = <0>; + }; + in-ports { + port { csys1_funnel_in_port0: endpoint { - slave-mode; }; };
@@ -37,22 +30,15 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - /* input port */ - port@0 { - reg = <0>; + in-ports { + port { etf1_in_port: endpoint { - slave-mode; remote-endpoint = <&csys1_funnel_out_port>; }; }; - - /* output port */ - port@1 { - reg = <0>; + }; + out-ports { + port { etf1_out_port: endpoint { remote-endpoint = <&csys2_funnel_in_port1>; }; @@ -67,20 +53,18 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>; - ports { - #address-cells = <1>; - #size-cells = <0>; - - /* output port */ - port@0 { - reg = <0>; + out-ports { + port { csys2_funnel_out_port: endpoint { remote-endpoint = <&replicator_in_port0>; }; }; + };
- /* input ports */ - port@1 { + in-ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { reg = <0>; csys2_funnel_in_port0: endpoint { slave-mode; @@ -88,7 +72,7 @@ }; };
- port@2 { + port@1 { reg = <1>; csys2_funnel_in_port1: endpoint { slave-mode; diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts index 2b2bf39..eb55c2f 100644 --- a/arch/arm64/boot/dts/arm/juno.dts +++ b/arch/arm64/boot/dts/arm/juno.dts @@ -257,14 +257,11 @@ remote-endpoint = <&main_funnel_in_port2>; };
-&main_funnel { - ports { - port@3 { - reg = <2>; - main_funnel_in_port2: endpoint { - slave-mode; - remote-endpoint = <&stm_out_port>; - }; +&main_funnel_in_ports { + port@2 { + reg = <2>; + main_funnel_in_port2: endpoint { + remote-endpoint = <&stm_out_port>; }; }; };