Re: [RFC PATCH 6/8] dts: coresight: Clean up the device tree graph bindings

From: Mathieu Poirier
Date: Fri Jun 01 2018 - 16:26:51 EST


On Fri, Jun 01, 2018 at 02:16:05PM +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. 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. Hence, we add an explicit
> property to denote the hardware port number, "coresight,hwid"
> which must be specified for each "endpoint".
>
> Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> Cc: Sudeep Holla <sudeep.holla@xxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> ---
> .../devicetree/bindings/arm/coresight.txt | 26 +++++++++---
> drivers/hwtracing/coresight/of_coresight.c | 46 ++++++++++++++++------
> 2 files changed, 54 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
> index bd36e40..385581a 100644
> --- a/Documentation/devicetree/bindings/arm/coresight.txt
> +++ b/Documentation/devicetree/bindings/arm/coresight.txt
> @@ -104,7 +104,11 @@ properties to uniquely identify the connection details.
> "slave-mode"
>
> * Hardware Port number at the component:
> - - The hardware port number is assumed to be the address of the "port" component.
> + - (Obsolete) The hardware port number is assumed to be the address of the "port" component.
> + - Each "endpoint" must define the hardware port of the local end of the
> + connection using the following property:
> + "coresight,hwid" - 32bit integer, hardware port number at the local end.
> +
>
>
> Example:
> @@ -120,6 +124,7 @@ Example:
> etb_in_port: endpoint@0 {
> slave-mode;
> remote-endpoint = <&replicator_out_port0>;
> + coresight,hwid = <0>;
> };
> };
> };
> @@ -134,6 +139,7 @@ Example:
> tpiu_in_port: endpoint@0 {
> slave-mode;
> remote-endpoint = <&replicator_out_port1>;
> + coresight,hwid = <0>;
> };
> };
> };
> @@ -154,6 +160,7 @@ Example:
> reg = <0>;
> replicator_out_port0: endpoint {
> remote-endpoint = <&etb_in_port>;
> + coresight,hwid = <0>;
> };
> };
>
> @@ -161,15 +168,17 @@ Example:
> reg = <1>;
> replicator_out_port1: endpoint {
> remote-endpoint = <&tpiu_in_port>;
> + coresight,hwid = <1>;
> };
> };
>
> /* replicator input port */
> port@2 {
> - reg = <0>;
> + reg = <1>;
> replicator_in_port0: endpoint {
> slave-mode;
> remote-endpoint = <&funnel_out_port0>;
> + coresight,hwid = <0>;
> };
> };
> };
> @@ -191,31 +200,35 @@ Example:
> funnel_out_port0: endpoint {
> remote-endpoint =
> <&replicator_in_port0>;
> + coresight,hwid = <0>;
> };
> };
>
> /* funnel input ports */
> port@1 {
> - reg = <0>;
> + reg = <1>;
> funnel_in_port0: endpoint {
> slave-mode;
> remote-endpoint = <&ptm0_out_port>;
> + coresight,hwid = <0>;
> };
> };
>
> port@2 {
> - reg = <1>;
> + reg = <2>;
> funnel_in_port1: endpoint {
> slave-mode;
> remote-endpoint = <&ptm1_out_port>;
> + coresight,hwid = <1>;
> };
> };
>
> port@3 {
> - reg = <2>;
> + reg = <3>;
> funnel_in_port2: endpoint {
> slave-mode;
> remote-endpoint = <&etm0_out_port>;
> + coresight,hwid = <2>;
> };
> };
>
> @@ -233,6 +246,7 @@ Example:
> port {
> ptm0_out_port: endpoint {
> remote-endpoint = <&funnel_in_port0>;
> + coresight,hwid = <0>;
> };
> };
> };
> @@ -247,6 +261,7 @@ Example:
> port {
> ptm1_out_port: endpoint {
> remote-endpoint = <&funnel_in_port1>;
> + coresight,hwid = <0>;
> };
> };
> };
> @@ -263,6 +278,7 @@ Example:
> port {
> stm_out_port: endpoint {
> remote-endpoint = <&main_funnel_in_port2>;
> + coresight,hwid = <0>;
> };
> };
> };
> diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
> index a3f3416..99d7a9c 100644
> --- a/drivers/hwtracing/coresight/of_coresight.c
> +++ b/drivers/hwtracing/coresight/of_coresight.c
> @@ -105,14 +105,37 @@ int of_coresight_get_cpu(const struct device_node *node)
> }
> EXPORT_SYMBOL_GPL(of_coresight_get_cpu);
>
> +/*
> + * of_graph_ep_coresight_get_port_id : Get the hardware port number for the
> + * given endpoint device node. Prefer the explicit "coresight,hwid" property
> + * over the endpoint register id (obsolete bindings).
> + */
> +static int of_graph_ep_coresight_get_port_id(struct device *dev,
> + struct device_node *ep_node)


static int of_coresight_endpoint_get_port_id(struct device *dev,
struct device_node *ep_node)

I think that makes more sense since this function is only visible in this file.

> +{
> + struct of_endpoint ep;
> + int rc, port_id;
> +
> +
> + if (!of_property_read_u32(ep_node, "coresight,hwid", &port_id))
> + return port_id;
> +
> + rc = of_graph_parse_endpoint(ep_node, &ep);
> + if (rc)
> + return rc;
> + dev_warn_once(dev,
> + "ep%d: Mandatory \"coresight,hwid\" property missing."
> + " DT uses obsolete coresight bindings\n", ep.port);
> + return ep.port;
> +}
> +
> struct coresight_platform_data *
> of_get_coresight_platform_data(struct device *dev,
> const struct device_node *node)
> {
> - int ret = 0;
> + int ret = 0, outport, inport;
> struct coresight_platform_data *pdata;
> struct coresight_connection *conn;
> - struct of_endpoint endpoint, rendpoint;
> struct device *rdev;
> struct device_node *ep = NULL;
> struct device_node *rparent = NULL;
> @@ -148,14 +171,10 @@ of_get_coresight_platform_data(struct device *dev,
> 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)
> + outport = of_graph_ep_coresight_get_port_id(dev, ep);
> + if (outport < 0)
> continue;
> -
> - /* The local out port number */
> - conn->outport = endpoint.port;
> + conn->outport = outport;
>
> /*
> * Get a handle on the remote endpoint and the device
> @@ -168,15 +187,16 @@ of_get_coresight_platform_data(struct device *dev,
> if (!rparent)
> continue;
>
> - if (of_graph_parse_endpoint(rep, &rendpoint))
> - continue;
> -
> rdev = of_coresight_get_endpoint_device(rparent);
> if (!rdev)
> return ERR_PTR(-EPROBE_DEFER);
>
> + inport = of_graph_ep_coresight_get_port_id(rdev, rep);
> + if (inport < 0)
> + continue;
> +
> conn->child_name = dev_name(rdev);
> - conn->child_port = rendpoint.port;
> + conn->child_port = inport;
> conn++;
> } while (ep);
> }
> --
> 2.7.4
>