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

Hi Mathieu,

On Tue, 3 Dec 2019 at 18:55, Mathieu Poirier mathieu.poirier@linaro.org wrote:

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

I think you have found a bug in [1] - this should already be happening - evidently got missed when I split out the sysfs links work from the main CTI set. Will redress in the main CTI set and update this accordingly.

Thanks

Mike

[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