[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ9a7Vjr73_CcfXULOHPwAQV2sCkeg0Vq4vSBKBiMhEuWE-VRQ@mail.gmail.com>
Date: Wed, 12 Apr 2023 15:55:52 +0100
From: Mike Leach <mike.leach@...aro.org>
To: James Clark <james.clark@....com>
Cc: coresight@...ts.linaro.org, quic_jinlmao@...cinc.com,
suzuki.poulose@....com,
Mathieu Poirier <mathieu.poirier@...aro.org>,
Leo Yan <leo.yan@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 08/13] coresight: Simplify connection fixup mechanism
On Tue, 4 Apr 2023 at 16:52, James Clark <james.clark@....com> wrote:
>
> There is some duplication between coresight_fixup_device_conns() and
> coresight_fixup_orphan_conns(). They both do the same thing except for
> the fact that coresight_fixup_orphan_conns() can't handle iterating over
> itself.
>
> By making it able to handle fixing up it's own connections the other
> function can be removed.
>
> Signed-off-by: James Clark <james.clark@....com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 84 ++++++++------------
> 1 file changed, 32 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 0b738960973b..8d377a59e0be 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1316,42 +1316,46 @@ static int coresight_orphan_match(struct device *dev, void *data)
> {
> int i, ret = 0;
> bool still_orphan = false;
> - struct coresight_device *csdev, *i_csdev;
> + struct coresight_device *dst_csdev = data;
> + struct coresight_device *src_csdev = to_coresight_device(dev);
> struct coresight_connection *conn;
> -
> - csdev = data;
> - i_csdev = to_coresight_device(dev);
> -
> - /* No need to check oneself */
> - if (csdev == i_csdev)
> - return 0;
> + bool fixup_self = (src_csdev == dst_csdev);
>
> /* Move on to another component if no connection is orphan */
> - if (!i_csdev->orphan)
> + if (!src_csdev->orphan)
> return 0;
> /*
> - * Circle throuch all the connection of that component. If we find
> - * an orphan connection whose name matches @csdev, link it.
> + * Circle through all the connections of that component. If we find
> + * an orphan connection whose name matches @dst_csdev, link it.
> */
> - for (i = 0; i < i_csdev->pdata->nr_outconns; i++) {
> - conn = i_csdev->pdata->out_conns[i];
> -
> - /* We have found at least one orphan connection */
> - if (conn->dest_dev == NULL) {
> - /* Does it match this newly added device? */
> - if (conn->dest_fwnode == csdev->dev.fwnode) {
> - ret = coresight_make_links(i_csdev,
> - conn, csdev);
> - if (ret)
> - return ret;
> - } else {
> - /* This component still has an orphan */
> - still_orphan = true;
> - }
> + for (i = 0; i < src_csdev->pdata->nr_outconns; i++) {
> + conn = src_csdev->pdata->out_conns[i];
> +
> + /* Skip the port if it's already connected. */
> + if (conn->dest_dev)
> + continue;
> +
> + /*
> + * If we are at the "new" device, which triggered this search,
> + * we must find the remote device from the fwnode in the
> + * connection.
> + */
> + if (fixup_self)
> + dst_csdev = coresight_find_csdev_by_fwnode(
> + conn->dest_fwnode);
> +
> + /* Does it match this newly added device? */
> + if (dst_csdev && conn->dest_fwnode == dst_csdev->dev.fwnode) {
> + ret = coresight_make_links(src_csdev, conn, dst_csdev);
> + if (ret)
> + return ret;
> + } else {
> + /* This component still has an orphan */
> + still_orphan = true;
> }
> }
>
> - i_csdev->orphan = still_orphan;
> + src_csdev->orphan = still_orphan;
>
> /*
> * Returning '0' in case we didn't encounter any error,
> @@ -1366,28 +1370,6 @@ static int coresight_fixup_orphan_conns(struct coresight_device *csdev)
> csdev, coresight_orphan_match);
> }
>
> -
> -static int coresight_fixup_device_conns(struct coresight_device *csdev)
> -{
> - int i, ret = 0;
> -
> - for (i = 0; i < csdev->pdata->nr_outconns; i++) {
> - struct coresight_connection *conn = csdev->pdata->out_conns[i];
> -
> - conn->dest_dev =
> - coresight_find_csdev_by_fwnode(conn->dest_fwnode);
> - if (conn->dest_dev && conn->dest_dev->has_conns_grp) {
> - ret = coresight_make_links(csdev, conn, conn->dest_dev);
> - if (ret)
> - break;
> - } else {
> - csdev->orphan = true;
> - }
> - }
> -
> - return ret;
> -}
> -
> static int coresight_remove_match(struct device *dev, void *data)
> {
> int i;
> @@ -1595,7 +1577,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
> csdev->subtype = desc->subtype;
> csdev->ops = desc->ops;
> csdev->access = desc->access;
> - csdev->orphan = false;
> + csdev->orphan = true;
>
> csdev->dev.type = &coresight_dev_type[desc->type];
> csdev->dev.groups = desc->groups;
> @@ -1645,8 +1627,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
> registered = true;
>
> ret = coresight_create_conns_sysfs_group(csdev);
> - if (!ret)
> - ret = coresight_fixup_device_conns(csdev);
> if (!ret)
> ret = coresight_fixup_orphan_conns(csdev);
>
> --
> 2.34.1
>
Reviewed-by: Mike Leach <mike.leach@...aro.org>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists