| Message ID | 20191119231912.12768-8-mike.leach@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | CoreSight CTI Driver | expand |
On Tue, Nov 19, 2019 at 11:19:05PM +0000, 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 | 250 +++++++++++++++++- > .../hwtracing/coresight/coresight-cti-sysfs.c | 11 + > 2 files changed, 257 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > index 790dd30b85f5..9c1ff432b487 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_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" > +#define CTI_DT_CTM_ID "arm,cti-ctm-id" > > /* > * Find a registered coresight device from a device fwnode. > @@ -68,6 +78,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) > return node->full_name; > return "unknown"; > } > + > +static bool of_cti_node_name_eq(const struct device_node *node, > + const char *name) > +{ > + return of_node_name_eq(node, name); > +} > #else > static int of_cti_get_cpu_at_node(const struct device_node *node) > { > @@ -78,6 +94,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) > { > return "unknown"; > } > + > +static bool of_cti_node_name_eq(const struct device_node *node, > + const char *name) > +{ > + return false; > +} > #endif > > static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) > @@ -94,6 +116,14 @@ static const char *cti_plat_get_node_name(struct fwnode_handle *fwnode) > return "unknown"; > } > > +static bool cti_plat_node_name_eq(struct fwnode_handle *fwnode, > + const char *name) > +{ > + if (is_of_node(fwnode)) > + return of_cti_node_name_eq(to_of_node(fwnode), name); > + return false; > +} > + > static int cti_plat_create_v8_etm_connection(struct device *dev, > struct cti_drvdata *drvdata) > { > @@ -205,6 +235,214 @@ static int cti_plat_create_v8_connections(struct device *dev, > return ret; > } > > +static int cti_plat_count_sig_elements(const struct fwnode_handle *fwnode, > + const char *name) > +{ > + int nr_elem = fwnode_property_count_u32(fwnode, name); > + > + return (nr_elem < 0 ? 0 : nr_elem); > +} > + > +static int cti_plat_read_trig_group(struct cti_trig_grp *tgrp, > + const struct fwnode_handle *fwnode, > + const char *grp_name) > +{ > + int idx, err = 0; > + u32 *values; > + > + if (!tgrp->nr_sigs) > + return 0; > + > + values = kcalloc(tgrp->nr_sigs, sizeof(u32), GFP_KERNEL); > + if (!values) > + return -ENOMEM; > + > + err = fwnode_property_read_u32_array(fwnode, grp_name, > + values, tgrp->nr_sigs); > + > + if (!err) { > + /* set the signal usage mask */ > + for (idx = 0; idx < tgrp->nr_sigs; idx++) > + tgrp->used_mask |= BIT(values[idx]); > + } > + > + kfree(values); > + return err; > +} > + > +static int cti_plat_read_trig_types(struct cti_trig_grp *tgrp, > + const struct fwnode_handle *fwnode, > + const char *type_name) > +{ > + int items, used = 0, err = 0, nr_sigs; > + u32 *values = NULL, 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 = cti_plat_count_sig_elements(fwnode, type_name); > + if (items > nr_sigs) > + return -EINVAL; > + > + /* need an array to store the values iff there are any */ > + if (items) { > + values = kcalloc(items, sizeof(u32), GFP_KERNEL); > + if (!values) > + return -ENOMEM; > + > + err = fwnode_property_read_u32_array(fwnode, type_name, > + values, items); > + if (err) > + goto read_trig_types_out; > + } > + > + /* > + * Match type id to signal index, 1st type to 1st index etc. > + * If fewer types than signals default remainder to GEN_IO. > + */ > + for (i = 0; i < nr_sigs; i++) { > + if (used < items) { > + tgrp->sig_types[i] = > + values[i] < CTI_TRIG_MAX ? values[i] : GEN_IO; > + used++; > + } else { > + tgrp->sig_types[i] = GEN_IO; > + } > + } > + > +read_trig_types_out: > + kfree(values); > + return err; > +} > + > +static int cti_plat_process_filter_sigs(struct cti_drvdata *drvdata, > + const struct fwnode_handle *fwnode) > +{ > + struct cti_trig_grp *tg = NULL; > + int err = 0, nr_filter_sigs; > + > + nr_filter_sigs = cti_plat_count_sig_elements(fwnode, > + 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 = cti_plat_read_trig_group(tg, fwnode, CTI_DT_FILTER_OUT_SIGS); > + if (!err) > + drvdata->config.trig_out_filter |= tg->used_mask; > + > + kfree(tg); > + return err; > +} > + > +static int cti_plat_create_connection(struct device *dev, > + struct cti_drvdata *drvdata, > + struct fwnode_handle *fwnode) > +{ > + struct cti_trig_con *tc = NULL; > + int cpuid = -1, err = 0; > + struct fwnode_handle *cs_fwnode = NULL; > + struct coresight_device *csdev = NULL; > + const char *assoc_name = "unknown"; > + char cpu_name_str[16]; > + int nr_sigs_in, nr_sigs_out; > + > + /* look to see how many in and out signals we have */ > + nr_sigs_in = cti_plat_count_sig_elements(fwnode, CTI_DT_TRIGIN_SIGS); > + nr_sigs_out = cti_plat_count_sig_elements(fwnode, 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 = cti_plat_read_trig_group(tc->con_in, fwnode, > + CTI_DT_TRIGIN_SIGS); > + if (err) > + goto create_con_err; > + > + err = cti_plat_read_trig_types(tc->con_in, fwnode, > + CTI_DT_TRIGIN_TYPES); > + if (err) > + goto create_con_err; > + > + err = cti_plat_read_trig_group(tc->con_out, fwnode, > + CTI_DT_TRIGOUT_SIGS); > + if (err) > + goto create_con_err; > + > + err = cti_plat_read_trig_types(tc->con_out, fwnode, > + CTI_DT_TRIGOUT_TYPES); > + if (err) > + goto create_con_err; > + > + err = cti_plat_process_filter_sigs(drvdata, fwnode); > + if (err) > + goto create_con_err; > + > + /* read the connection name if set - may be overridden by later */ > + fwnode_property_read_string(fwnode, CTI_DT_CONN_NAME, &assoc_name); > + > + /* associated cpu ? */ > + cpuid = cti_plat_get_cpu_at_node(fwnode); > + 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_fwnode = fwnode_find_reference(fwnode, > + CTI_DT_CSDEV_ASSOC, 0); > + if (!IS_ERR_OR_NULL(cs_fwnode)) { > + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); > + if (csdev) /* use device name if csdev found */ > + assoc_name = dev_name(&csdev->dev); > + else /* otherwise node name for later association */ > + assoc_name = cti_plat_get_node_name(cs_fwnode); > + fwnode_handle_put(cs_fwnode); > + } > + } > + /* set up a connection */ > + err = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); > + > +create_con_err: > + return err; > +} > + > +static int cti_plat_create_impdef_connections(struct device *dev, > + struct cti_drvdata *drvdata) > +{ > + int rc = 0; > + struct fwnode_handle *fwnode = dev_fwnode(dev); > + struct fwnode_handle *child = NULL; > + > + if (IS_ERR_OR_NULL(fwnode)) > + return -EINVAL; > + > + fwnode_for_each_child_node(fwnode, child) { > + if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) > + rc = cti_plat_create_connection(dev, drvdata, child); > + if (rc != 0) > + break; > + } > + fwnode_handle_put(child); As far as I can tell we don't need to call fwnode_handle_put()? With the above: Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > + > + return rc; > +} > + > /* get the hardware configuration & connection data. */ > int cti_plat_get_hw_data(struct device *dev, > struct cti_drvdata *drvdata) > @@ -212,12 +450,16 @@ int cti_plat_get_hw_data(struct device *dev, > int rc = 0; > struct cti_device *cti_dev = &drvdata->ctidev; > > + /* get any CTM ID - defaults to 0 */ > + device_property_read_u32(dev, CTI_DT_CTM_ID, &cti_dev->ctm_id); > + > /* check for a v8 architectural CTI device */ > - if (device_property_read_bool(dev, CTI_DT_V8ARCH)) { > + if (device_property_read_bool(dev, CTI_DT_V8ARCH)) > rc = cti_plat_create_v8_connections(dev, drvdata); > - if (rc) > - return rc; > - } > + else > + rc = cti_plat_create_impdef_connections(dev, drvdata); > + if (rc) > + return rc; > > /* if no connections, just add a single default based on max IN-OUT */ > if (cti_dev->nr_trig_con == 0) > diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > index 98de8a4768fc..f800402f73da 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c > @@ -56,9 +56,20 @@ static ssize_t enable_store(struct device *dev, > } > static DEVICE_ATTR_RW(enable); > > +static ssize_t ctmid_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + return scnprintf(buf, PAGE_SIZE, "%d\n", drvdata->ctidev.ctm_id); > +} > +static DEVICE_ATTR_RO(ctmid); > + > /* attribute and group sysfs tables. */ > static struct attribute *coresight_cti_attrs[] = { > &dev_attr_enable.attr, > + &dev_attr_ctmid.attr, > NULL, > }; > > -- > 2.17.1 >
On 25/11/2019 21:22, Mathieu Poirier wrote: > On Tue, Nov 19, 2019 at 11:19:05PM +0000, 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 | 250 +++++++++++++++++- >> .../hwtracing/coresight/coresight-cti-sysfs.c | 11 + >> 2 files changed, 257 insertions(+), 4 deletions(-) >> +static int cti_plat_create_impdef_connections(struct device *dev, >> + struct cti_drvdata *drvdata) >> +{ >> + int rc = 0; >> + struct fwnode_handle *fwnode = dev_fwnode(dev); >> + struct fwnode_handle *child = NULL; >> + >> + if (IS_ERR_OR_NULL(fwnode)) >> + return -EINVAL; >> + >> + fwnode_for_each_child_node(fwnode, child) { >> + if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) >> + rc = cti_plat_create_connection(dev, drvdata, child); >> + if (rc != 0) >> + break; >> + } >> + fwnode_handle_put(child); > > As far as I can tell we don't need to call fwnode_handle_put()? Actually we do, if we break the scan in between, at least for of_nodes. I had to literally look it down all the way down to confirm this. So for CONFIG_OF it ends up in of_get_next_available_child(), which drops the ref on "prev" and grabs the "next". So in case we break the loop, we must drop the ref on the child. Otherwise, the last result would be NULL, which would have dropped the ref on the "last" valid entry. And this is harmless with a NULL ptr. Suzuki
On 19/11/2019 23:19, 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 | 250 +++++++++++++++++- > .../hwtracing/coresight/coresight-cti-sysfs.c | 11 + > 2 files changed, 257 insertions(+), 4 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > index 790dd30b85f5..9c1ff432b487 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_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" > +#define CTI_DT_CTM_ID "arm,cti-ctm-id" > > /* > * Find a registered coresight device from a device fwnode. > @@ -68,6 +78,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) > return node->full_name; > return "unknown"; > } > + > +static bool of_cti_node_name_eq(const struct device_node *node, > + const char *name) > +{ > + return of_node_name_eq(node, name); > +} > #else > static int of_cti_get_cpu_at_node(const struct device_node *node) > { > @@ -78,6 +94,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) > { > return "unknown"; > } > + > +static bool of_cti_node_name_eq(const struct device_node *node, > + const char *name) > +{ > + return false; > +} > #endif nit: You don't need this wrapper of_node_name_eq() is already defined to return false for !CONFIG_OF. > > static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) > @@ -94,6 +116,14 @@ static const char *cti_plat_get_node_name(struct fwnode_handle *fwnode) > return "unknown"; > } > > +static bool cti_plat_node_name_eq(struct fwnode_handle *fwnode, > + const char *name) > +{ > + if (is_of_node(fwnode)) > + return of_cti_node_name_eq(to_of_node(fwnode), name); As mentioned above you could simply use of_node_name_eq() here. > + return false; > +} > + > static int cti_plat_create_v8_etm_connection(struct device *dev, > struct cti_drvdata *drvdata) > { > @@ -205,6 +235,214 @@ static int cti_plat_create_v8_connections(struct device *dev, > return ret; > } > > +static int cti_plat_count_sig_elements(const struct fwnode_handle *fwnode, > + const char *name) > +{ > + int nr_elem = fwnode_property_count_u32(fwnode, name); > + > + return (nr_elem < 0 ? 0 : nr_elem); > +} > + > +static int cti_plat_read_trig_group(struct cti_trig_grp *tgrp, > + const struct fwnode_handle *fwnode, > + const char *grp_name) > +{ > + int idx, err = 0; > + u32 *values; > + > + if (!tgrp->nr_sigs) > + return 0; > + > + values = kcalloc(tgrp->nr_sigs, sizeof(u32), GFP_KERNEL); > + if (!values) > + return -ENOMEM; > + > + err = fwnode_property_read_u32_array(fwnode, grp_name, > + values, tgrp->nr_sigs); > + > + if (!err) { > + /* set the signal usage mask */ > + for (idx = 0; idx < tgrp->nr_sigs; idx++) > + tgrp->used_mask |= BIT(values[idx]); > + } > + > + kfree(values); > + return err; > +} > + > +static int cti_plat_read_trig_types(struct cti_trig_grp *tgrp, > + const struct fwnode_handle *fwnode, > + const char *type_name) > +{ > + int items, used = 0, err = 0, nr_sigs; > + u32 *values = NULL, 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 = cti_plat_count_sig_elements(fwnode, type_name); > + if (items > nr_sigs) > + return -EINVAL; > + > + /* need an array to store the values iff there are any */ > + if (items) { > + values = kcalloc(items, sizeof(u32), GFP_KERNEL); > + if (!values) > + return -ENOMEM; > + > + err = fwnode_property_read_u32_array(fwnode, type_name, > + values, items); > + if (err) > + goto read_trig_types_out; > + } > + > + /* > + * Match type id to signal index, 1st type to 1st index etc. > + * If fewer types than signals default remainder to GEN_IO. > + */ > + for (i = 0; i < nr_sigs; i++) { > + if (used < items) { > + tgrp->sig_types[i] = > + values[i] < CTI_TRIG_MAX ? values[i] : GEN_IO; > + used++; Do we really need "used" here ? Couldn't this be : if (i < items) { } > + } else { + /* Mark the undefined connections as GEN_IO */ ? > + tgrp->sig_types[i] = GEN_IO; > + } > + } ... > + > + /* read the connection name if set - may be overridden by later */ > + fwnode_property_read_string(fwnode, CTI_DT_CONN_NAME, &assoc_name); > + > + /* associated cpu ? */ > + cpuid = cti_plat_get_cpu_at_node(fwnode); > + 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_fwnode = fwnode_find_reference(fwnode, > + CTI_DT_CSDEV_ASSOC, 0); > + if (!IS_ERR_OR_NULL(cs_fwnode)) { --- Cut - here -- > + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); > + if (csdev) /* use device name if csdev found */ > + assoc_name = dev_name(&csdev->dev); > + else /* otherwise node name for later association */ > + assoc_name = cti_plat_get_node_name(cs_fwnode); --- end - here -- I believe we do this for arm_v8 architected connections too. May be make this a helper ? > + fwnode_handle_put(cs_fwnode); > + } > + } > + /* set up a connection */ > + err = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); > + > +create_con_err: > + return err; > +} > + > +static int cti_plat_create_impdef_connections(struct device *dev, > + struct cti_drvdata *drvdata) > +{ > + int rc = 0; > + struct fwnode_handle *fwnode = dev_fwnode(dev); > + struct fwnode_handle *child = NULL; > + > + if (IS_ERR_OR_NULL(fwnode)) > + return -EINVAL; > + > + fwnode_for_each_child_node(fwnode, child) { > + if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) > + rc = cti_plat_create_connection(dev, drvdata, child); > + if (rc != 0) > + break; minor nit: To make it more obvious : if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) { rc = cti_plat_create_connection(dev, drvdata, child); if (rc) break; } Suzuki
On Fri, 29 Nov 2019 at 07:16, Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com> wrote: > > On 25/11/2019 21:22, Mathieu Poirier wrote: > > On Tue, Nov 19, 2019 at 11:19:05PM +0000, 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 | 250 +++++++++++++++++- > >> .../hwtracing/coresight/coresight-cti-sysfs.c | 11 + > >> 2 files changed, 257 insertions(+), 4 deletions(-) > > >> +static int cti_plat_create_impdef_connections(struct device *dev, > >> + struct cti_drvdata *drvdata) > >> +{ > >> + int rc = 0; > >> + struct fwnode_handle *fwnode = dev_fwnode(dev); > >> + struct fwnode_handle *child = NULL; > >> + > >> + if (IS_ERR_OR_NULL(fwnode)) > >> + return -EINVAL; > >> + > >> + fwnode_for_each_child_node(fwnode, child) { > >> + if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) > >> + rc = cti_plat_create_connection(dev, drvdata, child); > >> + if (rc != 0) > >> + break; > >> + } > >> + fwnode_handle_put(child); > > > > As far as I can tell we don't need to call fwnode_handle_put()? > > Actually we do, if we break the scan in between, at least for of_nodes. > I had to literally look it down all the way down to confirm this. > > So for CONFIG_OF it ends up in of_get_next_available_child(), which > drops the ref on "prev" and grabs the "next". So in case we break > the loop, we must drop the ref on the child. Well spotted. > Otherwise, the last > result would be NULL, which would have dropped the ref on the "last" > valid entry. And this is harmless with a NULL ptr. > > Suzuki
Hi Suzuki, On Fri, 29 Nov 2019 at 14:18, Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com> wrote: > > On 19/11/2019 23:19, 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 | 250 +++++++++++++++++- > > .../hwtracing/coresight/coresight-cti-sysfs.c | 11 + > > 2 files changed, 257 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > > index 790dd30b85f5..9c1ff432b487 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_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" > > +#define CTI_DT_CTM_ID "arm,cti-ctm-id" > > > > /* > > * Find a registered coresight device from a device fwnode. > > @@ -68,6 +78,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) > > return node->full_name; > > return "unknown"; > > } > > + > > +static bool of_cti_node_name_eq(const struct device_node *node, > > + const char *name) > > +{ > > + return of_node_name_eq(node, name); > > +} > > #else > > static int of_cti_get_cpu_at_node(const struct device_node *node) > > { > > @@ -78,6 +94,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) > > { > > return "unknown"; > > } > > + > > +static bool of_cti_node_name_eq(const struct device_node *node, > > + const char *name) > > +{ > > + return false; > > +} > > #endif > > nit: You don't need this wrapper of_node_name_eq() is already defined to > return false for !CONFIG_OF. > > OK. > > > > static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) > > @@ -94,6 +116,14 @@ static const char *cti_plat_get_node_name(struct fwnode_handle *fwnode) > > return "unknown"; > > } > > > > +static bool cti_plat_node_name_eq(struct fwnode_handle *fwnode, > > + const char *name) > > +{ > > + if (is_of_node(fwnode)) > > + return of_cti_node_name_eq(to_of_node(fwnode), name); > > As mentioned above you could simply use of_node_name_eq() here. > > > + return false; > > +} > > + > > static int cti_plat_create_v8_etm_connection(struct device *dev, > > struct cti_drvdata *drvdata) > > { > > @@ -205,6 +235,214 @@ static int cti_plat_create_v8_connections(struct device *dev, > > return ret; > > } > > > > +static int cti_plat_count_sig_elements(const struct fwnode_handle *fwnode, > > + const char *name) > > +{ > > + int nr_elem = fwnode_property_count_u32(fwnode, name); > > + > > + return (nr_elem < 0 ? 0 : nr_elem); > > +} > > + > > +static int cti_plat_read_trig_group(struct cti_trig_grp *tgrp, > > + const struct fwnode_handle *fwnode, > > + const char *grp_name) > > +{ > > + int idx, err = 0; > > + u32 *values; > > + > > + if (!tgrp->nr_sigs) > > + return 0; > > + > > + values = kcalloc(tgrp->nr_sigs, sizeof(u32), GFP_KERNEL); > > + if (!values) > > + return -ENOMEM; > > + > > + err = fwnode_property_read_u32_array(fwnode, grp_name, > > + values, tgrp->nr_sigs); > > + > > + if (!err) { > > + /* set the signal usage mask */ > > + for (idx = 0; idx < tgrp->nr_sigs; idx++) > > + tgrp->used_mask |= BIT(values[idx]); > > + } > > + > > + kfree(values); > > + return err; > > +} > > + > > +static int cti_plat_read_trig_types(struct cti_trig_grp *tgrp, > > + const struct fwnode_handle *fwnode, > > + const char *type_name) > > +{ > > + int items, used = 0, err = 0, nr_sigs; > > + u32 *values = NULL, 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 = cti_plat_count_sig_elements(fwnode, type_name); > > + if (items > nr_sigs) > > + return -EINVAL; > > + > > + /* need an array to store the values iff there are any */ > > + if (items) { > > + values = kcalloc(items, sizeof(u32), GFP_KERNEL); > > + if (!values) > > + return -ENOMEM; > > + > > + err = fwnode_property_read_u32_array(fwnode, type_name, > > + values, items); > > + if (err) > > + goto read_trig_types_out; > > + } > > + > > + /* > > + * Match type id to signal index, 1st type to 1st index etc. > > + * If fewer types than signals default remainder to GEN_IO. > > + */ > > + for (i = 0; i < nr_sigs; i++) { > > + if (used < items) { > > + tgrp->sig_types[i] = > > + values[i] < CTI_TRIG_MAX ? values[i] : GEN_IO; > > + used++; > > Do we really need "used" here ? Couldn't this be : > No we don't - I'll fix it. > if (i < items) { > > } > > > + } else { > > + /* Mark the undefined connections as GEN_IO */ ? > > > + tgrp->sig_types[i] = GEN_IO; > > + } > > + } > > ... > > > + > > + /* read the connection name if set - may be overridden by later */ > > + fwnode_property_read_string(fwnode, CTI_DT_CONN_NAME, &assoc_name); > > + > > + /* associated cpu ? */ > > + cpuid = cti_plat_get_cpu_at_node(fwnode); > > + 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_fwnode = fwnode_find_reference(fwnode, > > + CTI_DT_CSDEV_ASSOC, 0); > > > + if (!IS_ERR_OR_NULL(cs_fwnode)) { > > --- Cut - here -- > > + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); > > + if (csdev) /* use device name if csdev found */ > > + assoc_name = dev_name(&csdev->dev); > > + else /* otherwise node name for later association */ > > + assoc_name = cti_plat_get_node_name(cs_fwnode); > > --- end - here -- > > I believe we do this for arm_v8 architected connections too. May be make > this a helper ? > OK > > + fwnode_handle_put(cs_fwnode); > > + } > > + } > > + /* set up a connection */ > > + err = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); > > + > > +create_con_err: > > + return err; > > +} > > + > > +static int cti_plat_create_impdef_connections(struct device *dev, > > + struct cti_drvdata *drvdata) > > +{ > > + int rc = 0; > > + struct fwnode_handle *fwnode = dev_fwnode(dev); > > + struct fwnode_handle *child = NULL; > > + > > + if (IS_ERR_OR_NULL(fwnode)) > > + return -EINVAL; > > + > > + fwnode_for_each_child_node(fwnode, child) { > > + if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) > > + rc = cti_plat_create_connection(dev, drvdata, child); > > + if (rc != 0) > > + break; > > minor nit: To make it more obvious : > > if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) { > rc = cti_plat_create_connection(dev, drvdata, > child); > if (rc) > break; > } > > Suzuki Thanks Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c index 790dd30b85f5..9c1ff432b487 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_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" +#define CTI_DT_CTM_ID "arm,cti-ctm-id" /* * Find a registered coresight device from a device fwnode. @@ -68,6 +78,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) return node->full_name; return "unknown"; } + +static bool of_cti_node_name_eq(const struct device_node *node, + const char *name) +{ + return of_node_name_eq(node, name); +} #else static int of_cti_get_cpu_at_node(const struct device_node *node) { @@ -78,6 +94,12 @@ static const char *of_cti_get_node_name(const struct device_node *node) { return "unknown"; } + +static bool of_cti_node_name_eq(const struct device_node *node, + const char *name) +{ + return false; +} #endif static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) @@ -94,6 +116,14 @@ static const char *cti_plat_get_node_name(struct fwnode_handle *fwnode) return "unknown"; } +static bool cti_plat_node_name_eq(struct fwnode_handle *fwnode, + const char *name) +{ + if (is_of_node(fwnode)) + return of_cti_node_name_eq(to_of_node(fwnode), name); + return false; +} + static int cti_plat_create_v8_etm_connection(struct device *dev, struct cti_drvdata *drvdata) { @@ -205,6 +235,214 @@ static int cti_plat_create_v8_connections(struct device *dev, return ret; } +static int cti_plat_count_sig_elements(const struct fwnode_handle *fwnode, + const char *name) +{ + int nr_elem = fwnode_property_count_u32(fwnode, name); + + return (nr_elem < 0 ? 0 : nr_elem); +} + +static int cti_plat_read_trig_group(struct cti_trig_grp *tgrp, + const struct fwnode_handle *fwnode, + const char *grp_name) +{ + int idx, err = 0; + u32 *values; + + if (!tgrp->nr_sigs) + return 0; + + values = kcalloc(tgrp->nr_sigs, sizeof(u32), GFP_KERNEL); + if (!values) + return -ENOMEM; + + err = fwnode_property_read_u32_array(fwnode, grp_name, + values, tgrp->nr_sigs); + + if (!err) { + /* set the signal usage mask */ + for (idx = 0; idx < tgrp->nr_sigs; idx++) + tgrp->used_mask |= BIT(values[idx]); + } + + kfree(values); + return err; +} + +static int cti_plat_read_trig_types(struct cti_trig_grp *tgrp, + const struct fwnode_handle *fwnode, + const char *type_name) +{ + int items, used = 0, err = 0, nr_sigs; + u32 *values = NULL, 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 = cti_plat_count_sig_elements(fwnode, type_name); + if (items > nr_sigs) + return -EINVAL; + + /* need an array to store the values iff there are any */ + if (items) { + values = kcalloc(items, sizeof(u32), GFP_KERNEL); + if (!values) + return -ENOMEM; + + err = fwnode_property_read_u32_array(fwnode, type_name, + values, items); + if (err) + goto read_trig_types_out; + } + + /* + * Match type id to signal index, 1st type to 1st index etc. + * If fewer types than signals default remainder to GEN_IO. + */ + for (i = 0; i < nr_sigs; i++) { + if (used < items) { + tgrp->sig_types[i] = + values[i] < CTI_TRIG_MAX ? values[i] : GEN_IO; + used++; + } else { + tgrp->sig_types[i] = GEN_IO; + } + } + +read_trig_types_out: + kfree(values); + return err; +} + +static int cti_plat_process_filter_sigs(struct cti_drvdata *drvdata, + const struct fwnode_handle *fwnode) +{ + struct cti_trig_grp *tg = NULL; + int err = 0, nr_filter_sigs; + + nr_filter_sigs = cti_plat_count_sig_elements(fwnode, + 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 = cti_plat_read_trig_group(tg, fwnode, CTI_DT_FILTER_OUT_SIGS); + if (!err) + drvdata->config.trig_out_filter |= tg->used_mask; + + kfree(tg); + return err; +} + +static int cti_plat_create_connection(struct device *dev, + struct cti_drvdata *drvdata, + struct fwnode_handle *fwnode) +{ + struct cti_trig_con *tc = NULL; + int cpuid = -1, err = 0; + struct fwnode_handle *cs_fwnode = NULL; + struct coresight_device *csdev = NULL; + const char *assoc_name = "unknown"; + char cpu_name_str[16]; + int nr_sigs_in, nr_sigs_out; + + /* look to see how many in and out signals we have */ + nr_sigs_in = cti_plat_count_sig_elements(fwnode, CTI_DT_TRIGIN_SIGS); + nr_sigs_out = cti_plat_count_sig_elements(fwnode, 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 = cti_plat_read_trig_group(tc->con_in, fwnode, + CTI_DT_TRIGIN_SIGS); + if (err) + goto create_con_err; + + err = cti_plat_read_trig_types(tc->con_in, fwnode, + CTI_DT_TRIGIN_TYPES); + if (err) + goto create_con_err; + + err = cti_plat_read_trig_group(tc->con_out, fwnode, + CTI_DT_TRIGOUT_SIGS); + if (err) + goto create_con_err; + + err = cti_plat_read_trig_types(tc->con_out, fwnode, + CTI_DT_TRIGOUT_TYPES); + if (err) + goto create_con_err; + + err = cti_plat_process_filter_sigs(drvdata, fwnode); + if (err) + goto create_con_err; + + /* read the connection name if set - may be overridden by later */ + fwnode_property_read_string(fwnode, CTI_DT_CONN_NAME, &assoc_name); + + /* associated cpu ? */ + cpuid = cti_plat_get_cpu_at_node(fwnode); + 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_fwnode = fwnode_find_reference(fwnode, + CTI_DT_CSDEV_ASSOC, 0); + if (!IS_ERR_OR_NULL(cs_fwnode)) { + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); + if (csdev) /* use device name if csdev found */ + assoc_name = dev_name(&csdev->dev); + else /* otherwise node name for later association */ + assoc_name = cti_plat_get_node_name(cs_fwnode); + fwnode_handle_put(cs_fwnode); + } + } + /* set up a connection */ + err = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); + +create_con_err: + return err; +} + +static int cti_plat_create_impdef_connections(struct device *dev, + struct cti_drvdata *drvdata) +{ + int rc = 0; + struct fwnode_handle *fwnode = dev_fwnode(dev); + struct fwnode_handle *child = NULL; + + if (IS_ERR_OR_NULL(fwnode)) + return -EINVAL; + + fwnode_for_each_child_node(fwnode, child) { + if (cti_plat_node_name_eq(child, CTI_DT_CONNS)) + rc = cti_plat_create_connection(dev, drvdata, child); + if (rc != 0) + break; + } + fwnode_handle_put(child); + + return rc; +} + /* get the hardware configuration & connection data. */ int cti_plat_get_hw_data(struct device *dev, struct cti_drvdata *drvdata) @@ -212,12 +450,16 @@ int cti_plat_get_hw_data(struct device *dev, int rc = 0; struct cti_device *cti_dev = &drvdata->ctidev; + /* get any CTM ID - defaults to 0 */ + device_property_read_u32(dev, CTI_DT_CTM_ID, &cti_dev->ctm_id); + /* check for a v8 architectural CTI device */ - if (device_property_read_bool(dev, CTI_DT_V8ARCH)) { + if (device_property_read_bool(dev, CTI_DT_V8ARCH)) rc = cti_plat_create_v8_connections(dev, drvdata); - if (rc) - return rc; - } + else + rc = cti_plat_create_impdef_connections(dev, drvdata); + if (rc) + return rc; /* if no connections, just add a single default based on max IN-OUT */ if (cti_dev->nr_trig_con == 0) diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c index 98de8a4768fc..f800402f73da 100644 --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c @@ -56,9 +56,20 @@ static ssize_t enable_store(struct device *dev, } static DEVICE_ATTR_RW(enable); +static ssize_t ctmid_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent); + + return scnprintf(buf, PAGE_SIZE, "%d\n", drvdata->ctidev.ctm_id); +} +static DEVICE_ATTR_RO(ctmid); + /* attribute and group sysfs tables. */ static struct attribute *coresight_cti_attrs[] = { &dev_attr_enable.attr, + &dev_attr_ctmid.attr, NULL, };
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 | 250 +++++++++++++++++- .../hwtracing/coresight/coresight-cti-sysfs.c | 11 + 2 files changed, 257 insertions(+), 4 deletions(-)