Hi Suzuki
On Mon, 17 Jun 2019 at 14:39, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 05/06/2019 20:03, Mike Leach wrote:
The v8 architecture defines the relationship between a PE, its optional ETM and a CTI. Unlike non-architectural CTIs which are implementation defined, this has a fixed set of connections which can therefore be represented as a simple tag in the device tree.
This patch defines the tags needed to create an entry for this PE/ETM/CTI relationship, and provides functionality to implement the connection model in the CTI driver.
Signed-off-by: Mike Leach mike.leach@linaro.org
.../coresight/coresight-cti-platform.c | 191 +++++++++++++++++- drivers/hwtracing/coresight/coresight-cti.c | 10 +- drivers/hwtracing/coresight/coresight-cti.h | 6 + 3 files changed, 200 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c index 068932ac9e90..2fb7d356e9c9 100644 --- a/drivers/hwtracing/coresight/coresight-cti-platform.c +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c @@ -7,8 +7,38 @@
#include "coresight-cti.h"
-#ifdef CONFIG_OF +/* Number of CTI signals in the v8 architecturally defined connection */ +#define NR_V8PE_IN_SIGS 2 +#define NR_V8PE_OUT_SIGS 3 +#define NR_V8ETM_INOUT_SIGS 4
+/* 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"
+/*
- Find a registered coresight device from a device fwnode.
- The node info is associated with the AMBA parent, but the
- csdev keeps a copy so iterate round the coresight bus to
- find the device.
- */
+static struct coresight_device * +cti_get_assoc_csdev_by_fwnode(struct fwnode_handle *r_fwnode) +{
struct device *dev;struct coresight_device *csdev = NULL;dev = bus_find_device(&coresight_bustype, NULL, r_fwnode,coresight_device_fwnode_match);if (dev) {csdev = to_coresight_device(dev);put_device(dev);}return csdev;+}
+#ifdef CONFIG_OF /*
- CTI can be bound to a CPU, or a system device.
- Reflect this in the return value and do not default to cpu 0
@@ -29,6 +59,157 @@ static int of_cti_get_cpu(const struct device_node *node) return (cpu < 0) ? -1 : cpu; }
+static int of_cti_create_v8_etm_connection(struct cti_drvdata *drvdata,
struct device_node *np)+{
struct cti_trig_grp *etm_in = NULL, *etm_out = NULL;int *etm_sig_in_types = NULL, *etm_sig_out_types = NULL;int ret = 0, i;struct device_node *cs_np;const char *assoc_name = NULL;struct coresight_device *csdev;struct fwnode_handle *r_fwnode;/* Can optionally have an etm node - return if not */cs_np = of_parse_phandle(np, CTI_DT_CSDEV_ASSOC, 0);if (!cs_np)return 0;/* allocate memory */ret = -ENOMEM;etm_in = kzalloc(sizeof(struct cti_trig_grp), GFP_KERNEL);if (!etm_in)goto of_create_v8_etm_out;etm_out = kzalloc(sizeof(struct cti_trig_grp), GFP_KERNEL);if (!etm_out)goto of_create_v8_etm_out;etm_sig_in_types =kzalloc(sizeof(int) * NR_V8ETM_INOUT_SIGS, GFP_KERNEL);if (!etm_sig_in_types)goto of_create_v8_etm_out;etm_sig_out_types =kzalloc(sizeof(int) * NR_V8ETM_INOUT_SIGS, GFP_KERNEL);Could we not use devm_kzalloc() everywhere above ? ^
Also, I assume this would change if you re-shuffle the struct definitions.
re-structured in earlier patch to use devm and now calls a common create fn.
if (!etm_sig_out_types)goto of_create_v8_etm_out;/* build connection data */etm_in->nr_sigs = NR_V8ETM_INOUT_SIGS;etm_in->used_mask = 0xF0; /* sigs <4,5,6,7> */The used bits are 7-4, while the number of SIGS = 4. Does the nr_sigs need to be 8 and indicate that sigs 7-4 are only connected ?
nr_sigs is for _this_ etm connection, the mask shows which they are, not the total number in the cti. Some of sigs 0-3 are connected to the CPU.
etm_in->sig_types = etm_sig_in_types;etm_out->nr_sigs = NR_V8ETM_INOUT_SIGS;etm_out->used_mask = 0xF0; /* sigs <4,5,6,7> */etm_out->sig_types = etm_sig_out_types;
for (i = 0; i < NR_V8ETM_INOUT_SIGS; i++) {etm_sig_in_types[i] = CTITRIG_ETM_EXTOUT;etm_sig_out_types[i] = CTITRIG_ETM_EXTIN;}
r_fwnode = of_fwnode_handle(cs_np);csdev = cti_get_assoc_csdev_by_fwnode(r_fwnode);if (csdev)assoc_name = dev_name(&csdev->dev);elseassoc_name = cs_np->full_name;ret = cti_add_connection_entry(drvdata, etm_in, etm_out,csdev, assoc_name);+of_create_v8_etm_out:
of_node_put(cs_np);if (ret) {kfree(etm_in);kfree(etm_out);kfree(etm_sig_in_types);kfree(etm_sig_out_types);}If you use devm_kzalloc(), you could ignore freeing them out in the error path.
return ret;+}
+/*
- Create an architecturally defined v8 connection
- must have a cpu, can have an ETM.
- */
+static int of_cti_create_v8_connections(struct device *dev,
struct cti_drvdata *drvdata,struct device_node *np)+{
struct cti_device *cti_dev = &drvdata->ctidev;struct cti_trig_grp *in = NULL, *out = NULL;int cpuid = 0;char cpu_name_str[16];int *sig_in_types = NULL, *sig_out_types = NULL;int ret = -ENOMEM;/* Must have a cpu node */cpuid = of_cti_get_cpu(np);if (cpuid < 0) {dev_warn(dev, "CTI v8 DT binding no cpu\n");return -EINVAL;}cti_dev->cpu = cpuid;/* Allocate the v8 cpu memory */in = kzalloc(sizeof(struct cti_trig_grp), GFP_KERNEL);if (!in)goto of_create_v8_out;out = kzalloc(sizeof(struct cti_trig_grp), GFP_KERNEL);if (!out)goto of_create_v8_out;sig_in_types = kzalloc(sizeof(int) * NR_V8PE_IN_SIGS, GFP_KERNEL);if (!sig_in_types)goto of_create_v8_out;sig_out_types = kzalloc(sizeof(int) * NR_V8PE_OUT_SIGS, GFP_KERNEL);if (!sig_out_types)goto of_create_v8_out;/* Create the v8 PE CTI connection */in->nr_sigs = NR_V8PE_IN_SIGS;in->used_mask = 0x3; /* sigs <0 1> */in->sig_types = sig_in_types;sig_in_types[0] = CTITRIG_PE_DBGTRIGGER;sig_in_types[1] = CTITRIG_PE_PMUIRQ;out->nr_sigs = NR_V8PE_OUT_SIGS;out->used_mask = 0x7; /* sigs <0 1 2 > */out->sig_types = sig_out_types;sig_out_types[0] = CTITRIG_PE_EDBGREQ;sig_out_types[1] = CTITRIG_PE_DBGRESTART;sig_out_types[2] = CTITRIG_PE_CTIIRQ;scnprintf(cpu_name_str, 16, "cpu%d", cpuid);s/16/sizeof(cpu_name_str) ?
OK
ret = cti_add_connection_entry(drvdata, in, out, NULL, cpu_name_str);if (ret)goto of_create_v8_out;/* Create the v8 ETM associated connection */ret = of_cti_create_v8_etm_connection(drvdata, np);if (ret) {/** Need to remove PE connection just created which* frees all associated memory.*/cti_free_conn_info(drvdata);return ret;}/* filter pe_edbgreq - PE trigout sig <0> */drvdata->config.trig_out_filter |= 0x1;+of_create_v8_out:
if (ret) {kfree(in);kfree(out);kfree(sig_in_types);kfree(sig_out_types);}return ret;+}
- /* get the hardware configuration & connection data. */ int of_cti_get_hw_data(struct device *dev, struct device_node *np,
@@ -37,6 +218,13 @@ 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)) {I would prefer to use fwnode_property_* variants of the property accessors, wherever possible, thinking about the "ACPI" support picture. Of course, I understand that, we have not discussed about the bindings for the connections and stuff. But if we reuse the DT property names, we are all set for it, rather than a rework and we don't loose much here.
The Of/ACPI split is curently in the coresight_cti_get_platform_data() fn, and the of fns here are generally passed in an of_node. Looking at the differences in the main coresight platform between OF/ACPI I wasn't confident that I could predict what structures were likely for ACPI, and at what point they would necessarily diverge, and common code becomes unsupportabel / maintainable. For now I prefer to write code for what is defined and re-work later once the complete problem is understood (by me at least!).
Thanks
Mike
rc = of_cti_create_v8_connections(dev, drvdata, np);if (rc)return rc;}Suzuki
-- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK