[PATCH v4 0/6] Describe CoreSight topology using sysfs links.

[PATCH v4 0/6] Describe CoreSight topology using sysfs links.

[Mike Leach]

The connections between CoreSight sources, links and sinks is not obvious
without documentation or access to the device tree / ACPI definitions for
the platform.

This patchset provides sysfs links to enable the user to follow the trace
patch from source to sink.

Components in the trace path are updated to have a connections sysfs
group, which collates all the links for that component.

The CTI components which exist aside from the main trace patch, also
have an added connections directory showing connections to other
CoreSight devices.

This patchset applies on top of the recent CTI v9 patchset [1].

Adaptation of an original patchset [2] from Suzuki, reusing 2 patches
unchanged with update to 3rd adapt to the new common code for trace
path and CTI component links & add a default connections group.

Tested on Juno r1, DB410c; kernel 5.6-rc1

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2020-February/709923.html
[2] https://lists.linaro.org/pipermail/coresight/2019-May/002803.html

Changes since v3:
1) Rebased onto 5.6-rc1 kernel with CTI set[1].

Changes since v2:
1) Fixed issues with signature ordering noted by Suzuki.
2) Alterations to main CTI set[1] to overcome issue noted by Matthieu.

Changes since v1:
1) Code from original v4 CTI set moved here so that all connections related
code in this set.
2) Connections directory mandatory for all CoreSight components and
generated as part of the registration process.

Mike Leach (3):
  coresight: Add generic sysfs link creation functions.
  coresight: cti: Add in sysfs links to other coresight devices.
  coresight: docs: Add information about the topology representations.

Suzuki K Poulose (3):
  coresight: Pass coresight_device for coresight_release_platform_data
  coresight: add return value for fixup connections
  coresight: Expose device connections via sysfs

 .../trace/coresight/coresight-ect.rst         |   5 +-
 Documentation/trace/coresight/coresight.rst   |  85 ++++++++
 drivers/hwtracing/coresight/Makefile          |   3 +-
 drivers/hwtracing/coresight/coresight-cti.c   |  41 +++-
 .../hwtracing/coresight/coresight-platform.c  |   2 +-
 drivers/hwtracing/coresight/coresight-priv.h  |  12 +-
 drivers/hwtracing/coresight/coresight-sysfs.c | 204 ++++++++++++++++++
 drivers/hwtracing/coresight/coresight.c       |  75 ++++---
 include/linux/coresight.h                     |  22 ++
 9 files changed, 420 insertions(+), 29 deletions(-)
 create mode 100644 drivers/hwtracing/coresight/coresight-sysfs.c

-- 
2.17.1

[PATCH v4 1/6] coresight: Pass coresight_device for coresight_release_platform_data

[Mike Leach]

From: Suzuki K Poulose <suzuki.poulose@arm.com>

As we prepare to expose the links between the devices in
sysfs, pass the coresight_device instance to the
coresight_release_platform_data in order to free up the connections
when the device is removed.

No functional changes as such in this patch.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/coresight-platform.c | 2 +-
 drivers/hwtracing/coresight/coresight-priv.h     | 3 ++-
 drivers/hwtracing/coresight/coresight.c          | 7 ++++---
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 43418a2126ff..4b78e1ac5285 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -822,7 +822,7 @@ coresight_get_platform_data(struct device *dev)
 error:
 	if (!IS_ERR_OR_NULL(pdata))
 		/* Cleanup the connection information */
-		coresight_release_platform_data(pdata);
+		coresight_release_platform_data(NULL, pdata);
 	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(coresight_get_platform_data);
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 890f9a5c97c6..1cad642f27aa 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -211,7 +211,8 @@ static inline void *coresight_get_uci_data(const struct amba_id *id)
 	return 0;
 }
 
-void coresight_release_platform_data(struct coresight_platform_data *pdata);
+void coresight_release_platform_data(struct coresight_device *csdev,
+				     struct coresight_platform_data *pdata);
 struct coresight_device *
 coresight_find_csdev_by_fwnode(struct fwnode_handle *r_fwnode);
 void coresight_set_assoc_ectdev_mutex(struct coresight_device *csdev,
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index c71553c09f8e..10e756410d3c 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -1213,7 +1213,8 @@ postcore_initcall(coresight_init);
  * coresight_release_platform_data: Release references to the devices connected
  * to the output port of this device.
  */
-void coresight_release_platform_data(struct coresight_platform_data *pdata)
+void coresight_release_platform_data(struct coresight_device *csdev,
+				     struct coresight_platform_data *pdata)
 {
 	int i;
 
@@ -1316,7 +1317,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 	kfree(csdev);
 err_out:
 	/* Cleanup the connection information */
-	coresight_release_platform_data(desc->pdata);
+	coresight_release_platform_data(NULL, desc->pdata);
 	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(coresight_register);
@@ -1326,7 +1327,7 @@ void coresight_unregister(struct coresight_device *csdev)
 	etm_perf_del_symlink_sink(csdev);
 	/* Remove references of that device in the topology */
 	coresight_remove_conns(csdev);
-	coresight_release_platform_data(csdev->pdata);
+	coresight_release_platform_data(csdev, csdev->pdata);
 	device_unregister(&csdev->dev);
 }
 EXPORT_SYMBOL_GPL(coresight_unregister);
-- 
2.17.1

[PATCH v4 2/6] coresight: add return value for fixup connections

[Mike Leach]

From: Suzuki K Poulose <suzuki.poulose@arm.com>

Handle failures in fixing up connections for a newly registered
device. This will be useful to handle cases where we fail to expose
the links via sysfs for the connections.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/coresight.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index 10e756410d3c..07f66a3968f1 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -1073,18 +1073,14 @@ static int coresight_orphan_match(struct device *dev, void *data)
 	return 0;
 }
 
-static void coresight_fixup_orphan_conns(struct coresight_device *csdev)
+static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
 {
-	/*
-	 * No need to check for a return value as orphan connection(s)
-	 * are hooked-up with each newly added component.
-	 */
-	bus_for_each_dev(&coresight_bustype, NULL,
+	return bus_for_each_dev(&coresight_bustype, NULL,
 			 csdev, coresight_orphan_match);
 }
 
 
-static void coresight_fixup_device_conns(struct coresight_device *csdev)
+static int coresight_fixup_device_conns(struct coresight_device *csdev)
 {
 	int i;
 
@@ -1096,6 +1092,8 @@ static void coresight_fixup_device_conns(struct coresight_device *csdev)
 		if (!conn->child_dev)
 			csdev->orphan = true;
 	}
+
+	return 0;
 }
 
 static int coresight_remove_match(struct device *dev, void *data)
@@ -1305,11 +1303,17 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 
 	mutex_lock(&coresight_mutex);
 
-	coresight_fixup_device_conns(csdev);
-	coresight_fixup_orphan_conns(csdev);
-	cti_add_assoc_to_csdev(csdev);
+	ret = coresight_fixup_device_conns(csdev);
+	if (!ret)
+		ret = coresight_fixup_orphan_conns(csdev);
+	if (!ret)
+		cti_add_assoc_to_csdev(csdev);
 
 	mutex_unlock(&coresight_mutex);
+	if (ret) {
+		coresight_unregister(csdev);
+		return ERR_PTR(ret);
+	}
 
 	return csdev;
 
-- 
2.17.1

[PATCH v4 3/6] coresight: Add generic sysfs link creation functions.

[Mike Leach]

To allow the connections between coresight components to be represented
in sysfs, generic methods for creating sysfs links between two coresight
devices are added.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/Makefile          |   3 +-
 drivers/hwtracing/coresight/coresight-priv.h  |   4 +
 drivers/hwtracing/coresight/coresight-sysfs.c | 124 ++++++++++++++++++
 include/linux/coresight.h                     |  20 +++
 4 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 drivers/hwtracing/coresight/coresight-sysfs.c

diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 0e3e72f0f510..19497d1d92bf 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -2,7 +2,8 @@
 #
 # Makefile for CoreSight drivers.
 #
-obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o coresight-platform.o
+obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o \
+			   coresight-platform.o coresight-sysfs.o
 obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \
 					     coresight-tmc-etf.o \
 					     coresight-tmc-etr.o
diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index 1cad642f27aa..a4a658d46045 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -153,6 +153,10 @@ struct coresight_device *coresight_get_sink_by_id(u32 id);
 struct list_head *coresight_build_path(struct coresight_device *csdev,
 				       struct coresight_device *sink);
 void coresight_release_path(struct list_head *path);
+int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
+void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
+int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
+void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
 
 #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
 extern int etm_readl_cp14(u32 off, unsigned int *val);
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
new file mode 100644
index 000000000000..17d565941e5e
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, Linaro Limited, All rights reserved.
+ * Author: Mike Leach <mike.leach@linaro.org>
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+
+#include "coresight-priv.h"
+
+/*
+ * Connections group - links attribute.
+ * Count of created links between coresight components in the group.
+ */
+static ssize_t nr_links_show(struct device *dev,
+			     struct device_attribute *attr,
+			     char *buf)
+{
+	struct coresight_device *csdev = to_coresight_device(dev);
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", csdev->nr_links);
+}
+static DEVICE_ATTR_RO(nr_links);
+
+static struct attribute *coresight_conns_attrs[] = {
+	&dev_attr_nr_links.attr,
+	NULL,
+};
+
+static struct attribute_group coresight_conns_group = {
+	.attrs = coresight_conns_attrs,
+	.name = "connections",
+};
+
+/*
+ * Create connections group for CoreSight devices.
+ * This group will then be used to collate the sysfs links between
+ * devices.
+ */
+int coresight_create_conns_sysfs_group(struct coresight_device *csdev)
+{
+	int ret = 0;
+
+	if (!csdev)
+		return -EINVAL;
+
+	ret = sysfs_create_group(&csdev->dev.kobj, &coresight_conns_group);
+	if (ret)
+		return ret;
+
+	csdev->has_conns_grp = true;
+	return ret;
+}
+
+void coresight_remove_conns_sysfs_group(struct coresight_device *csdev)
+{
+	if (!csdev)
+		return;
+
+	if (csdev->has_conns_grp) {
+		sysfs_remove_group(&csdev->dev.kobj, &coresight_conns_group);
+		csdev->has_conns_grp = false;
+	}
+}
+
+int coresight_add_sysfs_link(struct coresight_sysfs_link *info)
+{
+	int ret = 0;
+
+	if (!info)
+		return -EINVAL;
+	if (!info->orig || !info->target ||
+	    !info->orig_name || !info->target_name)
+		return -EINVAL;
+	if (!info->orig->has_conns_grp || !info->target->has_conns_grp)
+		return -EINVAL;
+
+	/* first link orig->target */
+	ret = sysfs_add_link_to_group(&info->orig->dev.kobj,
+				      coresight_conns_group.name,
+				      &info->target->dev.kobj,
+				      info->orig_name);
+	if (ret)
+		return ret;
+
+	/* second link target->orig */
+	ret = sysfs_add_link_to_group(&info->target->dev.kobj,
+				      coresight_conns_group.name,
+				      &info->orig->dev.kobj,
+				      info->target_name);
+
+	/* error in second link - remove first - otherwise inc counts */
+	if (ret) {
+		sysfs_remove_link_from_group(&info->orig->dev.kobj,
+					     coresight_conns_group.name,
+					     info->orig_name);
+	} else {
+		info->orig->nr_links++;
+		info->target->nr_links++;
+	}
+
+	return ret;
+}
+
+void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
+{
+	if (!info)
+		return;
+	if (!info->orig || !info->target ||
+	    !info->orig_name || !info->target_name)
+		return;
+
+	sysfs_remove_link_from_group(&info->orig->dev.kobj,
+				     coresight_conns_group.name,
+				     info->orig_name);
+
+	sysfs_remove_link_from_group(&info->target->dev.kobj,
+				     coresight_conns_group.name,
+				     info->target_name);
+
+	info->orig->nr_links--;
+	info->target->nr_links--;
+}
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 193cc9dbf448..a2ec25e02ca9 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -148,6 +148,20 @@ struct coresight_connection {
 	struct coresight_device *child_dev;
 };
 
+/**
+ * struct coresight_sysfs_link - representation of a connection in sysfs.
+ * @orig:		Originating (master) coresight device for the link.
+ * @orig_name:		Name to use for the link orig->target.
+ * @target:		Target (slave) coresight device for the link.
+ * @target_name:	Name to use for the link target->orig.
+ */
+struct coresight_sysfs_link {
+	struct coresight_device *orig;
+	const char *orig_name;
+	struct coresight_device *target;
+	const char *target_name;
+};
+
 /**
  * struct coresight_device - representation of a device as used by the framework
  * @pdata:	Platform data with device connections associated to this device.
@@ -165,6 +179,9 @@ struct coresight_connection {
  * @ea:		Device attribute for sink representation under PMU directory.
  * @ect_dev:	Associated cross trigger device. Not part of the trace data
  *		path or connections.
+ * @nr_links:   number of sysfs links created to other components from this
+ *		device. These will appear in the "connections" group.
+ * @has_conns_grp: Have added a "connections" group for sysfs links.
  */
 struct coresight_device {
 	struct coresight_platform_data *pdata;
@@ -180,6 +197,9 @@ struct coresight_device {
 	struct dev_ext_attribute *ea;
 	/* cross trigger handling */
 	struct coresight_device *ect_dev;
+	/* sysfs links between components */
+	int nr_links;
+	bool has_conns_grp;
 };
 
 /*
-- 
2.17.1

Re: [PATCH v4 3/6] coresight: Add generic sysfs link creation functions.

[Mathieu Poirier]

On Tue, Feb 11, 2020 at 10:58:05AM +0000, Mike Leach wrote:
> To allow the connections between coresight components to be represented
> in sysfs, generic methods for creating sysfs links between two coresight
> devices are added.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/Makefile          |   3 +-
>  drivers/hwtracing/coresight/coresight-priv.h  |   4 +
>  drivers/hwtracing/coresight/coresight-sysfs.c | 124 ++++++++++++++++++
>  include/linux/coresight.h                     |  20 +++
>  4 files changed, 150 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/hwtracing/coresight/coresight-sysfs.c
> 
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 0e3e72f0f510..19497d1d92bf 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -2,7 +2,8 @@
>  #
>  # Makefile for CoreSight drivers.
>  #
> -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o coresight-platform.o
> +obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o \
> +			   coresight-platform.o coresight-sysfs.o
>  obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \
>  					     coresight-tmc-etf.o \
>  					     coresight-tmc-etr.o
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 1cad642f27aa..a4a658d46045 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -153,6 +153,10 @@ struct coresight_device *coresight_get_sink_by_id(u32 id);
>  struct list_head *coresight_build_path(struct coresight_device *csdev,
>  				       struct coresight_device *sink);
>  void coresight_release_path(struct list_head *path);
> +int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
> +void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
> +int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
> +void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
>  
>  #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
>  extern int etm_readl_cp14(u32 off, unsigned int *val);
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> new file mode 100644
> index 000000000000..17d565941e5e
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, Linaro Limited, All rights reserved.
> + * Author: Mike Leach <mike.leach@linaro.org>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +
> +#include "coresight-priv.h"
> +
> +/*
> + * Connections group - links attribute.
> + * Count of created links between coresight components in the group.
> + */
> +static ssize_t nr_links_show(struct device *dev,
> +			     struct device_attribute *attr,
> +			     char *buf)
> +{
> +	struct coresight_device *csdev = to_coresight_device(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", csdev->nr_links);
> +}
> +static DEVICE_ATTR_RO(nr_links);
> +
> +static struct attribute *coresight_conns_attrs[] = {
> +	&dev_attr_nr_links.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group coresight_conns_group = {
> +	.attrs = coresight_conns_attrs,
> +	.name = "connections",
> +};
> +
> +/*
> + * Create connections group for CoreSight devices.
> + * This group will then be used to collate the sysfs links between
> + * devices.
> + */
> +int coresight_create_conns_sysfs_group(struct coresight_device *csdev)
> +{
> +	int ret = 0;
> +
> +	if (!csdev)
> +		return -EINVAL;
> +
> +	ret = sysfs_create_group(&csdev->dev.kobj, &coresight_conns_group);
> +	if (ret)
> +		return ret;
> +
> +	csdev->has_conns_grp = true;

The only place coresight_create_conns_sysfs_group() is used is in
coresight_register() where an error is returned to driver probe() functions if
an error occurs.  Have you found places where csdev->has_conns_grp is needed?
If not please remove.

> +	return ret;
> +}
> +
> +void coresight_remove_conns_sysfs_group(struct coresight_device *csdev)
> +{
> +	if (!csdev)
> +		return;
> +
> +	if (csdev->has_conns_grp) {
> +		sysfs_remove_group(&csdev->dev.kobj, &coresight_conns_group);
> +		csdev->has_conns_grp = false;
> +	}
> +}
> +
> +int coresight_add_sysfs_link(struct coresight_sysfs_link *info)
> +{
> +	int ret = 0;
> +
> +	if (!info)
> +		return -EINVAL;
> +	if (!info->orig || !info->target ||
> +	    !info->orig_name || !info->target_name)
> +		return -EINVAL;
> +	if (!info->orig->has_conns_grp || !info->target->has_conns_grp)
> +		return -EINVAL;
> +
> +	/* first link orig->target */
> +	ret = sysfs_add_link_to_group(&info->orig->dev.kobj,
> +				      coresight_conns_group.name,
> +				      &info->target->dev.kobj,
> +				      info->orig_name);
> +	if (ret)
> +		return ret;
> +
> +	/* second link target->orig */
> +	ret = sysfs_add_link_to_group(&info->target->dev.kobj,
> +				      coresight_conns_group.name,
> +				      &info->orig->dev.kobj,
> +				      info->target_name);
> +
> +	/* error in second link - remove first - otherwise inc counts */
> +	if (ret) {
> +		sysfs_remove_link_from_group(&info->orig->dev.kobj,
> +					     coresight_conns_group.name,
> +					     info->orig_name);
> +	} else {
> +		info->orig->nr_links++;
> +		info->target->nr_links++;
> +	}
> +
> +	return ret;
> +}
> +
> +void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
> +{
> +	if (!info)
> +		return;
> +	if (!info->orig || !info->target ||
> +	    !info->orig_name || !info->target_name)
> +		return;
> +
> +	sysfs_remove_link_from_group(&info->orig->dev.kobj,
> +				     coresight_conns_group.name,
> +				     info->orig_name);
> +
> +	sysfs_remove_link_from_group(&info->target->dev.kobj,
> +				     coresight_conns_group.name,
> +				     info->target_name);
> +
> +	info->orig->nr_links--;
> +	info->target->nr_links--;
> +}
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 193cc9dbf448..a2ec25e02ca9 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -148,6 +148,20 @@ struct coresight_connection {
>  	struct coresight_device *child_dev;
>  };
>  
> +/**
> + * struct coresight_sysfs_link - representation of a connection in sysfs.
> + * @orig:		Originating (master) coresight device for the link.
> + * @orig_name:		Name to use for the link orig->target.
> + * @target:		Target (slave) coresight device for the link.
> + * @target_name:	Name to use for the link target->orig.
> + */
> +struct coresight_sysfs_link {
> +	struct coresight_device *orig;
> +	const char *orig_name;
> +	struct coresight_device *target;
> +	const char *target_name;
> +};
> +
>  /**
>   * struct coresight_device - representation of a device as used by the framework
>   * @pdata:	Platform data with device connections associated to this device.
> @@ -165,6 +179,9 @@ struct coresight_connection {
>   * @ea:		Device attribute for sink representation under PMU directory.
>   * @ect_dev:	Associated cross trigger device. Not part of the trace data
>   *		path or connections.
> + * @nr_links:   number of sysfs links created to other components from this
> + *		device. These will appear in the "connections" group.
> + * @has_conns_grp: Have added a "connections" group for sysfs links.
>   */
>  struct coresight_device {
>  	struct coresight_platform_data *pdata;
> @@ -180,6 +197,9 @@ struct coresight_device {
>  	struct dev_ext_attribute *ea;
>  	/* cross trigger handling */
>  	struct coresight_device *ect_dev;
> +	/* sysfs links between components */
> +	int nr_links;
> +	bool has_conns_grp;
>  };

With the above: 

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>  
>  /*
> -- 
> 2.17.1
> 

Re: [PATCH v4 3/6] coresight: Add generic sysfs link creation functions.

[Mike Leach]

Hi Mathieu,

On Fri, 14 Feb 2020 at 23:17, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> On Tue, Feb 11, 2020 at 10:58:05AM +0000, Mike Leach wrote:
> > To allow the connections between coresight components to be represented
> > in sysfs, generic methods for creating sysfs links between two coresight
> > devices are added.
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > ---
> >  drivers/hwtracing/coresight/Makefile          |   3 +-
> >  drivers/hwtracing/coresight/coresight-priv.h  |   4 +
> >  drivers/hwtracing/coresight/coresight-sysfs.c | 124 ++++++++++++++++++
> >  include/linux/coresight.h                     |  20 +++
> >  4 files changed, 150 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/hwtracing/coresight/coresight-sysfs.c
> >
> > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> > index 0e3e72f0f510..19497d1d92bf 100644
> > --- a/drivers/hwtracing/coresight/Makefile
> > +++ b/drivers/hwtracing/coresight/Makefile
> > @@ -2,7 +2,8 @@
> >  #
> >  # Makefile for CoreSight drivers.
> >  #
> > -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o coresight-platform.o
> > +obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o \
> > +                        coresight-platform.o coresight-sysfs.o
> >  obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \
> >                                            coresight-tmc-etf.o \
> >                                            coresight-tmc-etr.o
> > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> > index 1cad642f27aa..a4a658d46045 100644
> > --- a/drivers/hwtracing/coresight/coresight-priv.h
> > +++ b/drivers/hwtracing/coresight/coresight-priv.h
> > @@ -153,6 +153,10 @@ struct coresight_device *coresight_get_sink_by_id(u32 id);
> >  struct list_head *coresight_build_path(struct coresight_device *csdev,
> >                                      struct coresight_device *sink);
> >  void coresight_release_path(struct list_head *path);
> > +int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
> > +void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
> > +int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
> > +void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
> >
> >  #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
> >  extern int etm_readl_cp14(u32 off, unsigned int *val);
> > diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> > new file mode 100644
> > index 000000000000..17d565941e5e
> > --- /dev/null
> > +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> > @@ -0,0 +1,124 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2019, Linaro Limited, All rights reserved.
> > + * Author: Mike Leach <mike.leach@linaro.org>
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +
> > +#include "coresight-priv.h"
> > +
> > +/*
> > + * Connections group - links attribute.
> > + * Count of created links between coresight components in the group.
> > + */
> > +static ssize_t nr_links_show(struct device *dev,
> > +                          struct device_attribute *attr,
> > +                          char *buf)
> > +{
> > +     struct coresight_device *csdev = to_coresight_device(dev);
> > +
> > +     return scnprintf(buf, PAGE_SIZE, "%d\n", csdev->nr_links);
> > +}
> > +static DEVICE_ATTR_RO(nr_links);
> > +
> > +static struct attribute *coresight_conns_attrs[] = {
> > +     &dev_attr_nr_links.attr,
> > +     NULL,
> > +};
> > +
> > +static struct attribute_group coresight_conns_group = {
> > +     .attrs = coresight_conns_attrs,
> > +     .name = "connections",
> > +};
> > +
> > +/*
> > + * Create connections group for CoreSight devices.
> > + * This group will then be used to collate the sysfs links between
> > + * devices.
> > + */
> > +int coresight_create_conns_sysfs_group(struct coresight_device *csdev)
> > +{
> > +     int ret = 0;
> > +
> > +     if (!csdev)
> > +             return -EINVAL;
> > +
> > +     ret = sysfs_create_group(&csdev->dev.kobj, &coresight_conns_group);
> > +     if (ret)
> > +             return ret;
> > +
> > +     csdev->has_conns_grp = true;
>
> The only place coresight_create_conns_sysfs_group() is used is in
> coresight_register() where an error is returned to driver probe() functions if
> an error occurs.  Have you found places where csdev->has_conns_grp is needed?
> If not please remove.
>

There is a sequence of calls in coresight_register() which occur after
the coresight_create_conns_sysfs_group() -
any one of which may throw an error resulting in an immediate
coresight_unregister() and hence
coresight_release_platform_data() => coresight_remove_conns_sysfs_group().
There is also an alternate path through coresight_register() which
also results in coresight_release_platform_data() occurring before we
have created the group.
The flag ensures that both paths clean up the sysfs connection data
correctly and safely.

Regards

Mike

> > +     return ret;
> > +}
> > +
> > +void coresight_remove_conns_sysfs_group(struct coresight_device *csdev)
> > +{
> > +     if (!csdev)
> > +             return;
> > +
> > +     if (csdev->has_conns_grp) {
> > +             sysfs_remove_group(&csdev->dev.kobj, &coresight_conns_group);
> > +             csdev->has_conns_grp = false;
> > +     }
> > +}
> > +
> > +int coresight_add_sysfs_link(struct coresight_sysfs_link *info)
> > +{
> > +     int ret = 0;
> > +
> > +     if (!info)
> > +             return -EINVAL;
> > +     if (!info->orig || !info->target ||
> > +         !info->orig_name || !info->target_name)
> > +             return -EINVAL;
> > +     if (!info->orig->has_conns_grp || !info->target->has_conns_grp)
> > +             return -EINVAL;
> > +
> > +     /* first link orig->target */
> > +     ret = sysfs_add_link_to_group(&info->orig->dev.kobj,
> > +                                   coresight_conns_group.name,
> > +                                   &info->target->dev.kobj,
> > +                                   info->orig_name);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* second link target->orig */
> > +     ret = sysfs_add_link_to_group(&info->target->dev.kobj,
> > +                                   coresight_conns_group.name,
> > +                                   &info->orig->dev.kobj,
> > +                                   info->target_name);
> > +
> > +     /* error in second link - remove first - otherwise inc counts */
> > +     if (ret) {
> > +             sysfs_remove_link_from_group(&info->orig->dev.kobj,
> > +                                          coresight_conns_group.name,
> > +                                          info->orig_name);
> > +     } else {
> > +             info->orig->nr_links++;
> > +             info->target->nr_links++;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
> > +{
> > +     if (!info)
> > +             return;
> > +     if (!info->orig || !info->target ||
> > +         !info->orig_name || !info->target_name)
> > +             return;
> > +
> > +     sysfs_remove_link_from_group(&info->orig->dev.kobj,
> > +                                  coresight_conns_group.name,
> > +                                  info->orig_name);
> > +
> > +     sysfs_remove_link_from_group(&info->target->dev.kobj,
> > +                                  coresight_conns_group.name,
> > +                                  info->target_name);
> > +
> > +     info->orig->nr_links--;
> > +     info->target->nr_links--;
> > +}
> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > index 193cc9dbf448..a2ec25e02ca9 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -148,6 +148,20 @@ struct coresight_connection {
> >       struct coresight_device *child_dev;
> >  };
> >
> > +/**
> > + * struct coresight_sysfs_link - representation of a connection in sysfs.
> > + * @orig:            Originating (master) coresight device for the link.
> > + * @orig_name:               Name to use for the link orig->target.
> > + * @target:          Target (slave) coresight device for the link.
> > + * @target_name:     Name to use for the link target->orig.
> > + */
> > +struct coresight_sysfs_link {
> > +     struct coresight_device *orig;
> > +     const char *orig_name;
> > +     struct coresight_device *target;
> > +     const char *target_name;
> > +};
> > +
> >  /**
> >   * struct coresight_device - representation of a device as used by the framework
> >   * @pdata:   Platform data with device connections associated to this device.
> > @@ -165,6 +179,9 @@ struct coresight_connection {
> >   * @ea:              Device attribute for sink representation under PMU directory.
> >   * @ect_dev: Associated cross trigger device. Not part of the trace data
> >   *           path or connections.
> > + * @nr_links:   number of sysfs links created to other components from this
> > + *           device. These will appear in the "connections" group.
> > + * @has_conns_grp: Have added a "connections" group for sysfs links.
> >   */
> >  struct coresight_device {
> >       struct coresight_platform_data *pdata;
> > @@ -180,6 +197,9 @@ struct coresight_device {
> >       struct dev_ext_attribute *ea;
> >       /* cross trigger handling */
> >       struct coresight_device *ect_dev;
> > +     /* sysfs links between components */
> > +     int nr_links;
> > +     bool has_conns_grp;
> >  };
>
> With the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> >
> >  /*
> > --
> > 2.17.1
> >



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

Re: [PATCH v4 3/6] coresight: Add generic sysfs link creation functions.

[Mathieu Poirier]

On Tue, 25 Feb 2020 at 08:46, Mike Leach <mike.leach@linaro.org> wrote:
>
> Hi Mathieu,
>
> On Fri, 14 Feb 2020 at 23:17, Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > On Tue, Feb 11, 2020 at 10:58:05AM +0000, Mike Leach wrote:
> > > To allow the connections between coresight components to be represented
> > > in sysfs, generic methods for creating sysfs links between two coresight
> > > devices are added.
> > >
> > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > ---
> > >  drivers/hwtracing/coresight/Makefile          |   3 +-
> > >  drivers/hwtracing/coresight/coresight-priv.h  |   4 +
> > >  drivers/hwtracing/coresight/coresight-sysfs.c | 124 ++++++++++++++++++
> > >  include/linux/coresight.h                     |  20 +++
> > >  4 files changed, 150 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/hwtracing/coresight/coresight-sysfs.c
> > >
> > > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> > > index 0e3e72f0f510..19497d1d92bf 100644
> > > --- a/drivers/hwtracing/coresight/Makefile
> > > +++ b/drivers/hwtracing/coresight/Makefile
> > > @@ -2,7 +2,8 @@
> > >  #
> > >  # Makefile for CoreSight drivers.
> > >  #
> > > -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o coresight-platform.o
> > > +obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o \
> > > +                        coresight-platform.o coresight-sysfs.o
> > >  obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \
> > >                                            coresight-tmc-etf.o \
> > >                                            coresight-tmc-etr.o
> > > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> > > index 1cad642f27aa..a4a658d46045 100644
> > > --- a/drivers/hwtracing/coresight/coresight-priv.h
> > > +++ b/drivers/hwtracing/coresight/coresight-priv.h
> > > @@ -153,6 +153,10 @@ struct coresight_device *coresight_get_sink_by_id(u32 id);
> > >  struct list_head *coresight_build_path(struct coresight_device *csdev,
> > >                                      struct coresight_device *sink);
> > >  void coresight_release_path(struct list_head *path);
> > > +int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
> > > +void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
> > > +int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
> > > +void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
> > >
> > >  #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
> > >  extern int etm_readl_cp14(u32 off, unsigned int *val);
> > > diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> > > new file mode 100644
> > > index 000000000000..17d565941e5e
> > > --- /dev/null
> > > +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> > > @@ -0,0 +1,124 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2019, Linaro Limited, All rights reserved.
> > > + * Author: Mike Leach <mike.leach@linaro.org>
> > > + */
> > > +
> > > +#include <linux/device.h>
> > > +#include <linux/kernel.h>
> > > +
> > > +#include "coresight-priv.h"
> > > +
> > > +/*
> > > + * Connections group - links attribute.
> > > + * Count of created links between coresight components in the group.
> > > + */
> > > +static ssize_t nr_links_show(struct device *dev,
> > > +                          struct device_attribute *attr,
> > > +                          char *buf)
> > > +{
> > > +     struct coresight_device *csdev = to_coresight_device(dev);
> > > +
> > > +     return scnprintf(buf, PAGE_SIZE, "%d\n", csdev->nr_links);
> > > +}
> > > +static DEVICE_ATTR_RO(nr_links);
> > > +
> > > +static struct attribute *coresight_conns_attrs[] = {
> > > +     &dev_attr_nr_links.attr,
> > > +     NULL,
> > > +};
> > > +
> > > +static struct attribute_group coresight_conns_group = {
> > > +     .attrs = coresight_conns_attrs,
> > > +     .name = "connections",
> > > +};
> > > +
> > > +/*
> > > + * Create connections group for CoreSight devices.
> > > + * This group will then be used to collate the sysfs links between
> > > + * devices.
> > > + */
> > > +int coresight_create_conns_sysfs_group(struct coresight_device *csdev)
> > > +{
> > > +     int ret = 0;
> > > +
> > > +     if (!csdev)
> > > +             return -EINVAL;
> > > +
> > > +     ret = sysfs_create_group(&csdev->dev.kobj, &coresight_conns_group);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     csdev->has_conns_grp = true;
> >
> > The only place coresight_create_conns_sysfs_group() is used is in
> > coresight_register() where an error is returned to driver probe() functions if
> > an error occurs.  Have you found places where csdev->has_conns_grp is needed?
> > If not please remove.
> >
>
> There is a sequence of calls in coresight_register() which occur after
> the coresight_create_conns_sysfs_group() -
> any one of which may throw an error resulting in an immediate
> coresight_unregister() and hence
> coresight_release_platform_data() => coresight_remove_conns_sysfs_group().
> There is also an alternate path through coresight_register() which
> also results in coresight_release_platform_data() occurring before we
> have created the group.
> The flag ensures that both paths clean up the sysfs connection data
> correctly and safely.

I see the real problem is with the WARN() that is thrown in
sysfs_remove_group() when a group has not been created, and we can't
have that.  I think what you did is fine.

Mathieu

>
> Regards
>
> Mike
>
> > > +     return ret;
> > > +}
> > > +
> > > +void coresight_remove_conns_sysfs_group(struct coresight_device *csdev)
> > > +{
> > > +     if (!csdev)
> > > +             return;
> > > +
> > > +     if (csdev->has_conns_grp) {
> > > +             sysfs_remove_group(&csdev->dev.kobj, &coresight_conns_group);
> > > +             csdev->has_conns_grp = false;
> > > +     }
> > > +}
> > > +
> > > +int coresight_add_sysfs_link(struct coresight_sysfs_link *info)
> > > +{
> > > +     int ret = 0;
> > > +
> > > +     if (!info)
> > > +             return -EINVAL;
> > > +     if (!info->orig || !info->target ||
> > > +         !info->orig_name || !info->target_name)
> > > +             return -EINVAL;
> > > +     if (!info->orig->has_conns_grp || !info->target->has_conns_grp)
> > > +             return -EINVAL;
> > > +
> > > +     /* first link orig->target */
> > > +     ret = sysfs_add_link_to_group(&info->orig->dev.kobj,
> > > +                                   coresight_conns_group.name,
> > > +                                   &info->target->dev.kobj,
> > > +                                   info->orig_name);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     /* second link target->orig */
> > > +     ret = sysfs_add_link_to_group(&info->target->dev.kobj,
> > > +                                   coresight_conns_group.name,
> > > +                                   &info->orig->dev.kobj,
> > > +                                   info->target_name);
> > > +
> > > +     /* error in second link - remove first - otherwise inc counts */
> > > +     if (ret) {
> > > +             sysfs_remove_link_from_group(&info->orig->dev.kobj,
> > > +                                          coresight_conns_group.name,
> > > +                                          info->orig_name);
> > > +     } else {
> > > +             info->orig->nr_links++;
> > > +             info->target->nr_links++;
> > > +     }
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
> > > +{
> > > +     if (!info)
> > > +             return;
> > > +     if (!info->orig || !info->target ||
> > > +         !info->orig_name || !info->target_name)
> > > +             return;
> > > +
> > > +     sysfs_remove_link_from_group(&info->orig->dev.kobj,
> > > +                                  coresight_conns_group.name,
> > > +                                  info->orig_name);
> > > +
> > > +     sysfs_remove_link_from_group(&info->target->dev.kobj,
> > > +                                  coresight_conns_group.name,
> > > +                                  info->target_name);
> > > +
> > > +     info->orig->nr_links--;
> > > +     info->target->nr_links--;
> > > +}
> > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > > index 193cc9dbf448..a2ec25e02ca9 100644
> > > --- a/include/linux/coresight.h
> > > +++ b/include/linux/coresight.h
> > > @@ -148,6 +148,20 @@ struct coresight_connection {
> > >       struct coresight_device *child_dev;
> > >  };
> > >
> > > +/**
> > > + * struct coresight_sysfs_link - representation of a connection in sysfs.
> > > + * @orig:            Originating (master) coresight device for the link.
> > > + * @orig_name:               Name to use for the link orig->target.
> > > + * @target:          Target (slave) coresight device for the link.
> > > + * @target_name:     Name to use for the link target->orig.
> > > + */
> > > +struct coresight_sysfs_link {
> > > +     struct coresight_device *orig;
> > > +     const char *orig_name;
> > > +     struct coresight_device *target;
> > > +     const char *target_name;
> > > +};
> > > +
> > >  /**
> > >   * struct coresight_device - representation of a device as used by the framework
> > >   * @pdata:   Platform data with device connections associated to this device.
> > > @@ -165,6 +179,9 @@ struct coresight_connection {
> > >   * @ea:              Device attribute for sink representation under PMU directory.
> > >   * @ect_dev: Associated cross trigger device. Not part of the trace data
> > >   *           path or connections.
> > > + * @nr_links:   number of sysfs links created to other components from this
> > > + *           device. These will appear in the "connections" group.
> > > + * @has_conns_grp: Have added a "connections" group for sysfs links.
> > >   */
> > >  struct coresight_device {
> > >       struct coresight_platform_data *pdata;
> > > @@ -180,6 +197,9 @@ struct coresight_device {
> > >       struct dev_ext_attribute *ea;
> > >       /* cross trigger handling */
> > >       struct coresight_device *ect_dev;
> > > +     /* sysfs links between components */
> > > +     int nr_links;
> > > +     bool has_conns_grp;
> > >  };
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >
> > >
> > >  /*
> > > --
> > > 2.17.1
> > >
>
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK

[PATCH v4 4/6] coresight: Expose device connections via sysfs

[Mike Leach]

From: Suzuki K Poulose <suzuki.poulose@arm.com>

Coresight device connections are a bit complicated and is not
exposed currently to the user. One has to look at the platform
descriptions (DT bindings or ACPI bindings) to make an understanding.
Given the new naming scheme, it will be helpful to have this information
to choose the appropriate devices for tracing. This patch exposes
the device connections via links in the sysfs directories.

e.g, for a connection devA[OutputPort_X] -> devB[InputPort_Y]
is represented as two symlinks:

  /sys/bus/coresight/.../devA/out:X -> /sys/bus/coresight/.../devB
  /sys/bus/coresight/.../devB/in:Y  -> /sys/bus/coresight/.../devA

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Revised to use the generic sysfs links functions & link structures.
Provides a connections sysfs group to hold the links.

Co-developed-by: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/coresight-priv.h  |  5 ++
 drivers/hwtracing/coresight/coresight-sysfs.c | 80 +++++++++++++++++++
 drivers/hwtracing/coresight/coresight.c       | 46 ++++++++---
 include/linux/coresight.h                     |  2 +
 4 files changed, 121 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index a4a658d46045..5a36f0f50899 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -157,6 +157,11 @@ int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
 void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
 int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
 void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
+int coresight_make_links(struct coresight_device *orig,
+			 struct coresight_connection *conn,
+			 struct coresight_device *target);
+void coresight_remove_links(struct coresight_device *orig,
+			    struct coresight_connection *conn);
 
 #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
 extern int etm_readl_cp14(u32 off, unsigned int *val);
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index 17d565941e5e..0f18332b9f19 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -122,3 +122,83 @@ void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
 	info->orig->nr_links--;
 	info->target->nr_links--;
 }
+
+/*
+ * coresight_make_links: Make a link for a connection from a @orig
+ * device to @target, represented by @conn.
+ *
+ *   e.g, for devOrig[output_X] -> devTarget[input_Y] is represented
+ *   as two symbolic links :
+ *
+ *	/sys/.../devOrig/out:X	-> /sys/.../devTarget/
+ *	/sys/.../devTarget/in:Y	-> /sys/.../devOrig/
+ *
+ * The link names are allocated for a device where it appears. i.e, the
+ * "out" link on the master and "in" link on the slave device.
+ * The link info is stored in the connection record for avoiding
+ * the reconstruction of names for removal.
+ */
+int coresight_make_links(struct coresight_device *orig,
+			 struct coresight_connection *conn,
+			 struct coresight_device *target)
+{
+	int ret = -ENOMEM;
+	char *outs = NULL, *ins = NULL;
+	struct coresight_sysfs_link *link = NULL;
+
+	do {
+		outs = devm_kasprintf(&orig->dev, GFP_KERNEL,
+				      "out:%d", conn->outport);
+		if (!outs)
+			break;
+		ins = devm_kasprintf(&target->dev, GFP_KERNEL,
+				     "in:%d", conn->child_port);
+		if (!ins)
+			break;
+		link = devm_kzalloc(&orig->dev,
+				    sizeof(struct coresight_sysfs_link),
+				    GFP_KERNEL);
+		if (!link)
+			break;
+
+		link->orig = orig;
+		link->target = target;
+		link->orig_name = outs;
+		link->target_name = ins;
+
+		ret = coresight_add_sysfs_link(link);
+		if (ret)
+			break;
+
+		conn->link = link;
+
+		/*
+		 * Install the device connection. This also indicates that
+		 * the links are operational on both ends.
+		 */
+		conn->child_dev = target;
+		return 0;
+	} while (0);
+
+	return ret;
+}
+
+/*
+ * coresight_remove_links: Remove the sysfs links for a given connection @conn,
+ * from @orig device to @target device. See coresight_make_links() for more
+ * details.
+ */
+void coresight_remove_links(struct coresight_device *orig,
+			    struct coresight_connection *conn)
+{
+	if (!orig || !conn->link)
+		return;
+
+	coresight_remove_sysfs_link(conn->link);
+
+	devm_kfree(&conn->child_dev->dev, conn->link->target_name);
+	devm_kfree(&orig->dev, conn->link->orig_name);
+	devm_kfree(&orig->dev, conn->link);
+	conn->link = NULL;
+	conn->child_dev = NULL;
+}
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index 07f66a3968f1..4f10cfa9dc18 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -1031,7 +1031,7 @@ static void coresight_device_release(struct device *dev)
 
 static int coresight_orphan_match(struct device *dev, void *data)
 {
-	int i;
+	int i, ret = 0;
 	bool still_orphan = false;
 	struct coresight_device *csdev, *i_csdev;
 	struct coresight_connection *conn;
@@ -1056,19 +1056,23 @@ static int coresight_orphan_match(struct device *dev, void *data)
 		/* We have found at least one orphan connection */
 		if (conn->child_dev == NULL) {
 			/* Does it match this newly added device? */
-			if (conn->child_fwnode == csdev->dev.fwnode)
-				conn->child_dev = csdev;
-			else
+			if (conn->child_fwnode == csdev->dev.fwnode) {
+				ret = coresight_make_links(i_csdev,
+							   conn, csdev);
+				if (ret)
+					return ret;
+			} else {
 				/* This component still has an orphan */
 				still_orphan = true;
+			}
 		}
 	}
 
 	i_csdev->orphan = still_orphan;
 
 	/*
-	 * Returning '0' ensures that all known component on the
-	 * bus will be checked.
+	 * Returning '0' in case we didn't encounter any error,
+	 * ensures that all known component on the bus will be checked.
 	 */
 	return 0;
 }
@@ -1082,15 +1086,21 @@ static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
 
 static int coresight_fixup_device_conns(struct coresight_device *csdev)
 {
-	int i;
+	int i, ret = 0;
 
 	for (i = 0; i < csdev->pdata->nr_outport; i++) {
 		struct coresight_connection *conn = &csdev->pdata->conns[i];
 
 		conn->child_dev =
 			coresight_find_csdev_by_fwnode(conn->child_fwnode);
-		if (!conn->child_dev)
+		if (conn->child_dev) {
+			ret = coresight_make_links(csdev, conn,
+						   conn->child_dev);
+			if (ret)
+				break;
+		} else {
 			csdev->orphan = true;
+		}
 	}
 
 	return 0;
@@ -1121,7 +1131,7 @@ static int coresight_remove_match(struct device *dev, void *data)
 
 		if (csdev->dev.fwnode == conn->child_fwnode) {
 			iterator->orphan = true;
-			conn->child_dev = NULL;
+			coresight_remove_links(iterator, conn);
 			/*
 			 * Drop the reference to the handle for the remote
 			 * device acquired in parsing the connections from
@@ -1215,13 +1225,23 @@ void coresight_release_platform_data(struct coresight_device *csdev,
 				     struct coresight_platform_data *pdata)
 {
 	int i;
+	struct coresight_connection *conns = pdata->conns;
 
 	for (i = 0; i < pdata->nr_outport; i++) {
-		if (pdata->conns[i].child_fwnode) {
-			fwnode_handle_put(pdata->conns[i].child_fwnode);
+		/* If we have made the links, remove them now */
+		if (csdev && conns[i].child_dev)
+			coresight_remove_links(csdev, &conns[i]);
+		/*
+		 * Drop the refcount and clear the handle as this device
+		 * is going away
+		 */
+		if (conns[i].child_fwnode) {
+			fwnode_handle_put(conns[i].child_fwnode);
 			pdata->conns[i].child_fwnode = NULL;
 		}
 	}
+	if (csdev)
+		coresight_remove_conns_sysfs_group(csdev);
 }
 
 struct coresight_device *coresight_register(struct coresight_desc *desc)
@@ -1303,7 +1323,9 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 
 	mutex_lock(&coresight_mutex);
 
-	ret = coresight_fixup_device_conns(csdev);
+	ret = coresight_create_conns_sysfs_group(csdev);
+	if (!ret)
+		ret = coresight_fixup_device_conns(csdev);
 	if (!ret)
 		ret = coresight_fixup_orphan_conns(csdev);
 	if (!ret)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index a2ec25e02ca9..ccd17304d7bd 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -140,12 +140,14 @@ struct coresight_desc {
  * @chid_fwnode: remote component's fwnode handle.
  * @child_dev:	a @coresight_device representation of the component
 		connected to @outport.
+ * @link: Representation of the connection as a sysfs link.
  */
 struct coresight_connection {
 	int outport;
 	int child_port;
 	struct fwnode_handle *child_fwnode;
 	struct coresight_device *child_dev;
+	struct coresight_sysfs_link *link;
 };
 
 /**
-- 
2.17.1

Re: [PATCH v4 4/6] coresight: Expose device connections via sysfs

[Mathieu Poirier]

Hi Mike,

On Tue, Feb 11, 2020 at 10:58:06AM +0000, Mike Leach wrote:
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> Coresight device connections are a bit complicated and is not
> exposed currently to the user. One has to look at the platform
> descriptions (DT bindings or ACPI bindings) to make an understanding.
> Given the new naming scheme, it will be helpful to have this information
> to choose the appropriate devices for tracing. This patch exposes
> the device connections via links in the sysfs directories.
> 
> e.g, for a connection devA[OutputPort_X] -> devB[InputPort_Y]
> is represented as two symlinks:
> 
>   /sys/bus/coresight/.../devA/out:X -> /sys/bus/coresight/.../devB
>   /sys/bus/coresight/.../devB/in:Y  -> /sys/bus/coresight/.../devA
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> Revised to use the generic sysfs links functions & link structures.
> Provides a connections sysfs group to hold the links.
> 
> Co-developed-by: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>

Because this patch is "From:" Suzuki, this should be:

Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Mike Leach <mike.leach@linaro.org>

You can expand on Suzuki's contribution or the modifications you've done to it
in the changelog.

With the above:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> ---
>  drivers/hwtracing/coresight/coresight-priv.h  |  5 ++
>  drivers/hwtra cing/coresight/coresight-sysfs.c | 80 +++++++++++++++++++
>  drivers/hwtracing/coresight/coresight.c       | 46 ++++++++---
>  include/linux/coresight.h                     |  2 +
>  4 files changed, 121 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index a4a658d46045..5a36f0f50899 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -157,6 +157,11 @@ int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
>  void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
>  int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
>  void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
> +int coresight_make_links(struct coresight_device *orig,
> +			 struct coresight_connection *conn,
> +			 struct coresight_device *target);
> +void coresight_remove_links(struct coresight_device *orig,
> +			    struct coresight_connection *conn);
>  
>  #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
>  extern int etm_readl_cp14(u32 off, unsigned int *val);
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index 17d565941e5e..0f18332b9f19 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -122,3 +122,83 @@ void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
>  	info->orig->nr_links--;
>  	info->target->nr_links--;
>  }
> +
> +/*
> + * coresight_make_links: Make a link for a connection from a @orig
> + * device to @target, represented by @conn.
> + *
> + *   e.g, for devOrig[output_X] -> devTarget[input_Y] is represented
> + *   as two symbolic links :
> + *
> + *	/sys/.../devOrig/out:X	-> /sys/.../devTarget/
> + *	/sys/.../devTarget/in:Y	-> /sys/.../devOrig/
> + *
> + * The link names are allocated for a device where it appears. i.e, the
> + * "out" link on the master and "in" link on the slave device.
> + * The link info is stored in the connection record for avoiding
> + * the reconstruction of names for removal.
> + */
> +int coresight_make_links(struct coresight_device *orig,
> +			 struct coresight_connection *conn,
> +			 struct coresight_device *target)
> +{
> +	int ret = -ENOMEM;
> +	char *outs = NULL, *ins = NULL;
> +	struct coresight_sysfs_link *link = NULL;
> +
> +	do {
> +		outs = devm_kasprintf(&orig->dev, GFP_KERNEL,
> +				      "out:%d", conn->outport);
> +		if (!outs)
> +			break;
> +		ins = devm_kasprintf(&target->dev, GFP_KERNEL,
> +				     "in:%d", conn->child_port);
> +		if (!ins)
> +			break;
> +		link = devm_kzalloc(&orig->dev,
> +				    sizeof(struct coresight_sysfs_link),
> +				    GFP_KERNEL);
> +		if (!link)
> +			break;
> +
> +		link->orig = orig;
> +		link->target = target;
> +		link->orig_name = outs;
> +		link->target_name = ins;
> +
> +		ret = coresight_add_sysfs_link(link);
> +		if (ret)
> +			break;
> +
> +		conn->link = link;
> +
> +		/*
> +		 * Install the device connection. This also indicates that
> +		 * the links are operational on both ends.
> +		 */
> +		conn->child_dev = target;
> +		return 0;
> +	} while (0);
> +
> +	return ret;
> +}
> +
> +/*
> + * coresight_remove_links: Remove the sysfs links for a given connection @conn,
> + * from @orig device to @target device. See coresight_make_links() for more
> + * details.
> + */
> +void coresight_remove_links(struct coresight_device *orig,
> +			    struct coresight_connection *conn)
> +{
> +	if (!orig || !conn->link)
> +		return;
> +
> +	coresight_remove_sysfs_link(conn->link);
> +
> +	devm_kfree(&conn->child_dev->dev, conn->link->target_name);
> +	devm_kfree(&orig->dev, conn->link->orig_name);
> +	devm_kfree(&orig->dev, conn->link);
> +	conn->link = NULL;
> +	conn->child_dev = NULL;
> +}
> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> index 07f66a3968f1..4f10cfa9dc18 100644
> --- a/drivers/hwtracing/coresight/coresight.c
> +++ b/drivers/hwtracing/coresight/coresight.c
> @@ -1031,7 +1031,7 @@ static void coresight_device_release(struct device *dev)
>  
>  static int coresight_orphan_match(struct device *dev, void *data)
>  {
> -	int i;
> +	int i, ret = 0;
>  	bool still_orphan = false;
>  	struct coresight_device *csdev, *i_csdev;
>  	struct coresight_connection *conn;
> @@ -1056,19 +1056,23 @@ static int coresight_orphan_match(struct device *dev, void *data)
>  		/* We have found at least one orphan connection */
>  		if (conn->child_dev == NULL) {
>  			/* Does it match this newly added device? */
> -			if (conn->child_fwnode == csdev->dev.fwnode)
> -				conn->child_dev = csdev;
> -			else
> +			if (conn->child_fwnode == csdev->dev.fwnode) {
> +				ret = coresight_make_links(i_csdev,
> +							   conn, csdev);
> +				if (ret)
> +					return ret;
> +			} else {
>  				/* This component still has an orphan */
>  				still_orphan = true;
> +			}
>  		}
>  	}
>  
>  	i_csdev->orphan = still_orphan;
>  
>  	/*
> -	 * Returning '0' ensures that all known component on the
> -	 * bus will be checked.
> +	 * Returning '0' in case we didn't encounter any error,
> +	 * ensures that all known component on the bus will be checked.
>  	 */
>  	return 0;
>  }
> @@ -1082,15 +1086,21 @@ static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
>  
>  static int coresight_fixup_device_conns(struct coresight_device *csdev)
>  {
> -	int i;
> +	int i, ret = 0;
>  
>  	for (i = 0; i < csdev->pdata->nr_outport; i++) {
>  		struct coresight_connection *conn = &csdev->pdata->conns[i];
>  
>  		conn->child_dev =
>  			coresight_find_csdev_by_fwnode(conn->child_fwnode);
> -		if (!conn->child_dev)
> +		if (conn->child_dev) {
> +			ret = coresight_make_links(csdev, conn,
> +						   conn->child_dev);
> +			if (ret)
> +				break;
> +		} else {
>  			csdev->orphan = true;
> +		}
>  	}
>  
>  	return 0;
> @@ -1121,7 +1131,7 @@ static int coresight_remove_match(struct device *dev, void *data)
>  
>  		if (csdev->dev.fwnode == conn->child_fwnode) {
>  			iterator->orphan = true;
> -			conn->child_dev = NULL;
> +			coresight_remove_links(iterator, conn);
>  			/*
>  			 * Drop the reference to the handle for the remote
>  			 * device acquired in parsing the connections from
> @@ -1215,13 +1225,23 @@ void coresight_release_platform_data(struct coresight_device *csdev,
>  				     struct coresight_platform_data *pdata)
>  {
>  	int i;
> +	struct coresight_connection *conns = pdata->conns;
>  
>  	for (i = 0; i < pdata->nr_outport; i++) {
> -		if (pdata->conns[i].child_fwnode) {
> -			fwnode_handle_put(pdata->conns[i].child_fwnode);
> +		/* If we have made the links, remove them now */
> +		if (csdev && conns[i].child_dev)
> +			coresight_remove_links(csdev, &conns[i]);
> +		/*
> +		 * Drop the refcount and clear the handle as this device
> +		 * is going away
> +		 */
> +		if (conns[i].child_fwnode) {
> +			fwnode_handle_put(conns[i].child_fwnode);
>  			pdata->conns[i].child_fwnode = NULL;
>  		}
>  	}
> +	if (csdev)
> +		coresight_remove_conns_sysfs_group(csdev);
>  }
>  
>  struct coresight_device *coresight_register(struct coresight_desc *desc)
> @@ -1303,7 +1323,9 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>  
>  	mutex_lock(&coresight_mutex);
>  
> -	ret = coresight_fixup_device_conns(csdev);
> +	ret = coresight_create_conns_sysfs_group(csdev);
> +	if (!ret)
> +		ret = coresight_fixup_device_conns(csdev);
>  	if (!ret)
>  		ret = coresight_fixup_orphan_conns(csdev);
>  	if (!ret)
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index a2ec25e02ca9..ccd17304d7bd 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -140,12 +140,14 @@ struct coresight_desc {
>   * @chid_fwnode: remote component's fwnode handle.
>   * @child_dev:	a @coresight_device representation of the component
>  		connected to @outport.
> + * @link: Representation of the connection as a sysfs link.
>   */
>  struct coresight_connection {
>  	int outport;
>  	int child_port;
>  	struct fwnode_handle *child_fwnode;
>  	struct coresight_device *child_dev;
> +	struct coresight_sysfs_link *link;
>  };
>  
>  /**
> -- 
> 2.17.1
> 

Re: [PATCH v4 4/6] coresight: Expose device connections via sysfs

[Mike Leach]

Hi Mathieu,

On Fri, 14 Feb 2020 at 22:31, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> Hi Mike,
>
> On Tue, Feb 11, 2020 at 10:58:06AM +0000, Mike Leach wrote:
> > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> >
> > Coresight device connections are a bit complicated and is not
> > exposed currently to the user. One has to look at the platform
> > descriptions (DT bindings or ACPI bindings) to make an understanding.
> > Given the new naming scheme, it will be helpful to have this information
> > to choose the appropriate devices for tracing. This patch exposes
> > the device connections via links in the sysfs directories.
> >
> > e.g, for a connection devA[OutputPort_X] -> devB[InputPort_Y]
> > is represented as two symlinks:
> >
> >   /sys/bus/coresight/.../devA/out:X -> /sys/bus/coresight/.../devB
> >   /sys/bus/coresight/.../devB/in:Y  -> /sys/bus/coresight/.../devA
> >
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >
> > Revised to use the generic sysfs links functions & link structures.
> > Provides a connections sysfs group to hold the links.
> >
> > Co-developed-by: Mike Leach <mike.leach@linaro.org>
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
>
> Because this patch is "From:" Suzuki, this should be:
>
> Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
>

if I do the above then checkpatch.pl:-

WARNING: Co-developed-by: should not be used to attribute nominal
patch author 'Suzuki K Poulose <suzuki.poulose@arm.com>'
#25:
Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
WARNING: Co-developed-by and Signed-off-by: name/email do not match
#25:
Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Mike Leach <mike.leach@linaro.org>

Regards

Mike

> You can expand on Suzuki's contribution or the modifications you've done to it
> in the changelog.
>
> With the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> > ---
> >  drivers/hwtracing/coresight/coresight-priv.h  |  5 ++
> >  drivers/hwtra cing/coresight/coresight-sysfs.c | 80 +++++++++++++++++++
> >  drivers/hwtracing/coresight/coresight.c       | 46 ++++++++---
> >  include/linux/coresight.h                     |  2 +
> >  4 files changed, 121 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> > index a4a658d46045..5a36f0f50899 100644
> > --- a/drivers/hwtracing/coresight/coresight-priv.h
> > +++ b/drivers/hwtracing/coresight/coresight-priv.h
> > @@ -157,6 +157,11 @@ int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
> >  void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
> >  int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
> >  void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
> > +int coresight_make_links(struct coresight_device *orig,
> > +                      struct coresight_connection *conn,
> > +                      struct coresight_device *target);
> > +void coresight_remove_links(struct coresight_device *orig,
> > +                         struct coresight_connection *conn);
> >
> >  #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
> >  extern int etm_readl_cp14(u32 off, unsigned int *val);
> > diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> > index 17d565941e5e..0f18332b9f19 100644
> > --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> > +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> > @@ -122,3 +122,83 @@ void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
> >       info->orig->nr_links--;
> >       info->target->nr_links--;
> >  }
> > +
> > +/*
> > + * coresight_make_links: Make a link for a connection from a @orig
> > + * device to @target, represented by @conn.
> > + *
> > + *   e.g, for devOrig[output_X] -> devTarget[input_Y] is represented
> > + *   as two symbolic links :
> > + *
> > + *   /sys/.../devOrig/out:X  -> /sys/.../devTarget/
> > + *   /sys/.../devTarget/in:Y -> /sys/.../devOrig/
> > + *
> > + * The link names are allocated for a device where it appears. i.e, the
> > + * "out" link on the master and "in" link on the slave device.
> > + * The link info is stored in the connection record for avoiding
> > + * the reconstruction of names for removal.
> > + */
> > +int coresight_make_links(struct coresight_device *orig,
> > +                      struct coresight_connection *conn,
> > +                      struct coresight_device *target)
> > +{
> > +     int ret = -ENOMEM;
> > +     char *outs = NULL, *ins = NULL;
> > +     struct coresight_sysfs_link *link = NULL;
> > +
> > +     do {
> > +             outs = devm_kasprintf(&orig->dev, GFP_KERNEL,
> > +                                   "out:%d", conn->outport);
> > +             if (!outs)
> > +                     break;
> > +             ins = devm_kasprintf(&target->dev, GFP_KERNEL,
> > +                                  "in:%d", conn->child_port);
> > +             if (!ins)
> > +                     break;
> > +             link = devm_kzalloc(&orig->dev,
> > +                                 sizeof(struct coresight_sysfs_link),
> > +                                 GFP_KERNEL);
> > +             if (!link)
> > +                     break;
> > +
> > +             link->orig = orig;
> > +             link->target = target;
> > +             link->orig_name = outs;
> > +             link->target_name = ins;
> > +
> > +             ret = coresight_add_sysfs_link(link);
> > +             if (ret)
> > +                     break;
> > +
> > +             conn->link = link;
> > +
> > +             /*
> > +              * Install the device connection. This also indicates that
> > +              * the links are operational on both ends.
> > +              */
> > +             conn->child_dev = target;
> > +             return 0;
> > +     } while (0);
> > +
> > +     return ret;
> > +}
> > +
> > +/*
> > + * coresight_remove_links: Remove the sysfs links for a given connection @conn,
> > + * from @orig device to @target device. See coresight_make_links() for more
> > + * details.
> > + */
> > +void coresight_remove_links(struct coresight_device *orig,
> > +                         struct coresight_connection *conn)
> > +{
> > +     if (!orig || !conn->link)
> > +             return;
> > +
> > +     coresight_remove_sysfs_link(conn->link);
> > +
> > +     devm_kfree(&conn->child_dev->dev, conn->link->target_name);
> > +     devm_kfree(&orig->dev, conn->link->orig_name);
> > +     devm_kfree(&orig->dev, conn->link);
> > +     conn->link = NULL;
> > +     conn->child_dev = NULL;
> > +}
> > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> > index 07f66a3968f1..4f10cfa9dc18 100644
> > --- a/drivers/hwtracing/coresight/coresight.c
> > +++ b/drivers/hwtracing/coresight/coresight.c
> > @@ -1031,7 +1031,7 @@ static void coresight_device_release(struct device *dev)
> >
> >  static int coresight_orphan_match(struct device *dev, void *data)
> >  {
> > -     int i;
> > +     int i, ret = 0;
> >       bool still_orphan = false;
> >       struct coresight_device *csdev, *i_csdev;
> >       struct coresight_connection *conn;
> > @@ -1056,19 +1056,23 @@ static int coresight_orphan_match(struct device *dev, void *data)
> >               /* We have found at least one orphan connection */
> >               if (conn->child_dev == NULL) {
> >                       /* Does it match this newly added device? */
> > -                     if (conn->child_fwnode == csdev->dev.fwnode)
> > -                             conn->child_dev = csdev;
> > -                     else
> > +                     if (conn->child_fwnode == csdev->dev.fwnode) {
> > +                             ret = coresight_make_links(i_csdev,
> > +                                                        conn, csdev);
> > +                             if (ret)
> > +                                     return ret;
> > +                     } else {
> >                               /* This component still has an orphan */
> >                               still_orphan = true;
> > +                     }
> >               }
> >       }
> >
> >       i_csdev->orphan = still_orphan;
> >
> >       /*
> > -      * Returning '0' ensures that all known component on the
> > -      * bus will be checked.
> > +      * Returning '0' in case we didn't encounter any error,
> > +      * ensures that all known component on the bus will be checked.
> >        */
> >       return 0;
> >  }
> > @@ -1082,15 +1086,21 @@ static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
> >
> >  static int coresight_fixup_device_conns(struct coresight_device *csdev)
> >  {
> > -     int i;
> > +     int i, ret = 0;
> >
> >       for (i = 0; i < csdev->pdata->nr_outport; i++) {
> >               struct coresight_connection *conn = &csdev->pdata->conns[i];
> >
> >               conn->child_dev =
> >                       coresight_find_csdev_by_fwnode(conn->child_fwnode);
> > -             if (!conn->child_dev)
> > +             if (conn->child_dev) {
> > +                     ret = coresight_make_links(csdev, conn,
> > +                                                conn->child_dev);
> > +                     if (ret)
> > +                             break;
> > +             } else {
> >                       csdev->orphan = true;
> > +             }
> >       }
> >
> >       return 0;
> > @@ -1121,7 +1131,7 @@ static int coresight_remove_match(struct device *dev, void *data)
> >
> >               if (csdev->dev.fwnode == conn->child_fwnode) {
> >                       iterator->orphan = true;
> > -                     conn->child_dev = NULL;
> > +                     coresight_remove_links(iterator, conn);
> >                       /*
> >                        * Drop the reference to the handle for the remote
> >                        * device acquired in parsing the connections from
> > @@ -1215,13 +1225,23 @@ void coresight_release_platform_data(struct coresight_device *csdev,
> >                                    struct coresight_platform_data *pdata)
> >  {
> >       int i;
> > +     struct coresight_connection *conns = pdata->conns;
> >
> >       for (i = 0; i < pdata->nr_outport; i++) {
> > -             if (pdata->conns[i].child_fwnode) {
> > -                     fwnode_handle_put(pdata->conns[i].child_fwnode);
> > +             /* If we have made the links, remove them now */
> > +             if (csdev && conns[i].child_dev)
> > +                     coresight_remove_links(csdev, &conns[i]);
> > +             /*
> > +              * Drop the refcount and clear the handle as this device
> > +              * is going away
> > +              */
> > +             if (conns[i].child_fwnode) {
> > +                     fwnode_handle_put(conns[i].child_fwnode);
> >                       pdata->conns[i].child_fwnode = NULL;
> >               }
> >       }
> > +     if (csdev)
> > +             coresight_remove_conns_sysfs_group(csdev);
> >  }
> >
> >  struct coresight_device *coresight_register(struct coresight_desc *desc)
> > @@ -1303,7 +1323,9 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
> >
> >       mutex_lock(&coresight_mutex);
> >
> > -     ret = coresight_fixup_device_conns(csdev);
> > +     ret = coresight_create_conns_sysfs_group(csdev);
> > +     if (!ret)
> > +             ret = coresight_fixup_device_conns(csdev);
> >       if (!ret)
> >               ret = coresight_fixup_orphan_conns(csdev);
> >       if (!ret)
> > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > index a2ec25e02ca9..ccd17304d7bd 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -140,12 +140,14 @@ struct coresight_desc {
> >   * @chid_fwnode: remote component's fwnode handle.
> >   * @child_dev:       a @coresight_device representation of the component
> >               connected to @outport.
> > + * @link: Representation of the connection as a sysfs link.
> >   */
> >  struct coresight_connection {
> >       int outport;
> >       int child_port;
> >       struct fwnode_handle *child_fwnode;
> >       struct coresight_device *child_dev;
> > +     struct coresight_sysfs_link *link;
> >  };
> >
> >  /**
> > --
> > 2.17.1
> >



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

Re: [PATCH v4 4/6] coresight: Expose device connections via sysfs

[Mathieu Poirier]

On Wed, Feb 26, 2020 at 02:32:16PM +0000, Mike Leach wrote:
> Hi Mathieu,
> 
> On Fri, 14 Feb 2020 at 22:31, Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > Hi Mike,
> >
> > On Tue, Feb 11, 2020 at 10:58:06AM +0000, Mike Leach wrote:
> > > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> > >
> > > Coresight device connections are a bit complicated and is not
> > > exposed currently to the user. One has to look at the platform
> > > descriptions (DT bindings or ACPI bindings) to make an understanding.
> > > Given the new naming scheme, it will be helpful to have this information
> > > to choose the appropriate devices for tracing. This patch exposes
> > > the device connections via links in the sysfs directories.
> > >
> > > e.g, for a connection devA[OutputPort_X] -> devB[InputPort_Y]
> > > is represented as two symlinks:
> > >
> > >   /sys/bus/coresight/.../devA/out:X -> /sys/bus/coresight/.../devB
> > >   /sys/bus/coresight/.../devB/in:Y  -> /sys/bus/coresight/.../devA
> > >
> > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > >
> > > Revised to use the generic sysfs links functions & link structures.
> > > Provides a connections sysfs group to hold the links.
> > >
> > > Co-developed-by: Mike Leach <mike.leach@linaro.org>
> > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> >
> > Because this patch is "From:" Suzuki, this should be:
> >
> > Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> >
> 
> if I do the above then checkpatch.pl:-
> 
> WARNING: Co-developed-by: should not be used to attribute nominal
> patch author 'Suzuki K Poulose <suzuki.poulose@arm.com>'
> #25:
> Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> WARNING: Co-developed-by and Signed-off-by: name/email do not match
> #25:
> Co-developed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>

Got that - thanks

> 
> Regards
> 
> Mike
> 
> > You can expand on Suzuki's contribution or the modifications you've done to it
> > in the changelog.
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >
> > > ---
> > >  drivers/hwtracing/coresight/coresight-priv.h  |  5 ++
> > >  drivers/hwtra cing/coresight/coresight-sysfs.c | 80 +++++++++++++++++++
> > >  drivers/hwtracing/coresight/coresight.c       | 46 ++++++++---
> > >  include/linux/coresight.h                     |  2 +
> > >  4 files changed, 121 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> > > index a4a658d46045..5a36f0f50899 100644
> > > --- a/drivers/hwtracing/coresight/coresight-priv.h
> > > +++ b/drivers/hwtracing/coresight/coresight-priv.h
> > > @@ -157,6 +157,11 @@ int coresight_add_sysfs_link(struct coresight_sysfs_link *info);
> > >  void coresight_remove_sysfs_link(struct coresight_sysfs_link *info);
> > >  int coresight_create_conns_sysfs_group(struct coresight_device *csdev);
> > >  void coresight_remove_conns_sysfs_group(struct coresight_device *csdev);
> > > +int coresight_make_links(struct coresight_device *orig,
> > > +                      struct coresight_connection *conn,
> > > +                      struct coresight_device *target);
> > > +void coresight_remove_links(struct coresight_device *orig,
> > > +                         struct coresight_connection *conn);
> > >
> > >  #ifdef CONFIG_CORESIGHT_SOURCE_ETM3X
> > >  extern int etm_readl_cp14(u32 off, unsigned int *val);
> > > diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> > > index 17d565941e5e..0f18332b9f19 100644
> > > --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> > > +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> > > @@ -122,3 +122,83 @@ void coresight_remove_sysfs_link(struct coresight_sysfs_link *info)
> > >       info->orig->nr_links--;
> > >       info->target->nr_links--;
> > >  }
> > > +
> > > +/*
> > > + * coresight_make_links: Make a link for a connection from a @orig
> > > + * device to @target, represented by @conn.
> > > + *
> > > + *   e.g, for devOrig[output_X] -> devTarget[input_Y] is represented
> > > + *   as two symbolic links :
> > > + *
> > > + *   /sys/.../devOrig/out:X  -> /sys/.../devTarget/
> > > + *   /sys/.../devTarget/in:Y -> /sys/.../devOrig/
> > > + *
> > > + * The link names are allocated for a device where it appears. i.e, the
> > > + * "out" link on the master and "in" link on the slave device.
> > > + * The link info is stored in the connection record for avoiding
> > > + * the reconstruction of names for removal.
> > > + */
> > > +int coresight_make_links(struct coresight_device *orig,
> > > +                      struct coresight_connection *conn,
> > > +                      struct coresight_device *target)
> > > +{
> > > +     int ret = -ENOMEM;
> > > +     char *outs = NULL, *ins = NULL;
> > > +     struct coresight_sysfs_link *link = NULL;
> > > +
> > > +     do {
> > > +             outs = devm_kasprintf(&orig->dev, GFP_KERNEL,
> > > +                                   "out:%d", conn->outport);
> > > +             if (!outs)
> > > +                     break;
> > > +             ins = devm_kasprintf(&target->dev, GFP_KERNEL,
> > > +                                  "in:%d", conn->child_port);
> > > +             if (!ins)
> > > +                     break;
> > > +             link = devm_kzalloc(&orig->dev,
> > > +                                 sizeof(struct coresight_sysfs_link),
> > > +                                 GFP_KERNEL);
> > > +             if (!link)
> > > +                     break;
> > > +
> > > +             link->orig = orig;
> > > +             link->target = target;
> > > +             link->orig_name = outs;
> > > +             link->target_name = ins;
> > > +
> > > +             ret = coresight_add_sysfs_link(link);
> > > +             if (ret)
> > > +                     break;
> > > +
> > > +             conn->link = link;
> > > +
> > > +             /*
> > > +              * Install the device connection. This also indicates that
> > > +              * the links are operational on both ends.
> > > +              */
> > > +             conn->child_dev = target;
> > > +             return 0;
> > > +     } while (0);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +/*
> > > + * coresight_remove_links: Remove the sysfs links for a given connection @conn,
> > > + * from @orig device to @target device. See coresight_make_links() for more
> > > + * details.
> > > + */
> > > +void coresight_remove_links(struct coresight_device *orig,
> > > +                         struct coresight_connection *conn)
> > > +{
> > > +     if (!orig || !conn->link)
> > > +             return;
> > > +
> > > +     coresight_remove_sysfs_link(conn->link);
> > > +
> > > +     devm_kfree(&conn->child_dev->dev, conn->link->target_name);
> > > +     devm_kfree(&orig->dev, conn->link->orig_name);
> > > +     devm_kfree(&orig->dev, conn->link);
> > > +     conn->link = NULL;
> > > +     conn->child_dev = NULL;
> > > +}
> > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> > > index 07f66a3968f1..4f10cfa9dc18 100644
> > > --- a/drivers/hwtracing/coresight/coresight.c
> > > +++ b/drivers/hwtracing/coresight/coresight.c
> > > @@ -1031,7 +1031,7 @@ static void coresight_device_release(struct device *dev)
> > >
> > >  static int coresight_orphan_match(struct device *dev, void *data)
> > >  {
> > > -     int i;
> > > +     int i, ret = 0;
> > >       bool still_orphan = false;
> > >       struct coresight_device *csdev, *i_csdev;
> > >       struct coresight_connection *conn;
> > > @@ -1056,19 +1056,23 @@ static int coresight_orphan_match(struct device *dev, void *data)
> > >               /* We have found at least one orphan connection */
> > >               if (conn->child_dev == NULL) {
> > >                       /* Does it match this newly added device? */
> > > -                     if (conn->child_fwnode == csdev->dev.fwnode)
> > > -                             conn->child_dev = csdev;
> > > -                     else
> > > +                     if (conn->child_fwnode == csdev->dev.fwnode) {
> > > +                             ret = coresight_make_links(i_csdev,
> > > +                                                        conn, csdev);
> > > +                             if (ret)
> > > +                                     return ret;
> > > +                     } else {
> > >                               /* This component still has an orphan */
> > >                               still_orphan = true;
> > > +                     }
> > >               }
> > >       }
> > >
> > >       i_csdev->orphan = still_orphan;
> > >
> > >       /*
> > > -      * Returning '0' ensures that all known component on the
> > > -      * bus will be checked.
> > > +      * Returning '0' in case we didn't encounter any error,
> > > +      * ensures that all known component on the bus will be checked.
> > >        */
> > >       return 0;
> > >  }
> > > @@ -1082,15 +1086,21 @@ static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
> > >
> > >  static int coresight_fixup_device_conns(struct coresight_device *csdev)
> > >  {
> > > -     int i;
> > > +     int i, ret = 0;
> > >
> > >       for (i = 0; i < csdev->pdata->nr_outport; i++) {
> > >               struct coresight_connection *conn = &csdev->pdata->conns[i];
> > >
> > >               conn->child_dev =
> > >                       coresight_find_csdev_by_fwnode(conn->child_fwnode);
> > > -             if (!conn->child_dev)
> > > +             if (conn->child_dev) {
> > > +                     ret = coresight_make_links(csdev, conn,
> > > +                                                conn->child_dev);
> > > +                     if (ret)
> > > +                             break;
> > > +             } else {
> > >                       csdev->orphan = true;
> > > +             }
> > >       }
> > >
> > >       return 0;
> > > @@ -1121,7 +1131,7 @@ static int coresight_remove_match(struct device *dev, void *data)
> > >
> > >               if (csdev->dev.fwnode == conn->child_fwnode) {
> > >                       iterator->orphan = true;
> > > -                     conn->child_dev = NULL;
> > > +                     coresight_remove_links(iterator, conn);
> > >                       /*
> > >                        * Drop the reference to the handle for the remote
> > >                        * device acquired in parsing the connections from
> > > @@ -1215,13 +1225,23 @@ void coresight_release_platform_data(struct coresight_device *csdev,
> > >                                    struct coresight_platform_data *pdata)
> > >  {
> > >       int i;
> > > +     struct coresight_connection *conns = pdata->conns;
> > >
> > >       for (i = 0; i < pdata->nr_outport; i++) {
> > > -             if (pdata->conns[i].child_fwnode) {
> > > -                     fwnode_handle_put(pdata->conns[i].child_fwnode);
> > > +             /* If we have made the links, remove them now */
> > > +             if (csdev && conns[i].child_dev)
> > > +                     coresight_remove_links(csdev, &conns[i]);
> > > +             /*
> > > +              * Drop the refcount and clear the handle as this device
> > > +              * is going away
> > > +              */
> > > +             if (conns[i].child_fwnode) {
> > > +                     fwnode_handle_put(conns[i].child_fwnode);
> > >                       pdata->conns[i].child_fwnode = NULL;
> > >               }
> > >       }
> > > +     if (csdev)
> > > +             coresight_remove_conns_sysfs_group(csdev);
> > >  }
> > >
> > >  struct coresight_device *coresight_register(struct coresight_desc *desc)
> > > @@ -1303,7 +1323,9 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
> > >
> > >       mutex_lock(&coresight_mutex);
> > >
> > > -     ret = coresight_fixup_device_conns(csdev);
> > > +     ret = coresight_create_conns_sysfs_group(csdev);
> > > +     if (!ret)
> > > +             ret = coresight_fixup_device_conns(csdev);
> > >       if (!ret)
> > >               ret = coresight_fixup_orphan_conns(csdev);
> > >       if (!ret)
> > > diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> > > index a2ec25e02ca9..ccd17304d7bd 100644
> > > --- a/include/linux/coresight.h
> > > +++ b/include/linux/coresight.h
> > > @@ -140,12 +140,14 @@ struct coresight_desc {
> > >   * @chid_fwnode: remote component's fwnode handle.
> > >   * @child_dev:       a @coresight_device representation of the component
> > >               connected to @outport.
> > > + * @link: Representation of the connection as a sysfs link.
> > >   */
> > >  struct coresight_connection {
> > >       int outport;
> > >       int child_port;
> > >       struct fwnode_handle *child_fwnode;
> > >       struct coresight_device *child_dev;
> > > +     struct coresight_sysfs_link *link;
> > >  };
> > >
> > >  /**
> > > --
> > > 2.17.1
> > >
> 
> 
> 
> -- 
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK

[PATCH v4 5/6] coresight: cti: Add in sysfs links to other coresight devices.

[Mike Leach]

Adds in sysfs links for connections where the connected device is another
coresight device. This allows examination of the coresight topology.

Non-coresight connections remain just as a reference name.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 drivers/hwtracing/coresight/coresight-cti.c | 41 ++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
index 9e18e176831c..f620e9460e7d 100644
--- a/drivers/hwtracing/coresight/coresight-cti.c
+++ b/drivers/hwtracing/coresight/coresight-cti.c
@@ -441,6 +441,37 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
 	return err;
 }
 
+static void cti_add_sysfs_link(struct cti_drvdata *drvdata,
+			       struct cti_trig_con *tc)
+{
+	struct coresight_sysfs_link link_info;
+
+	link_info.orig = drvdata->csdev;
+	link_info.orig_name = tc->con_dev_name;
+	link_info.target = tc->con_dev;
+	link_info.target_name = dev_name(&drvdata->csdev->dev);
+	coresight_add_sysfs_link(&link_info);
+}
+
+static void cti_remove_all_sysfs_links(struct cti_drvdata *drvdata)
+{
+	struct cti_trig_con *tc;
+	struct cti_device *ctidev = &drvdata->ctidev;
+	struct coresight_sysfs_link link_info;
+
+	/* origin device and target link name constant for this cti */
+	link_info.orig = drvdata->csdev;
+	link_info.target_name = dev_name(&drvdata->csdev->dev);
+
+	list_for_each_entry(tc, &ctidev->trig_cons, node) {
+		if (tc->con_dev) {
+			link_info.target = tc->con_dev;
+			link_info.orig_name = tc->con_dev_name;
+			coresight_remove_sysfs_link(&link_info);
+		}
+	}
+}
+
 /*
  * Look for a matching connection device name in the list of connections.
  * If found then swap in the csdev name, set trig con association pointer
@@ -452,6 +483,8 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
 {
 	struct cti_trig_con *tc;
 	const char *csdev_name;
+	struct cti_drvdata *drvdata = container_of(ctidev, struct cti_drvdata,
+						   ctidev);
 
 	list_for_each_entry(tc, &ctidev->trig_cons, node) {
 		if (tc->con_dev_name) {
@@ -462,6 +495,7 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
 					devm_kstrdup(&csdev->dev, csdev_name,
 						     GFP_KERNEL);
 				tc->con_dev = csdev;
+				cti_add_sysfs_link(drvdata, tc);
 				return true;
 			}
 		}
@@ -546,10 +580,12 @@ static void cti_update_conn_xrefs(struct cti_drvdata *drvdata)
 	struct cti_device *ctidev = &drvdata->ctidev;
 
 	list_for_each_entry(tc, &ctidev->trig_cons, node) {
-		if (tc->con_dev)
+		if (tc->con_dev) {
 			/* set tc->con_dev->ect_dev */
 			coresight_set_assoc_ectdev_mutex(tc->con_dev,
 							 drvdata->csdev);
+			cti_add_sysfs_link(drvdata, tc);
+		}
 	}
 }
 
@@ -602,6 +638,9 @@ static void cti_device_release(struct device *dev)
 	mutex_lock(&ect_mutex);
 	cti_remove_conn_xrefs(drvdata);
 
+	/* clear the dynamic sysfs associate with connections */
+	cti_remove_all_sysfs_links(drvdata);
+
 	/* remove from the list */
 	list_for_each_entry_safe(ect_item, ect_tmp, &ect_net, node) {
 		if (ect_item == drvdata) {
-- 
2.17.1

Re: [PATCH v4 5/6] coresight: cti: Add in sysfs links to other coresight devices.

[Mathieu Poirier]

On Tue, Feb 11, 2020 at 10:58:07AM +0000, Mike Leach wrote:
> Adds in sysfs links for connections where the connected device is another
> coresight device. This allows examination of the coresight topology.
> 
> Non-coresight connections remain just as a reference name.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-cti.c | 41 ++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
> index 9e18e176831c..f620e9460e7d 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.c
> +++ b/drivers/hwtracing/coresight/coresight-cti.c
> @@ -441,6 +441,37 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
>  	return err;
>  }
>  
> +static void cti_add_sysfs_link(struct cti_drvdata *drvdata,
> +			       struct cti_trig_con *tc)
> +{
> +	struct coresight_sysfs_link link_info;
> +
> +	link_info.orig = drvdata->csdev;
> +	link_info.orig_name = tc->con_dev_name;
> +	link_info.target = tc->con_dev;
> +	link_info.target_name = dev_name(&drvdata->csdev->dev);
> +	coresight_add_sysfs_link(&link_info);

I understand there isn't much to do if a problem occurs so just catch the error
and add a comment to assert you're doing this on purpose.

> +}
> +
> +static void cti_remove_all_sysfs_links(struct cti_drvdata *drvdata)
> +{
> +	struct cti_trig_con *tc;
> +	struct cti_device *ctidev = &drvdata->ctidev;
> +	struct coresight_sysfs_link link_info;
> +
> +	/* origin device and target link name constant for this cti */
> +	link_info.orig = drvdata->csdev;
> +	link_info.target_name = dev_name(&drvdata->csdev->dev);
> +
> +	list_for_each_entry(tc, &ctidev->trig_cons, node) {
> +		if (tc->con_dev) {
> +			link_info.target = tc->con_dev;
> +			link_info.orig_name = tc->con_dev_name;
> +			coresight_remove_sysfs_link(&link_info);
> +		}
> +	}
> +}
> +
>  /*
>   * Look for a matching connection device name in the list of connections.
>   * If found then swap in the csdev name, set trig con association pointer
> @@ -452,6 +483,8 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
>  {
>  	struct cti_trig_con *tc;
>  	const char *csdev_name;
> +	struct cti_drvdata *drvdata = container_of(ctidev, struct cti_drvdata,
> +						   ctidev);
>  
>  	list_for_each_entry(tc, &ctidev->trig_cons, node) {
>  		if (tc->con_dev_name) {
> @@ -462,6 +495,7 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
>  					devm_kstrdup(&csdev->dev, csdev_name,
>  						     GFP_KERNEL);
>  				tc->con_dev = csdev;
> +				cti_add_sysfs_link(drvdata, tc);
>  				return true;
>  			}
>  		}
> @@ -546,10 +580,12 @@ static void cti_update_conn_xrefs(struct cti_drvdata *drvdata)
>  	struct cti_device *ctidev = &drvdata->ctidev;
>  
>  	list_for_each_entry(tc, &ctidev->trig_cons, node) {
> -		if (tc->con_dev)
> +		if (tc->con_dev) {
>  			/* set tc->con_dev->ect_dev */
>  			coresight_set_assoc_ectdev_mutex(tc->con_dev,
>  							 drvdata->csdev);
> +			cti_add_sysfs_link(drvdata, tc);
> +		}
>  	}
>  }
>  
> @@ -602,6 +638,9 @@ static void cti_device_release(struct device *dev)
>  	mutex_lock(&ect_mutex);
>  	cti_remove_conn_xrefs(drvdata);
>  
> +	/* clear the dynamic sysfs associate with connections */

s/associate/associated

> +	cti_remove_all_sysfs_links(drvdata);
> +
>  	/* remove from the list */
>  	list_for_each_entry_safe(ect_item, ect_tmp, &ect_net, node) {
>  		if (ect_item == drvdata) {

With the above:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> -- 
> 2.17.1
> 

Re: [PATCH v4 5/6] coresight: cti: Add in sysfs links to other coresight devices.

[Mike Leach]

Hi Mathieu,

On Fri, 14 Feb 2020 at 22:58, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> On Tue, Feb 11, 2020 at 10:58:07AM +0000, Mike Leach wrote:
> > Adds in sysfs links for connections where the connected device is another
> > coresight device. This allows examination of the coresight topology.
> >
> > Non-coresight connections remain just as a reference name.
> >
> > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > ---
> >  drivers/hwtracing/coresight/coresight-cti.c | 41 ++++++++++++++++++++-
> >  1 file changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
> > index 9e18e176831c..f620e9460e7d 100644
> > --- a/drivers/hwtracing/coresight/coresight-cti.c
> > +++ b/drivers/hwtracing/coresight/coresight-cti.c
> > @@ -441,6 +441,37 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
> >       return err;
> >  }
> >
> > +static void cti_add_sysfs_link(struct cti_drvdata *drvdata,
> > +                            struct cti_trig_con *tc)
> > +{
> > +     struct coresight_sysfs_link link_info;
> > +
> > +     link_info.orig = drvdata->csdev;
> > +     link_info.orig_name = tc->con_dev_name;
> > +     link_info.target = tc->con_dev;
> > +     link_info.target_name = dev_name(&drvdata->csdev->dev);
> > +     coresight_add_sysfs_link(&link_info);
>
> I understand there isn't much to do if a problem occurs so just catch the error
> and add a comment to assert you're doing this on purpose.
>

When I revisited this code I saw an imbalance between the case where
the CTI is created first and the associated CSdev is created first.
The result could be shutdown path where the CTI removes sysfs links
after the csdev has been removed - which really shouldn't happen.
Also we could try to remove a sysfs link that we failed to set in the
first place - again not ideal

I've reworked this code to fix this, and now if the sysfs link fails
to be set, then we do not set the association between CTI and csdev
components.
This is not sufficient to fail either component from registering, as
we may have successfully registered previous associations with the
same CTI.

It seems unlikely that the sysfs link could fail, but if it does, is
it worth using a dev_warn() to at least log the failure?

Regards

Mike


> > +}
> > +
> > +static void cti_remove_all_sysfs_links(struct cti_drvdata *drvdata)
> > +{
> > +     struct cti_trig_con *tc;
> > +     struct cti_device *ctidev = &drvdata->ctidev;
> > +     struct coresight_sysfs_link link_info;
> > +
> > +     /* origin device and target link name constant for this cti */
> > +     link_info.orig = drvdata->csdev;
> > +     link_info.target_name = dev_name(&drvdata->csdev->dev);
> > +
> > +     list_for_each_entry(tc, &ctidev->trig_cons, node) {
> > +             if (tc->con_dev) {
> > +                     link_info.target = tc->con_dev;
> > +                     link_info.orig_name = tc->con_dev_name;
> > +                     coresight_remove_sysfs_link(&link_info);
> > +             }
> > +     }
> > +}
> > +
> >  /*
> >   * Look for a matching connection device name in the list of connections.
> >   * If found then swap in the csdev name, set trig con association pointer
> > @@ -452,6 +483,8 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
> >  {
> >       struct cti_trig_con *tc;
> >       const char *csdev_name;
> > +     struct cti_drvdata *drvdata = container_of(ctidev, struct cti_drvdata,
> > +                                                ctidev);
> >
> >       list_for_each_entry(tc, &ctidev->trig_cons, node) {
> >               if (tc->con_dev_name) {
> > @@ -462,6 +495,7 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
> >                                       devm_kstrdup(&csdev->dev, csdev_name,
> >                                                    GFP_KERNEL);
> >                               tc->con_dev = csdev;
> > +                             cti_add_sysfs_link(drvdata, tc);
> >                               return true;
> >                       }
> >               }
> > @@ -546,10 +580,12 @@ static void cti_update_conn_xrefs(struct cti_drvdata *drvdata)
> >       struct cti_device *ctidev = &drvdata->ctidev;
> >
> >       list_for_each_entry(tc, &ctidev->trig_cons, node) {
> > -             if (tc->con_dev)
> > +             if (tc->con_dev) {
> >                       /* set tc->con_dev->ect_dev */
> >                       coresight_set_assoc_ectdev_mutex(tc->con_dev,
> >                                                        drvdata->csdev);
> > +                     cti_add_sysfs_link(drvdata, tc);
> > +             }
> >       }
> >  }
> >
> > @@ -602,6 +638,9 @@ static void cti_device_release(struct device *dev)
> >       mutex_lock(&ect_mutex);
> >       cti_remove_conn_xrefs(drvdata);
> >
> > +     /* clear the dynamic sysfs associate with connections */
>
> s/associate/associated
>
> > +     cti_remove_all_sysfs_links(drvdata);
> > +
> >       /* remove from the list */
> >       list_for_each_entry_safe(ect_item, ect_tmp, &ect_net, node) {
> >               if (ect_item == drvdata) {
>
> With the above:
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
>
> > --
> > 2.17.1
> >



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

Re: [PATCH v4 5/6] coresight: cti: Add in sysfs links to other coresight devices.

[Mathieu Poirier]

On Wed, Feb 26, 2020 at 01:37:17PM +0000, Mike Leach wrote:
> Hi Mathieu,
> 
> On Fri, 14 Feb 2020 at 22:58, Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
> >
> > On Tue, Feb 11, 2020 at 10:58:07AM +0000, Mike Leach wrote:
> > > Adds in sysfs links for connections where the connected device is another
> > > coresight device. This allows examination of the coresight topology.
> > >
> > > Non-coresight connections remain just as a reference name.
> > >
> > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > > ---
> > >  drivers/hwtracing/coresight/coresight-cti.c | 41 ++++++++++++++++++++-
> > >  1 file changed, 40 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
> > > index 9e18e176831c..f620e9460e7d 100644
> > > --- a/drivers/hwtracing/coresight/coresight-cti.c
> > > +++ b/drivers/hwtracing/coresight/coresight-cti.c
> > > @@ -441,6 +441,37 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
> > >       return err;
> > >  }
> > >
> > > +static void cti_add_sysfs_link(struct cti_drvdata *drvdata,
> > > +                            struct cti_trig_con *tc)
> > > +{
> > > +     struct coresight_sysfs_link link_info;
> > > +
> > > +     link_info.orig = drvdata->csdev;
> > > +     link_info.orig_name = tc->con_dev_name;
> > > +     link_info.target = tc->con_dev;
> > > +     link_info.target_name = dev_name(&drvdata->csdev->dev);
> > > +     coresight_add_sysfs_link(&link_info);
> >
> > I understand there isn't much to do if a problem occurs so just catch the error
> > and add a comment to assert you're doing this on purpose.
> >
> 
> When I revisited this code I saw an imbalance between the case where
> the CTI is created first and the associated CSdev is created first.
> The result could be shutdown path where the CTI removes sysfs links
> after the csdev has been removed - which really shouldn't happen.
> Also we could try to remove a sysfs link that we failed to set in the
> first place - again not ideal
> 
> I've reworked this code to fix this, and now if the sysfs link fails
> to be set, then we do not set the association between CTI and csdev
> components.

Ok

> This is not sufficient to fail either component from registering, as
> we may have successfully registered previous associations with the
> same CTI.
>

That is also my opinion.
 
> It seems unlikely that the sysfs link could fail, but if it does, is
> it worth using a dev_warn() to at least log the failure?
>

Yes, that would surely be helpful. 
 
> Regards
> 
> Mike
> 
> 
> > > +}
> > > +
> > > +static void cti_remove_all_sysfs_links(struct cti_drvdata *drvdata)
> > > +{
> > > +     struct cti_trig_con *tc;
> > > +     struct cti_device *ctidev = &drvdata->ctidev;
> > > +     struct coresight_sysfs_link link_info;
> > > +
> > > +     /* origin device and target link name constant for this cti */
> > > +     link_info.orig = drvdata->csdev;
> > > +     link_info.target_name = dev_name(&drvdata->csdev->dev);
> > > +
> > > +     list_for_each_entry(tc, &ctidev->trig_cons, node) {
> > > +             if (tc->con_dev) {
> > > +                     link_info.target = tc->con_dev;
> > > +                     link_info.orig_name = tc->con_dev_name;
> > > +                     coresight_remove_sysfs_link(&link_info);
> > > +             }
> > > +     }
> > > +}
> > > +
> > >  /*
> > >   * Look for a matching connection device name in the list of connections.
> > >   * If found then swap in the csdev name, set trig con association pointer
> > > @@ -452,6 +483,8 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
> > >  {
> > >       struct cti_trig_con *tc;
> > >       const char *csdev_name;
> > > +     struct cti_drvdata *drvdata = container_of(ctidev, struct cti_drvdata,
> > > +                                                ctidev);
> > >
> > >       list_for_each_entry(tc, &ctidev->trig_cons, node) {
> > >               if (tc->con_dev_name) {
> > > @@ -462,6 +495,7 @@ cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
> > >                                       devm_kstrdup(&csdev->dev, csdev_name,
> > >                                                    GFP_KERNEL);
> > >                               tc->con_dev = csdev;
> > > +                             cti_add_sysfs_link(drvdata, tc);
> > >                               return true;
> > >                       }
> > >               }
> > > @@ -546,10 +580,12 @@ static void cti_update_conn_xrefs(struct cti_drvdata *drvdata)
> > >       struct cti_device *ctidev = &drvdata->ctidev;
> > >
> > >       list_for_each_entry(tc, &ctidev->trig_cons, node) {
> > > -             if (tc->con_dev)
> > > +             if (tc->con_dev) {
> > >                       /* set tc->con_dev->ect_dev */
> > >                       coresight_set_assoc_ectdev_mutex(tc->con_dev,
> > >                                                        drvdata->csdev);
> > > +                     cti_add_sysfs_link(drvdata, tc);
> > > +             }
> > >       }
> > >  }
> > >
> > > @@ -602,6 +638,9 @@ static void cti_device_release(struct device *dev)
> > >       mutex_lock(&ect_mutex);
> > >       cti_remove_conn_xrefs(drvdata);
> > >
> > > +     /* clear the dynamic sysfs associate with connections */
> >
> > s/associate/associated
> >
> > > +     cti_remove_all_sysfs_links(drvdata);
> > > +
> > >       /* remove from the list */
> > >       list_for_each_entry_safe(ect_item, ect_tmp, &ect_net, node) {
> > >               if (ect_item == drvdata) {
> >
> > With the above:
> >
> > Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >
> > > --
> > > 2.17.1
> > >
> 
> 
> 
> -- 
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK

[PATCH v4 6/6] coresight: docs: Add information about the topology representations.

[Mike Leach]

Update the CoreSight documents to describe the new connections directory
and the links between CoreSight devices in this directory.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 .../trace/coresight/coresight-ect.rst         |  5 +-
 Documentation/trace/coresight/coresight.rst   | 85 +++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/coresight/coresight-ect.rst b/Documentation/trace/coresight/coresight-ect.rst
index 067cee91c857..3539af895832 100644
--- a/Documentation/trace/coresight/coresight-ect.rst
+++ b/Documentation/trace/coresight/coresight-ect.rst
@@ -71,7 +71,7 @@ capable of generating or using trigger signals.::
 
   >$ ls /sys/bus/coresight/devices/etm0/cti_cpu0
   channels  ctmid  enable  nr_trigger_cons mgmt  power  regs  subsystem
-  triggers0 triggers1  uevent
+  connections triggers0 triggers1  uevent
 
 *Key file items are:-*
    * ``enable``: enables/disables the CTI.
@@ -84,6 +84,9 @@ capable of generating or using trigger signals.::
    * ``channels``: Contains the channel API - CTI main programming interface.
    * ``regs``: Gives access to the raw programmable CTI regs.
    * ``mgmt``: the standard CoreSight management registers.
+   * ``connections``: Links to connected *CoreSight* devices. The number of
+     links can be 0 to ``nr_trigger_cons``. Actual number given by ``nr_links``
+     in this directory.
 
 
 triggers<N> directories
diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst
index 108600ee1e12..0b73acb44efa 100644
--- a/Documentation/trace/coresight/coresight.rst
+++ b/Documentation/trace/coresight/coresight.rst
@@ -241,6 +241,91 @@ to the newer scheme, to give a confirmation that what you see on your
 system is not unexpected. One must use the "names" as they appear on
 the system under specified locations.
 
+Topology Representation
+-----------------------
+
+Each CoreSight component has a ``connections`` directory which will contain
+links to other CoreSight components. This allows the user to explore the trace
+topology and for larger systems, determine the most appropriate sink for a
+given source. The connection information can also be used to establish
+which CTI devices are connected to a given component. This directory contains a
+``nr_links`` attribute detailing the number of links in the directory.
+
+For an ETM source, in this case ``etm0`` on a Juno platform, a typical
+arrangement will be::
+
+  linaro-developer:~# ls - l /sys/bus/coresight/devices/etm0/connections
+  <file details>  cti_cpu0 -> ../../../23020000.cti/cti_cpu0
+  <file details>  nr_links
+  <file details>  out:0 -> ../../../230c0000.funnel/funnel2
+
+Following the out port to ``funnel2``::
+
+  linaro-developer:~# ls -l /sys/bus/coresight/devices/funnel2/connections
+  <file details> in:0 -> ../../../23040000.etm/etm0
+  <file details> in:1 -> ../../../23140000.etm/etm3
+  <file details> in:2 -> ../../../23240000.etm/etm4
+  <file details> in:3 -> ../../../23340000.etm/etm5
+  <file details> nr_links
+  <file details> out:0 -> ../../../20040000.funnel/funnel0
+
+And again to ``funnel0``::
+
+  linaro-developer:~# ls -l /sys/bus/coresight/devices/funnel0/connections
+  <file details> in:0 -> ../../../220c0000.funnel/funnel1
+  <file details> in:1 -> ../../../230c0000.funnel/funnel2
+  <file details> nr_links
+  <file details> out:0 -> ../../../20010000.etf/tmc_etf0
+
+Finding the first sink ``tmc_etf0``. This can be used to collect data
+as a sink, or as a link to propagate further along the chain::
+
+  linaro-developer:~# ls -l /sys/bus/coresight/devices/tmc_etf0/connections
+  <file details> cti_sys0 -> ../../../20020000.cti/cti_sys0
+  <file details> in:0 -> ../../../20040000.funnel/funnel0
+  <file details> nr_links
+  <file details> out:0 -> ../../../20150000.funnel/funnel4
+
+via ``funnel4``::
+
+  linaro-developer:~# ls -l /sys/bus/coresight/devices/funnel4/connections
+  <file details> in:0 -> ../../../20010000.etf/tmc_etf0
+  <file details> in:1 -> ../../../20140000.etf/tmc_etf1
+  <file details> nr_links
+  <file details> out:0 -> ../../../20120000.replicator/replicator0
+
+and a ``replicator0``::
+
+  linaro-developer:~# ls -l /sys/bus/coresight/devices/replicator0/connections
+  <file details> in:0 -> ../../../20150000.funnel/funnel4
+  <file details> nr_links
+  <file details> out:0 -> ../../../20030000.tpiu/tpiu0
+  <file details> out:1 -> ../../../20070000.etr/tmc_etr0
+
+Arriving at the final sink in the chain, ``tmc_etr0``::
+
+  linaro-developer:~# ls -l /sys/bus/coresight/devices/tmc_etr0/connections
+  <file details> cti_sys0 -> ../../../20020000.cti/cti_sys0
+  <file details> in:0 -> ../../../20120000.replicator/replicator0
+  <file details> nr_links
+
+As described below, when using sysfs it is sufficient to enable a sink and
+a source for successful trace. The framework will correctly enable all
+intermediate links as required.
+
+Note: ``cti_sys0`` appears in two of the connections lists above.
+CTIs can connect to multiple devices and are arranged in a star topology
+via the CTM. See (:doc:`coresight-ect`) [#fourth]_ for further details.
+Looking at this device we see 4 connections::
+
+  linaro-developer:~# ls -l /sys/bus/coresight/devices/cti_sys0/connections
+  <file details> nr_links
+  <file details> stm0 -> ../../../20100000.stm/stm0
+  <file details> tmc_etf0 -> ../../../20010000.etf/tmc_etf0
+  <file details> tmc_etr0 -> ../../../20070000.etr/tmc_etr0
+  <file details> tpiu0 -> ../../../20030000.tpiu/tpiu0
+
+
 How to use the tracer modules
 -----------------------------
 
-- 
2.17.1

Re: [PATCH v4 6/6] coresight: docs: Add information about the topology representations.

[Mathieu Poirier]

On Tue, Feb 11, 2020 at 10:58:08AM +0000, Mike Leach wrote:
> Update the CoreSight documents to describe the new connections directory
> and the links between CoreSight devices in this directory.
> 
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  .../trace/coresight/coresight-ect.rst         |  5 +-
>  Documentation/trace/coresight/coresight.rst   | 85 +++++++++++++++++++
>  2 files changed, 89 insertions(+), 1 deletion(-)
> 

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> diff --git a/Documentation/trace/coresight/coresight-ect.rst b/Documentation/trace/coresight/coresight-ect.rst
> index 067cee91c857..3539af895832 100644
> --- a/Documentation/trace/coresight/coresight-ect.rst
> +++ b/Documentation/trace/coresight/coresight-ect.rst
> @@ -71,7 +71,7 @@ capable of generating or using trigger signals.::
>  
>    >$ ls /sys/bus/coresight/devices/etm0/cti_cpu0
>    channels  ctmid  enable  nr_trigger_cons mgmt  power  regs  subsystem
> -  triggers0 triggers1  uevent
> +  connections triggers0 triggers1  uevent
>  
>  *Key file items are:-*
>     * ``enable``: enables/disables the CTI.
> @@ -84,6 +84,9 @@ capable of generating or using trigger signals.::
>     * ``channels``: Contains the channel API - CTI main programming interface.
>     * ``regs``: Gives access to the raw programmable CTI regs.
>     * ``mgmt``: the standard CoreSight management registers.
> +   * ``connections``: Links to connected *CoreSight* devices. The number of
> +     links can be 0 to ``nr_trigger_cons``. Actual number given by ``nr_links``
> +     in this directory.
>  
>  
>  triggers<N> directories
> diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst
> index 108600ee1e12..0b73acb44efa 100644
> --- a/Documentation/trace/coresight/coresight.rst
> +++ b/Documentation/trace/coresight/coresight.rst
> @@ -241,6 +241,91 @@ to the newer scheme, to give a confirmation that what you see on your
>  system is not unexpected. One must use the "names" as they appear on
>  the system under specified locations.
>  
> +Topology Representation
> +-----------------------
> +
> +Each CoreSight component has a ``connections`` directory which will contain
> +links to other CoreSight components. This allows the user to explore the trace
> +topology and for larger systems, determine the most appropriate sink for a
> +given source. The connection information can also be used to establish
> +which CTI devices are connected to a given component. This directory contains a
> +``nr_links`` attribute detailing the number of links in the directory.
> +
> +For an ETM source, in this case ``etm0`` on a Juno platform, a typical
> +arrangement will be::
> +
> +  linaro-developer:~# ls - l /sys/bus/coresight/devices/etm0/connections
> +  <file details>  cti_cpu0 -> ../../../23020000.cti/cti_cpu0
> +  <file details>  nr_links
> +  <file details>  out:0 -> ../../../230c0000.funnel/funnel2
> +
> +Following the out port to ``funnel2``::
> +
> +  linaro-developer:~# ls -l /sys/bus/coresight/devices/funnel2/connections
> +  <file details> in:0 -> ../../../23040000.etm/etm0
> +  <file details> in:1 -> ../../../23140000.etm/etm3
> +  <file details> in:2 -> ../../../23240000.etm/etm4
> +  <file details> in:3 -> ../../../23340000.etm/etm5
> +  <file details> nr_links
> +  <file details> out:0 -> ../../../20040000.funnel/funnel0
> +
> +And again to ``funnel0``::
> +
> +  linaro-developer:~# ls -l /sys/bus/coresight/devices/funnel0/connections
> +  <file details> in:0 -> ../../../220c0000.funnel/funnel1
> +  <file details> in:1 -> ../../../230c0000.funnel/funnel2
> +  <file details> nr_links
> +  <file details> out:0 -> ../../../20010000.etf/tmc_etf0
> +
> +Finding the first sink ``tmc_etf0``. This can be used to collect data
> +as a sink, or as a link to propagate further along the chain::
> +
> +  linaro-developer:~# ls -l /sys/bus/coresight/devices/tmc_etf0/connections
> +  <file details> cti_sys0 -> ../../../20020000.cti/cti_sys0
> +  <file details> in:0 -> ../../../20040000.funnel/funnel0
> +  <file details> nr_links
> +  <file details> out:0 -> ../../../20150000.funnel/funnel4
> +
> +via ``funnel4``::
> +
> +  linaro-developer:~# ls -l /sys/bus/coresight/devices/funnel4/connections
> +  <file details> in:0 -> ../../../20010000.etf/tmc_etf0
> +  <file details> in:1 -> ../../../20140000.etf/tmc_etf1
> +  <file details> nr_links
> +  <file details> out:0 -> ../../../20120000.replicator/replicator0
> +
> +and a ``replicator0``::
> +
> +  linaro-developer:~# ls -l /sys/bus/coresight/devices/replicator0/connections
> +  <file details> in:0 -> ../../../20150000.funnel/funnel4
> +  <file details> nr_links
> +  <file details> out:0 -> ../../../20030000.tpiu/tpiu0
> +  <file details> out:1 -> ../../../20070000.etr/tmc_etr0
> +
> +Arriving at the final sink in the chain, ``tmc_etr0``::
> +
> +  linaro-developer:~# ls -l /sys/bus/coresight/devices/tmc_etr0/connections
> +  <file details> cti_sys0 -> ../../../20020000.cti/cti_sys0
> +  <file details> in:0 -> ../../../20120000.replicator/replicator0
> +  <file details> nr_links
> +
> +As described below, when using sysfs it is sufficient to enable a sink and
> +a source for successful trace. The framework will correctly enable all
> +intermediate links as required.
> +
> +Note: ``cti_sys0`` appears in two of the connections lists above.
> +CTIs can connect to multiple devices and are arranged in a star topology
> +via the CTM. See (:doc:`coresight-ect`) [#fourth]_ for further details.
> +Looking at this device we see 4 connections::
> +
> +  linaro-developer:~# ls -l /sys/bus/coresight/devices/cti_sys0/connections
> +  <file details> nr_links
> +  <file details> stm0 -> ../../../20100000.stm/stm0
> +  <file details> tmc_etf0 -> ../../../20010000.etf/tmc_etf0
> +  <file details> tmc_etr0 -> ../../../20070000.etr/tmc_etr0
> +  <file details> tpiu0 -> ../../../20030000.tpiu/tpiu0
> +
> +
>  How to use the tracer modules
>  -----------------------------
>  
> -- 
> 2.17.1
>