Re: [PATCH v2 5/9] coresight: Dynamically add connections

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