On Wed, Jan 09, 2019 at 10:54:36PM +0000, Mike Leach wrote:
Adds in code to the CoreSight core to prepare for the addition of the CTI driver.
Adds functionality to add cross reference association between CTI and other CoreSight devices.
Adds functionality to enable associated CTI when CoreSight devices are enabled by the trace path code.
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight.c | 51 ++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 2b0df1a0a8df..01ad708ca7a7 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -214,6 +214,36 @@ void coresight_disclaim_device(void __iomem *base) CS_LOCK(base); } +/* enable or disable an associated CTI device of the supplied CS device */ +static void +coresight_control_assoc_ectdev(struct coresight_device *csdev, int enable) +{
- int ect_ret = 0;
- struct coresight_device *ect_csdev = csdev->ect_dev;
- if (!ect_csdev)
return;
- if (enable) {
if (ect_ops(ect_csdev)->enable)
ect_ret = ect_ops(ect_csdev)->enable(ect_csdev, NULL);
- } else {
if (ect_ops(ect_csdev)->disable)
ect_ret = ect_ops(ect_csdev)->disable(ect_csdev, NULL);
- }
We can't say if this code is correct since we don't know what the enable and disable functions do. I suggest starting by introducing the driver and then using it.
- dev_info(&csdev->dev, "%s assoc CTI %s for %s\n",
enable ? "Enabling" : "Disabling",
dev_name(&ect_csdev->dev), dev_name(&csdev->dev));
- /* even if the control failed - we don't want to fail possible trace
* capture.
*/
s/even/Even
And the format for multi-line comments isn't correct. It should be:
/* * Even if the control failed - we don't want to fail possible trace * capture. */
Shouldn't we want the trace session to fail if enabling the CTI failed? In my opinion we should.
- if (ect_ret)
dev_info(&csdev->dev, "Associated ECT device (%s) %s failed\n",
dev_name(&ect_csdev->dev), enable ? "enable" : "disable");
+}
static int coresight_enable_sink(struct coresight_device *csdev, u32 mode, void *data) { @@ -227,6 +257,7 @@ static int coresight_enable_sink(struct coresight_device *csdev, ret = sink_ops(csdev)->enable(csdev, mode, data); if (ret) return ret;
csdev->enable = true; }coresight_control_assoc_ectdev(csdev, 1);
@@ -240,6 +271,7 @@ static void coresight_disable_sink(struct coresight_device *csdev) if (atomic_dec_return(csdev->refcnt) == 0) { if (sink_ops(csdev)->disable) { sink_ops(csdev)->disable(csdev);
} }coresight_control_assoc_ectdev(csdev, 0); csdev->enable = false;
@@ -277,6 +309,7 @@ static int coresight_enable_link(struct coresight_device *csdev, atomic_dec(&csdev->refcnt[refport]); return ret; }
} }coresight_control_assoc_ectdev(csdev, 1);
@@ -312,8 +345,10 @@ static void coresight_disable_link(struct coresight_device *csdev, } if (atomic_dec_return(&csdev->refcnt[refport]) == 0) {
if (link_ops(csdev)->disable)
if (link_ops(csdev)->disable) { link_ops(csdev)->disable(csdev, inport, outport);
coresight_control_assoc_ectdev(csdev, 0);
}}
for (i = 0; i < nr_conns; i++) @@ -338,6 +373,7 @@ static int coresight_enable_source(struct coresight_device *csdev, u32 mode) ret = source_ops(csdev)->enable(csdev, NULL, mode); if (ret) return ret;
} csdev->enable = true; }coresight_control_assoc_ectdev(csdev, 1);
@@ -360,6 +396,7 @@ static bool coresight_disable_source(struct coresight_device *csdev) if (atomic_dec_return(csdev->refcnt) == 0) { if (source_ops(csdev)->disable) source_ops(csdev)->disable(csdev, NULL);
csdev->enable = false; } return !csdev->enable;coresight_control_assoc_ectdev(csdev, 0);
@@ -743,6 +780,7 @@ int coresight_enable(struct coresight_device *csdev) goto out; }
- dev_info(&csdev->dev, "calling coresight_build_path()\n");
Please remove.
path = coresight_build_path(csdev, sink); if (IS_ERR(path)) { pr_err("building path(s) failed\n"); @@ -750,10 +788,12 @@ int coresight_enable(struct coresight_device *csdev) goto out; }
- dev_info(&csdev->dev, "calling coresight_enable_path()\n");
Please remove.
ret = coresight_enable_path(path, CS_MODE_SYSFS, NULL); if (ret) goto err_path;
- dev_info(&csdev->dev, "calling coresight_enable_source()\n");
Please remove.
ret = coresight_enable_source(csdev, CS_MODE_SYSFS); if (ret) goto err_source; @@ -924,12 +964,20 @@ static struct device_type coresight_dev_type[] = { { .name = "helper", },
- {
.name = "ect",
- },
}; static void coresight_device_release(struct device *dev) { struct coresight_device *csdev = to_coresight_device(dev);
- /* additional info to release for CTI */
- if ((csdev->type == CORESIGHT_DEV_TYPE_ECT) &&
(csdev->subtype.ect_subtype == CORESIGHT_DEV_SUBTYPE_ECT_CTI))
cti_device_release(dev);
Same comment as above regarding the usage of unimplemented functions. A function needs to be introduced before it is used (or in the same patch).
kfree(csdev->conns); kfree(csdev->refcnt); kfree(csdev); @@ -1171,6 +1219,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) coresight_fixup_device_conns(csdev); coresight_fixup_orphan_conns(csdev);
- cti_add_assoc_to_csdev(csdev);
Same comment as above.
mutex_unlock(&coresight_mutex); -- 2.19.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight