| Message ID | 1528235011-30691-1-git-send-email-suzuki.poulose@arm.com |
|---|---|
| Headers | show |
| Series | coresight: Update device tree bindings | expand |
Hi Suzuki, On Wednesday 06 June 2018 03:13 AM, Suzuki K Poulose wrote: > commit 6403587a930c ("coresight: use put_device() instead of kfree()") > introduced a memory leak where, if we fail to register the device > for coresight_device, we don't free the "coresight_device" object, > which was allocated via kzalloc(). Fix this by jumping to the > appropriate error path. put_device() will decrement the last reference and then free the memory by calling dev->release. Internally put_device() -> kobject_put() -> kobject_cleanup() which is responsible to call 'dev -> release' and also free other kobject resources. If you will see the coresight_device_release. There we are releasing all allocated memory. Still if you call kfree() again then it'll be redundancy. ~arvind > > Fixes: commit 6403587a930c ("coresight: use put_device() instead of kfree()") > 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> > --- > drivers/hwtracing/coresight/coresight.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index 4969b32..2893cfe 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -1020,7 +1020,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > ret = device_register(&csdev->dev); > if (ret) { > put_device(&csdev->dev); > - goto err_kzalloc_csdev; > + goto err_kzalloc_refcnts; > } > > mutex_lock(&coresight_mutex); -- 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
On 06/06/2018 07:44 AM, Arvind Yadav wrote: > Hi Suzuki, > > > On Wednesday 06 June 2018 03:13 AM, Suzuki K Poulose wrote: >> commit 6403587a930c ("coresight: use put_device() instead of kfree()") >> introduced a memory leak where, if we fail to register the device >> for coresight_device, we don't free the "coresight_device" object, >> which was allocated via kzalloc(). Fix this by jumping to the >> appropriate error path. > put_device() will decrement the last reference and then > free the memory by calling dev->release. Internally > put_device() -> kobject_put() -> kobject_cleanup() which is > responsible to call 'dev -> release' and also free other kobject > resources. If you will see the coresight_device_release. There > we are releasing all allocated memory. Still if you call kfree() again > then it'll be redundancy. You're right. I think it would be good to have a comment explaining this to prevent this fix popping up in the future :-). I will add it Thanks 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
On Tue, Jun 05, 2018 at 10:43:15PM +0100, Suzuki K Poulose wrote: > Nobody uses the "clk" field in struct coresight_platform_data. > Remove it. > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > include/linux/coresight.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index e5421b8..69a5c9f 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -87,7 +87,6 @@ struct coresight_dev_subtype { > * @child_ports:child component port number the current component is > connected to. > * @nr_outport: number of output ports for this component. > - * @clk: The clock this component is associated to. > */ > struct coresight_platform_data { > int cpu; > @@ -97,7 +96,6 @@ struct coresight_platform_data { > const char **child_names; > int *child_ports; > int nr_outport; > - struct clk *clk; > }; I'm going to queue this up for the next rc. No need to send it as part of another revision. Thanks, Mathieu > > /** > -- > 2.7.4 > -- 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
On Tue, Jun 05, 2018 at 10:43:13PM +0100, Suzuki K Poulose wrote: > The coresight driver doesn't drop the references on the > remote endpoint/port nodes. Add the missing of_node_put() > calls. To make it easier to handle different corner cases > cleanly, move the parsing of an endpoint to separate > function. Please split this as those are two different things. > > Reported-by: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/hwtracing/coresight/of_coresight.c | 139 +++++++++++++++++------------ > 1 file changed, 84 insertions(+), 55 deletions(-) > > diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c > index a33a92e..8a23c63 100644 > --- a/drivers/hwtracing/coresight/of_coresight.c > +++ b/drivers/hwtracing/coresight/of_coresight.c > @@ -111,17 +111,80 @@ 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_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 { > + /* > + * 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)) > + break; > + > + /* 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); That's a clever way of coding a classic 'goto' block. > + > + if (rparent) > + of_node_put(rparent); > + if (rport) > + of_node_put(rport); Perfect - thank you for that. > + > + 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) > @@ -129,63 +192,29 @@ 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 not output connections, we are done */ /not/no > + 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 */ > + do { > + /* Get a handle on a port */ > + ep = of_graph_get_next_endpoint(node, ep); > + if (!ep) > + break; > + ret = of_coresight_parse_endpoint(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); > + } while (ep); > > return pdata; > } > -- > 2.7.4 > -- 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
On Tue, Jun 05, 2018 at 10:43:14PM +0100, Suzuki K Poulose wrote: > 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. > > Cc: 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 8a23c63..ada4f07 100644 > --- a/drivers/hwtracing/coresight/of_coresight.c > +++ b/drivers/hwtracing/coresight/of_coresight.c > @@ -129,7 +129,7 @@ static int of_coresight_parse_endpoint(struct device_node *ep, > 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 { > @@ -144,16 +144,16 @@ static int of_coresight_parse_endpoint(struct device_node *ep, > 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 */ > @@ -165,15 +165,15 @@ static int of_coresight_parse_endpoint(struct device_node *ep, > > pdata->outports[*i] = endpoint.port; > pdata->child_names[*i] = dev_name(rdev); > - 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); > > return ret; > } Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> (Please add to the next iteration so that I don't have to review again) > -- > 2.7.4 > -- 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
On Tue, Jun 05, 2018 at 10:43:16PM +0100, Suzuki K Poulose wrote: > 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. > > Cc: 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 | 51 ++++++++++++------------------ > include/linux/coresight.h | 9 ++---- > 3 files changed, 23 insertions(+), 58 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index 2893cfe..69e9136 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -953,13 +953,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) { > @@ -988,22 +986,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_kzalloc_conns; > - } > - > - 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; > @@ -1032,8 +1015,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > > return csdev; > > -err_kzalloc_conns: > - kfree(refcnts); > err_kzalloc_refcnts: > kfree(csdev); > err_kzalloc_csdev: > diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c > index ada4f07..d01a9ce 100644 > --- a/drivers/hwtracing/coresight/of_coresight.c > +++ b/drivers/hwtracing/coresight/of_coresight.c > @@ -70,26 +70,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_kzalloc(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_kzalloc(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_kzalloc(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; > } > @@ -113,24 +100,24 @@ 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. > * -Errno - Fatal error, abort the scanning. > */ > static int of_coresight_parse_endpoint(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 { > /* > @@ -163,11 +150,11 @@ static int of_coresight_parse_endpoint(struct device_node *ep, > break; > } > > - pdata->outports[*i] = endpoint.port; > - pdata->child_names[*i] = dev_name(rdev); > - pdata->child_ports[*i] = rendpoint.port; > - /* Move the index */ > - (*i)++; > + conn->outport = endpoint.port; > + conn->child_name = dev_name(rdev); > + conn->child_port = rendpoint.port; > + /* Move the connection record */ > + (*pconn)++; > } while (0); > > if (rparent) > @@ -182,8 +169,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); > @@ -205,13 +193,14 @@ of_get_coresight_platform_data(struct device *dev, > if (ret) > return ERR_PTR(ret); > > + conn = pdata->conns; > /* 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; > - ret = of_coresight_parse_endpoint(ep, pdata, &i); > + ret = of_coresight_parse_endpoint(ep, &conn); > if (ret) > return ERR_PTR(ret); > } while (ep); > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index 69a5c9f..2a75a15 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -82,20 +82,15 @@ struct 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; > }; Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > /** > -- > 2.7.4 > -- 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
On Tue, Jun 05, 2018 at 10:43:17PM +0100, Suzuki K Poulose wrote: > If we fail to find the input / output port for a LINK component > while enabling a path, we should fail gracefully rather than > assuming port "0". > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/hwtracing/coresight/coresight.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index 69e9136..3c1c058 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -107,7 +107,7 @@ static int coresight_find_link_inport(struct coresight_device *csdev, > dev_err(&csdev->dev, "couldn't find inport, parent: %s, child: %s\n", > dev_name(&parent->dev), dev_name(&csdev->dev)); > > - return 0; > + return -ENODEV; > } > > static int coresight_find_link_outport(struct coresight_device *csdev, > @@ -125,7 +125,7 @@ static int coresight_find_link_outport(struct coresight_device *csdev, > dev_err(&csdev->dev, "couldn't find outport, parent: %s, child: %s\n", > dev_name(&csdev->dev), dev_name(&child->dev)); > > - return 0; > + return -ENODEV; > } > > static int coresight_enable_sink(struct coresight_device *csdev, u32 mode) > @@ -178,6 +178,9 @@ static int coresight_enable_link(struct coresight_device *csdev, > else > refport = 0; > > + if (refport < 0) > + return refport; > + > if (atomic_inc_return(&csdev->refcnt[refport]) == 1) { > if (link_ops(csdev)->enable) { > ret = link_ops(csdev)->enable(csdev, inport, outport); Queued for the next rc - no need to resend. Thanks, Mathieu > -- > 2.7.4 > -- 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
On Tue, Jun 05, 2018 at 10:43:21PM +0100, 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> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > Changes since V1: > - Add support Juno for r1 & r2. > --- Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > arch/arm64/boot/dts/arm/juno-base.dtsi | 82 ++++++++++++++++++++++--------- > arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 26 +++++++--- > arch/arm64/boot/dts/arm/juno.dts | 5 +- > 3 files changed, 81 insertions(+), 32 deletions(-) > > diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi > index eb749c5..33b41ba 100644 > --- a/arch/arm64/boot/dts/arm/juno-base.dtsi > +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi > @@ -122,15 +122,18 @@ > port@0 { > reg = <0>; > etf0_in_port: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&main_funnel_out_port>; > + coresight,hwid = <0>; > }; > }; > > /* output port */ > port@1 { > - reg = <0>; > + reg = <1>; > etf0_out_port: endpoint { > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -145,8 +148,9 @@ > power-domains = <&scpi_devpd 0>; > port { > tpiu_in_port: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&replicator_out_port0>; > + coresight,hwid = <0>; > }; > }; > }; > @@ -168,23 +172,27 @@ > reg = <0>; > main_funnel_out_port: endpoint { > remote-endpoint = <&etf0_in_port>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > > /* input ports */ > port@1 { > - reg = <0>; > + reg = <1>; > main_funnel_in_port0: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster0_funnel_out_port>; > + coresight,hwid = <0>; > }; > }; > > port@2 { > - reg = <1>; > + reg = <2>; > main_funnel_in_port1: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster1_funnel_out_port>; > + coresight,hwid = <1>; > }; > }; > }; > @@ -200,8 +208,9 @@ > power-domains = <&scpi_devpd 0>; > port { > etr_in_port: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&replicator_out_port1>; > + coresight,hwid = <0>; > }; > }; > }; > @@ -217,6 +226,8 @@ > power-domains = <&scpi_devpd 0>; > port { > stm_out_port: endpoint { > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -240,6 +251,8 @@ > port { > cluster0_etm0_out_port: endpoint { > remote-endpoint = <&cluster0_funnel_in_port0>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -259,22 +272,26 @@ > reg = <0>; > cluster0_funnel_out_port: endpoint { > remote-endpoint = <&main_funnel_in_port0>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > > port@1 { > - reg = <0>; > + reg = <1>; > cluster0_funnel_in_port0: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster0_etm0_out_port>; > + coresight,hwid = <0>; > }; > }; > > port@2 { > - reg = <1>; > + reg = <2>; > cluster0_funnel_in_port1: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster0_etm1_out_port>; > + coresight,hwid = <1>; > }; > }; > }; > @@ -299,6 +316,8 @@ > port { > cluster0_etm1_out_port: endpoint { > remote-endpoint = <&cluster0_funnel_in_port1>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -322,6 +341,8 @@ > port { > cluster1_etm0_out_port: endpoint { > remote-endpoint = <&cluster1_funnel_in_port0>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -341,36 +362,42 @@ > reg = <0>; > cluster1_funnel_out_port: endpoint { > remote-endpoint = <&main_funnel_in_port1>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > > port@1 { > - reg = <0>; > + reg = <1>; > cluster1_funnel_in_port0: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster1_etm0_out_port>; > + coresight,hwid = <0>; > }; > }; > > port@2 { > - reg = <1>; > + reg = <2>; > cluster1_funnel_in_port1: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster1_etm1_out_port>; > + coresight,hwid = <1>; > }; > }; > port@3 { > - reg = <2>; > + reg = <3>; > cluster1_funnel_in_port2: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster1_etm2_out_port>; > + coresight,hwid = <2>; > }; > }; > port@4 { > - reg = <3>; > + reg = <4>; > cluster1_funnel_in_port3: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&cluster1_etm3_out_port>; > + coresight,hwid = <3>; > }; > }; > }; > @@ -395,6 +422,8 @@ > port { > cluster1_etm1_out_port: endpoint { > remote-endpoint = <&cluster1_funnel_in_port1>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -418,6 +447,8 @@ > port { > cluster1_etm2_out_port: endpoint { > remote-endpoint = <&cluster1_funnel_in_port2>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -441,6 +472,8 @@ > port { > cluster1_etm3_out_port: endpoint { > remote-endpoint = <&cluster1_funnel_in_port3>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > }; > @@ -462,6 +495,8 @@ > reg = <0>; > replicator_out_port0: endpoint { > remote-endpoint = <&tpiu_in_port>; > + coresight,hwid = <0>; > + direction = <1>; > }; > }; > > @@ -469,14 +504,17 @@ > reg = <1>; > replicator_out_port1: endpoint { > remote-endpoint = <&etr_in_port>; > + coresight,hwid = <1>; > + direction = <1>; > }; > }; > > /* replicator input port */ > port@2 { > - reg = <0>; > + reg = <2>; > replicator_in_port0: endpoint { > - slave-mode; > + direction = <0>; > + coresight,hwid = <0>; > }; > }; > }; > diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi > index 0c43fb3..146a5d9 100644 > --- a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi > +++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi > @@ -15,15 +15,18 @@ > port@0 { > reg = <0>; > csys1_funnel_out_port: endpoint { > + coresight,hwid = <0>; > + direction = <1>; > remote-endpoint = <&etf1_in_port>; > }; > }; > > /* input port */ > port@1 { > - reg = <0>; > + reg = <1>; > csys1_funnel_in_port0: endpoint { > - slave-mode; > + coresight,hwid = <0>; > + direction = <0>; > }; > }; > > @@ -45,15 +48,18 @@ > port@0 { > reg = <0>; > etf1_in_port: endpoint { > - slave-mode; > + direction = <0>; > + coresight,hwid = <0>; > remote-endpoint = <&csys1_funnel_out_port>; > }; > }; > > /* output port */ > port@1 { > - reg = <0>; > + reg = <1>; > etf1_out_port: endpoint { > + coresight,hwid = <0>; > + direction = <1>; > remote-endpoint = <&csys2_funnel_in_port1>; > }; > }; > @@ -75,23 +81,27 @@ > port@0 { > reg = <0>; > csys2_funnel_out_port: endpoint { > + coresight,hwid = <0>; > + direction = <1>; > remote-endpoint = <&replicator_in_port0>; > }; > }; > > /* input ports */ > port@1 { > - reg = <0>; > + reg = <1>; > csys2_funnel_in_port0: endpoint { > - slave-mode; > + direction = <0>; > + coresight,hwid = <0>; > remote-endpoint = <&etf0_out_port>; > }; > }; > > port@2 { > - reg = <1>; > + reg = <2>; > csys2_funnel_in_port1: endpoint { > - slave-mode; > + direction = <0>; > + coresight,hwid = <1>; > remote-endpoint = <&etf1_out_port>; > }; > }; > diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts > index c9236c4..27b8036 100644 > --- a/arch/arm64/boot/dts/arm/juno.dts > +++ b/arch/arm64/boot/dts/arm/juno.dts > @@ -260,10 +260,11 @@ > &main_funnel { > ports { > port@3 { > - reg = <2>; > + reg = <3>; > main_funnel_in_port2: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&stm_out_port>; > + coresight,hwid = <2>; > }; > }; > }; > -- > 2.7.4 > -- 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
On 8 June 2018 at 15:49, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On Tue, Jun 05, 2018 at 10:43:21PM +0100, 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> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> Changes since V1: >> - Add support Juno for r1 & r2. >> --- > > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> Sudeep please hold on before applying this as there is more work to be done on this set. > >> arch/arm64/boot/dts/arm/juno-base.dtsi | 82 ++++++++++++++++++++++--------- >> arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 26 +++++++--- >> arch/arm64/boot/dts/arm/juno.dts | 5 +- >> 3 files changed, 81 insertions(+), 32 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi >> index eb749c5..33b41ba 100644 >> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi >> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi >> @@ -122,15 +122,18 @@ >> port@0 { >> reg = <0>; >> etf0_in_port: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&main_funnel_out_port>; >> + coresight,hwid = <0>; >> }; >> }; >> >> /* output port */ >> port@1 { >> - reg = <0>; >> + reg = <1>; >> etf0_out_port: endpoint { >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -145,8 +148,9 @@ >> power-domains = <&scpi_devpd 0>; >> port { >> tpiu_in_port: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&replicator_out_port0>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> @@ -168,23 +172,27 @@ >> reg = <0>; >> main_funnel_out_port: endpoint { >> remote-endpoint = <&etf0_in_port>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> >> /* input ports */ >> port@1 { >> - reg = <0>; >> + reg = <1>; >> main_funnel_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster0_funnel_out_port>; >> + coresight,hwid = <0>; >> }; >> }; >> >> port@2 { >> - reg = <1>; >> + reg = <2>; >> main_funnel_in_port1: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster1_funnel_out_port>; >> + coresight,hwid = <1>; >> }; >> }; >> }; >> @@ -200,8 +208,9 @@ >> power-domains = <&scpi_devpd 0>; >> port { >> etr_in_port: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&replicator_out_port1>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> @@ -217,6 +226,8 @@ >> power-domains = <&scpi_devpd 0>; >> port { >> stm_out_port: endpoint { >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -240,6 +251,8 @@ >> port { >> cluster0_etm0_out_port: endpoint { >> remote-endpoint = <&cluster0_funnel_in_port0>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -259,22 +272,26 @@ >> reg = <0>; >> cluster0_funnel_out_port: endpoint { >> remote-endpoint = <&main_funnel_in_port0>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> >> port@1 { >> - reg = <0>; >> + reg = <1>; >> cluster0_funnel_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster0_etm0_out_port>; >> + coresight,hwid = <0>; >> }; >> }; >> >> port@2 { >> - reg = <1>; >> + reg = <2>; >> cluster0_funnel_in_port1: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster0_etm1_out_port>; >> + coresight,hwid = <1>; >> }; >> }; >> }; >> @@ -299,6 +316,8 @@ >> port { >> cluster0_etm1_out_port: endpoint { >> remote-endpoint = <&cluster0_funnel_in_port1>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -322,6 +341,8 @@ >> port { >> cluster1_etm0_out_port: endpoint { >> remote-endpoint = <&cluster1_funnel_in_port0>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -341,36 +362,42 @@ >> reg = <0>; >> cluster1_funnel_out_port: endpoint { >> remote-endpoint = <&main_funnel_in_port1>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> >> port@1 { >> - reg = <0>; >> + reg = <1>; >> cluster1_funnel_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster1_etm0_out_port>; >> + coresight,hwid = <0>; >> }; >> }; >> >> port@2 { >> - reg = <1>; >> + reg = <2>; >> cluster1_funnel_in_port1: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster1_etm1_out_port>; >> + coresight,hwid = <1>; >> }; >> }; >> port@3 { >> - reg = <2>; >> + reg = <3>; >> cluster1_funnel_in_port2: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster1_etm2_out_port>; >> + coresight,hwid = <2>; >> }; >> }; >> port@4 { >> - reg = <3>; >> + reg = <4>; >> cluster1_funnel_in_port3: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&cluster1_etm3_out_port>; >> + coresight,hwid = <3>; >> }; >> }; >> }; >> @@ -395,6 +422,8 @@ >> port { >> cluster1_etm1_out_port: endpoint { >> remote-endpoint = <&cluster1_funnel_in_port1>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -418,6 +447,8 @@ >> port { >> cluster1_etm2_out_port: endpoint { >> remote-endpoint = <&cluster1_funnel_in_port2>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -441,6 +472,8 @@ >> port { >> cluster1_etm3_out_port: endpoint { >> remote-endpoint = <&cluster1_funnel_in_port3>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> }; >> @@ -462,6 +495,8 @@ >> reg = <0>; >> replicator_out_port0: endpoint { >> remote-endpoint = <&tpiu_in_port>; >> + coresight,hwid = <0>; >> + direction = <1>; >> }; >> }; >> >> @@ -469,14 +504,17 @@ >> reg = <1>; >> replicator_out_port1: endpoint { >> remote-endpoint = <&etr_in_port>; >> + coresight,hwid = <1>; >> + direction = <1>; >> }; >> }; >> >> /* replicator input port */ >> port@2 { >> - reg = <0>; >> + reg = <2>; >> replicator_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> diff --git a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi >> index 0c43fb3..146a5d9 100644 >> --- a/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi >> +++ b/arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi >> @@ -15,15 +15,18 @@ >> port@0 { >> reg = <0>; >> csys1_funnel_out_port: endpoint { >> + coresight,hwid = <0>; >> + direction = <1>; >> remote-endpoint = <&etf1_in_port>; >> }; >> }; >> >> /* input port */ >> port@1 { >> - reg = <0>; >> + reg = <1>; >> csys1_funnel_in_port0: endpoint { >> - slave-mode; >> + coresight,hwid = <0>; >> + direction = <0>; >> }; >> }; >> >> @@ -45,15 +48,18 @@ >> port@0 { >> reg = <0>; >> etf1_in_port: endpoint { >> - slave-mode; >> + direction = <0>; >> + coresight,hwid = <0>; >> remote-endpoint = <&csys1_funnel_out_port>; >> }; >> }; >> >> /* output port */ >> port@1 { >> - reg = <0>; >> + reg = <1>; >> etf1_out_port: endpoint { >> + coresight,hwid = <0>; >> + direction = <1>; >> remote-endpoint = <&csys2_funnel_in_port1>; >> }; >> }; >> @@ -75,23 +81,27 @@ >> port@0 { >> reg = <0>; >> csys2_funnel_out_port: endpoint { >> + coresight,hwid = <0>; >> + direction = <1>; >> remote-endpoint = <&replicator_in_port0>; >> }; >> }; >> >> /* input ports */ >> port@1 { >> - reg = <0>; >> + reg = <1>; >> csys2_funnel_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> + coresight,hwid = <0>; >> remote-endpoint = <&etf0_out_port>; >> }; >> }; >> >> port@2 { >> - reg = <1>; >> + reg = <2>; >> csys2_funnel_in_port1: endpoint { >> - slave-mode; >> + direction = <0>; >> + coresight,hwid = <1>; >> remote-endpoint = <&etf1_out_port>; >> }; >> }; >> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts >> index c9236c4..27b8036 100644 >> --- a/arch/arm64/boot/dts/arm/juno.dts >> +++ b/arch/arm64/boot/dts/arm/juno.dts >> @@ -260,10 +260,11 @@ >> &main_funnel { >> ports { >> port@3 { >> - reg = <2>; >> + reg = <3>; >> main_funnel_in_port2: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&stm_out_port>; >> + coresight,hwid = <2>; >> }; >> }; >> }; >> -- >> 2.7.4 >> -- 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
On 08/06/18 20:55, Mathieu Poirier wrote: > On Tue, Jun 05, 2018 at 10:43:13PM +0100, Suzuki K Poulose wrote: >> The coresight driver doesn't drop the references on the >> remote endpoint/port nodes. Add the missing of_node_put() >> calls. To make it easier to handle different corner cases >> cleanly, move the parsing of an endpoint to separate >> function. > > Please split this as those are two different things. Mathieu, I can do that, its only that if someone were to backport the fix for an older kernel, they would need to pick up these two patches, which is still fine. > >> >> Reported-by: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> drivers/hwtracing/coresight/of_coresight.c | 139 +++++++++++++++++------------ >> 1 file changed, 84 insertions(+), 55 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c >> index a33a92e..8a23c63 100644 >> --- a/drivers/hwtracing/coresight/of_coresight.c >> +++ b/drivers/hwtracing/coresight/of_coresight.c >> +static int of_coresight_parse_endpoint(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 { ... >> + } while (0); > > That's a clever way of coding a classic 'goto' block. > >> + >> + if (rparent) >> + of_node_put(rparent); >> + if (rport) >> + of_node_put(rport); > > Perfect - thank you for that. > >> 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 not output connections, we are done */ > > /not/no Thanks for spotting. 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
On 08/06/18 22:52, Mathieu Poirier wrote: > On 8 June 2018 at 15:49, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >> On Tue, Jun 05, 2018 at 10:43:21PM +0100, 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> >>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >>> --- >>> Changes since V1: >>> - Add support Juno for r1 & r2. >>> --- >> >> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > Sudeep please hold on before applying this as there is more work to be > done on this set. Mathieu, I will wait for Rob / Frank to have a say on the bindings before I post the next update. Cheers 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
On Fri, Jun 08, 2018 at 03:52:58PM -0600, Mathieu Poirier wrote: > On 8 June 2018 at 15:49, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > On Tue, Jun 05, 2018 at 10:43:21PM +0100, 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> > >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > >> --- > >> Changes since V1: > >> - Add support Juno for r1 & r2. > >> --- > > > > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > Sudeep please hold on before applying this as there is more work to be > done on this set. > Sure, I plan to apply the DT changes only after the driver changes are queued by you or whichever tree it's channelled through. I was already told the same to Suzuki. -- Regards, Sudeep -- 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
Hi Stefan, Can you take a look at the patch? Thanks. Shawn On Tue, Jun 05, 2018 at 10:43:26PM +0100, Suzuki K Poulose wrote: > Switch to the updated coresight bindings. > > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > Cc: Fabio Estevam <fabio.estevam@nxp.com> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > arch/arm/boot/dts/imx7d.dtsi | 5 ++++- > arch/arm/boot/dts/imx7s.dtsi | 41 ++++++++++++++++++++++++++++++----------- > 2 files changed, 34 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi > index 200714e..5faff17 100644 > --- a/arch/arm/boot/dts/imx7d.dtsi > +++ b/arch/arm/boot/dts/imx7d.dtsi > @@ -87,7 +87,9 @@ > > port { > etm1_out_port: endpoint { > + direction = <1>; > remote-endpoint = <&ca_funnel_in_port1>; > + coresight,hwid = <0>; > }; > }; > }; > @@ -174,8 +176,9 @@ > port@1 { > reg = <1>; > ca_funnel_in_port1: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&etm1_out_port>; > + coresight,hwid = <1>; > }; > }; > }; > diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi > index 4d42335..8e90915 100644 > --- a/arch/arm/boot/dts/imx7s.dtsi > +++ b/arch/arm/boot/dts/imx7s.dtsi > @@ -151,23 +151,28 @@ > port@0 { > reg = <0>; > replicator_out_port0: endpoint { > + direction = <1>; > remote-endpoint = <&tpiu_in_port>; > + coresight,hwid = <0>; > }; > }; > > port@1 { > reg = <1>; > replicator_out_port1: endpoint { > + direction = <1>; > remote-endpoint = <&etr_in_port>; > + coresight,hwid = <1>; > }; > }; > > /* replicator input port */ > port@2 { > - reg = <0>; > + reg = <2>; > replicator_in_port0: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&etf_out_port>; > + coresight,hwid = <0>; > }; > }; > }; > @@ -203,16 +208,19 @@ > port@0 { > reg = <0>; > ca_funnel_in_port0: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&etm0_out_port>; > + coresight,hwid = <0>; > }; > }; > > /* funnel output port */ > port@2 { > - reg = <0>; > + reg = <2>; > ca_funnel_out_port0: endpoint { > + direction = <1>; > remote-endpoint = <&hugo_funnel_in_port0>; > + coresight,hwid = <0>; > }; > }; > > @@ -229,7 +237,9 @@ > > port { > etm0_out_port: endpoint { > + direction = <1>; > remote-endpoint = <&ca_funnel_in_port0>; > + coresight,hwid = <0>; > }; > }; > }; > @@ -248,22 +258,26 @@ > port@0 { > reg = <0>; > hugo_funnel_in_port0: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&ca_funnel_out_port0>; > + coresight,hwid = <0>; > }; > }; > > port@1 { > reg = <1>; > hugo_funnel_in_port1: endpoint { > - slave-mode; /* M4 input */ > + direction = <0>; /* M4 input */ > + coresight,hwid = <1>; > }; > }; > > port@2 { > - reg = <0>; > + reg = <2>; > hugo_funnel_out_port0: endpoint { > + direction = <1>; > remote-endpoint = <&etf_in_port>; > + coresight,hwid = <0>; > }; > }; > > @@ -284,15 +298,18 @@ > port@0 { > reg = <0>; > etf_in_port: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&hugo_funnel_out_port0>; > + coresight,hwid = <0>; > }; > }; > > port@1 { > - reg = <0>; > + reg = <1>; > etf_out_port: endpoint { > + direction = <1>; > remote-endpoint = <&replicator_in_port0>; > + coresight,hwid = <0>; > }; > }; > }; > @@ -306,8 +323,9 @@ > > port { > etr_in_port: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&replicator_out_port1>; > + coresight,hwid = <0>; > }; > }; > }; > @@ -320,8 +338,9 @@ > > port { > tpiu_in_port: endpoint { > - slave-mode; > + direction = <0>; > remote-endpoint = <&replicator_out_port1>; > + coresight,hwid = <0>; > }; > }; > }; > -- > 2.7.4 > -- 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
On 19.06.2018 04:12, Shawn Guo wrote: > Hi Stefan, > > Can you take a look at the patch? Thanks. > > Shawn > > On Tue, Jun 05, 2018 at 10:43:26PM +0100, Suzuki K Poulose wrote: >> Switch to the updated coresight bindings. Looks good to me. Reviewed-by: Stefan Agner <stefan@agner.ch> -- Stefan >> >> Cc: Shawn Guo <shawnguo@kernel.org> >> Cc: Sascha Hauer <s.hauer@pengutronix.de> >> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> >> Cc: Fabio Estevam <fabio.estevam@nxp.com> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> arch/arm/boot/dts/imx7d.dtsi | 5 ++++- >> arch/arm/boot/dts/imx7s.dtsi | 41 ++++++++++++++++++++++++++++++----------- >> 2 files changed, 34 insertions(+), 12 deletions(-) >> >> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi >> index 200714e..5faff17 100644 >> --- a/arch/arm/boot/dts/imx7d.dtsi >> +++ b/arch/arm/boot/dts/imx7d.dtsi >> @@ -87,7 +87,9 @@ >> >> port { >> etm1_out_port: endpoint { >> + direction = <1>; >> remote-endpoint = <&ca_funnel_in_port1>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> @@ -174,8 +176,9 @@ >> port@1 { >> reg = <1>; >> ca_funnel_in_port1: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&etm1_out_port>; >> + coresight,hwid = <1>; >> }; >> }; >> }; >> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi >> index 4d42335..8e90915 100644 >> --- a/arch/arm/boot/dts/imx7s.dtsi >> +++ b/arch/arm/boot/dts/imx7s.dtsi >> @@ -151,23 +151,28 @@ >> port@0 { >> reg = <0>; >> replicator_out_port0: endpoint { >> + direction = <1>; >> remote-endpoint = <&tpiu_in_port>; >> + coresight,hwid = <0>; >> }; >> }; >> >> port@1 { >> reg = <1>; >> replicator_out_port1: endpoint { >> + direction = <1>; >> remote-endpoint = <&etr_in_port>; >> + coresight,hwid = <1>; >> }; >> }; >> >> /* replicator input port */ >> port@2 { >> - reg = <0>; >> + reg = <2>; >> replicator_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&etf_out_port>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> @@ -203,16 +208,19 @@ >> port@0 { >> reg = <0>; >> ca_funnel_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&etm0_out_port>; >> + coresight,hwid = <0>; >> }; >> }; >> >> /* funnel output port */ >> port@2 { >> - reg = <0>; >> + reg = <2>; >> ca_funnel_out_port0: endpoint { >> + direction = <1>; >> remote-endpoint = <&hugo_funnel_in_port0>; >> + coresight,hwid = <0>; >> }; >> }; >> >> @@ -229,7 +237,9 @@ >> >> port { >> etm0_out_port: endpoint { >> + direction = <1>; >> remote-endpoint = <&ca_funnel_in_port0>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> @@ -248,22 +258,26 @@ >> port@0 { >> reg = <0>; >> hugo_funnel_in_port0: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&ca_funnel_out_port0>; >> + coresight,hwid = <0>; >> }; >> }; >> >> port@1 { >> reg = <1>; >> hugo_funnel_in_port1: endpoint { >> - slave-mode; /* M4 input */ >> + direction = <0>; /* M4 input */ >> + coresight,hwid = <1>; >> }; >> }; >> >> port@2 { >> - reg = <0>; >> + reg = <2>; >> hugo_funnel_out_port0: endpoint { >> + direction = <1>; >> remote-endpoint = <&etf_in_port>; >> + coresight,hwid = <0>; >> }; >> }; >> >> @@ -284,15 +298,18 @@ >> port@0 { >> reg = <0>; >> etf_in_port: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&hugo_funnel_out_port0>; >> + coresight,hwid = <0>; >> }; >> }; >> >> port@1 { >> - reg = <0>; >> + reg = <1>; >> etf_out_port: endpoint { >> + direction = <1>; >> remote-endpoint = <&replicator_in_port0>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> @@ -306,8 +323,9 @@ >> >> port { >> etr_in_port: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&replicator_out_port1>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> @@ -320,8 +338,9 @@ >> >> port { >> tpiu_in_port: endpoint { >> - slave-mode; >> + direction = <0>; >> remote-endpoint = <&replicator_out_port1>; >> + coresight,hwid = <0>; >> }; >> }; >> }; >> -- >> 2.7.4 >> > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- 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
On Mon, 18 Jun 2018 at 20:13, Shawn Guo <shawnguo@kernel.org> wrote: > > Hi Stefan, > > Can you take a look at the patch? Thanks. > These bindings are still being discussed and patches related to them shouldn't be merged. The next iteration of this patchset will not included individual modifications to device tree files, that will be left for a later time when bindings have been agreed upon. Thanks, Mathieu > Shawn > > On Tue, Jun 05, 2018 at 10:43:26PM +0100, Suzuki K Poulose wrote: > > Switch to the updated coresight bindings. > > > > Cc: Shawn Guo <shawnguo@kernel.org> > > Cc: Sascha Hauer <s.hauer@pengutronix.de> > > Cc: Pengutronix Kernel Team <kernel@pengutronix.de> > > Cc: Fabio Estevam <fabio.estevam@nxp.com> > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > --- > > arch/arm/boot/dts/imx7d.dtsi | 5 ++++- > > arch/arm/boot/dts/imx7s.dtsi | 41 ++++++++++++++++++++++++++++++----------- > > 2 files changed, 34 insertions(+), 12 deletions(-) > > > > diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi > > index 200714e..5faff17 100644 > > --- a/arch/arm/boot/dts/imx7d.dtsi > > +++ b/arch/arm/boot/dts/imx7d.dtsi > > @@ -87,7 +87,9 @@ > > > > port { > > etm1_out_port: endpoint { > > + direction = <1>; > > remote-endpoint = <&ca_funnel_in_port1>; > > + coresight,hwid = <0>; > > }; > > }; > > }; > > @@ -174,8 +176,9 @@ > > port@1 { > > reg = <1>; > > ca_funnel_in_port1: endpoint { > > - slave-mode; > > + direction = <0>; > > remote-endpoint = <&etm1_out_port>; > > + coresight,hwid = <1>; > > }; > > }; > > }; > > diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi > > index 4d42335..8e90915 100644 > > --- a/arch/arm/boot/dts/imx7s.dtsi > > +++ b/arch/arm/boot/dts/imx7s.dtsi > > @@ -151,23 +151,28 @@ > > port@0 { > > reg = <0>; > > replicator_out_port0: endpoint { > > + direction = <1>; > > remote-endpoint = <&tpiu_in_port>; > > + coresight,hwid = <0>; > > }; > > }; > > > > port@1 { > > reg = <1>; > > replicator_out_port1: endpoint { > > + direction = <1>; > > remote-endpoint = <&etr_in_port>; > > + coresight,hwid = <1>; > > }; > > }; > > > > /* replicator input port */ > > port@2 { > > - reg = <0>; > > + reg = <2>; > > replicator_in_port0: endpoint { > > - slave-mode; > > + direction = <0>; > > remote-endpoint = <&etf_out_port>; > > + coresight,hwid = <0>; > > }; > > }; > > }; > > @@ -203,16 +208,19 @@ > > port@0 { > > reg = <0>; > > ca_funnel_in_port0: endpoint { > > - slave-mode; > > + direction = <0>; > > remote-endpoint = <&etm0_out_port>; > > + coresight,hwid = <0>; > > }; > > }; > > > > /* funnel output port */ > > port@2 { > > - reg = <0>; > > + reg = <2>; > > ca_funnel_out_port0: endpoint { > > + direction = <1>; > > remote-endpoint = <&hugo_funnel_in_port0>; > > + coresight,hwid = <0>; > > }; > > }; > > > > @@ -229,7 +237,9 @@ > > > > port { > > etm0_out_port: endpoint { > > + direction = <1>; > > remote-endpoint = <&ca_funnel_in_port0>; > > + coresight,hwid = <0>; > > }; > > }; > > }; > > @@ -248,22 +258,26 @@ > > port@0 { > > reg = <0>; > > hugo_funnel_in_port0: endpoint { > > - slave-mode; > > + direction = <0>; > > remote-endpoint = <&ca_funnel_out_port0>; > > + coresight,hwid = <0>; > > }; > > }; > > > > port@1 { > > reg = <1>; > > hugo_funnel_in_port1: endpoint { > > - slave-mode; /* M4 input */ > > + direction = <0>; /* M4 input */ > > + coresight,hwid = <1>; > > }; > > }; > > > > port@2 { > > - reg = <0>; > > + reg = <2>; > > hugo_funnel_out_port0: endpoint { > > + direction = <1>; > > remote-endpoint = <&etf_in_port>; > > + coresight,hwid = <0>; > > }; > > }; > > > > @@ -284,15 +298,18 @@ > > port@0 { > > reg = <0>; > > etf_in_port: endpoint { > > - slave-mode; > > + direction = <0>; > > remote-endpoint = <&hugo_funnel_out_port0>; > > + coresight,hwid = <0>; > > }; > > }; > > > > port@1 { > > - reg = <0>; > > + reg = <1>; > > etf_out_port: endpoint { > > + direction = <1>; > > remote-endpoint = <&replicator_in_port0>; > > + coresight,hwid = <0>; > > }; > > }; > > }; > > @@ -306,8 +323,9 @@ > > > > port { > > etr_in_port: endpoint { > > - slave-mode; > > + direction = <0>; > > remote-endpoint = <&replicator_out_port1>; > > + coresight,hwid = <0>; > > }; > > }; > > }; > > @@ -320,8 +338,9 @@ > > > > port { > > tpiu_in_port: endpoint { > > - slave-mode; > > + direction = <0>; > > remote-endpoint = <&replicator_out_port1>; > > + coresight,hwid = <0>; > > }; > > }; > > }; > > -- > > 2.7.4 > > -- 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
On 05/06/2018 22:43:29+0100, Suzuki K Poulose wrote: > Switch to the new coresight bindings for hardware ports > > Cc: Nicolas Ferre <nicolas.ferre@microchip.com> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > arch/arm/boot/dts/sama5d2.dtsi | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > Applied, thanks.
On 19/06/18 22:24, Alexandre Belloni wrote: > On 05/06/2018 22:43:29+0100, Suzuki K Poulose wrote: >> Switch to the new coresight bindings for hardware ports >> >> Cc: Nicolas Ferre <nicolas.ferre@microchip.com> >> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> arch/arm/boot/dts/sama5d2.dtsi | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> > Applied, thanks. > Alexandre, Please hold off applying this change, as we are yet to come to an agreement on this. Sorry for the trouble. See [0] [0] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-June/582269.html Kind regards 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
On 05/06/18 22:43, Suzuki K Poulose wrote: > 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. > ... > There were three options considered for the hardware port number scheme: > ... > 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. > > > This series implements Option (3) 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. .... > dts: juno: Update coresight bindings for hw port > dts: hisilicon: Update coresight bindings for hw ports > dts: spreadtrum: Update coresight bindings for hw ports > dts: qcom: Update coresight bindings for hw ports > dts: arm: hisilicon: Update coresight bindings for hardware port > dts: arm: imx7{d,s}: Update coresight binding for hardware ports > dts: arm: omap: Update coresight bindings for hardware ports > dts: arm: qcom: Update coresight bindings for hardware ports > dts: sama5d2: Update coresight bindings for hardware ports > dts: ste-dbx5x0: Update coresight bindings for hardware port > dts: tc2: Update coresight bindings for hardware ports > All, Pleas hold on with applying the DTS changes listed above. There are still some on going discussions on the bindings and we are yet to come to a conclusion [0]. And there are high chances that these might change. Sorry for the inconvenience. [0] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-June/582269.html Kind regards 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
On 20/06/2018 10:44:29+0100, Suzuki K Poulose wrote: > On 19/06/18 22:24, Alexandre Belloni wrote: > > On 05/06/2018 22:43:29+0100, Suzuki K Poulose wrote: > > > Switch to the new coresight bindings for hardware ports > > > > > > Cc: Nicolas Ferre <nicolas.ferre@microchip.com> > > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > > --- > > > arch/arm/boot/dts/sama5d2.dtsi | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > Applied, thanks. > > > > Alexandre, > > Please hold off applying this change, as we are yet to come to an > agreement on this. Sorry for the trouble. See [0] > I've removed it.
On Tue, Jun 5, 2018 at 11:45 PM Suzuki K Poulose <suzuki.poulose@arm.com> wrote: > Switch to the new coresight bindings > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> I think I was requested in another mail to hold back applying this patch until it's been discussed. I will apply once Mathieu (etc) ACKs the series. Yours, Linus Walleij -- 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
On 26/06/18 10:30, Linus Walleij wrote: > On Tue, Jun 5, 2018 at 11:45 PM Suzuki K Poulose <suzuki.poulose@arm.com> wrote: > >> Switch to the new coresight bindings >> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > I think I was requested in another mail to hold back applying this > patch until it's been discussed. > > I will apply once Mathieu (etc) ACKs the series. Linus, Yes, thats right. Please ignore this for now. 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
* Suzuki K Poulose <suzuki.poulose@arm.com> [180605 14:48]:
> Switch to the new coresight bindings for hardware ports
So is this patch safe for me to pick separately for v4.19?
Regards,
Tony
--
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
Hi Tony, On 07/03/2018 08:09 AM, Tony Lindgren wrote: > * Suzuki K Poulose <suzuki.poulose@arm.com> [180605 14:48]: >> Switch to the new coresight bindings for hardware ports > > So is this patch safe for me to pick separately for v4.19? No. Please ignore this for now. The bindings are still under discussion. Sorry for the inconvenience. 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
* Suzuki K Poulose <suzuki.poulose@arm.com> [180703 08:01]: > > > Hi Tony, > > On 07/03/2018 08:09 AM, Tony Lindgren wrote: > > * Suzuki K Poulose <suzuki.poulose@arm.com> [180605 14:48]: > > > Switch to the new coresight bindings for hardware ports > > > > So is this patch safe for me to pick separately for v4.19? > > No. Please ignore this for now. The bindings are still under > discussion. Sorry for the inconvenience. OK will do thanks. Tony -- 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
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. This series implements Option (3) 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. It also cleans up the platform parsing code to reduce the memory usage by reusing the platform description. I am not sure what is the best route to push these DTS changes. Thoughts ? Changes since RFC [0] : - 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. - Rename new helper functions for consistency Suzuki K Poulose (20): coresight: Fix memory leak in coresight_register coresight: of: Fix refcounting for graph nodes coresight: Fix remote endpoint parsing coresight: Cleanup platform description data coresight: platform: Cleanup coresight connection handling coresight: Handle errors in finding input/output ports coresight: dts: Document usage of graph bindings coresight: dts: Cleanup device tree graph bindings coresight: dts: Define new bindings for direction of data flow dts: juno: Update coresight bindings for hw port dts: hisilicon: Update coresight bindings for hw ports dts: spreadtrum: Update coresight bindings for hw ports dts: qcom: Update coresight bindings for hw ports dts: arm: hisilicon: Update coresight bindings for hardware port dts: arm: imx7{d,s}: Update coresight binding for hardware ports dts: arm: omap: Update coresight bindings for hardware ports dts: arm: qcom: Update coresight bindings for hardware ports dts: sama5d2: Update coresight bindings for hardware ports dts: ste-dbx5x0: Update coresight bindings for hardware port dts: tc2: Update coresight bindings for hardware ports .../devicetree/bindings/arm/coresight.txt | 76 +++++-- arch/arm/boot/dts/hip04.dtsi | 195 +++++++++++++----- arch/arm/boot/dts/imx7d.dtsi | 5 +- arch/arm/boot/dts/imx7s.dtsi | 41 ++-- arch/arm/boot/dts/omap3-beagle-xm.dts | 5 +- arch/arm/boot/dts/omap3-beagle.dts | 5 +- arch/arm/boot/dts/qcom-apq8064.dtsi | 37 +++- arch/arm/boot/dts/qcom-msm8974.dtsi | 60 ++++-- arch/arm/boot/dts/sama5d2.dtsi | 5 +- arch/arm/boot/dts/ste-dbx5x0.dtsi | 31 ++- arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts | 48 +++-- arch/arm64/boot/dts/arm/juno-base.dtsi | 82 +++++--- arch/arm64/boot/dts/arm/juno-cs-r1r2.dtsi | 26 ++- arch/arm64/boot/dts/arm/juno.dts | 5 +- .../arm64/boot/dts/hisilicon/hi6220-coresight.dtsi | 89 ++++++--- arch/arm64/boot/dts/qcom/msm8916.dtsi | 55 ++++-- arch/arm64/boot/dts/sprd/sc9836.dtsi | 40 ++-- arch/arm64/boot/dts/sprd/sc9860.dtsi | 101 +++++++--- drivers/hwtracing/coresight/coresight.c | 30 +-- drivers/hwtracing/coresight/of_coresight.c | 219 +++++++++++++-------- include/linux/coresight.h | 11 +- 21 files changed, 818 insertions(+), 348 deletions(-)