Messages in this thread | | | Date | Wed, 29 Mar 2023 13:02:31 +0100 | From | James Clark <> | Subject | Re: [PATCH v2 5/9] coresight: Dynamically add connections |
| |
On 16/03/2023 17:12, Suzuki K Poulose wrote: > On 10/03/2023 16:06, James Clark wrote: >> Add a function for adding connections dynamically. This also removes >> the 1:1 mapping between port number and the index into the connections >> array. The only place this mapping was used was in the warning for >> duplicate output ports, which has been replaced by a search. Other >> uses of the port number already use the port member variable. >> >> Being able to dynamically add connections will allow other devices like >> CTI to re-use the connection mechanism despite not having explicit >> connections described in the DT. >> >> Signed-off-by: James Clark <james.clark@arm.com> >> --- >> .../hwtracing/coresight/coresight-platform.c | 77 ++++++++++++++----- >> include/linux/coresight.h | 7 +- >> 2 files changed, 64 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-platform.c >> b/drivers/hwtracing/coresight/coresight-platform.c >> index c77238cdf448..16553f7dde12 100644 >> --- a/drivers/hwtracing/coresight/coresight-platform.c >> +++ b/drivers/hwtracing/coresight/coresight-platform.c >> @@ -27,8 +27,9 @@ static int coresight_alloc_conns(struct device *dev, >> struct coresight_platform_data *pdata) >> { >> if (pdata->nr_outconns) { >> - pdata->out_conns = devm_kcalloc(dev, pdata->nr_outconns, >> - sizeof(*pdata->out_conns), GFP_KERNEL); >> + pdata->out_conns = devm_krealloc_array( >> + dev, pdata->out_conns, pdata->nr_outconns, > > super minor nit: > pdata->out_conns = devm_krealloc_array(dev, >
This is actually clang-format's doing when using the kernel rules in the root of the repo. I started using it because of some other style comments I got before. Not sure if this time it's just done something bad or it's technically ok.
I formatted everything in V3 with it so it should at least all be consistent.
> >> + sizeof(*pdata->out_conns), GFP_KERNEL | __GFP_ZERO); >> if (!pdata->out_conns) >> return -ENOMEM; >> } >> @@ -36,6 +37,48 @@ static int coresight_alloc_conns(struct device *dev, >> return 0; >> } >> +/* >> + * Add a connection in the first free slot, or realloc >> + * if there is no space. @conn's contents is copied into the new slot. >> + * >> + * If the output port is already assigned on this device, return -EINVAL >> + */ >> +int coresight_add_conn(struct device *dev, >> + struct coresight_platform_data *pdata, >> + const struct coresight_connection *conn) >> +{ >> + int ret; >> + struct coresight_connection *free_conn = NULL; >> + struct coresight_connection *i; >> + >> + /* >> + * Search for a free slot, and while looking for one, warn >> + * on any existing duplicate output port. >> + */ >> + for (i = pdata->out_conns; i < pdata->out_conns + >> pdata->nr_outconns; >> + ++i) { > > minor nit: I see why you have gone against using "i" as index into > the array. But I think having that as the index is still better > readable. > for (i = 0; i < pdata->nr_outconns; i++) { > struct coresight_connection *c = &pdata->out_conns[i]; > >> + if (i->remote_fwnode && conn->port != -1 && >> + i->port == conn->port) { >> + dev_warn(dev, "Duplicate output port %d\n", i->port); >> + return -EINVAL; >> + } >> + if (!i->remote_fwnode && !free_conn) >> + free_conn = i; >> + } >> + >> + if (!free_conn) { > > and: > /* No free slots */ > if (i == pdata->nr_outconns) { > >> + pdata->nr_outconns++; >> + ret = coresight_alloc_conns(dev, pdata); >> + if (ret) >> + return ret; >> + free_conn = &pdata->out_conns[pdata->nr_outconns - 1]; >> + } >> + > > and: > pdata->out_conns[i] = *conn; > > > Otherwise looks good to me. > > Suzuki > >
| |