Re: [PATCH] coresight: cti: Fix remove sysfs link error.

Hi Mathieu,

On Tue, 21 Jul 2020 at 18:51, Mathieu Poirier mathieu.poirier@linaro.org wrote:

Hi Mike,

On Tue, Jul 21, 2020 at 12:17:36PM +0100, Mike Leach wrote:

...

CTI code to remove sysfs link to other devices on shutdown, incorrectly tries to remove a single ended link when these are all double ended. This implementation leaves elements in the link info structure undefined which results in a crash in recent tests for driver module unload.

This patch corrects the link removal code.

Fixes: 73274abb6557 (coresight: cti: Add in sysfs links to other coresight devices)

Please remove the extra new line, add a "Reported-by:" for Tingwei and remove the '.' at the end of the title. Up to now I've been removing those manually but it causes problems when Greg has to pickup a set directly from the mailing list.

Will do.

Also, I just noticed the other problem you shared with Tingwei regarding the call to cti_remove_conn_xrefs() in cti_remove(). I think it will be simpler if you send a 2 patch serie with those fixes.

The issue here is that this patch is completely independent of Tingwei's coresight module set, and applies immediately to coresight\next.

The additional change I suggested to completely fix the problem is entirely dependent on changes (i.e. the introduction of cti_remove()) made by Tingwei and thus dependent on that set.

Thus I cannot issue a 2 patch set without Tingwei re-working the module set, and the resulting set would be dependent on that set, leaving the coresight module set broken until a second set is applied.

My expectation was that Tingwei would include the cti_remove() suggested change as part of his re-work while rebasing on coresight/next, leaving this patch to be applied before / afterwards.

Alternatively, this patch could simply become another patch in Tingwei's coresight module series. (the issue it fixes doesn't appear to have adverse effects when coresight drivers are built in).

Regards

Mike

Thanks, Mathieu

...

Signed-off-by: Mike Leach mike.leach@linaro.org

drivers/hwtracing/coresight/coresight-cti.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 3ccc703dc940..19d0b031a7df 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -511,12 +511,15 @@ static bool cti_add_sysfs_link(struct cti_drvdata *drvdata, return !link_err; }

-static void cti_remove_sysfs_link(struct cti_trig_con *tc) +static void cti_remove_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_remove_sysfs_link(&link_info);
  

}

@@ -607,7 +610,7 @@ void cti_remove_assoc_from_csdev(struct coresight_device *csdev) ctidev = &ctidrv->ctidev; list_for_each_entry(tc, &ctidev->trig_cons, node) { if (tc->con_dev == csdev->ect_dev) {

•                         cti_remove_sysfs_link(tc);
  

•                         cti_remove_sysfs_link(ctidrv, tc);
                          tc->con_dev = NULL;
                          break;
                  }
  

@@ -651,7 +654,7 @@ static void cti_remove_conn_xrefs(struct cti_drvdata *drvdata) if (tc->con_dev) { coresight_set_assoc_ectdev_mutex(tc->con_dev, NULL);

•                 cti_remove_sysfs_link(tc);
  

•                 cti_remove_sysfs_link(drvdata, tc);
                  tc->con_dev = NULL;
          }
  }
  

-- 2.17.1