Hi Mathieu,
On Mon, 17 Aug 2020 at 17:38, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Fri, Aug 07, 2020 at 07:11:48PM +0800, Tingwei Zhang wrote:
If associated ect device is not enabled at first place, disable routine should not be called. Add ect_enabled flag to check whether ect device is enabled. Fix the issue in below case. Ect device is not available when associated coresight device enabled and the association is established after coresight device is enabled.
Signed-off-by: Mike Leach mike.leach@linaro.org Signed-off-by: Tingwei Zhang tingwei@codeaurora.org
drivers/hwtracing/coresight/coresight.c | 11 ++++++++--- include/linux/coresight.h | 1 + 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index d066411aa794..27ad8317e3cf 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -245,13 +245,18 @@ coresight_control_assoc_ectdev(struct coresight_device *csdev, bool enable)
if (!ect_csdev) return 0;
if ((!ect_ops(ect_csdev)->enable) || (!ect_ops(ect_csdev)->disable))return 0; if (enable) {
if (ect_ops(ect_csdev)->enable)ect_ret = ect_ops(ect_csdev)->enable(ect_csdev);
ect_ret = ect_ops(ect_csdev)->enable(ect_csdev);if (!ect_ret)csdev->ect_enabled = true; } else {
if (ect_ops(ect_csdev)->disable)
if (csdev->ect_enabled) { ect_ret = ect_ops(ect_csdev)->disable(ect_csdev);csdev->ect_enabled = false;} } /* output warning if ECT enable is preventing trace operation */diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 3bb738f9a326..7d3c87e5b97c 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -208,6 +208,7 @@ struct coresight_device { /* sysfs links between components */ int nr_links; bool has_conns_grp;
bool ect_enabled; /* true only if associated ect device is enabled */We have cti_config::enable_req_count and cti_config::hw_enabled, both used in cti_enable_hw() and cti_disable_hw(). I would have thought they'd be sufficient to address the counting problems. If they are not I would much rather see a solution confined to the cti driver than in the core itself.
This is related to the fact that under sysfs it is possible under sysfs to enable an etm e.g. etm1 without the cti module present, then insert the CTI module, then enable another ETM e.g etm2. This is an issue that is caused by the possibility of module load and unload, and though inadvisable from a system usage point of view - Tingwei correctly points out that it could happen.
At the point that the first ETM is enabled, the associated ect pointer would be NULL, and thus no attempt to enable the ect/CTI is made. The CTI module on load will set the ect pointers on all registered csdev devices, including ones that are already enabled - etm1. So when we come to disable etm1, it will try to disable a CTI that it did not enable - a fact that cannot be counted in the CTI driver as it was not there when the etm was enabled. So we have a flag in csdev to record if this csdev did in fact enable the associated device, so it is clear to disable it on shutdown.
Regards
Mike
Thanks, Mathieu
};
/*
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project