[bug report] coresight: Dynamically add connections

[James Clark]

On 02/04/2025 12:00 pm, Dan Carpenter wrote:
> Hello James Clark,
> 
> Commit 3d4ff657e454 ("coresight: Dynamically add connections") from
> Apr 25, 2023 (linux-next), leads to the following Smatch static
> checker warning:
> 
> 	drivers/hwtracing/coresight/coresight-platform.c:279 of_coresight_parse_endpoint()
> 	warn: inconsistent refcounting 'rep->kobj.kref.refcount.refs.counter':
> 
> drivers/hwtracing/coresight/coresight-platform.c
>      196 static int of_coresight_parse_endpoint(struct device *dev,
>      197                                        struct device_node *ep,
>      198                                        struct coresight_platform_data *pdata)
>      199 {
>      200         int ret = 0;
>      201         struct of_endpoint endpoint, rendpoint;
>      202         struct device_node *rparent = NULL;
>      203         struct device_node *rep = NULL;
>      204         struct device *rdev = NULL;
>      205         struct fwnode_handle *rdev_fwnode;
>      206         struct coresight_connection conn = {};
>      207         struct coresight_connection *new_conn;
>      208
>      209         do {
>      210                 /* Parse the local port details */
>      211                 if (of_graph_parse_endpoint(ep, &endpoint))
>      212                         break;
>      213                 /*
>      214                  * Get a handle on the remote endpoint and the device it is
>      215                  * attached to.
>      216                  */
>      217                 rep = of_graph_get_remote_endpoint(ep);
>      218                 if (!rep)
>      219                         break;
>      220                 rparent = of_coresight_get_port_parent(rep);
>      221                 if (!rparent)
>      222                         break;
>      223                 if (of_graph_parse_endpoint(rep, &rendpoint))
>      224                         break;
>      225
>      226                 rdev_fwnode = of_fwnode_handle(rparent);
>      227                 /* If the remote device is not available, defer probing */
>      228                 rdev = coresight_find_device_by_fwnode(rdev_fwnode);
>      229                 if (!rdev) {
>      230                         ret = -EPROBE_DEFER;
>      231                         break;
>      232                 }
>      233
>      234                 conn.src_port = endpoint.port;
>      235                 /*
>      236                  * Hold the refcount to the target device. This could be
>      237                  * released via:
>      238                  * 1) coresight_release_platform_data() if the probe fails or
>      239                  *    this device is unregistered.
>      240                  * 2) While removing the target device via
>      241                  *    coresight_remove_match()
>      242                  */
>      243                 conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>      244                 conn.dest_port = rendpoint.port;
>      245
>      246                 /*
>      247                  * Get the firmware node of the filter source through the
>      248                  * reference. This could be used to filter the source in
>      249                  * building path.
>      250                  */
>      251                 conn.filter_src_fwnode =
>      252                         fwnode_find_reference(&ep->fwnode, "filter-source", 0);
>      253                 if (IS_ERR(conn.filter_src_fwnode)) {
>      254                         conn.filter_src_fwnode = NULL;
>      255                 } else {
>      256                         conn.filter_src_dev =
>      257                          coresight_find_csdev_by_fwnode(conn.filter_src_fwnode);
>      258                         if (conn.filter_src_dev &&
>      259                             !coresight_is_device_source(conn.filter_src_dev)) {
>      260                                 dev_warn(dev, "port %d: Filter handle is not a trace source : %s\n",
>      261                                          conn.src_port, dev_name(&conn.filter_src_dev->dev));
>      262                                 conn.filter_src_dev = NULL;
>      263                                 conn.filter_src_fwnode = NULL;
>      264                         }
>      265                 }
>      266
>      267                 new_conn = coresight_add_out_conn(dev, pdata, &conn);
>      268                 if (IS_ERR_VALUE(new_conn)) {
>      269                         fwnode_handle_put(conn.dest_fwnode);
>      270                         return PTR_ERR(new_conn);
> 
> Should this be a break statement instead of return statement?
> 

Hi Dan,

Yes it probably should, I'll send a patch.

Thanks
James