Re: [RFC PATCH v4 07/15] coresight: cti: Add device tree support for custom CTI.

On Mon, Aug 19, 2019 at 10:13:53PM +0100, Mike Leach wrote:

Adds support for CTIs whose connections are implementation defined at hardware design time, and not constrained by v8 architecture.

These CTIs have no standard connection setup, all the settings have to be defined in the device tree files. The patch creates a set of connections and trigger signals based on the information provided.

Signed-off-by: Mike Leach mike.leach@linaro.org

.../coresight/coresight-cti-platform.c | 209 +++++++++++++++++- 1 file changed, 204 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c index 3ee5b8f8d245..b7649f75f08b 100644 --- a/drivers/hwtracing/coresight/coresight-cti-platform.c +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c @@ -13,9 +13,19 @@ #define NR_V8PE_OUT_SIGS 3 #define NR_V8ETM_INOUT_SIGS 4 +/* CTI device tree trigger connection node keyword */ +#define CTI_DT_CONNS "trig-conns"

/* CTI device tree connection property keywords */ #define CTI_DT_V8ARCH "arm,cti-v8-arch" #define CTI_DT_CSDEV_ASSOC "arm,cs-dev-assoc" +#define CTI_DT_CTM_ID "arm,cti-ctm-id" +#define CTI_DT_TRIGIN_SIGS "arm,trig-in-sigs" +#define CTI_DT_TRIGOUT_SIGS "arm,trig-out-sigs" +#define CTI_DT_TRIGIN_TYPES "arm,trig-in-types" +#define CTI_DT_TRIGOUT_TYPES "arm,trig-out-types" +#define CTI_DT_FILTER_OUT_SIGS "arm,trig-filters" +#define CTI_DT_CONN_NAME "arm,trig-conn-name" /*

• Find a registered coresight device from a device fwnode.

@@ -156,6 +166,188 @@ static int of_cti_create_v8_connections(struct device *dev, return ret; }

+static int of_count_sig_elements(const struct device_node *np,

• 		 const char *name)
  

+{

• int nr_elem = of_property_count_elems_of_size(np, name, 4);
• return (nr_elem < 0 ? 0 : nr_elem);

+}

+static int of_cti_read_trig_group(struct cti_trig_grp *tgrp,

• 		  struct device_node *np,
  

• 		  const char *grp_name)
  

+{

• int items = tgrp->nr_sigs, err = 0;
• u32 value, pidx;
• if (!items)

• return 0;
  

• /* set the signal usage mask */
• for (pidx = 0; pidx < items; pidx++) {

• err = of_property_read_u32_index(np, grp_name, pidx, &value);
  

• if (err)
  

• 	return err;
  

• tgrp->used_mask |= BIT(value);
  

• }
• return 0;

+}

+static int of_cti_read_trig_types(struct cti_trig_grp *tgrp,

• 		  struct device_node *np,
  

• 		  const char *type_name)
  

+{

• int items, used = 0, err = 0, nr_sigs;
• u32 value, i;
• /* allocate an array according to number of signals in connection */
• nr_sigs = tgrp->nr_sigs;
• if (!nr_sigs)

• return 0;
  

• /* see if any types have been included in the device description */
• items = of_count_sig_elements(np, type_name);
• if (items > nr_sigs)

• return -EINVAL;
  

• /*

• * Match type id to signal index, 1st type to 1st index etc.
  

• * If fewer types than signals default remainer to GEN_IO.
  

• */
  

• for (i = 0; i < nr_sigs; i++) {

• if (used < items) {
  

• 	err = of_property_read_u32_index(np, type_name,
  

• 					 i, &value);
  

• 	if (!err)
  

• 		tgrp->sig_types[i] = value;
  

Should we check that 'value' falls within what we expect? That would mean adding a MAX element to the defines in coresight-cti-dt.h. Another acceptable way would be to add a comment to say where this is checked when the configuration is being applied.

• 	else
  

• 		tgrp->sig_types[i] = GEN_IO;
  

• 	used++;
  

• } else
  

• 	tgrp->sig_types[i] = GEN_IO;
  

Even if checkpatch doesn't complain you need curly braces around the else condition.

• }
• return 0;

+}

+static int of_cti_process_filter_sigs(struct cti_drvdata *drvdata,

• 		      struct device_node *np)
  

+{

• struct cti_trig_grp *tg = NULL;
• int err = 0, nr_filter_sigs;
• nr_filter_sigs = of_count_sig_elements(np, CTI_DT_FILTER_OUT_SIGS);
• if (nr_filter_sigs == 0)

• return 0;
  

• if (nr_filter_sigs > drvdata->config.nr_trig_max)

• return -EINVAL;
  

• tg = kzalloc(sizeof(*tg), GFP_KERNEL);
• if (!tg)

• return -ENOMEM;
  

• err = of_cti_read_trig_group(tg, np, CTI_DT_FILTER_OUT_SIGS);
• if (!err)

• drvdata->config.trig_out_filter |= tg->used_mask;
  

• kfree(tg);
• return err;

+}

+static int of_cti_create_connection(struct device *dev,

• 		    struct cti_drvdata *drvdata,
  

• 		    struct device_node *np)
  

+{

• struct cti_trig_con *tc = NULL;
• int cpuid = -1, err = 0;
• struct device_node *cs_np = NULL;
• struct coresight_device *csdev = NULL;
• const char *assoc_name = "unknown";
• char cpu_name_str[16];
• struct fwnode_handle *r_fwnode;
• int nr_sigs_in, nr_sigs_out;
• /* look to see how many in and out signals we have */
• nr_sigs_in = of_count_sig_elements(np, CTI_DT_TRIGIN_SIGS);
• nr_sigs_out = of_count_sig_elements(np, CTI_DT_TRIGOUT_SIGS);
• if ((nr_sigs_in > drvdata->config.nr_trig_max) ||

•    (nr_sigs_out > drvdata->config.nr_trig_max))
  

• return -EINVAL;
  

• tc = cti_allocate_trig_con(dev, nr_sigs_in, nr_sigs_out);
• if (!tc)

• return -ENOMEM;
  

• /* look for the signals properties. */
• err = of_cti_read_trig_group(tc->con_in, np, CTI_DT_TRIGIN_SIGS);
• if (err)

• goto of_create_con_err;
  

• err = of_cti_read_trig_types(tc->con_in, np, CTI_DT_TRIGIN_TYPES);
• if (err)

• goto of_create_con_err;
  

• err = of_cti_read_trig_group(tc->con_out, np, CTI_DT_TRIGOUT_SIGS);
• if (err)

• goto of_create_con_err;
  

• err = of_cti_read_trig_types(tc->con_out, np, CTI_DT_TRIGOUT_TYPES);
• if (err)

• goto of_create_con_err;
  

• err = of_cti_process_filter_sigs(drvdata, np);
• if (err)

• goto of_create_con_err;
  

• /* read the connection name if set - may be overridden by later */
• of_property_read_string(np, CTI_DT_CONN_NAME, &assoc_name);
• /* associated cpu ? */
• cpuid = of_cti_get_cpu_at_node(np);
• if (cpuid >= 0) {

• drvdata->ctidev.cpu = cpuid;
  

• scnprintf(cpu_name_str, sizeof(cpu_name_str), "cpu%d", cpuid);
  

• assoc_name = cpu_name_str;
  

• } else {

• /* associated device ? */
  

• cs_np = of_parse_phandle(np,  CTI_DT_CSDEV_ASSOC, 0);
  

• if (cs_np) {
  

• 	r_fwnode = of_fwnode_handle(cs_np);
  

• 	csdev = cti_get_assoc_csdev_by_fwnode(r_fwnode);
  

• 	if (csdev) /* use device name if csdev found */
  

• 		assoc_name = dev_name(&csdev->dev);
  

• 	else  /* otherwise node name for later association */
  

• 		assoc_name = cs_np->full_name;
  

• 	of_node_put(cs_np);
  

• }
  

• }
• /* set up a connection */
• err = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name);

+of_create_con_err:

• return err;

+}

+static int of_cti_create_impdef_connections(struct device *dev,

• 			    struct cti_drvdata *drvdata,
  

• 			    struct device_node *np)
  

+{

• int rc = 0;
• struct device_node *nc = NULL;
• for_each_child_of_node(np, nc) {

• if (of_node_cmp(nc->name, CTI_DT_CONNS) != 0)
  

• 	continue;
  

• rc = of_cti_create_connection(dev, drvdata, nc);
  

• if (rc != 0)
  

• 	break;
  

• }
• return rc;

+}

/* get the hardware configuration & connection data. */ int of_cti_get_hw_data(struct device *dev, struct device_node *np, @@ -164,12 +356,19 @@ int of_cti_get_hw_data(struct device *dev, int rc = 0; struct cti_device *cti_dev = &drvdata->ctidev;

• /* check for a v8 architectural CTI device */
• if (of_property_read_bool(np, CTI_DT_V8ARCH)) {

• /* get any CTM ID - defaults to 0 */
• of_property_read_u32(np, CTI_DT_CTM_ID, &cti_dev->ctm_id);

cti_dev->ctm_id isn't used in this patch - it should be introduced later when there is a need for it.

• /*

• * Check for a v8 architectural CTI device,
  

• * otherwise implementation defined.
  

• */
  

• if (of_property_read_bool(np, CTI_DT_V8ARCH)) rc = of_cti_create_v8_connections(dev, drvdata, np);

• if (rc)
  

• 	return rc;
  

• }

• else

• rc = of_cti_create_impdef_connections(dev, drvdata, np);
  

If you followed my advice about moving of_cti_create_v8_connections() to cti_create_v8_connections() then this should be cti_create_impdef_connections() and the is_of_node() check done in there.

• if (rc)

• return rc;
  

/* if no connections, just add a single default based on max IN-OUT */ if (cti_dev->nr_trig_con == 0) -- 2.17.1