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

Hi Mike,

On Wed, Nov 27, 2019 at 10:36:21PM +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 | 44 +++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index c3d63cc53bdd..b6422ae7c233 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -440,6 +440,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 and return

@@ -447,7 +478,7 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op, */ static bool cti_match_con_name(struct cti_device *ctidev, const char *node_name,

•    const char *csdev_name)
  

•    const char *csdev_name, struct cti_trig_con **tc)
  

{ struct cti_trig_con *trig_con; @@ -458,6 +489,7 @@ cti_match_con_name(struct cti_device *ctidev, const char *node_name, kfree(trig_con->con_dev_name); trig_con->con_dev_name = kstrdup(csdev_name, GFP_KERNEL);

• 		*tc = trig_con;
  	return true;
  }
  

  }

@@ -474,6 +506,7 @@ void cti_add_assoc_to_csdev(struct coresight_device *csdev) struct cti_drvdata *ect_item; struct cti_device *ctidev; const char *node_name = NULL, *csdev_name;

• struct cti_trig_con *tc = NULL;

/* protect the list */ mutex_lock(&ect_mutex); @@ -494,12 +527,14 @@ void cti_add_assoc_to_csdev(struct coresight_device *csdev) /* for each CTI in list... */ list_for_each_entry(ect_item, &ect_net, node) { ctidev = &ect_item->ctidev;

• if (cti_match_con_name(ctidev, node_name, csdev_name)) {
  

• if (cti_match_con_name(ctidev, node_name, csdev_name, &tc)) {
  /*
   * if we found a matching name then update the
   * association pointers.
   */
  csdev->ect_dev = ect_item->csdev;
  

• 	tc->con_dev = csdev;
  

I think the "tc->con_dev = csdev;" should have been done in patch 8 [1] of your previous set. In [1] I suggest to introduce function cti_match_con_name() with the @tc parameter. When a match is found just do "*tc = trig_con;" and do the kfree() and kstrdup() right here so that all manipulation on tc->xyz are done at the same place. Then this hunk will simply be concerned with adding the sysfs link.

Let me know what you think, Mathieu

[1]. "coresight: cti: Enable CTI associated with devices."

• 	cti_add_sysfs_link(ect_item, tc);
  goto cti_add_done;
  

  } }

@@ -519,8 +554,10 @@ 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) {
  tc->con_dev->ect_dev = drvdata->csdev;
  

• 	cti_add_sysfs_link(drvdata, tc);
  

• }
  

  }

} @@ -562,6 +599,7 @@ static void cti_device_release(struct device *dev) mutex_lock(&ect_mutex); /* clear the dynamic sysfs associate with connections */

• cti_remove_all_sysfs_links(drvdata); cti_destroy_cons_sysfs(&drvdata->ctidev);

/* remove from the list */ -- 2.20.1