Messages in this thread | | | Date | Thu, 16 Mar 2023 17:12:06 +0000 | Subject | Re: [PATCH v2 5/9] coresight: Dynamically add connections | From | Suzuki K Poulose <> |
| |
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,
> + 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
| |