As using devm_... for allocation, there is no need to explicitly free
up tc->con_dev_name, also the lifetime of the connection is linked to
the lifetime of csdev, so we can drop the devm_kstrdup in the csdev
case so this becomes

/* match: so swap in csdev name & dev */
               tc->con_dev_name = dev_name(&csdev->dev);
                tc->con_dev = csdev;
                return true;

Same true for similar link in patch 1, removing 2 un-needed
allocations, leaving 1 to be fixed up with error checking

Mike


On Fri, 21 Feb 2020 at 16:51, Mathieu Poirier
[off-list ref] wrote:

On Fri, Feb 21, 2020 at 12:20:17AM +0000, Suzuki K Poulose wrote:

quoted

Hi Mike

Sorry for the delay. one minor comment below.

On 02/10/2020 09:39 PM, Mike Leach wrote:

quoted

The CoreSight subsystem enables a path of devices from source to sink.
Any CTI devices associated with the path devices must be enabled at the
same time.

This patch adds an associated coresight_device element to the main
coresight device structure, and uses this to create associations between
the CTI and other devices based on the device tree data. The associated
device element is used to enable CTI in conjunction with the path elements.

CTI devices are reference counted so where a single CTI is associated with
multiple elements on the path, it will be enabled on the first associated
device enable, and disabled with the last associated device disable.

Signed-off-by: Mike Leach <redacted>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
  drivers/hwtracing/coresight/coresight-cti.c  | 129 +++++++++++++++++++
  drivers/hwtracing/coresight/coresight-cti.h  |   1 +
  drivers/hwtracing/coresight/coresight-priv.h |  12 ++
  drivers/hwtracing/coresight/coresight.c      |  71 +++++++++-
  include/linux/coresight.h                    |   4 +
  5 files changed, 212 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c
index 77c2af247917..c4494923d030 100644
--- a/drivers/hwtracing/coresight/coresight-cti.c
+++ b/drivers/hwtracing/coresight/coresight-cti.c

@@ -4,6 +4,7 @@
   * Author: Mike Leach <mike.leach@linaro.org>
   */
+#include <linux/property.h>
  #include "coresight-cti.h"
  /**

@@ -440,6 +441,131 @@ int cti_channel_setop(struct device *dev, enum cti_chan_set_op op,
    return err;
  }
+/*
+ * 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
+ * and return found.
+ */
+static bool
+cti_match_fixup_csdev(struct cti_device *ctidev, const char *node_name,
+                 struct coresight_device *csdev)
+{
+   struct cti_trig_con *tc;
+   const char *csdev_name;
+
+   list_for_each_entry(tc, &ctidev->trig_cons, node) {
+           if (tc->con_dev_name) {
+                   if (!strcmp(node_name, tc->con_dev_name)) {
+                           /* match: so swap in csdev name & dev */
+                           csdev_name = dev_name(&csdev->dev);
+                           tc->con_dev_name =
+                                   devm_kstrdup(&csdev->dev, csdev_name,
+                                                GFP_KERNEL);

In the extreme rare case of an allocation failure, we may want to
check if the allocation was successful or not, rather than silently
ignoring it. With that fixed,

Line 419 and 423 in patch 1 need the same attention.

quoted

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel