Hi Mathieu,
On Mon, 17 Aug 2020 at 17:02, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Fri, Aug 07, 2020 at 07:11:45PM +0800, Tingwei Zhang wrote:
Add static cti_assoc_ops to coresight core driver. Let cti driver register the add_assoc and remove_assoc call back. Avoid coresight core driver to depend on cti driver.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-cti.c | 32 ++++++++++++++++++-- drivers/hwtracing/coresight/coresight-priv.h | 14 ++++----- drivers/hwtracing/coresight/coresight.c | 21 +++++++++++-- 3 files changed, 53 insertions(+), 14 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 3ccc703dc940..1f470c47ba70 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -589,7 +589,6 @@ void cti_add_assoc_to_csdev(struct coresight_device *csdev) cti_add_done: mutex_unlock(&ect_mutex); } -EXPORT_SYMBOL_GPL(cti_add_assoc_to_csdev);
/*
- Removing the associated devices is easier.
@@ -616,7 +615,15 @@ void cti_remove_assoc_from_csdev(struct coresight_device *csdev) } mutex_unlock(&ect_mutex); } -EXPORT_SYMBOL_GPL(cti_remove_assoc_from_csdev);
Mike and Tingwei,
It seems to me that functions cti_add_assoc_to_csdev() and cti_remote_assoc_from_csdev() don't have to be exported. They could be called in cti_probe() and cti_device_release() with the ect_mutex held, the same way cti_update_conn_xrefs() and cti_remove_conn_xrefs() are.
The different functions handle two different initialisation & two different teardown ordering scenarios a) csdev - e.g. ETM is discovered & initialised before the associated CTI. Therefore the CTI has to establish interconnections on creation - calls cti_update_conn_xrefs() . b) CTI is discovered before csdev - therefore the csdev calls cti_add_assoc_to_csdev(). c) csdev is torn down before CTI - therefore csdev calls cti_remove_assoc_from_csdev() d) CTI is torn down before csdev - therefore calls cti_remove_conn_xrefs()
We can't call cti_add_assoc_to_csdev() in cti probe as the csdev does not exist yet. Bear in mind that one CTI can actually be associated with multiple different csdev devices - especially in the none CPU bound infrastructure such as funnels and sinks. This has always been the case since the first CTI set - but is even more important now the modules can be removed / added as required.
Regards
Mike
If my assessment is correct this patch would be simpler.
Thanks, Mathieu
+/*
- Operations to add and remove associated CTI.
- Register to coresight core driver as call back function.
- */
+static struct cti_assoc_op cti_assoc_ops = {
.add = cti_add_assoc_to_csdev,.remove = cti_remove_assoc_from_csdev+};
/*
- Update the cross references where the associated device was found
@@ -972,4 +979,23 @@ static struct amba_driver cti_driver = { .probe = cti_probe, .id_table = cti_ids, }; -builtin_amba_driver(cti_driver);
+static int __init cti_init(void) +{
int ret;ret = amba_driver_register(&cti_driver);if (ret)pr_info("Error registering cti driver\n");coresight_set_cti_ops(&cti_assoc_ops);return ret;+}
+static void __exit cti_exit(void) +{
coresight_remove_cti_ops();amba_driver_unregister(&cti_driver);+}
+module_init(cti_init); +module_exit(cti_exit); diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index dcb8aeb6af62..6cde6cf42554 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -173,15 +173,13 @@ static inline int etm_readl_cp14(u32 off, unsigned int *val) { return 0; } static inline int etm_writel_cp14(u32 off, u32 val) { return 0; } #endif
-#ifdef CONFIG_CORESIGHT_CTI -extern void cti_add_assoc_to_csdev(struct coresight_device *csdev); -extern void cti_remove_assoc_from_csdev(struct coresight_device *csdev); +struct cti_assoc_op {
void (*add)(struct coresight_device *csdev);void (*remove)(struct coresight_device *csdev);+};
-#else -static inline void cti_add_assoc_to_csdev(struct coresight_device *csdev) {} -static inline void -cti_remove_assoc_from_csdev(struct coresight_device *csdev) {} -#endif +extern void coresight_set_cti_ops(const struct cti_assoc_op *cti_op); +extern void coresight_remove_cti_ops(void);
/*
- Macros and inline functions to handle CoreSight UCI data and driver
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 0b0e31577b9b..31b9ec8d3b9c 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -57,6 +57,20 @@ const u32 coresight_barrier_pkt[4] = { 0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff}; EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
+static const struct cti_assoc_op *cti_assoc_ops;
+void coresight_set_cti_ops(const struct cti_assoc_op *cti_op) +{
cti_assoc_ops = cti_op;+} +EXPORT_SYMBOL_GPL(coresight_set_cti_ops);
+void coresight_remove_cti_ops(void) +{
cti_assoc_ops = NULL;+} +EXPORT_SYMBOL_GPL(coresight_remove_cti_ops);
static int coresight_id_match(struct device *dev, void *data) { int trace_id, i_trace_id; @@ -1242,7 +1256,8 @@ static void coresight_device_release(struct device *dev) { struct coresight_device *csdev = to_coresight_device(dev);
cti_remove_assoc_from_csdev(csdev);
if (cti_assoc_ops && cti_assoc_ops->remove)cti_assoc_ops->remove(csdev); fwnode_handle_put(csdev->dev.fwnode); kfree(csdev->refcnt); kfree(csdev);@@ -1553,8 +1568,8 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) ret = coresight_fixup_device_conns(csdev); if (!ret) ret = coresight_fixup_orphan_conns(csdev);
if (!ret)cti_add_assoc_to_csdev(csdev);
if (!ret && cti_assoc_ops && cti_assoc_ops->add)cti_assoc_ops->add(csdev); mutex_unlock(&coresight_mutex); if (ret) {-- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project