| Message ID | 20191119231912.12768-7-mike.leach@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | CoreSight CTI Driver | expand |
On Tue, Nov 19, 2019 at 11:19:04PM +0000, 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 | 205 ++++++++++++++++++ > 1 file changed, 205 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > index 665be86c585d..790dd30b85f5 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-platform.c > +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c > @@ -3,10 +3,208 @@ > * Copyright (c) 2019, The Linaro Limited. All rights reserved. > */ > > +#include <dt-bindings/arm/coresight-cti-dt.h> > #include <linux/of.h> > > #include "coresight-cti.h" > > +/* 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" > + > +/* > + * 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_by_fwnode(&coresight_bustype, r_fwnode); > + 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. > + * CPU can be declared at the device top level or in a connections node > + * so need to check relative to node not device. > + */ > +static int of_cti_get_cpu_at_node(const struct device_node *node) > +{ > + int cpu; > + struct device_node *dn; > + > + if (node == NULL) > + return -1; > + > + dn = of_parse_phandle(node, "cpu", 0); > + /* CTI affinity defaults to no cpu */ > + if (!dn) > + return -1; > + cpu = of_cpu_node_to_id(dn); > + of_node_put(dn); > + > + /* No Affinity if no cpu nodes are found */ > + return (cpu < 0) ? -1 : cpu; > +} > + > +static const char *of_cti_get_node_name(const struct device_node *node) > +{ > + if (node) > + return node->full_name; > + return "unknown"; > +} > +#else > +static int of_cti_get_cpu_at_node(const struct device_node *node) > +{ > + return -1; > +} > + > +static const char *of_cti_get_node_name(const struct device_node *node) > +{ > + return "unknown"; > +} > +#endif > + > +static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) > +{ > + if (is_of_node(fwnode)) > + return of_cti_get_cpu_at_node(to_of_node(fwnode)); > + return -1; > +} > + > +static const char *cti_plat_get_node_name(struct fwnode_handle *fwnode) > +{ > + if (is_of_node(fwnode)) > + return of_cti_get_node_name(to_of_node(fwnode)); > + return "unknown"; > +} > + > +static int cti_plat_create_v8_etm_connection(struct device *dev, > + struct cti_drvdata *drvdata) > +{ > + int ret = -ENOMEM, i; > + struct fwnode_handle *root_fwnode, *cs_fwnode; > + const char *assoc_name = NULL; > + struct coresight_device *csdev; > + struct cti_trig_con *tc = NULL; > + > + root_fwnode = dev_fwnode(dev); > + if (IS_ERR_OR_NULL(root_fwnode)) > + return -EINVAL; > + > + /* Can optionally have an etm node - return if not */ > + cs_fwnode = fwnode_find_reference(root_fwnode, CTI_DT_CSDEV_ASSOC, 0); > + if (IS_ERR_OR_NULL(cs_fwnode)) > + return 0; > + > + /* allocate memory */ > + tc = cti_allocate_trig_con(dev, NR_V8ETM_INOUT_SIGS, > + NR_V8ETM_INOUT_SIGS); > + if (!tc) > + goto create_v8_etm_out; > + > + /* build connection data */ > + tc->con_in->used_mask = 0xF0; /* sigs <4,5,6,7> */ > + tc->con_out->used_mask = 0xF0; /* sigs <4,5,6,7> */ > + > + /* > + * The EXTOUT type signals from the ETM are connected to a set of input > + * triggers on the CTI, the EXTIN being connected to output triggers. > + */ > + for (i = 0; i < NR_V8ETM_INOUT_SIGS; i++) { > + tc->con_in->sig_types[i] = ETM_EXTOUT; > + tc->con_out->sig_types[i] = ETM_EXTIN; > + } > + > + /* > + * We look to see if the ETM coresight device associated with this > + * handle has been registered with the system - i.e. probed before > + * this CTI. If so csdev will be non NULL and we can use the device > + * name and pass the csdev to the connection entry function where > + * the association will be recorded. > + * If not, then simply record the name in the connection data, the > + * probing of the ETM will call into the CTI driver API to update the > + * association then. > + */ > + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); > + if (csdev) > + assoc_name = dev_name(&csdev->dev); > + else > + assoc_name = cti_plat_get_node_name(cs_fwnode); > + ret = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); > + > +create_v8_etm_out: > + fwnode_handle_put(cs_fwnode); > + return ret; > +} > + > +/* > + * Create an architecturally defined v8 connection > + * must have a cpu, can have an ETM. > + */ > +static int cti_plat_create_v8_connections(struct device *dev, > + struct cti_drvdata *drvdata) > +{ > + struct cti_device *cti_dev = &drvdata->ctidev; > + struct cti_trig_con *tc = NULL; > + int cpuid = 0; > + char cpu_name_str[16]; > + int ret = -ENOMEM; > + > + /* Must have a cpu node */ > + cpuid = cti_plat_get_cpu_at_node(dev_fwnode(dev)); > + if (cpuid < 0) { > + dev_warn(dev, "CTI v8 DT binding no cpu\n"); > + return -EINVAL; > + } > + cti_dev->cpu = cpuid; > + > + /* Allocate the v8 cpu connection memory */ > + tc = cti_allocate_trig_con(dev, NR_V8PE_IN_SIGS, NR_V8PE_OUT_SIGS); > + if (!tc) > + goto of_create_v8_out; > + > + /* Set the v8 PE CTI connection data */ > + tc->con_in->used_mask = 0x3; /* sigs <0 1> */ > + tc->con_in->sig_types[0] = PE_DBGTRIGGER; > + tc->con_in->sig_types[1] = PE_PMUIRQ; > + tc->con_out->used_mask = 0x7; /* sigs <0 1 2 > */ > + tc->con_out->sig_types[0] = PE_EDBGREQ; > + tc->con_out->sig_types[1] = PE_DBGRESTART; > + tc->con_out->sig_types[2] = PE_CTIIRQ; > + scnprintf(cpu_name_str, sizeof(cpu_name_str), "cpu%d", cpuid); > + > + ret = cti_add_connection_entry(dev, drvdata, tc, NULL, cpu_name_str); > + if (ret) > + goto of_create_v8_out; > + > + /* Create the v8 ETM associated connection */ > + ret = cti_plat_create_v8_etm_connection(dev, drvdata); > + if (ret) > + goto of_create_v8_out; > + > + /* filter pe_edbgreq - PE trigout sig <0> */ > + drvdata->config.trig_out_filter |= 0x1; > + > +of_create_v8_out: > + return ret; > +} > + > /* get the hardware configuration & connection data. */ > int cti_plat_get_hw_data(struct device *dev, > struct cti_drvdata *drvdata) > @@ -14,6 +212,13 @@ int cti_plat_get_hw_data(struct device *dev, > int rc = 0; > struct cti_device *cti_dev = &drvdata->ctidev; > > + /* check for a v8 architectural CTI device */ > + if (device_property_read_bool(dev, CTI_DT_V8ARCH)) { > + rc = cti_plat_create_v8_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) > rc = cti_add_default_connection(dev, drvdata); Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> > -- > 2.17.1 >
On 19/11/2019 23:19, 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 | 205 ++++++++++++++++++ > 1 file changed, 205 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > index 665be86c585d..790dd30b85f5 100644 > --- a/drivers/hwtracing/coresight/coresight-cti-platform.c > +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c > @@ -3,10 +3,208 @@ > * Copyright (c) 2019, The Linaro Limited. All rights reserved. > */ > > +#include <dt-bindings/arm/coresight-cti-dt.h> > #include <linux/of.h> > > #include "coresight-cti.h" > > +/* 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" > + > +/* > + * 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) To be frank this has nothing to do with the CTI and is in a way a good candidate for a CoreSight generic function. We do similar stuff in coresight_fixup_device_conns(). So this could be : struct coresight_device * coresight_find_device_by_fwnode(const struct fwnode_handle *fwnode) > +{ > + struct device *dev; > + struct coresight_device *csdev = NULL; > + > + dev = bus_find_device_by_fwnode(&coresight_bustype, r_fwnode); > + if (dev) { > + csdev = to_coresight_device(dev); > + put_device(dev); > + } > + return csdev; > +} > + And used in coresight_fixup_conns(). > +#ifdef CONFIG_OF > +/* > + * CTI can be bound to a CPU, or a system device. > + * CPU can be declared at the device top level or in a connections node > + * so need to check relative to node not device. > + */ > +static int of_cti_get_cpu_at_node(const struct device_node *node) > +{ > + int cpu; > + struct device_node *dn; > + > + if (node == NULL) > + return -1; > + > + dn = of_parse_phandle(node, "cpu", 0); > + /* CTI affinity defaults to no cpu */ > + if (!dn) > + return -1; > + cpu = of_cpu_node_to_id(dn); > + of_node_put(dn); > + > + /* No Affinity if no cpu nodes are found */ > + return (cpu < 0) ? -1 : cpu; > +} > + > +static const char *of_cti_get_node_name(const struct device_node *node) > +{ > + if (node) > + return node->full_name; > + return "unknown"; > +} > +#else > +static int of_cti_get_cpu_at_node(const struct device_node *node) > +{ > + return -1; > +} > + > +static const char *of_cti_get_node_name(const struct device_node *node) > +{ > + return "unknown"; > +} > +#endif > + > +static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) > +{ You may simply reuse coresight_get_cpu() below, instead of adding this duplicate set of functions. See below. > +static int cti_plat_create_v8_etm_connection(struct device *dev, > + struct cti_drvdata *drvdata) > +{ > + int ret = -ENOMEM, i; > + struct fwnode_handle *root_fwnode, *cs_fwnode; > + const char *assoc_name = NULL; > + struct coresight_device *csdev; > + struct cti_trig_con *tc = NULL; > + > + root_fwnode = dev_fwnode(dev); > + if (IS_ERR_OR_NULL(root_fwnode)) > + return -EINVAL; > + > + /* Can optionally have an etm node - return if not */ > + cs_fwnode = fwnode_find_reference(root_fwnode, CTI_DT_CSDEV_ASSOC, 0); > + if (IS_ERR_OR_NULL(cs_fwnode)) > + return 0; > + > + /* allocate memory */ > + tc = cti_allocate_trig_con(dev, NR_V8ETM_INOUT_SIGS, > + NR_V8ETM_INOUT_SIGS); > + if (!tc) > + goto create_v8_etm_out; > + > + /* build connection data */ > + tc->con_in->used_mask = 0xF0; /* sigs <4,5,6,7> */ > + tc->con_out->used_mask = 0xF0; /* sigs <4,5,6,7> */ > + > + /* > + * The EXTOUT type signals from the ETM are connected to a set of input > + * triggers on the CTI, the EXTIN being connected to output triggers. > + */ > + for (i = 0; i < NR_V8ETM_INOUT_SIGS; i++) { > + tc->con_in->sig_types[i] = ETM_EXTOUT; > + tc->con_out->sig_types[i] = ETM_EXTIN; > + } > + > + /* > + * We look to see if the ETM coresight device associated with this > + * handle has been registered with the system - i.e. probed before > + * this CTI. If so csdev will be non NULL and we can use the device > + * name and pass the csdev to the connection entry function where > + * the association will be recorded. > + * If not, then simply record the name in the connection data, the > + * probing of the ETM will call into the CTI driver API to update the > + * association then. > + */ > + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); > + if (csdev) > + assoc_name = dev_name(&csdev->dev); Does it make sense to defer the probing until the ETM device turn up ? Its fine either way. > + else > + assoc_name = cti_plat_get_node_name(cs_fwnode); > + ret = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); > + > +create_v8_etm_out: > + fwnode_handle_put(cs_fwnode); > + return ret; > +} > + > +/* > + * Create an architecturally defined v8 connection > + * must have a cpu, can have an ETM. > + */ > +static int cti_plat_create_v8_connections(struct device *dev, > + struct cti_drvdata *drvdata) > +{ > + struct cti_device *cti_dev = &drvdata->ctidev; > + struct cti_trig_con *tc = NULL; > + int cpuid = 0; > + char cpu_name_str[16]; > + int ret = -ENOMEM; > + > + /* Must have a cpu node */ > + cpuid = cti_plat_get_cpu_at_node(dev_fwnode(dev)); Could we reuse coresight_get_cpu(dev) instead ? I understand that the ACPI bindings have not been defined and it may be slightly different from what we have now for the ETMs (i.e, ETM node as child of the CPU node). But I don't see why we can't force it for the CTIs either. In the worst case, you could still reuse the of_coresgith_get_cpu(dev) instead of writing your own for the OF case. > + if (cpuid < 0) { > + dev_warn(dev, "CTI v8 DT binding no cpu\n"); This may be better off without mentioning the DT. e.g, "CTI Arm v8 architected connection: missing CPU\n" > + return -EINVAL; > + } > + cti_dev->cpu = cpuid; > + > + /* Allocate the v8 cpu connection memory */ > + tc = cti_allocate_trig_con(dev, NR_V8PE_IN_SIGS, NR_V8PE_OUT_SIGS); > + if (!tc) > + goto of_create_v8_out; > + > + /* Set the v8 PE CTI connection data */ > + tc->con_in->used_mask = 0x3; /* sigs <0 1> */ > + tc->con_in->sig_types[0] = PE_DBGTRIGGER; > + tc->con_in->sig_types[1] = PE_PMUIRQ; > + tc->con_out->used_mask = 0x7; /* sigs <0 1 2 > */ > + tc->con_out->sig_types[0] = PE_EDBGREQ; > + tc->con_out->sig_types[1] = PE_DBGRESTART; > + tc->con_out->sig_types[2] = PE_CTIIRQ; > + scnprintf(cpu_name_str, sizeof(cpu_name_str), "cpu%d", cpuid); > + > + ret = cti_add_connection_entry(dev, drvdata, tc, NULL, cpu_name_str); > + if (ret) > + goto of_create_v8_out; > + > + /* Create the v8 ETM associated connection */ > + ret = cti_plat_create_v8_etm_connection(dev, drvdata); > + if (ret) > + goto of_create_v8_out; > + > + /* filter pe_edbgreq - PE trigout sig <0> */ > + drvdata->config.trig_out_filter |= 0x1; > + > +of_create_v8_out: > + return ret; > +} > + > /* get the hardware configuration & connection data. */ > int cti_plat_get_hw_data(struct device *dev, > struct cti_drvdata *drvdata) > @@ -14,6 +212,13 @@ int cti_plat_get_hw_data(struct device *dev, > int rc = 0; > struct cti_device *cti_dev = &drvdata->ctidev; > > + /* check for a v8 architectural CTI device */ minor nit: Check for Arm v8 architected CTI connection ? > + if (device_property_read_bool(dev, CTI_DT_V8ARCH)) { > + rc = cti_plat_create_v8_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) > rc = cti_add_default_connection(dev, drvdata); > Suzuki
Hi Suzuki, On Fri, 29 Nov 2019 at 11:33, Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com> wrote: > > On 19/11/2019 23:19, 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 | 205 ++++++++++++++++++ > > 1 file changed, 205 insertions(+) > > > > diff --git a/drivers/hwtracing/coresight/coresight-cti-platform.c b/drivers/hwtracing/coresight/coresight-cti-platform.c > > index 665be86c585d..790dd30b85f5 100644 > > --- a/drivers/hwtracing/coresight/coresight-cti-platform.c > > +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c > > @@ -3,10 +3,208 @@ > > * Copyright (c) 2019, The Linaro Limited. All rights reserved. > > */ > > > > +#include <dt-bindings/arm/coresight-cti-dt.h> > > #include <linux/of.h> > > > > #include "coresight-cti.h" > > > > +/* 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" > > + > > +/* > > + * 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) > > To be frank this has nothing to do with the CTI and is in a way > a good candidate for a CoreSight generic function. We do similar > stuff in coresight_fixup_device_conns(). So this could be : > > struct coresight_device * > coresight_find_device_by_fwnode(const struct fwnode_handle *fwnode) > > > +{ > > + struct device *dev; > > + struct coresight_device *csdev = NULL; > > + > > + dev = bus_find_device_by_fwnode(&coresight_bustype, r_fwnode); > > + if (dev) { > > + csdev = to_coresight_device(dev); > > + put_device(dev); > > + } > > + return csdev; > > +} > > + > > And used in coresight_fixup_conns(). > OK - I'll look at that. > > +#ifdef CONFIG_OF > > +/* > > + * CTI can be bound to a CPU, or a system device. > > + * CPU can be declared at the device top level or in a connections node > > + * so need to check relative to node not device. > > + */ > > +static int of_cti_get_cpu_at_node(const struct device_node *node) > > +{ > > + int cpu; > > + struct device_node *dn; > > + > > + if (node == NULL) > > + return -1; > > + > > + dn = of_parse_phandle(node, "cpu", 0); > > + /* CTI affinity defaults to no cpu */ > > + if (!dn) > > + return -1; > > + cpu = of_cpu_node_to_id(dn); > > + of_node_put(dn); > > + > > + /* No Affinity if no cpu nodes are found */ > > + return (cpu < 0) ? -1 : cpu; > > +} > > + > > +static const char *of_cti_get_node_name(const struct device_node *node) > > +{ > > + if (node) > > + return node->full_name; > > + return "unknown"; > > +} > > +#else > > +static int of_cti_get_cpu_at_node(const struct device_node *node) > > +{ > > + return -1; > > +} > > + > > +static const char *of_cti_get_node_name(const struct device_node *node) > > +{ > > + return "unknown"; > > +} > > +#endif > > + > > +static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) > > +{ > > You may simply reuse coresight_get_cpu() below, instead of adding this > duplicate set of functions. See below. > > No we can't. coresight_get_cpu gets the 'cpu' entry relative to the device node, this gets the 'cpu' relative to the supplied node. This is very important for the case where a none v8 architected PE is attached to a CTI. This will use the devicetree form:- cti@<addr> { [ some stuff ] trig_conns@1 { cpu = <&CPU0> [trigger signal connection info for this cpu] } } trig_conns is a child node and we must look for 'cpu' relative to it. > > +static int cti_plat_create_v8_etm_connection(struct device *dev, > > + struct cti_drvdata *drvdata) > > +{ > > + int ret = -ENOMEM, i; > > + struct fwnode_handle *root_fwnode, *cs_fwnode; > > + const char *assoc_name = NULL; > > + struct coresight_device *csdev; > > + struct cti_trig_con *tc = NULL; > > + > > + root_fwnode = dev_fwnode(dev); > > + if (IS_ERR_OR_NULL(root_fwnode)) > > + return -EINVAL; > > + > > + /* Can optionally have an etm node - return if not */ > > + cs_fwnode = fwnode_find_reference(root_fwnode, CTI_DT_CSDEV_ASSOC, 0); > > + if (IS_ERR_OR_NULL(cs_fwnode)) > > + return 0; > > + > > + /* allocate memory */ > > + tc = cti_allocate_trig_con(dev, NR_V8ETM_INOUT_SIGS, > > + NR_V8ETM_INOUT_SIGS); > > + if (!tc) > > + goto create_v8_etm_out; > > + > > + /* build connection data */ > > + tc->con_in->used_mask = 0xF0; /* sigs <4,5,6,7> */ > > + tc->con_out->used_mask = 0xF0; /* sigs <4,5,6,7> */ > > + > > + /* > > + * The EXTOUT type signals from the ETM are connected to a set of input > > + * triggers on the CTI, the EXTIN being connected to output triggers. > > + */ > > + for (i = 0; i < NR_V8ETM_INOUT_SIGS; i++) { > > + tc->con_in->sig_types[i] = ETM_EXTOUT; > > + tc->con_out->sig_types[i] = ETM_EXTIN; > > + } > > + > > + /* > > + * We look to see if the ETM coresight device associated with this > > + * handle has been registered with the system - i.e. probed before > > + * this CTI. If so csdev will be non NULL and we can use the device > > + * name and pass the csdev to the connection entry function where > > + * the association will be recorded. > > + * If not, then simply record the name in the connection data, the > > + * probing of the ETM will call into the CTI driver API to update the > > + * association then. > > + */ > > + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); > > + if (csdev) > > + assoc_name = dev_name(&csdev->dev); > > Does it make sense to defer the probing until the ETM device turn up ? > Its fine either way. > Not really as the ETM is optional but the PE still has a CTI. > > + else > > + assoc_name = cti_plat_get_node_name(cs_fwnode); > > + ret = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); > > + > > +create_v8_etm_out: > > + fwnode_handle_put(cs_fwnode); > > + return ret; > > +} > > + > > +/* > > + * Create an architecturally defined v8 connection > > + * must have a cpu, can have an ETM. > > + */ > > +static int cti_plat_create_v8_connections(struct device *dev, > > + struct cti_drvdata *drvdata) > > +{ > > + struct cti_device *cti_dev = &drvdata->ctidev; > > + struct cti_trig_con *tc = NULL; > > + int cpuid = 0; > > + char cpu_name_str[16]; > > + int ret = -ENOMEM; > > + > > + /* Must have a cpu node */ > > + cpuid = cti_plat_get_cpu_at_node(dev_fwnode(dev)); > > Could we reuse coresight_get_cpu(dev) instead ? I understand that the > ACPI bindings have not been defined and it may be slightly different > from what we have now for the ETMs (i.e, ETM node as child of the CPU > node). But I don't see why we can't force it for the CTIs either. > In the worst case, you could still reuse the of_coresgith_get_cpu(dev) > instead of writing your own for the OF case. > See comments above - in theory here we could use coresight_get_cpu(), but for consistency it is better to use that same function throughout in case someone decided to "fix" it later. I probably need to beef up the comments around cti_plat_get_cpu_at_node / of_cti_get_cpu_at_node. > > > + if (cpuid < 0) { > > + dev_warn(dev, "CTI v8 DT binding no cpu\n"); > > This may be better off without mentioning the DT. e.g, > > "CTI Arm v8 architected connection: missing CPU\n" > OK > > > + return -EINVAL; > > + } > > + cti_dev->cpu = cpuid; > > + > > + /* Allocate the v8 cpu connection memory */ > > + tc = cti_allocate_trig_con(dev, NR_V8PE_IN_SIGS, NR_V8PE_OUT_SIGS); > > + if (!tc) > > + goto of_create_v8_out; > > + > > + /* Set the v8 PE CTI connection data */ > > + tc->con_in->used_mask = 0x3; /* sigs <0 1> */ > > + tc->con_in->sig_types[0] = PE_DBGTRIGGER; > > + tc->con_in->sig_types[1] = PE_PMUIRQ; > > + tc->con_out->used_mask = 0x7; /* sigs <0 1 2 > */ > > + tc->con_out->sig_types[0] = PE_EDBGREQ; > > + tc->con_out->sig_types[1] = PE_DBGRESTART; > > + tc->con_out->sig_types[2] = PE_CTIIRQ; > > + scnprintf(cpu_name_str, sizeof(cpu_name_str), "cpu%d", cpuid); > > + > > + ret = cti_add_connection_entry(dev, drvdata, tc, NULL, cpu_name_str); > > + if (ret) > > + goto of_create_v8_out; > > + > > + /* Create the v8 ETM associated connection */ > > + ret = cti_plat_create_v8_etm_connection(dev, drvdata); > > + if (ret) > > + goto of_create_v8_out; > > + > > + /* filter pe_edbgreq - PE trigout sig <0> */ > > + drvdata->config.trig_out_filter |= 0x1; > > + > > +of_create_v8_out: > > + return ret; > > +} > > + > > /* get the hardware configuration & connection data. */ > > int cti_plat_get_hw_data(struct device *dev, > > struct cti_drvdata *drvdata) > > @@ -14,6 +212,13 @@ int cti_plat_get_hw_data(struct device *dev, > > int rc = 0; > > struct cti_device *cti_dev = &drvdata->ctidev; > > > > + /* check for a v8 architectural CTI device */ > minor nit: Check for Arm v8 architected CTI connection ? > > > + if (device_property_read_bool(dev, CTI_DT_V8ARCH)) { > > + rc = cti_plat_create_v8_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) > > rc = cti_add_default_connection(dev, drvdata); > > > > > Suzuki Thanks Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
On 03/12/2019 10:59, Mike Leach wrote: > Hi Suzuki, > > On Fri, 29 Nov 2019 at 11:33, Suzuki Kuruppassery Poulose > <suzuki.poulose@arm.com> wrote: >> >> On 19/11/2019 23:19, 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> >>> --- >>> +#ifdef CONFIG_OF >>> +/* >>> + * CTI can be bound to a CPU, or a system device. >>> + * CPU can be declared at the device top level or in a connections node >>> + * so need to check relative to node not device. >>> + */ >>> +static int of_cti_get_cpu_at_node(const struct device_node *node) >>> +{ >>> + int cpu; >>> + struct device_node *dn; >>> + >>> + if (node == NULL) >>> + return -1; >>> + >>> + dn = of_parse_phandle(node, "cpu", 0); >>> + /* CTI affinity defaults to no cpu */ >>> + if (!dn) >>> + return -1; >>> + cpu = of_cpu_node_to_id(dn); >>> + of_node_put(dn); >>> + >>> + /* No Affinity if no cpu nodes are found */ >>> + return (cpu < 0) ? -1 : cpu; >>> +} >>> + >>> +static const char *of_cti_get_node_name(const struct device_node *node) >>> +{ >>> + if (node) >>> + return node->full_name; >>> + return "unknown"; >>> +} >>> +#else >>> +static int of_cti_get_cpu_at_node(const struct device_node *node) >>> +{ >>> + return -1; >>> +} >>> + >>> +static const char *of_cti_get_node_name(const struct device_node *node) >>> +{ >>> + return "unknown"; >>> +} >>> +#endif >>> + >>> +static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) >>> +{ >> >> You may simply reuse coresight_get_cpu() below, instead of adding this >> duplicate set of functions. See below. >> >> > > No we can't. coresight_get_cpu gets the 'cpu' entry relative to the > device node, this gets the 'cpu' relative to the supplied node. > This is very important for the case where a none v8 architected PE is > attached to a CTI. This will use the devicetree form:- > > cti@<addr> { > [ some stuff ] > trig_conns@1 { > cpu = <&CPU0> > [trigger signal connection info for this cpu] > } > } > > trig_conns is a child node and we must look for 'cpu' relative to it. Ok. May be we could refactor the function to find the 'CPU' node relative to the given "fwnode" and let the coresight_get_cpu() use it ? int coresight_get_cpu(struct device *dev) { return coresight_get_fwnode_cpu(dev_fwnode(dev)); } That way it is clear what we are dealing with. i.e, fwnode of any level (device or an intermediate node). >>> + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); >>> + if (csdev) >>> + assoc_name = dev_name(&csdev->dev); >> >> Does it make sense to defer the probing until the ETM device turn up ? >> Its fine either way. >> > > Not really as the ETM is optional but the PE still has a CTI. Ah, you're right. Please ignore my comment. Kind regards Suzuki
Hi Suzuki, On Tue, 3 Dec 2019 at 11:28, Suzuki Kuruppassery Poulose <suzuki.poulose@arm.com> wrote: > > On 03/12/2019 10:59, Mike Leach wrote: > > Hi Suzuki, > > > > On Fri, 29 Nov 2019 at 11:33, Suzuki Kuruppassery Poulose > > <suzuki.poulose@arm.com> wrote: > >> > >> On 19/11/2019 23:19, 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> > >>> --- > > > >>> +#ifdef CONFIG_OF > >>> +/* > >>> + * CTI can be bound to a CPU, or a system device. > >>> + * CPU can be declared at the device top level or in a connections node > >>> + * so need to check relative to node not device. > >>> + */ > >>> +static int of_cti_get_cpu_at_node(const struct device_node *node) > >>> +{ > >>> + int cpu; > >>> + struct device_node *dn; > >>> + > >>> + if (node == NULL) > >>> + return -1; > >>> + > >>> + dn = of_parse_phandle(node, "cpu", 0); > >>> + /* CTI affinity defaults to no cpu */ > >>> + if (!dn) > >>> + return -1; > >>> + cpu = of_cpu_node_to_id(dn); > >>> + of_node_put(dn); > >>> + > >>> + /* No Affinity if no cpu nodes are found */ > >>> + return (cpu < 0) ? -1 : cpu; > >>> +} > >>> + > >>> +static const char *of_cti_get_node_name(const struct device_node *node) > >>> +{ > >>> + if (node) > >>> + return node->full_name; > >>> + return "unknown"; > >>> +} > >>> +#else > >>> +static int of_cti_get_cpu_at_node(const struct device_node *node) > >>> +{ > >>> + return -1; > >>> +} > >>> + > >>> +static const char *of_cti_get_node_name(const struct device_node *node) > >>> +{ > >>> + return "unknown"; > >>> +} > >>> +#endif > >>> + > >>> +static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) > >>> +{ > >> > >> You may simply reuse coresight_get_cpu() below, instead of adding this > >> duplicate set of functions. See below. > >> > >> > > > > No we can't. coresight_get_cpu gets the 'cpu' entry relative to the > > device node, this gets the 'cpu' relative to the supplied node. > > This is very important for the case where a none v8 architected PE is > > attached to a CTI. This will use the devicetree form:- > > > > cti@<addr> { > > [ some stuff ] > > trig_conns@1 { > > cpu = <&CPU0> > > [trigger signal connection info for this cpu] > > } > > } > > > > trig_conns is a child node and we must look for 'cpu' relative to it. > > Ok. May be we could refactor the function to find the 'CPU' node > relative to the given "fwnode" and let the coresight_get_cpu() use it ? > > int coresight_get_cpu(struct device *dev) > { > return coresight_get_fwnode_cpu(dev_fwnode(dev)); > } > > That way it is clear what we are dealing with. i.e, fwnode of any level > (device or an intermediate node). > At present the generic coresight_get_cpu() deals with both DT and ACPI bindings. To refactor this would require re-factoring both binding types - and at present we have no definition for ACPI bindings for CTI and hence no way of knowing how the embedded cpu node is going to be represented. I think we have to take just the DT binding as is for now as a CTI specific element and consider if it is worth re-factoring once the ACPI bindings are defined Regards Mike > >>> + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); > >>> + if (csdev) > >>> + assoc_name = dev_name(&csdev->dev); > >> > >> Does it make sense to defer the probing until the ETM device turn up ? > >> Its fine either way. > >> > > > > Not really as the ETM is optional but the PE still has a CTI. > > Ah, you're right. Please ignore my comment. > > Kind regards > Suzuki -- 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 665be86c585d..790dd30b85f5 100644 --- a/drivers/hwtracing/coresight/coresight-cti-platform.c +++ b/drivers/hwtracing/coresight/coresight-cti-platform.c @@ -3,10 +3,208 @@ * Copyright (c) 2019, The Linaro Limited. All rights reserved. */ +#include <dt-bindings/arm/coresight-cti-dt.h> #include <linux/of.h> #include "coresight-cti.h" +/* 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" + +/* + * 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_by_fwnode(&coresight_bustype, r_fwnode); + 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. + * CPU can be declared at the device top level or in a connections node + * so need to check relative to node not device. + */ +static int of_cti_get_cpu_at_node(const struct device_node *node) +{ + int cpu; + struct device_node *dn; + + if (node == NULL) + return -1; + + dn = of_parse_phandle(node, "cpu", 0); + /* CTI affinity defaults to no cpu */ + if (!dn) + return -1; + cpu = of_cpu_node_to_id(dn); + of_node_put(dn); + + /* No Affinity if no cpu nodes are found */ + return (cpu < 0) ? -1 : cpu; +} + +static const char *of_cti_get_node_name(const struct device_node *node) +{ + if (node) + return node->full_name; + return "unknown"; +} +#else +static int of_cti_get_cpu_at_node(const struct device_node *node) +{ + return -1; +} + +static const char *of_cti_get_node_name(const struct device_node *node) +{ + return "unknown"; +} +#endif + +static int cti_plat_get_cpu_at_node(struct fwnode_handle *fwnode) +{ + if (is_of_node(fwnode)) + return of_cti_get_cpu_at_node(to_of_node(fwnode)); + return -1; +} + +static const char *cti_plat_get_node_name(struct fwnode_handle *fwnode) +{ + if (is_of_node(fwnode)) + return of_cti_get_node_name(to_of_node(fwnode)); + return "unknown"; +} + +static int cti_plat_create_v8_etm_connection(struct device *dev, + struct cti_drvdata *drvdata) +{ + int ret = -ENOMEM, i; + struct fwnode_handle *root_fwnode, *cs_fwnode; + const char *assoc_name = NULL; + struct coresight_device *csdev; + struct cti_trig_con *tc = NULL; + + root_fwnode = dev_fwnode(dev); + if (IS_ERR_OR_NULL(root_fwnode)) + return -EINVAL; + + /* Can optionally have an etm node - return if not */ + cs_fwnode = fwnode_find_reference(root_fwnode, CTI_DT_CSDEV_ASSOC, 0); + if (IS_ERR_OR_NULL(cs_fwnode)) + return 0; + + /* allocate memory */ + tc = cti_allocate_trig_con(dev, NR_V8ETM_INOUT_SIGS, + NR_V8ETM_INOUT_SIGS); + if (!tc) + goto create_v8_etm_out; + + /* build connection data */ + tc->con_in->used_mask = 0xF0; /* sigs <4,5,6,7> */ + tc->con_out->used_mask = 0xF0; /* sigs <4,5,6,7> */ + + /* + * The EXTOUT type signals from the ETM are connected to a set of input + * triggers on the CTI, the EXTIN being connected to output triggers. + */ + for (i = 0; i < NR_V8ETM_INOUT_SIGS; i++) { + tc->con_in->sig_types[i] = ETM_EXTOUT; + tc->con_out->sig_types[i] = ETM_EXTIN; + } + + /* + * We look to see if the ETM coresight device associated with this + * handle has been registered with the system - i.e. probed before + * this CTI. If so csdev will be non NULL and we can use the device + * name and pass the csdev to the connection entry function where + * the association will be recorded. + * If not, then simply record the name in the connection data, the + * probing of the ETM will call into the CTI driver API to update the + * association then. + */ + csdev = cti_get_assoc_csdev_by_fwnode(cs_fwnode); + if (csdev) + assoc_name = dev_name(&csdev->dev); + else + assoc_name = cti_plat_get_node_name(cs_fwnode); + ret = cti_add_connection_entry(dev, drvdata, tc, csdev, assoc_name); + +create_v8_etm_out: + fwnode_handle_put(cs_fwnode); + return ret; +} + +/* + * Create an architecturally defined v8 connection + * must have a cpu, can have an ETM. + */ +static int cti_plat_create_v8_connections(struct device *dev, + struct cti_drvdata *drvdata) +{ + struct cti_device *cti_dev = &drvdata->ctidev; + struct cti_trig_con *tc = NULL; + int cpuid = 0; + char cpu_name_str[16]; + int ret = -ENOMEM; + + /* Must have a cpu node */ + cpuid = cti_plat_get_cpu_at_node(dev_fwnode(dev)); + if (cpuid < 0) { + dev_warn(dev, "CTI v8 DT binding no cpu\n"); + return -EINVAL; + } + cti_dev->cpu = cpuid; + + /* Allocate the v8 cpu connection memory */ + tc = cti_allocate_trig_con(dev, NR_V8PE_IN_SIGS, NR_V8PE_OUT_SIGS); + if (!tc) + goto of_create_v8_out; + + /* Set the v8 PE CTI connection data */ + tc->con_in->used_mask = 0x3; /* sigs <0 1> */ + tc->con_in->sig_types[0] = PE_DBGTRIGGER; + tc->con_in->sig_types[1] = PE_PMUIRQ; + tc->con_out->used_mask = 0x7; /* sigs <0 1 2 > */ + tc->con_out->sig_types[0] = PE_EDBGREQ; + tc->con_out->sig_types[1] = PE_DBGRESTART; + tc->con_out->sig_types[2] = PE_CTIIRQ; + scnprintf(cpu_name_str, sizeof(cpu_name_str), "cpu%d", cpuid); + + ret = cti_add_connection_entry(dev, drvdata, tc, NULL, cpu_name_str); + if (ret) + goto of_create_v8_out; + + /* Create the v8 ETM associated connection */ + ret = cti_plat_create_v8_etm_connection(dev, drvdata); + if (ret) + goto of_create_v8_out; + + /* filter pe_edbgreq - PE trigout sig <0> */ + drvdata->config.trig_out_filter |= 0x1; + +of_create_v8_out: + return ret; +} + /* get the hardware configuration & connection data. */ int cti_plat_get_hw_data(struct device *dev, struct cti_drvdata *drvdata) @@ -14,6 +212,13 @@ int cti_plat_get_hw_data(struct device *dev, int rc = 0; struct cti_device *cti_dev = &drvdata->ctidev; + /* check for a v8 architectural CTI device */ + if (device_property_read_bool(dev, CTI_DT_V8ARCH)) { + rc = cti_plat_create_v8_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) rc = cti_add_default_connection(dev, drvdata);
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 | 205 ++++++++++++++++++ 1 file changed, 205 insertions(+) -- 2.17.1