Allow to build coresight as modules. This gives developers the feasibility to test their code without reboot.
This series is based on below two series.
- "coresight: allow to build components as modules" https://lkml.org/lkml/2018/6/5/989 - "coresight: make drivers modular" https://lkml.org/lkml/2020/1/17/468
Change from v6: Correct module description for CATU (Mike) Check ect_ret equals 0 and set ect_enabled flag (Mike) Add Tested-by and Reviewed-by from Mike
Change from v5: Add below CTI clean up change from Mike into series -https://lists.linaro.org/pipermail/coresight/2020-July/004349.html Increase module reference count when enabling CTI device (Mike)
Change from v4: Fix error handling in coresight_grab_devicei() (Greg) Add coresight: cti: Fix remove sysfs link error from Mike -https://lists.linaro.org/pipermail/coresight/2020-July/004275.html Move cti_remove_conn_xrefs() into cti_remove() (Mike) Align patch subject to coresight: <component>: <description> (Mike)
Change from v3: Rebase to coresight-next (Mike and Mathieu) Reorder try_get_module() (Suzuki) Clean up etmdrvdata[] in device remote path (Mike) Move cti_remove_conn_xrefs to cti_remove (Mike)
Change from v2: Rebase to 5.8-rc5. Export coresight_add_sysfs_link and coresight_remove_sysfs_link Fix one cut and paste error on MODULE_DESCRIPTION of CTI
Change from v1: Use try_module_get() to avoid module to be unloaded when device is used in active trace session. (Mathieu P)
Change from above two series. This series adds the support to dynamically remove module when the device in that module is enabled and used by some trace path. It disables all trace paths with that device and release the trace path.
Kim Phillips (7): coresight: use IS_ENABLED for CONFIGs that may be modules coresight: etm3x: allow etm3x to be built as a module coresight: etm4x: allow etm4x to be built as a module coresight: etb: allow etb to be built as a module coresight: tpiu: allow tpiu to be built as a module coresight: tmc: allow tmc to be built as a module coresight: allow funnel and replicator drivers to be built as modules
Mian Yousaf Kaukab (4): coresight: export global symbols coresight: funnel: remove multiple init calls from funnel driver coresight: replicator: remove multiple init calls coresight: tmc-etr: add function to register catu ops
Mike Leach (1): coresight: cti: Fix remove sysfs link error
Tingwei Zhang (13): coresight: cpu_debug: add module name in Kconfig coresight: cpu_debug: define MODULE_DEVICE_TABLE coresight: add coresight prefix to barrier_pkt coresight: add try_get_module() in coresight_grab_device() coresight: stm: allow to build coresight-stm as a module coresight: etm: perf: Fix warning caused by etm_setup_aux failure coresight: cti: add function to register cti associate ops coresight: cti: Fix bug clearing sysfs links on callback coresight: cti: don't disable ect device if it's not enabled coresight: cti: increase reference count when enabling cti coresight: cti: allow cti to be built as a module coresight: catu: allow catu drivers to be built as modules coresight: allow the coresight core driver to be built as a module
drivers/hwtracing/coresight/Kconfig | 54 +++++-- drivers/hwtracing/coresight/Makefile | 22 +-- drivers/hwtracing/coresight/coresight-catu.c | 37 ++++- drivers/hwtracing/coresight/coresight-catu.h | 2 - .../{coresight.c => coresight-core.c} | 134 +++++++++++++++--- .../hwtracing/coresight/coresight-cpu-debug.c | 2 + .../{coresight-cti.c => coresight-cti-core.c} | 62 ++++++-- drivers/hwtracing/coresight/coresight-etb10.c | 22 ++- .../hwtracing/coresight/coresight-etm-perf.c | 13 +- .../hwtracing/coresight/coresight-etm-perf.h | 5 +- ...resight-etm3x.c => coresight-etm3x-core.c} | 27 +++- ...resight-etm4x.c => coresight-etm4x-core.c} | 26 +++- .../hwtracing/coresight/coresight-funnel.c | 62 +++++++- .../hwtracing/coresight/coresight-platform.c | 1 + drivers/hwtracing/coresight/coresight-priv.h | 24 ++-- .../coresight/coresight-replicator.c | 63 +++++++- drivers/hwtracing/coresight/coresight-stm.c | 20 ++- drivers/hwtracing/coresight/coresight-sysfs.c | 2 + .../{coresight-tmc.c => coresight-tmc-core.c} | 19 ++- .../hwtracing/coresight/coresight-tmc-etf.c | 2 +- .../hwtracing/coresight/coresight-tmc-etr.c | 21 ++- drivers/hwtracing/coresight/coresight-tmc.h | 3 + drivers/hwtracing/coresight/coresight-tpiu.c | 19 ++- include/linux/coresight.h | 3 +- 24 files changed, 559 insertions(+), 86 deletions(-) rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (93%) rename drivers/hwtracing/coresight/{coresight-cti.c => coresight-cti-core.c} (94%) rename drivers/hwtracing/coresight/{coresight-etm3x.c => coresight-etm3x-core.c} (97%) rename drivers/hwtracing/coresight/{coresight-etm4x.c => coresight-etm4x-core.c} (98%) rename drivers/hwtracing/coresight/{coresight-tmc.c => coresight-tmc-core.c} (96%)
Provide name of cpu_debug module in Kconfig help section.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 02dbb5ca3bcf..4663fd1bbffc 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -110,6 +110,9 @@ config CORESIGHT_CPU_DEBUG properly, please refer Documentation/trace/coresight/coresight-cpu-debug.rst for detailed description and the example for usage.
+ To compile this driver as a module, choose M here: the + module will be called coresight-cpu-debug. + config CORESIGHT_CTI bool "CoreSight Cross Trigger Interface (CTI) driver" depends on ARM || ARM64
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Provide name of cpu_debug module in Kconfig help section.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 02dbb5ca3bcf..4663fd1bbffc 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -110,6 +110,9 @@ config CORESIGHT_CPU_DEBUG properly, please refer Documentation/trace/coresight/coresight-cpu-debug.rst for detailed description and the example for usage.
To compile this driver as a module, choose M here: the
module will be called coresight-cpu-debug.
- config CORESIGHT_CTI bool "CoreSight Cross Trigger Interface (CTI) driver" depends on ARM || ARM64
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Define a MODULE_DEVICE_TABLE for cpu_debug so module can be auto loaded on boot.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-cpu-debug.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index 96544b348c27..1d0880b3764a 100644 --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c @@ -665,6 +665,8 @@ static const struct amba_id debug_ids[] = { {}, };
+MODULE_DEVICE_TABLE(amba, debug_ids); + static struct amba_driver debug_driver = { .drv = { .name = "coresight-cpu-debug",
From: Kim Phillips kim.phillips@arm.com
Checking for ifdef CONFIG_x fails if CONFIG_x=m. Use IS_ENABLED that is true for both built-ins and modules, instead. Required when building coresight components as modules.
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etm-perf.h | 2 +- drivers/hwtracing/coresight/coresight-priv.h | 2 +- include/linux/coresight.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 015213abe00a..05f89723e282 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -57,7 +57,7 @@ struct etm_event_data { struct list_head * __percpu *path; };
-#ifdef CONFIG_CORESIGHT +#if IS_ENABLED(CONFIG_CORESIGHT) int etm_perf_symlink(struct coresight_device *csdev, bool link); int etm_perf_add_symlink_sink(struct coresight_device *csdev); void etm_perf_del_symlink_sink(struct coresight_device *csdev); diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index f2dc625ea585..d801a2755432 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -165,7 +165,7 @@ int coresight_make_links(struct coresight_device *orig, void coresight_remove_links(struct coresight_device *orig, struct coresight_connection *conn);
-#ifdef CONFIG_CORESIGHT_SOURCE_ETM3X +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM3X) extern int etm_readl_cp14(u32 off, unsigned int *val); extern int etm_writel_cp14(u32 off, u32 val); #else diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 58fffdecdbfd..3bb738f9a326 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -324,7 +324,7 @@ struct coresight_ops { const struct coresight_ops_ect *ect_ops; };
-#ifdef CONFIG_CORESIGHT +#if IS_ENABLED(CONFIG_CORESIGHT) extern struct coresight_device * coresight_register(struct coresight_desc *desc); extern void coresight_unregister(struct coresight_device *csdev);
Add coresight prefix to make it specific. It will be a export symbol.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-etb10.c | 2 +- drivers/hwtracing/coresight/coresight-priv.h | 8 ++++---- drivers/hwtracing/coresight/coresight-tmc-etf.c | 2 +- drivers/hwtracing/coresight/coresight.c | 3 ++- 4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 03e3f2590191..04ee9cda988d 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -525,7 +525,7 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev,
cur = buf->cur; offset = buf->offset; - barrier = barrier_pkt; + barrier = coresight_barrier_pkt;
for (i = 0; i < to_read; i += 4) { buf_ptr = buf->data_pages[cur] + offset; diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index d801a2755432..dcb8aeb6af62 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -66,8 +66,8 @@ static DEVICE_ATTR_RO(name) #define coresight_simple_reg64(type, name, lo_off, hi_off) \ __coresight_simple_func(type, NULL, name, lo_off, hi_off)
-extern const u32 barrier_pkt[4]; -#define CORESIGHT_BARRIER_PKT_SIZE (sizeof(barrier_pkt)) +extern const u32 coresight_barrier_pkt[4]; +#define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt))
enum etm_addr_type { ETM_ADDR_TYPE_NONE, @@ -104,10 +104,10 @@ struct cs_buffers { static inline void coresight_insert_barrier_packet(void *buf) { if (buf) - memcpy(buf, barrier_pkt, CORESIGHT_BARRIER_PKT_SIZE); + memcpy(buf, coresight_barrier_pkt, + CORESIGHT_BARRIER_PKT_SIZE); }
- static inline void CS_LOCK(void __iomem *addr) { do { diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 6375504ba8b0..44402d413ebb 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -519,7 +519,7 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
cur = buf->cur; offset = buf->offset; - barrier = barrier_pkt; + barrier = coresight_barrier_pkt;
/* for every byte to read */ for (i = 0; i < to_read; i += 4) { diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index e9c90f2de34a..310b1b825dd1 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -53,7 +53,8 @@ static struct list_head *stm_path; * beginning of the data collected in a buffer. That way the decoder knows that * it needs to look for another sync sequence. */ -const u32 barrier_pkt[4] = {0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff}; +const u32 coresight_barrier_pkt[4] = { + 0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff};
static int coresight_id_match(struct device *dev, void *data) {
From: Mian Yousaf Kaukab ykaukab@suse.de
Export symbols used among coresight modules.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 1 + drivers/hwtracing/coresight/coresight-sysfs.c | 2 ++ drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++++ drivers/hwtracing/coresight/coresight.c | 8 ++++++++ 4 files changed, 17 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 1a3169e69bb1..dcb0592418ae 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -517,6 +517,7 @@ int etm_perf_symlink(struct coresight_device *csdev, bool link)
return 0; } +EXPORT_SYMBOL_GPL(etm_perf_symlink);
static ssize_t etm_perf_sink_name_show(struct device *dev, struct device_attribute *dattr, diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index 82afeaf2ccc4..34d2a2d31d00 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -102,6 +102,7 @@ int coresight_add_sysfs_link(struct coresight_sysfs_link *info)
return ret; } +EXPORT_SYMBOL_GPL(coresight_add_sysfs_link);
void coresight_remove_sysfs_link(struct coresight_sysfs_link *info) { @@ -122,6 +123,7 @@ void coresight_remove_sysfs_link(struct coresight_sysfs_link *info) info->orig->nr_links--; info->target->nr_links--; } +EXPORT_SYMBOL_GPL(coresight_remove_sysfs_link);
/* * coresight_make_links: Make a link for a connection from a @orig diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index b29c2db94d96..ad991a37e2d2 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -255,6 +255,7 @@ void tmc_free_sg_table(struct tmc_sg_table *sg_table) tmc_free_table_pages(sg_table); tmc_free_data_pages(sg_table); } +EXPORT_SYMBOL_GPL(tmc_free_sg_table);
/* * Alloc pages for the table. Since this will be used by the device, @@ -340,6 +341,7 @@ struct tmc_sg_table *tmc_alloc_sg_table(struct device *dev,
return sg_table; } +EXPORT_SYMBOL_GPL(tmc_alloc_sg_table);
/* * tmc_sg_table_sync_data_range: Sync the data buffer written @@ -360,6 +362,7 @@ void tmc_sg_table_sync_data_range(struct tmc_sg_table *table, PAGE_SIZE, DMA_FROM_DEVICE); } } +EXPORT_SYMBOL_GPL(tmc_sg_table_sync_data_range);
/* tmc_sg_sync_table: Sync the page table */ void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table) @@ -372,6 +375,7 @@ void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table) dma_sync_single_for_device(real_dev, table_pages->daddrs[i], PAGE_SIZE, DMA_TO_DEVICE); } +EXPORT_SYMBOL_GPL(tmc_sg_table_sync_table);
/* * tmc_sg_table_get_data: Get the buffer pointer for data @offset @@ -401,6 +405,7 @@ ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table, *bufpp = page_address(data_pages->pages[pg_idx]) + pg_offset; return len; } +EXPORT_SYMBOL_GPL(tmc_sg_table_get_data);
#ifdef ETR_SG_DEBUG /* Map a dma address to virtual address */ @@ -766,6 +771,7 @@ tmc_etr_get_catu_device(struct tmc_drvdata *drvdata)
return NULL; } +EXPORT_SYMBOL_GPL(tmc_etr_get_catu_device);
static inline int tmc_etr_enable_catu(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf) diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 310b1b825dd1..b7151c5f81b1 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -55,6 +55,7 @@ static struct list_head *stm_path; */ const u32 coresight_barrier_pkt[4] = { 0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff}; +EXPORT_SYMBOL_GPL(coresight_barrier_pkt);
static int coresight_id_match(struct device *dev, void *data) { @@ -180,6 +181,7 @@ int coresight_claim_device_unlocked(void __iomem *base) coresight_clear_claim_tags(base); return -EBUSY; } +EXPORT_SYMBOL_GPL(coresight_claim_device_unlocked);
int coresight_claim_device(void __iomem *base) { @@ -191,6 +193,7 @@ int coresight_claim_device(void __iomem *base)
return rc; } +EXPORT_SYMBOL_GPL(coresight_claim_device);
/* * coresight_disclaim_device_unlocked : Clear the claim tags for the device. @@ -209,6 +212,7 @@ void coresight_disclaim_device_unlocked(void __iomem *base) */ WARN_ON_ONCE(1); } +EXPORT_SYMBOL_GPL(coresight_disclaim_device_unlocked);
void coresight_disclaim_device(void __iomem *base) { @@ -216,6 +220,7 @@ void coresight_disclaim_device(void __iomem *base) coresight_disclaim_device_unlocked(base); CS_LOCK(base); } +EXPORT_SYMBOL_GPL(coresight_disclaim_device);
/* enable or disable an associated CTI device of the supplied CS device */ static int @@ -468,6 +473,7 @@ void coresight_disable_path(struct list_head *path) { coresight_disable_path_from(path, NULL); } +EXPORT_SYMBOL_GPL(coresight_disable_path);
int coresight_enable_path(struct list_head *path, u32 mode, void *sink_data) { @@ -1377,6 +1383,7 @@ int coresight_timeout(void __iomem *addr, u32 offset, int position, int value)
return -EAGAIN; } +EXPORT_SYMBOL_GPL(coresight_timeout);
struct bus_type coresight_bustype = { .name = "coresight", @@ -1553,6 +1560,7 @@ bool coresight_loses_context_with_cpu(struct device *dev) return fwnode_property_present(dev_fwnode(dev), "arm,coresight-loses-context-with-cpu"); } +EXPORT_SYMBOL_GPL(coresight_loses_context_with_cpu);
/* * coresight_alloc_device_name - Get an index for a given device in the
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Mian Yousaf Kaukab ykaukab@suse.de
Export symbols used among coresight modules.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-etm-perf.c | 1 + drivers/hwtracing/coresight/coresight-sysfs.c | 2 ++ drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++++ drivers/hwtracing/coresight/coresight.c | 8 ++++++++ 4 files changed, 17 insertions(+)
...
struct bus_type coresight_bustype = { .name = "coresight", @@ -1553,6 +1560,7 @@ bool coresight_loses_context_with_cpu(struct device *dev) return fwnode_property_present(dev_fwnode(dev), "arm,coresight-loses-context-with-cpu"); } +EXPORT_SYMBOL_GPL(coresight_loses_context_with_cpu);
minor nit: This function is a good candidate for moving to the header file as a static inline, rather than being exported.
Either ways,
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
On Wed, Aug 05, 2020 at 07:13:43PM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Mian Yousaf Kaukab ykaukab@suse.de
Export symbols used among coresight modules.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-etm-perf.c | 1 + drivers/hwtracing/coresight/coresight-sysfs.c | 2 ++ drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++++ drivers/hwtracing/coresight/coresight.c | 8 ++++++++ 4 files changed, 17 insertions(+)
...
struct bus_type coresight_bustype = { .name = "coresight", @@ -1553,6 +1560,7 @@ bool coresight_loses_context_with_cpu(struct device *dev) return fwnode_property_present(dev_fwnode(dev), "arm,coresight-loses-context-with-cpu"); } +EXPORT_SYMBOL_GPL(coresight_loses_context_with_cpu);
minor nit: This function is a good candidate for moving to the header file as a static inline, rather than being exported.
That's a good point. I'll move it.
Either ways,
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
When coresight device is in an active session, driver module of that device should not be removed. Use try_get_module() in coresight_grab_device() to prevent module to be unloaded. Use get_device()/put_device() to protect device data in the middle of active session.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight.c | 39 +++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index b7151c5f81b1..1626bc885dfe 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -640,7 +640,7 @@ struct coresight_device *coresight_get_sink_by_id(u32 id) * don't appear on the trace path, they should be handled along with the * the master device. */ -static void coresight_grab_device(struct coresight_device *csdev) +static int coresight_grab_device(struct coresight_device *csdev) { int i;
@@ -648,10 +648,30 @@ static void coresight_grab_device(struct coresight_device *csdev) struct coresight_device *child;
child = csdev->pdata->conns[i].child_dev; - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) + if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { + if (!try_module_get(child->dev.parent->driver->owner)) + goto err; + get_device(child->dev.parent); pm_runtime_get_sync(child->dev.parent); + } } + if (!try_module_get(csdev->dev.parent->driver->owner)) + goto err; + get_device(csdev->dev.parent); pm_runtime_get_sync(csdev->dev.parent); + return 0; +err: + for (i--; i >= 0; i--) { + struct coresight_device *child; + + child = csdev->pdata->conns[i].child_dev; + if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { + pm_runtime_put(child->dev.parent); + put_device(child->dev.parent); + module_put(child->dev.parent->driver->owner); + } + } + return -ENODEV; }
/* @@ -663,12 +683,17 @@ static void coresight_drop_device(struct coresight_device *csdev) int i;
pm_runtime_put(csdev->dev.parent); + put_device(csdev->dev.parent); + module_put(csdev->dev.parent->driver->owner); for (i = 0; i < csdev->pdata->nr_outport; i++) { struct coresight_device *child;
child = csdev->pdata->conns[i].child_dev; - if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) + if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { pm_runtime_put(child->dev.parent); + put_device(child->dev.parent); + module_put(child->dev.parent->driver->owner); + } } }
@@ -687,7 +712,7 @@ static int _coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink, struct list_head *path) { - int i; + int i, ret; bool found = false; struct coresight_node *node;
@@ -721,7 +746,11 @@ static int _coresight_build_path(struct coresight_device *csdev, if (!node) return -ENOMEM;
- coresight_grab_device(csdev); + ret = coresight_grab_device(csdev); + if (ret) { + kfree(node); + return ret; + } node->csdev = csdev; list_add(&node->link, path);
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
When coresight device is in an active session, driver module of that device should not be removed. Use try_get_module() in coresight_grab_device() to prevent module to be unloaded. Use get_device()/put_device() to protect device data in the middle of active session.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight.c | 39 +++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index b7151c5f81b1..1626bc885dfe 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -640,7 +640,7 @@ struct coresight_device *coresight_get_sink_by_id(u32 id)
- don't appear on the trace path, they should be handled along with the
- the master device.
*/ -static void coresight_grab_device(struct coresight_device *csdev) +static int coresight_grab_device(struct coresight_device *csdev) { int i;
Please could you add a pair of helper functions to hold/drop reference to a device/driver ? i.e,
static inline bool coresight_get_ref(struct coresight_device *csdev) { struct device *dev = csdev->dev.parent;
/* Make sure the driver cant be removed */ if (!try_module_get(dev->driver->owner)) return false; /* Make sure the device can't go away */ get_device(dev); pm_runtime_get_sync(dev); return true; }
static inline void coresight_put_ref(struct coresight_device *csdev) { struct device *dev = csdev->dev.parent;
pm_runtime_put(dev); put_device(dev); module_put(dev->driver->owner); }
@@ -648,10 +648,30 @@ static void coresight_grab_device(struct coresight_device *csdev) struct coresight_device *child; child = csdev->pdata->conns[i].child_dev;
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) {
if (!try_module_get(child->dev.parent->driver->owner))
goto err;
get_device(child->dev.parent); pm_runtime_get_sync(child->dev.parent);
}}
This could then be : if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) if (!coresight_get_ref(child)) goto err;
- if (!try_module_get(csdev->dev.parent->driver->owner))
goto err;
- get_device(csdev->dev.parent); pm_runtime_get_sync(csdev->dev.parent);
if (coresight_get_ref(csdev)) return 0;
- return 0;
+err:
- for (i--; i >= 0; i--) {
struct coresight_device *child;
child = csdev->pdata->conns[i].child_dev;
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
pm_runtime_put(child->dev.parent);
put_device(child->dev.parent);
module_put(child->dev.parent->driver->owner);
}
And this could be :
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) coresight_put_ref(child);
- }
- return -ENODEV; }
/* @@ -663,12 +683,17 @@ static void coresight_drop_device(struct coresight_device *csdev) int i; pm_runtime_put(csdev->dev.parent);
- put_device(csdev->dev.parent);
- module_put(csdev->dev.parent->driver->owner);
coresight_put_ref(csdev);
for (i = 0; i < csdev->pdata->nr_outport; i++) { struct coresight_device *child; child = csdev->pdata->conns[i].child_dev;
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { pm_runtime_put(child->dev.parent);
put_device(child->dev.parent);
module_put(child->dev.parent->driver->owner);
}
coresight_put_ref(child);
} } @@ -687,7 +712,7 @@ static int _coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink, struct list_head *path) {
- int i;
- int i, ret; bool found = false; struct coresight_node *node;
@@ -721,7 +746,11 @@ static int _coresight_build_path(struct coresight_device *csdev, if (!node) return -ENOMEM;
- coresight_grab_device(csdev);
- ret = coresight_grab_device(csdev);
- if (ret) {
kfree(node);
return ret;
- } node->csdev = csdev; list_add(&node->link, path);
Cheers Suzuki
On Wed, Aug 05, 2020 at 06:55:40PM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
When coresight device is in an active session, driver module of that device should not be removed. Use try_get_module() in coresight_grab_device() to prevent module to be unloaded. Use get_device()/put_device() to protect device data in the middle of active session.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight.c | 39 +++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c
b/drivers/hwtracing/coresight/coresight.c
index b7151c5f81b1..1626bc885dfe 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -640,7 +640,7 @@ struct coresight_device
*coresight_get_sink_by_id(u32 id)
- don't appear on the trace path, they should be handled along with
the
- the master device.
*/ -static void coresight_grab_device(struct coresight_device *csdev) +static int coresight_grab_device(struct coresight_device *csdev) { int i;
Please could you add a pair of helper functions to hold/drop reference to a device/driver ? i.e,
static inline bool coresight_get_ref(struct coresight_device *csdev) { struct device *dev = csdev->dev.parent;
/* Make sure the driver cant be removed */ if (!try_module_get(dev->driver->owner)) return false; /* Make sure the device can't go away */ get_device(dev); pm_runtime_get_sync(dev); return true; }
Sure. I'll add it to next revision.
Thanks, Tingwei
static inline void coresight_put_ref(struct coresight_device *csdev) { struct device *dev = csdev->dev.parent;
pm_runtime_put(dev); put_device(dev); module_put(dev->driver->owner); }
@@ -648,10 +648,30 @@ static void coresight_grab_device(struct
coresight_device *csdev)
struct coresight_device *child; child = csdev->pdata->conns[i].child_dev;
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) {
if
(!try_module_get(child->dev.parent->driver->owner))
goto err;
get_device(child->dev.parent); pm_runtime_get_sync(child->dev.parent);
}}
This could then be : if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) if (!coresight_get_ref(child)) goto err;
- if (!try_module_get(csdev->dev.parent->driver->owner))
goto err;
- get_device(csdev->dev.parent); pm_runtime_get_sync(csdev->dev.parent);
if (coresight_get_ref(csdev)) return 0;
- return 0;
+err:
- for (i--; i >= 0; i--) {
struct coresight_device *child;
child = csdev->pdata->conns[i].child_dev;
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
pm_runtime_put(child->dev.parent);
put_device(child->dev.parent);
module_put(child->dev.parent->driver->owner);
}
And this could be :
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) coresight_put_ref(child);
- }
- return -ENODEV;
} /* @@ -663,12 +683,17 @@ static void coresight_drop_device(struct
coresight_device *csdev)
int i; pm_runtime_put(csdev->dev.parent);
- put_device(csdev->dev.parent);
- module_put(csdev->dev.parent->driver->owner);
coresight_put_ref(csdev);
for (i = 0; i < csdev->pdata->nr_outport; i++) { struct coresight_device *child; child = csdev->pdata->conns[i].child_dev;
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER)
if (child && child->type == CORESIGHT_DEV_TYPE_HELPER) { pm_runtime_put(child->dev.parent);
put_device(child->dev.parent);
module_put(child->dev.parent->driver->owner);
}
coresight_put_ref(child);
} } @@ -687,7 +712,7 @@ static int _coresight_build_path(struct
coresight_device *csdev,
struct coresight_device *sink, struct list_head *path)
{
- int i;
- int i, ret; bool found = false; struct coresight_node *node;
@@ -721,7 +746,11 @@ static int _coresight_build_path(struct
coresight_device *csdev,
if (!node) return -ENOMEM;
- coresight_grab_device(csdev);
- ret = coresight_grab_device(csdev);
- if (ret) {
kfree(node);
return ret;
- } node->csdev = csdev; list_add(&node->link, path);
Cheers Suzuki
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
- Kconfig becomes a tristate, to allow =m - add a stm_remove function, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-stm.c | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 4663fd1bbffc..6433f835fc97 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -86,7 +86,7 @@ config CORESIGHT_SOURCE_ETM4X data tracing may also be available.
config CORESIGHT_STM - bool "CoreSight System Trace Macrocell driver" + tristate "CoreSight System Trace Macrocell driver" depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64 select CORESIGHT_LINKS_AND_SINKS select STM @@ -96,6 +96,9 @@ config CORESIGHT_STM logging useful software events or data coming from various entities in the system, possibly running different OSs
+ To compile this driver as a module, choose M here: the + module will be called coresight-stm. + config CORESIGHT_CPU_DEBUG tristate "CoreSight CPU Debug driver" depends on ARM || ARM64 diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index 673d2f56ed1e..b74072e78436 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -948,6 +948,17 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit stm_remove(struct amba_device *adev) +{ + struct stm_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + coresight_unregister(drvdata->csdev); + + stm_unregister_device(&drvdata->stm); + + return 0; +} + #ifdef CONFIG_PM static int stm_runtime_suspend(struct device *dev) { @@ -980,6 +991,8 @@ static const struct amba_id stm_ids[] = { { 0, 0}, };
+MODULE_DEVICE_TABLE(amba, stm_ids); + static struct amba_driver stm_driver = { .drv = { .name = "coresight-stm", @@ -988,7 +1001,12 @@ static struct amba_driver stm_driver = { .suppress_bind_attrs = true, }, .probe = stm_probe, + .remove = stm_remove, .id_table = stm_ids, };
-builtin_amba_driver(stm_driver); +module_amba_driver(stm_driver); + +MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); +MODULE_DESCRIPTION("Arm CoreSight System Trace Macrocell driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
- Kconfig becomes a tristate, to allow =m
- add a stm_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
When coresight_build_path() fails on all the cpus, etm_setup_aux calls etm_free_aux() to free allocated event_data. WARN_ON(cpumask_empty(mask) will be triggered since cpu mask is empty. Check event_data->snk_config is not NULL first to avoid this warning.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index dcb0592418ae..3728c44e5763 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -126,10 +126,10 @@ static void free_sink_buffer(struct etm_event_data *event_data) cpumask_t *mask = &event_data->mask; struct coresight_device *sink;
- if (WARN_ON(cpumask_empty(mask))) + if (!event_data->snk_config) return;
- if (!event_data->snk_config) + if (WARN_ON(cpumask_empty(mask))) return;
cpu = cpumask_first(mask);
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
When coresight_build_path() fails on all the cpus, etm_setup_aux calls etm_free_aux() to free allocated event_data. WARN_ON(cpumask_empty(mask) will be triggered since cpu mask is empty. Check event_data->snk_config is not NULL first to avoid this warning.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
Please could you added a Fixes tag for this ?
Fixes: f5200aa9831f38 ("coresight: perf: Refactor function free_event_data()")
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index dcb0592418ae..3728c44e5763 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -126,10 +126,10 @@ static void free_sink_buffer(struct etm_event_data *event_data) cpumask_t *mask = &event_data->mask; struct coresight_device *sink;
- if (WARN_ON(cpumask_empty(mask)))
- if (!event_data->snk_config) return;
- if (!event_data->snk_config)
- if (WARN_ON(cpumask_empty(mask))) return;
cpu = cpumask_first(mask);
On Wed, Aug 05, 2020 at 06:18:35PM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
When coresight_build_path() fails on all the cpus, etm_setup_aux calls etm_free_aux() to free allocated event_data. WARN_ON(cpumask_empty(mask) will be triggered since cpu mask is empty. Check event_data->snk_config is not NULL first to avoid this warning.
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
Please could you added a Fixes tag for this ?
Fixes: f5200aa9831f38 ("coresight: perf: Refactor function free_event_data()")
I'll add Fixes tag.
Thanks, Tingwei
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index dcb0592418ae..3728c44e5763 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -126,10 +126,10 @@ static void free_sink_buffer(struct etm_event_data *event_data) cpumask_t *mask = &event_data->mask; struct coresight_device *sink;
- if (WARN_ON(cpumask_empty(mask)))
- if (!event_data->snk_config) return;
- if (!event_data->snk_config)
- if (WARN_ON(cpumask_empty(mask))) return;
cpu = cpumask_first(mask);
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm3x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m - append -core to source file name to allow module to be called coresight-etm3x by the Makefile - add an etm_remove function, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 +++- drivers/hwtracing/coresight/Makefile | 3 ++- ...resight-etm3x.c => coresight-etm3x-core.c} | 27 ++++++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) rename drivers/hwtracing/coresight/{coresight-etm3x.c => coresight-etm3x-core.c} (97%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 6433f835fc97..8fd9fd139cf3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -65,7 +65,7 @@ config CORESIGHT_SINK_ETBV10 special enhancement or added features.
config CORESIGHT_SOURCE_ETM3X - bool "CoreSight Embedded Trace Macrocell 3.x driver" + tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 select CORESIGHT_LINKS_AND_SINKS help @@ -74,6 +74,9 @@ config CORESIGHT_SOURCE_ETM3X This is primarily useful for instruction level tracing. Depending the ETM version data tracing may also be available.
+ To compile this driver as a module, choose M here: the + module will be called coresight-etm3x. + config CORESIGHT_SOURCE_ETM4X bool "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64 diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 19497d1d92bf..d619cfd0abd8 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -11,7 +11,8 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ coresight-replicator.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \ +obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o +coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \ coresight-etm3x-sysfs.o obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \ coresight-etm4x-sysfs.o diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c similarity index 97% rename from drivers/hwtracing/coresight/coresight-etm3x.c rename to drivers/hwtracing/coresight/coresight-etm3x-core.c index bf22dcfd3327..82b333c40006 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -895,6 +895,23 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etm_remove(struct amba_device *adev) +{ + struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + etm_perf_symlink(drvdata->csdev, false); + + if (--etm_count == 0) { + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); + if (hp_online) + cpuhp_remove_state_nocalls(hp_online); + } + + coresight_unregister(drvdata->csdev); + + return 0; +} + #ifdef CONFIG_PM static int etm_runtime_suspend(struct device *dev) { @@ -937,6 +954,8 @@ static const struct amba_id etm_ids[] = { { 0, 0}, };
+MODULE_DEVICE_TABLE(amba, etm_ids); + static struct amba_driver etm_driver = { .drv = { .name = "coresight-etm3x", @@ -945,6 +964,12 @@ static struct amba_driver etm_driver = { .suppress_bind_attrs = true, }, .probe = etm_probe, + .remove = etm_remove, .id_table = etm_ids, }; -builtin_amba_driver(etm_driver); +module_amba_driver(etm_driver); + +MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); +MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight Program Flow Trace driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm3x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight-etm3x by the Makefile
- add an etm_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 +++- drivers/hwtracing/coresight/Makefile | 3 ++- ...resight-etm3x.c => coresight-etm3x-core.c} | 27 ++++++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) rename drivers/hwtracing/coresight/{coresight-etm3x.c => coresight-etm3x-core.c} (97%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 6433f835fc97..8fd9fd139cf3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -65,7 +65,7 @@ config CORESIGHT_SINK_ETBV10 special enhancement or added features. config CORESIGHT_SOURCE_ETM3X
- bool "CoreSight Embedded Trace Macrocell 3.x driver"
- tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 select CORESIGHT_LINKS_AND_SINKS help
@@ -74,6 +74,9 @@ config CORESIGHT_SOURCE_ETM3X This is primarily useful for instruction level tracing. Depending the ETM version data tracing may also be available.
To compile this driver as a module, choose M here: the
module will be called coresight-etm3x.
- config CORESIGHT_SOURCE_ETM4X bool "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 19497d1d92bf..d619cfd0abd8 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -11,7 +11,8 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ coresight-replicator.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \ +obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o +coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \ coresight-etm3x-sysfs.o obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \ coresight-etm4x-sysfs.o diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c similarity index 97% rename from drivers/hwtracing/coresight/coresight-etm3x.c rename to drivers/hwtracing/coresight/coresight-etm3x-core.c index bf22dcfd3327..82b333c40006 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -895,6 +895,23 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return ret; } +static int __exit etm_remove(struct amba_device *adev) +{
- struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
- etm_perf_symlink(drvdata->csdev, false);
- if (--etm_count == 0) {
Could there be multiple instances of remove running in parallel ? I believe we need some sort of a protection here to avoid racing.
Or even better, I would recommend leaving the notifiers registered at module_init and removed at the cleanup of the module, just like we are doing for etm4x driver, and get rid of this silly scheme. Like here :
https://lkml.kernel.org/r/20200729051310.18436-1-saiprakash.ranjan@codeauror...
Cheers Suzuki
Hi,
On Wed, 5 Aug 2020 at 12:05, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm3x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight-etm3x by the Makefile
- add an etm_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 +++- drivers/hwtracing/coresight/Makefile | 3 ++- ...resight-etm3x.c => coresight-etm3x-core.c} | 27 ++++++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) rename drivers/hwtracing/coresight/{coresight-etm3x.c => coresight-etm3x-core.c} (97%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 6433f835fc97..8fd9fd139cf3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -65,7 +65,7 @@ config CORESIGHT_SINK_ETBV10 special enhancement or added features.
config CORESIGHT_SOURCE_ETM3X
bool "CoreSight Embedded Trace Macrocell 3.x driver"
tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 select CORESIGHT_LINKS_AND_SINKS help
@@ -74,6 +74,9 @@ config CORESIGHT_SOURCE_ETM3X This is primarily useful for instruction level tracing. Depending the ETM version data tracing may also be available.
To compile this driver as a module, choose M here: the
module will be called coresight-etm3x.
- config CORESIGHT_SOURCE_ETM4X bool "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 19497d1d92bf..d619cfd0abd8 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -11,7 +11,8 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ coresight-replicator.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \ +obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o +coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \ coresight-etm3x-sysfs.o obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \ coresight-etm4x-sysfs.o diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c similarity index 97% rename from drivers/hwtracing/coresight/coresight-etm3x.c rename to drivers/hwtracing/coresight/coresight-etm3x-core.c index bf22dcfd3327..82b333c40006 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -895,6 +895,23 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etm_remove(struct amba_device *adev) +{
struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
etm_perf_symlink(drvdata->csdev, false);
if (--etm_count == 0) {
Could there be multiple instances of remove running in parallel ? I believe we need some sort of a protection here to avoid racing.
Or even better, I would recommend leaving the notifiers registered at module_init and removed at the cleanup of the module, just like we are doing for etm4x driver, and get rid of this silly scheme.
I would agree that this needs addressing but this is an independent problem that could be better served by a separate patchset, rather than add feature creep to this set. The same schema is used in the CTI driver as well and needs fixing there.
Like here :
https://lkml.kernel.org/r/20200729051310.18436-1-saiprakash.ranjan@codeauror...
This conflicts with the init / exit fns created for module loading in the etm4x part of this set..
A decision needs to be made on which set gets applied first - my view is that the module set could go first, then a set fixing the PM registration issues for all three affected drivers to be applied next.
The change to making all the drivers module loadable provides the init functions needed to initalise the PM outside the probe functions, just like the patch mentioned above and makes a change to all three drivers easy to create.
Regards
Mike
Cheers Suzuki
On 08/05/2020 01:54 PM, Mike Leach wrote:
Hi,
On Wed, 5 Aug 2020 at 12:05, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm3x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight-etm3x by the Makefile
- add an etm_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
bool "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 19497d1d92bf..d619cfd0abd8 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -11,7 +11,8 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ coresight-replicator.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \ +obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o +coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \ coresight-etm3x-sysfs.o obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \ coresight-etm4x-sysfs.o diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c similarity index 97% rename from drivers/hwtracing/coresight/coresight-etm3x.c rename to drivers/hwtracing/coresight/coresight-etm3x-core.c index bf22dcfd3327..82b333c40006 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -895,6 +895,23 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etm_remove(struct amba_device *adev) +{
struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
etm_perf_symlink(drvdata->csdev, false);
if (--etm_count == 0) {
Could there be multiple instances of remove running in parallel ? I believe we need some sort of a protection here to avoid racing.
Or even better, I would recommend leaving the notifiers registered at module_init and removed at the cleanup of the module, just like we are doing for etm4x driver, and get rid of this silly scheme.
I would agree that this needs addressing but this is an independent problem that could be better served by a separate patchset, rather
I agree that it is an independent problem, but this series makes it more exposed, i.e, calling the probe/remove at runtime in any order. So, pushing this series before that is fixed is going to cause trouble, if they both end up in different releases.
than add feature creep to this set. The same schema is used in the CTI driver as well and needs fixing there.
Like here :
https://lkml.kernel.org/r/20200729051310.18436-1-saiprakash.ranjan@codeauror...
This conflicts with the init / exit fns created for module loading in the etm4x part of this set..
A decision needs to be made on which set gets applied first - my view is that the module set could go first, then a set fixing the PM registration issues for all three affected drivers to be applied next.
I think we should do the opposite. Fixing the hotplug is fairly straight forward, and need not block on this large series.
Cheers Suzuki
Hi Mike,
On 2020-08-05 18:24, Mike Leach wrote:
Hi,
On Wed, 5 Aug 2020 at 12:05, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm3x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight-etm3x by the Makefile
- add an etm_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 +++- drivers/hwtracing/coresight/Makefile | 3 ++- ...resight-etm3x.c => coresight-etm3x-core.c} | 27 ++++++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) rename drivers/hwtracing/coresight/{coresight-etm3x.c => coresight-etm3x-core.c} (97%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 6433f835fc97..8fd9fd139cf3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -65,7 +65,7 @@ config CORESIGHT_SINK_ETBV10 special enhancement or added features.
config CORESIGHT_SOURCE_ETM3X
bool "CoreSight Embedded Trace Macrocell 3.x driver"
tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 select CORESIGHT_LINKS_AND_SINKS help
@@ -74,6 +74,9 @@ config CORESIGHT_SOURCE_ETM3X This is primarily useful for instruction level tracing. Depending the ETM version data tracing may also be available.
To compile this driver as a module, choose M here: the
module will be called coresight-etm3x.
- config CORESIGHT_SOURCE_ETM4X bool "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 19497d1d92bf..d619cfd0abd8 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -11,7 +11,8 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ coresight-replicator.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \ +obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o +coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \ coresight-etm3x-sysfs.o obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \ coresight-etm4x-sysfs.o diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c similarity index 97% rename from drivers/hwtracing/coresight/coresight-etm3x.c rename to drivers/hwtracing/coresight/coresight-etm3x-core.c index bf22dcfd3327..82b333c40006 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -895,6 +895,23 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etm_remove(struct amba_device *adev) +{
struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
etm_perf_symlink(drvdata->csdev, false);
if (--etm_count == 0) {
Could there be multiple instances of remove running in parallel ? I believe we need some sort of a protection here to avoid racing.
Or even better, I would recommend leaving the notifiers registered at module_init and removed at the cleanup of the module, just like we are doing for etm4x driver, and get rid of this silly scheme.
I would agree that this needs addressing but this is an independent problem that could be better served by a separate patchset, rather than add feature creep to this set. The same schema is used in the CTI driver as well and needs fixing there.
Like here :
https://lkml.kernel.org/r/20200729051310.18436-1-saiprakash.ranjan@codeauror...
This conflicts with the init / exit fns created for module loading in the etm4x part of this set..
A decision needs to be made on which set gets applied first - my view is that the module set could go first, then a set fixing the PM registration issues for all three affected drivers to be applied next.
The etm4_count race is a bug fix which in my opinion should go first because this feature would likely to be merged for 5.10 and till then we would have a bug lying around if the fix patch for the race does not go in right?
Thanks, Sai
On Thu, Aug 06, 2020 at 01:44:56PM +0800, Sai Prakash Ranjan wrote:
Hi Mike,
On 2020-08-05 18:24, Mike Leach wrote:
Hi,
On Wed, 5 Aug 2020 at 12:05, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm3x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight-etm3x by the Makefile
- add an etm_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 +++- drivers/hwtracing/coresight/Makefile | 3 ++- ...resight-etm3x.c => coresight-etm3x-core.c} | 27
++++++++++++++++++-
3 files changed, 32 insertions(+), 3 deletions(-) rename drivers/hwtracing/coresight/{coresight-etm3x.c =>
coresight-etm3x-core.c} (97%)
diff --git a/drivers/hwtracing/coresight/Kconfig
b/drivers/hwtracing/coresight/Kconfig
index 6433f835fc97..8fd9fd139cf3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -65,7 +65,7 @@ config CORESIGHT_SINK_ETBV10 special enhancement or added features.
config CORESIGHT_SOURCE_ETM3X
bool "CoreSight Embedded Trace Macrocell 3.x driver"
tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 select CORESIGHT_LINKS_AND_SINKS help
@@ -74,6 +74,9 @@ config CORESIGHT_SOURCE_ETM3X This is primarily useful for instruction level tracing.
Depending
the ETM version data tracing may also be available.
To compile this driver as a module, choose M here: the
module will be called coresight-etm3x.
- config CORESIGHT_SOURCE_ETM4X bool "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64
diff --git a/drivers/hwtracing/coresight/Makefile
b/drivers/hwtracing/coresight/Makefile
index 19497d1d92bf..d619cfd0abd8 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -11,7 +11,8 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) +=
coresight-tpiu.o
obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ coresight-replicator.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o
coresight-etm-cp14.o \
+obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o +coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \ coresight-etm3x-sysfs.o obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \ coresight-etm4x-sysfs.o diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c
b/drivers/hwtracing/coresight/coresight-etm3x-core.c
similarity index 97% rename from drivers/hwtracing/coresight/coresight-etm3x.c rename to drivers/hwtracing/coresight/coresight-etm3x-core.c index bf22dcfd3327..82b333c40006 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -895,6 +895,23 @@ static int etm_probe(struct amba_device *adev,
const struct amba_id *id)
return ret;
}
+static int __exit etm_remove(struct amba_device *adev) +{
struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
etm_perf_symlink(drvdata->csdev, false);
if (--etm_count == 0) {
Could there be multiple instances of remove running in parallel ? I believe we need some sort of a protection here to avoid racing.
Or even better, I would recommend leaving the notifiers registered at module_init and removed at the cleanup of the module, just like we are doing for etm4x driver, and get rid of this silly scheme.
I would agree that this needs addressing but this is an independent problem that could be better served by a separate patchset, rather than add feature creep to this set. The same schema is used in the CTI driver as well and needs fixing there.
Like here :
https://lkml.kernel.org/r/20200729051310.18436-1-saiprakash.ranjan@codeaur ora.org
This conflicts with the init / exit fns created for module loading in the etm4x part of this set..
A decision needs to be made on which set gets applied first - my view is that the module set could go first, then a set fixing the PM registration issues for all three affected drivers to be applied next.
The etm4_count race is a bug fix which in my opinion should go first because this feature would likely to be merged for 5.10 and till then we would have a bug lying around if the fix patch for the race does not go in right?
Agree with you, Sai. I'll rebase this series to your change.
Thanks, Sai
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm4x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m - append -core to source file name to allow module to be called coresight-etm4x by the Makefile - add an etm4_remove function, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 +++- drivers/hwtracing/coresight/Makefile | 4 +-- ...resight-etm4x.c => coresight-etm4x-core.c} | 26 ++++++++++++++++++- 3 files changed, 31 insertions(+), 4 deletions(-) rename drivers/hwtracing/coresight/{coresight-etm4x.c => coresight-etm4x-core.c} (98%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 8fd9fd139cf3..d6e107bbd30b 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -78,7 +78,7 @@ config CORESIGHT_SOURCE_ETM3X module will be called coresight-etm3x.
config CORESIGHT_SOURCE_ETM4X - bool "CoreSight Embedded Trace Macrocell 4.x driver" + tristate "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64 select CORESIGHT_LINKS_AND_SINKS select PID_IN_CONTEXTIDR @@ -88,6 +88,9 @@ config CORESIGHT_SOURCE_ETM4X for instruction level tracing. Depending on the implemented version data tracing may also be available.
+ To compile this driver as a module, choose M here: the + module will be called coresight-etm4x. + config CORESIGHT_STM tristate "CoreSight System Trace Macrocell driver" depends on (ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64 diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index d619cfd0abd8..271dc255454f 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -14,8 +14,8 @@ obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \ coresight-etm3x-sysfs.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \ - coresight-etm4x-sysfs.o +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o +coresight-etm4x-y := coresight-etm4x-core.o coresight-etm4x-sysfs.o obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c similarity index 98% rename from drivers/hwtracing/coresight/coresight-etm4x.c rename to drivers/hwtracing/coresight/coresight-etm4x-core.c index 6d7d2169bfb2..8149853d5e9d 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1573,6 +1573,21 @@ static struct amba_cs_uci_id uci_id_etm4[] = { } };
+static int __exit etm4_remove(struct amba_device *adev) +{ + struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + etm_perf_symlink(drvdata->csdev, false); + + etmdrvdata[drvdata->cpu] = NULL; + etm4_pm_clear(); + + coresight_unregister(drvdata->csdev); + + return 0; +} + + static const struct amba_id etm4_ids[] = { CS_AMBA_ID(0x000bb95d), /* Cortex-A53 */ CS_AMBA_ID(0x000bb95e), /* Cortex-A57 */ @@ -1590,12 +1605,21 @@ static const struct amba_id etm4_ids[] = { {}, };
+MODULE_DEVICE_TABLE(amba, etm4_ids); + static struct amba_driver etm4x_driver = { .drv = { .name = "coresight-etm4x", + .owner = THIS_MODULE, .suppress_bind_attrs = true, }, .probe = etm4_probe, + .remove = etm4_remove, .id_table = etm4_ids, }; -builtin_amba_driver(etm4x_driver); +module_amba_driver(etm4x_driver); + +MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); +MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight Program Flow Trace v4 driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etm4x as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight-etm4x by the Makefile
- add an etm4_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
+static int __exit etm4_remove(struct amba_device *adev) +{
- struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev);
- etm_perf_symlink(drvdata->csdev, false);
- etmdrvdata[drvdata->cpu] = NULL;
I think we need to explicitly make this change visible to the other observers, to make sure that a PM notifier doesn't end up using the stale per-cpu pointer.
- etm4_pm_clear();
- coresight_unregister(drvdata->csdev);
- return 0;
+}
- static const struct amba_id etm4_ids[] = { CS_AMBA_ID(0x000bb95d), /* Cortex-A53 */ CS_AMBA_ID(0x000bb95e), /* Cortex-A57 */
@@ -1590,12 +1605,21 @@ static const struct amba_id etm4_ids[] = { {}, }; +MODULE_DEVICE_TABLE(amba, etm4_ids);
- static struct amba_driver etm4x_driver = { .drv = { .name = "coresight-etm4x",
.suppress_bind_attrs = true, }, .probe = etm4_probe,.owner = THIS_MODULE,
- .remove = etm4_remove, .id_table = etm4_ids, };
-builtin_amba_driver(etm4x_driver); +module_amba_driver(etm4x_driver);
Please note that this could conflict with the following patch :
https://lkml.kernel.org/r/20200729051310.18436-1-saiprakash.ranjan@codeauror...
Otherwise looks good to me.
Suzuki
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etb10 as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m - add an etb_remove function, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-etb10.c | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index d6e107bbd30b..996d84a1edb8 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -57,13 +57,16 @@ config CORESIGHT_SINK_TPIU the on-board coresight memory can handle.
config CORESIGHT_SINK_ETBV10 - bool "Coresight ETBv1.0 driver" + tristate "Coresight ETBv1.0 driver" depends on CORESIGHT_LINKS_AND_SINKS help This enables support for the Embedded Trace Buffer version 1.0 driver that complies with the generic implementation of the component without special enhancement or added features.
+ To compile this driver as a module, choose M here: the + module will be called coresight-etb10. + config CORESIGHT_SOURCE_ETM3X tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 04ee9cda988d..b40756497c9a 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -801,6 +801,16 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etb_remove(struct amba_device *adev) +{ + struct etb_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + misc_deregister(&drvdata->miscdev); + coresight_unregister(drvdata->csdev); + + return 0; +} + #ifdef CONFIG_PM static int etb_runtime_suspend(struct device *dev) { @@ -835,6 +845,8 @@ static const struct amba_id etb_ids[] = { { 0, 0}, };
+MODULE_DEVICE_TABLE(amba, etb_ids); + static struct amba_driver etb_driver = { .drv = { .name = "coresight-etb10", @@ -844,6 +856,12 @@ static struct amba_driver etb_driver = {
}, .probe = etb_probe, + .remove = etb_remove, .id_table = etb_ids, }; -builtin_amba_driver(etb_driver); +module_amba_driver(etb_driver); + +MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); +MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight Embedded Trace Buffer driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etb10 as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add an etb_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-etb10.c | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index d6e107bbd30b..996d84a1edb8 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -57,13 +57,16 @@ config CORESIGHT_SINK_TPIU the on-board coresight memory can handle. config CORESIGHT_SINK_ETBV10
- bool "Coresight ETBv1.0 driver"
- tristate "Coresight ETBv1.0 driver" depends on CORESIGHT_LINKS_AND_SINKS help This enables support for the Embedded Trace Buffer version 1.0 driver that complies with the generic implementation of the component without special enhancement or added features.
To compile this driver as a module, choose M here: the
module will be called coresight-etb10.
- config CORESIGHT_SOURCE_ETM3X tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 04ee9cda988d..b40756497c9a 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -801,6 +801,16 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return ret; } +static int __exit etb_remove(struct amba_device *adev) +{
- struct etb_drvdata *drvdata = dev_get_drvdata(&adev->dev);
- misc_deregister(&drvdata->miscdev);
- coresight_unregister(drvdata->csdev);
- return 0;
+}
I am worried about the dangling reference via the misc device to the drvdata. Not sure, if we need to grab the reference the module for each open fd on the misc device. The misc device infrastructure doesn't seem to do any of this implicitly. This is something worth checking. I will see if I can trigger this.
i.e,
CPU 0: CPU1:
open("/dev/etb") etb_open() ... etb_remove() { .. coresight_unregister() }
read() etb_read() deal with free'd drvdata ?
Cheers Suzuki
On Wed, Aug 05, 2020 at 11:43:36PM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etb10 as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add an etb_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-etb10.c | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index d6e107bbd30b..996d84a1edb8 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -57,13 +57,16 @@ config CORESIGHT_SINK_TPIU the on-board coresight memory can handle.
config CORESIGHT_SINK_ETBV10
- bool "Coresight ETBv1.0 driver"
- tristate "Coresight ETBv1.0 driver" depends on CORESIGHT_LINKS_AND_SINKS help This enables support for the Embedded Trace Buffer version 1.0 driver that complies with the generic implementation of the component
without special enhancement or added features.
To compile this driver as a module, choose M here: the
module will be called coresight-etb10.
config CORESIGHT_SOURCE_ETM3X tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 04ee9cda988d..b40756497c9a 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -801,6 +801,16 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etb_remove(struct amba_device *adev) +{
- struct etb_drvdata *drvdata = dev_get_drvdata(&adev->dev);
- misc_deregister(&drvdata->miscdev);
- coresight_unregister(drvdata->csdev);
- return 0;
+}
I am worried about the dangling reference via the misc device to the drvdata. Not sure, if we need to grab the reference the module for each open fd on the misc device. The misc device infrastructure doesn't seem to do any of this implicitly. This is something worth checking. I will see if I can trigger this.
Misc device is one char device. cdev_get() is called in chrdev_open() to grab reference of module. Look like we are safe here.
Thanks, Tingwei
i.e,
CPU 0: CPU1:
open("/dev/etb") etb_open() ... etb_remove() { .. coresight_unregister() }
read() etb_read() deal with free'd drvdata ?
Cheers Suzuki
On 08/06/2020 12:19 PM, Tingwei Zhang wrote:
On Wed, Aug 05, 2020 at 11:43:36PM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etb10 as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add an etb_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-etb10.c | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index d6e107bbd30b..996d84a1edb8 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -57,13 +57,16 @@ config CORESIGHT_SINK_TPIU the on-board coresight memory can handle.
config CORESIGHT_SINK_ETBV10
- bool "Coresight ETBv1.0 driver"
- tristate "Coresight ETBv1.0 driver" depends on CORESIGHT_LINKS_AND_SINKS help This enables support for the Embedded Trace Buffer version 1.0 driver that complies with the generic implementation of the component
without special enhancement or added features.
To compile this driver as a module, choose M here: the
module will be called coresight-etb10.
- config CORESIGHT_SOURCE_ETM3X tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 04ee9cda988d..b40756497c9a 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -801,6 +801,16 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etb_remove(struct amba_device *adev) +{
- struct etb_drvdata *drvdata = dev_get_drvdata(&adev->dev);
- misc_deregister(&drvdata->miscdev);
- coresight_unregister(drvdata->csdev);
- return 0;
+}
I am worried about the dangling reference via the misc device to the drvdata. Not sure, if we need to grab the reference the module for each open fd on the misc device. The misc device infrastructure doesn't seem to do any of this implicitly. This is something worth checking. I will see if I can trigger this.
Misc device is one char device. cdev_get() is called in chrdev_open() to grab reference of module. Look like we are safe here.
That is not the case, as the misc_device doesn't have any pointers to who the owner is, that triggered my question.
Actually, it is the misc_open() which holds a refcount on the f_ops (which is etb_fops for etb10) owner, thus we are safe here.
It may be a good idea to add this comment in etb_remove(), so that we don't have to dig this again.
with that:
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Cheers Suzuki
On Thu, Aug 06, 2020 at 10:56:57PM +0800, Suzuki K Poulose wrote:
On 08/06/2020 12:19 PM, Tingwei Zhang wrote:
On Wed, Aug 05, 2020 at 11:43:36PM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-etb10 as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add an etb_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-etb10.c | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index d6e107bbd30b..996d84a1edb8 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -57,13 +57,16 @@ config CORESIGHT_SINK_TPIU the on-board coresight memory can handle.
config CORESIGHT_SINK_ETBV10
- bool "Coresight ETBv1.0 driver"
- tristate "Coresight ETBv1.0 driver" depends on CORESIGHT_LINKS_AND_SINKS help This enables support for the Embedded Trace Buffer version 1.0
driver that complies with the generic implementation of the component without special enhancement or added features.
To compile this driver as a module, choose M here: the
module will be called coresight-etb10.
config CORESIGHT_SOURCE_ETM3X tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 04ee9cda988d..b40756497c9a 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -801,6 +801,16 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit etb_remove(struct amba_device *adev) +{
- struct etb_drvdata *drvdata = dev_get_drvdata(&adev->dev);
- misc_deregister(&drvdata->miscdev);
- coresight_unregister(drvdata->csdev);
- return 0;
+}
I am worried about the dangling reference via the misc device to the drvdata. Not sure, if we need to grab the reference the module for each open fd on the misc device. The misc device infrastructure doesn't seem to do any of this implicitly. This is something worth checking. I will see if I can trigger this.
Misc device is one char device. cdev_get() is called in chrdev_open() to grab reference of module. Look like we are safe here.
That is not the case, as the misc_device doesn't have any pointers to who the owner is, that triggered my question.
Actually, it is the misc_open() which holds a refcount on the f_ops (which is etb_fops for etb10) owner, thus we are safe here.
You are correct. It's in fops_get(c->fops.
It may be a good idea to add this comment in etb_remove(), so that we don't have to dig this again.
I'll add comment to etb_remove.
Thanks, Tingwei
with that:
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Cheers Suzuki
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-tpiu as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m - add a tpiu_remove function, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-tpiu.c | 19 ++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 996d84a1edb8..8fd9887fb03b 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -46,7 +46,7 @@ config CORESIGHT_CATU mode where the address is not translated.
config CORESIGHT_SINK_TPIU - bool "Coresight generic TPIU driver" + tristate "Coresight generic TPIU driver" depends on CORESIGHT_LINKS_AND_SINKS help This enables support for the Trace Port Interface Unit driver, @@ -56,6 +56,9 @@ config CORESIGHT_SINK_TPIU connected to an external host for use case capturing more traces than the on-board coresight memory can handle.
+ To compile this driver as a module, choose M here: the + module will be called coresight-tpiu. + config CORESIGHT_SINK_ETBV10 tristate "Coresight ETBv1.0 driver" depends on CORESIGHT_LINKS_AND_SINKS diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index f8583e4032a6..e4ddd9801535 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -173,6 +173,15 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR(drvdata->csdev); }
+static int __exit tpiu_remove(struct amba_device *adev) +{ + struct tpiu_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + coresight_unregister(drvdata->csdev); + + return 0; +} + #ifdef CONFIG_PM static int tpiu_runtime_suspend(struct device *dev) { @@ -216,6 +225,8 @@ static const struct amba_id tpiu_ids[] = { { 0, 0}, };
+MODULE_DEVICE_TABLE(amba, tpiu_ids); + static struct amba_driver tpiu_driver = { .drv = { .name = "coresight-tpiu", @@ -224,6 +235,12 @@ static struct amba_driver tpiu_driver = { .suppress_bind_attrs = true, }, .probe = tpiu_probe, + .remove = tpiu_remove, .id_table = tpiu_ids, }; -builtin_amba_driver(tpiu_driver); +module_amba_driver(tpiu_driver); + +MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); +MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight TPIU (Trace Port Interface Unit) driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-tpiu as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add a tpiu_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-tmc as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m - append -core to source file name to allow module to be called coresight-tmc by the Makefile - add an tmc_remove function, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 6 +++++- drivers/hwtracing/coresight/Makefile | 6 +++--- .../{coresight-tmc.c => coresight-tmc-core.c} | 19 ++++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) rename drivers/hwtracing/coresight/{coresight-tmc.c => coresight-tmc-core.c} (96%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index 8fd9887fb03b..fc48ae086746 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -25,7 +25,8 @@ config CORESIGHT_LINKS_AND_SINKS entity at run time to form a complete trace path.
config CORESIGHT_LINK_AND_SINK_TMC - bool "Coresight generic TMC driver" + tristate "Coresight generic TMC driver" + depends on CORESIGHT_LINKS_AND_SINKS help This enables support for the Trace Memory Controller driver. @@ -34,6 +35,9 @@ config CORESIGHT_LINK_AND_SINK_TMC complies with the generic implementation of the component without special enhancement or added features.
+ To compile this driver as a module, choose M here: the + module will be called coresight-tmc. + config CORESIGHT_CATU bool "Coresight Address Translation Unit (CATU) driver" depends on CORESIGHT_LINK_AND_SINK_TMC diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 271dc255454f..f2a568b969c4 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -4,9 +4,9 @@ # obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o \ coresight-platform.o coresight-sysfs.o -obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \ - coresight-tmc-etf.o \ - coresight-tmc-etr.o +obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o +coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \ + coresight-tmc-etr.o obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \ diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc-core.c similarity index 96% rename from drivers/hwtracing/coresight/coresight-tmc.c rename to drivers/hwtracing/coresight/coresight-tmc-core.c index 7040d583bed9..29d191ba7ccf 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -561,6 +561,16 @@ static void tmc_shutdown(struct amba_device *adev) spin_unlock_irqrestore(&drvdata->spinlock, flags); }
+static int __exit tmc_remove(struct amba_device *adev) +{ + struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + misc_deregister(&drvdata->miscdev); + coresight_unregister(drvdata->csdev); + + return 0; +} + static const struct amba_id tmc_ids[] = { CS_AMBA_ID(0x000bb961), /* Coresight SoC 600 TMC-ETR/ETS */ @@ -572,6 +582,8 @@ static const struct amba_id tmc_ids[] = { { 0, 0}, };
+MODULE_DEVICE_TABLE(amba, tmc_ids); + static struct amba_driver tmc_driver = { .drv = { .name = "coresight-tmc", @@ -580,6 +592,11 @@ static struct amba_driver tmc_driver = { }, .probe = tmc_probe, .shutdown = tmc_shutdown, + .remove = tmc_remove, .id_table = tmc_ids, }; -builtin_amba_driver(tmc_driver); +module_amba_driver(tmc_driver); + +MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); +MODULE_DESCRIPTION("Arm CoreSight Trace Memory Controller driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-tmc as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight-tmc by the Makefile
- add an tmc_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 6 +++++- drivers/hwtracing/coresight/Makefile | 6 +++--- .../{coresight-tmc.c => coresight-tmc-core.c} | 19 ++++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) rename drivers/hwtracing/coresight/{coresight-tmc.c => coresight-tmc-core.c} (96%)
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc-core.c similarity index 96% rename from drivers/hwtracing/coresight/coresight-tmc.c rename to drivers/hwtracing/coresight/coresight-tmc-core.c index 7040d583bed9..29d191ba7ccf 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -561,6 +561,16 @@ static void tmc_shutdown(struct amba_device *adev) spin_unlock_irqrestore(&drvdata->spinlock, flags); } +static int __exit tmc_remove(struct amba_device *adev) +{
- struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
- misc_deregister(&drvdata->miscdev);
- coresight_unregister(drvdata->csdev);
Same comment as the etb10 driver. Will check this out.
Suzuki
On 08/05/2020 04:49 PM, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-tmc as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to
be called coresight-tmc by the Makefile
- add an tmc_remove function, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 6 +++++- drivers/hwtracing/coresight/Makefile | 6 +++--- .../{coresight-tmc.c => coresight-tmc-core.c} | 19 ++++++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) rename drivers/hwtracing/coresight/{coresight-tmc.c => coresight-tmc-core.c} (96%)
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc-core.c similarity index 96% rename from drivers/hwtracing/coresight/coresight-tmc.c rename to drivers/hwtracing/coresight/coresight-tmc-core.c index 7040d583bed9..29d191ba7ccf 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -561,6 +561,16 @@ static void tmc_shutdown(struct amba_device *adev) spin_unlock_irqrestore(&drvdata->spinlock, flags); } +static int __exit tmc_remove(struct amba_device *adev) +{ + struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
+ misc_deregister(&drvdata->miscdev); + coresight_unregister(drvdata->csdev);
Same comment as the etb10 driver. Will check this out.
As with the etb10 driver, with the comment added here:
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Suzuki
From: Mian Yousaf Kaukab ykaukab@suse.de
Dynamic-funnel uses module_amba_driver to register. Whereas static-funnel uses builtin_platform_driver. Combine these init calls into a single module_init/exit pair in preparation to make the driver modular.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- .../hwtracing/coresight/coresight-funnel.c | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 900690a9f7f0..46b277ed8606 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -341,7 +341,6 @@ static struct platform_driver static_funnel_driver = { .suppress_bind_attrs = true, }, }; -builtin_platform_driver(static_funnel_driver);
static int dynamic_funnel_probe(struct amba_device *adev, const struct amba_id *id) @@ -372,4 +371,31 @@ static struct amba_driver dynamic_funnel_driver = { .probe = dynamic_funnel_probe, .id_table = dynamic_funnel_ids, }; -builtin_amba_driver(dynamic_funnel_driver); + +static int __init funnel_init(void) +{ + int ret; + + ret = platform_driver_register(&static_funnel_driver); + if (ret) { + pr_info("Error registering platform driver\n"); + return ret; + } + + ret = amba_driver_register(&dynamic_funnel_driver); + if (ret) { + pr_info("Error registering amba driver\n"); + platform_driver_unregister(&static_funnel_driver); + } + + return ret; +} + +static void __exit funnel_exit(void) +{ + platform_driver_unregister(&static_funnel_driver); + amba_driver_unregister(&dynamic_funnel_driver); +} + +module_init(funnel_init); +module_exit(funnel_exit);
From: Mian Yousaf Kaukab ykaukab@suse.de
Dynamic-replicator uses module_amba_driver to register. Whereas static-replicator uses builtin_platform_driver. Combine these init calls into a single module_init/exit pair in preparation to make the driver modular.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- .../coresight/coresight-replicator.c | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 78acf29c49ca..6b30bcee7458 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -360,7 +360,6 @@ static struct platform_driver static_replicator_driver = { .suppress_bind_attrs = true, }, }; -builtin_platform_driver(static_replicator_driver);
static int dynamic_replicator_probe(struct amba_device *adev, const struct amba_id *id) @@ -383,4 +382,31 @@ static struct amba_driver dynamic_replicator_driver = { .probe = dynamic_replicator_probe, .id_table = dynamic_replicator_ids, }; -builtin_amba_driver(dynamic_replicator_driver); + +static int __init replicator_init(void) +{ + int ret; + + ret = platform_driver_register(&static_replicator_driver); + if (ret) { + pr_info("Error registering platform driver\n"); + return ret; + } + + ret = amba_driver_register(&dynamic_replicator_driver); + if (ret) { + pr_info("Error registering amba driver\n"); + platform_driver_unregister(&static_replicator_driver); + } + + return ret; +} + +static void __exit replicator_exit(void) +{ + platform_driver_unregister(&static_replicator_driver); + amba_driver_unregister(&dynamic_replicator_driver); +} + +module_init(replicator_init); +module_exit(replicator_exit);
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-funnel and coresight-replicator as modules, for ease of development.
- Kconfig becomes a tristate, to allow =m - add funnel_remove and replicator_remove functions, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 ++- .../hwtracing/coresight/coresight-funnel.c | 32 ++++++++++++++++++ .../coresight/coresight-replicator.c | 33 +++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index fc48ae086746..f31778dd0b5d 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -17,13 +17,16 @@ menuconfig CORESIGHT
if CORESIGHT config CORESIGHT_LINKS_AND_SINKS - bool "CoreSight Link and Sink drivers" + tristate "CoreSight Link and Sink drivers" help This enables support for CoreSight link and sink drivers that are responsible for transporting and collecting the trace data respectively. Link and sinks are dynamically aggregated with a trace entity at run time to form a complete trace path.
+ To compile these drivers as modules, choose M here: the + modules will be called coresight-funnel and coresight-replicator. + config CORESIGHT_LINK_AND_SINK_TMC tristate "Coresight generic TMC driver"
diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 46b277ed8606..062694ef9879 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -274,6 +274,15 @@ static int funnel_probe(struct device *dev, struct resource *res) return ret; }
+static int __exit funnel_remove(struct device *dev) +{ + struct funnel_drvdata *drvdata = dev_get_drvdata(dev); + + coresight_unregister(drvdata->csdev); + + return 0; +} + #ifdef CONFIG_PM static int funnel_runtime_suspend(struct device *dev) { @@ -319,20 +328,31 @@ static int static_funnel_probe(struct platform_device *pdev) return ret; }
+static int __exit static_funnel_remove(struct platform_device *pdev) +{ + funnel_remove(&pdev->dev); + pm_runtime_disable(&pdev->dev); + return 0; +} + static const struct of_device_id static_funnel_match[] = { {.compatible = "arm,coresight-static-funnel"}, {} };
+MODULE_DEVICE_TABLE(of, static_funnel_match); + #ifdef CONFIG_ACPI static const struct acpi_device_id static_funnel_ids[] = { {"ARMHC9FE", 0}, {}, }; +MODULE_DEVICE_TABLE(acpi, static_funnel_ids); #endif
static struct platform_driver static_funnel_driver = { .probe = static_funnel_probe, + .probe = static_funnel_remove, .driver = { .name = "coresight-static-funnel", .of_match_table = static_funnel_match, @@ -348,6 +368,11 @@ static int dynamic_funnel_probe(struct amba_device *adev, return funnel_probe(&adev->dev, &adev->res); }
+static int __exit dynamic_funnel_remove(struct amba_device *adev) +{ + return funnel_remove(&adev->dev); +} + static const struct amba_id dynamic_funnel_ids[] = { { .id = 0x000bb908, @@ -361,6 +386,8 @@ static const struct amba_id dynamic_funnel_ids[] = { { 0, 0}, };
+MODULE_DEVICE_TABLE(amba, dynamic_funnel_ids); + static struct amba_driver dynamic_funnel_driver = { .drv = { .name = "coresight-dynamic-funnel", @@ -369,6 +396,7 @@ static struct amba_driver dynamic_funnel_driver = { .suppress_bind_attrs = true, }, .probe = dynamic_funnel_probe, + .remove = dynamic_funnel_remove, .id_table = dynamic_funnel_ids, };
@@ -399,3 +427,7 @@ static void __exit funnel_exit(void)
module_init(funnel_init); module_exit(funnel_exit); + +MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight Funnel Driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 6b30bcee7458..05db434c79d0 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -291,6 +291,14 @@ static int replicator_probe(struct device *dev, struct resource *res) return ret; }
+static int __exit replicator_remove(struct device *dev) +{ + struct replicator_drvdata *drvdata = dev_get_drvdata(dev); + + coresight_unregister(drvdata->csdev); + return 0; +} + static int static_replicator_probe(struct platform_device *pdev) { int ret; @@ -310,6 +318,13 @@ static int static_replicator_probe(struct platform_device *pdev) return ret; }
+static int __exit static_replicator_remove(struct platform_device *pdev) +{ + replicator_remove(&pdev->dev); + pm_runtime_disable(&pdev->dev); + return 0; +} + #ifdef CONFIG_PM static int replicator_runtime_suspend(struct device *dev) { @@ -342,18 +357,22 @@ static const struct of_device_id static_replicator_match[] = { {.compatible = "arm,coresight-static-replicator"}, {} }; +MODULE_DEVICE_TABLE(of, static_replicator_match);
#ifdef CONFIG_ACPI static const struct acpi_device_id static_replicator_acpi_ids[] = { {"ARMHC985", 0}, /* ARM CoreSight Static Replicator */ {} }; +MODULE_DEVICE_TABLE(acpi, static_replicator_acpi_ids); #endif
static struct platform_driver static_replicator_driver = { .probe = static_replicator_probe, + .remove = static_replicator_remove, .driver = { .name = "coresight-static-replicator", + .owner = THIS_MODULE, .of_match_table = of_match_ptr(static_replicator_match), .acpi_match_table = ACPI_PTR(static_replicator_acpi_ids), .pm = &replicator_dev_pm_ops, @@ -367,19 +386,28 @@ static int dynamic_replicator_probe(struct amba_device *adev, return replicator_probe(&adev->dev, &adev->res); }
+static int __exit dynamic_replicator_remove(struct amba_device *adev) +{ + return replicator_remove(&adev->dev); +} + static const struct amba_id dynamic_replicator_ids[] = { CS_AMBA_ID(0x000bb909), CS_AMBA_ID(0x000bb9ec), /* Coresight SoC-600 */ {}, };
+MODULE_DEVICE_TABLE(amba, dynamic_replicator_ids); + static struct amba_driver dynamic_replicator_driver = { .drv = { .name = "coresight-dynamic-replicator", .pm = &replicator_dev_pm_ops, + .owner = THIS_MODULE, .suppress_bind_attrs = true, }, .probe = dynamic_replicator_probe, + .remove = dynamic_replicator_remove, .id_table = dynamic_replicator_ids, };
@@ -410,3 +438,8 @@ static void __exit replicator_exit(void)
module_init(replicator_init); module_exit(replicator_exit); + +MODULE_AUTHOR("Pratik Patel pratikp@codeaurora.org"); +MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight Replicator Driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-funnel and coresight-replicator as modules, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add funnel_remove and replicator_remove functions, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
nit: Please split the changes to multiple drivers into their respective patches. Also, I don't see any harm in merging the "remove multiple init calls" patches with the respective "allow module" for the respective drivers.
Otherwise looks good to me.
Suzuki
On Thu, Aug 06, 2020 at 12:04:24AM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Kim Phillips kim.phillips@arm.com
Allow to build coresight-funnel and coresight-replicator as modules, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add funnel_remove and replicator_remove functions, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
nit: Please split the changes to multiple drivers into their respective patches. Also, I don't see any harm in merging the "remove multiple init calls" patches with the respective "allow module" for the respective drivers.
I'll rearrange the patches.
Thanks, Tingwei
Otherwise looks good to me.
Suzuki
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); + +/* + * 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 1626bc885dfe..25578579559b 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; @@ -1224,7 +1238,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); @@ -1535,8 +1550,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) {
From: Mike Leach mike.leach@linaro.org
CTI code to remove sysfs link to other devices on shutdown, incorrectly tries to remove a single ended link when these are all double ended. This implementation leaves elements in the link info structure undefined which results in a crash in recent tests for driver module unload.
This patch corrects the link removal code.
Fixes: 73274abb6557 (coresight: cti: Add in sysfs links to other coresight devices) Reported-by: Tingwei Zhang tingwei@codeaurora.org Signed-off-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight-cti.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 1f470c47ba70..1c34db7d1b1b 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -511,12 +511,15 @@ static bool cti_add_sysfs_link(struct cti_drvdata *drvdata, return !link_err; }
-static void cti_remove_sysfs_link(struct cti_trig_con *tc) +static void cti_remove_sysfs_link(struct cti_drvdata *drvdata, + struct cti_trig_con *tc) { struct coresight_sysfs_link link_info;
+ link_info.orig = drvdata->csdev; link_info.orig_name = tc->con_dev_name; link_info.target = tc->con_dev; + link_info.target_name = dev_name(&drvdata->csdev->dev); coresight_remove_sysfs_link(&link_info); }
@@ -606,7 +609,7 @@ void cti_remove_assoc_from_csdev(struct coresight_device *csdev) ctidev = &ctidrv->ctidev; list_for_each_entry(tc, &ctidev->trig_cons, node) { if (tc->con_dev == csdev->ect_dev) { - cti_remove_sysfs_link(tc); + cti_remove_sysfs_link(ctidrv, tc); tc->con_dev = NULL; break; } @@ -658,7 +661,7 @@ static void cti_remove_conn_xrefs(struct cti_drvdata *drvdata) if (tc->con_dev) { coresight_set_assoc_ectdev_mutex(tc->con_dev, NULL); - cti_remove_sysfs_link(tc); + cti_remove_sysfs_link(drvdata, tc); tc->con_dev = NULL; } }
During module unload, a coresight driver module will call back into the CTI driver to remove any links between the two devices.
The current code has 2 issues:- 1) in the CTI driver the matching code is matching to the wrong device so misses all the links. 2) The callback is called too late in the unload process resulting in a crash.
This fixes both the issues.
Fixes: 177af8285b59 (coresight: cti: Enable CTI associated with devices) Reported-by: Tingwei Zhang tingwei@codeaurora.org Signed-off-by: Mike Leach mike.leach@linaro.org Signed-off-by: Tingwei Zhang tingwei@codeaurora.org --- drivers/hwtracing/coresight/coresight-cti.c | 2 +- drivers/hwtracing/coresight/coresight.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 1c34db7d1b1b..6bb06c5715db 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -608,7 +608,7 @@ void cti_remove_assoc_from_csdev(struct coresight_device *csdev) ctidrv = csdev_to_cti_drvdata(csdev->ect_dev); ctidev = &ctidrv->ctidev; list_for_each_entry(tc, &ctidev->trig_cons, node) { - if (tc->con_dev == csdev->ect_dev) { + if (tc->con_dev == csdev) { cti_remove_sysfs_link(ctidrv, tc); tc->con_dev = NULL; break; diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 25578579559b..8b55383cfcf1 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -1238,8 +1238,6 @@ static void coresight_device_release(struct device *dev) { struct coresight_device *csdev = to_coresight_device(dev);
- if (cti_assoc_ops && cti_assoc_ops->remove) - cti_assoc_ops->remove(csdev); fwnode_handle_put(csdev->dev.fwnode); kfree(csdev->refcnt); kfree(csdev); @@ -1574,6 +1572,8 @@ void coresight_unregister(struct coresight_device *csdev) { etm_perf_del_symlink_sink(csdev); /* Remove references of that device in the topology */ + if (cti_assoc_ops && cti_assoc_ops->remove) + cti_assoc_ops->remove(csdev); coresight_remove_conns(csdev); coresight_clear_default_sink(csdev); coresight_release_platform_data(csdev, csdev->pdata);
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Missing, From:.... if this is authored by Mike Leach.
During module unload, a coresight driver module will call back into the CTI driver to remove any links between the two devices.
The current code has 2 issues:-
- in the CTI driver the matching code is matching to the wrong device
so misses all the links. 2) The callback is called too late in the unload process resulting in a crash.
This fixes both the issues.
Fixes: 177af8285b59 (coresight: cti: Enable CTI associated with devices) Reported-by: Tingwei Zhang tingwei@codeaurora.org Signed-off-by: Mike Leach mike.leach@linaro.org Signed-off-by: Tingwei Zhang tingwei@codeaurora.org
drivers/hwtracing/coresight/coresight-cti.c | 2 +- drivers/hwtracing/coresight/coresight.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 1c34db7d1b1b..6bb06c5715db 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -608,7 +608,7 @@ void cti_remove_assoc_from_csdev(struct coresight_device *csdev) ctidrv = csdev_to_cti_drvdata(csdev->ect_dev); ctidev = &ctidrv->ctidev; list_for_each_entry(tc, &ctidev->trig_cons, node) {
if (tc->con_dev == csdev->ect_dev) {
if (tc->con_dev == csdev) { cti_remove_sysfs_link(ctidrv, tc); tc->con_dev = NULL; break;
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 25578579559b..8b55383cfcf1 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -1238,8 +1238,6 @@ static void coresight_device_release(struct device *dev) { struct coresight_device *csdev = to_coresight_device(dev);
- if (cti_assoc_ops && cti_assoc_ops->remove)
fwnode_handle_put(csdev->dev.fwnode); kfree(csdev->refcnt); kfree(csdev);cti_assoc_ops->remove(csdev);
@@ -1574,6 +1572,8 @@ void coresight_unregister(struct coresight_device *csdev) { etm_perf_del_symlink_sink(csdev); /* Remove references of that device in the topology */
- if (cti_assoc_ops && cti_assoc_ops->remove)
cti_assoc_ops->remove(csdev);
Acked-by: Suzuki K Poulose suzuki.poulose@arm.com
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 8b55383cfcf1..b35f5b270a94 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 */ };
/*
CTI device is enabled when associated coresight device is enabled. Increase the module and device reference count for CTI device when it's enabled. This can prevent CTI device be removed or module be unloaded when CTI device is enabled by an active trace session.
Signed-off-by: Mike Leach mike.leach@linaro.org Signed-off-by: Tingwei Zhang tingwei@codeaurora.org --- drivers/hwtracing/coresight/coresight.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index b35f5b270a94..78439b7cd8ba 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -242,19 +242,30 @@ coresight_control_assoc_ectdev(struct coresight_device *csdev, bool enable) { int ect_ret = 0; struct coresight_device *ect_csdev = csdev->ect_dev; + struct module *mod;
if (!ect_csdev) return 0; if ((!ect_ops(ect_csdev)->enable) || (!ect_ops(ect_csdev)->disable)) return 0;
+ mod = ect_csdev->dev.parent->driver->owner; if (enable) { - ect_ret = ect_ops(ect_csdev)->enable(ect_csdev); - if (!ect_ret) - csdev->ect_enabled = true; + if (try_module_get(mod)) { + ect_ret = ect_ops(ect_csdev)->enable(ect_csdev); + if (ect_ret) { + module_put(mod); + } else { + get_device(ect_csdev->dev.parent); + csdev->ect_enabled = true; + } + } else + ect_ret = -ENODEV; } else { if (csdev->ect_enabled) { ect_ret = ect_ops(ect_csdev)->disable(ect_csdev); + put_device(ect_csdev->dev.parent); + module_put(mod); csdev->ect_enabled = false; } }
Allow to build coresight-cti as a module, for ease of development.
- Kconfig becomes a tristate, to allow =m - append -core to source file name to allow module to be called coresight-cti by the Makefile - add an cti_remove function, for module unload - move cti_remove_conn_xrefs to cti_remove - add a MODULE_DEVICE_TABLE for autoloading on boot
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/Makefile | 4 ++-- .../{coresight-cti.c => coresight-cti-core.c} | 19 ++++++++++++++++++- .../hwtracing/coresight/coresight-platform.c | 1 + drivers/hwtracing/coresight/coresight.c | 1 + 5 files changed, 26 insertions(+), 4 deletions(-) rename drivers/hwtracing/coresight/{coresight-cti.c => coresight-cti-core.c} (98%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index f31778dd0b5d..b04aae2ceecc 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -136,7 +136,7 @@ config CORESIGHT_CPU_DEBUG module will be called coresight-cpu-debug.
config CORESIGHT_CTI - bool "CoreSight Cross Trigger Interface (CTI) driver" + tristate "CoreSight Cross Trigger Interface (CTI) driver" depends on ARM || ARM64 help This driver provides support for CoreSight CTI and CTM components. @@ -147,6 +147,9 @@ config CORESIGHT_CTI halt compared to disabling sources and sinks normally in driver software.
+ To compile this driver as a module, choose M here: the + module will be called coresight-cti. + config CORESIGHT_CTI_INTEGRATION_REGS bool "Access CTI CoreSight Integration Registers" depends on CORESIGHT_CTI diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index f2a568b969c4..0359d5a1588f 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -19,6 +19,6 @@ coresight-etm4x-y := coresight-etm4x-core.o coresight-etm4x-sysfs.o obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o obj-$(CONFIG_CORESIGHT_CATU) += coresight-catu.o -obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o \ - coresight-cti-platform.o \ +obj-$(CONFIG_CORESIGHT_CTI) += coresight-cti.o +coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ coresight-cti-sysfs.o diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti-core.c similarity index 98% rename from drivers/hwtracing/coresight/coresight-cti.c rename to drivers/hwtracing/coresight/coresight-cti-core.c index 6bb06c5715db..ca31da6cf1c0 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti-core.c @@ -838,7 +838,6 @@ static void cti_device_release(struct device *dev) struct cti_drvdata *ect_item, *ect_tmp;
mutex_lock(&ect_mutex); - cti_remove_conn_xrefs(drvdata); cti_pm_release(drvdata);
/* remove from the list */ @@ -853,6 +852,18 @@ static void cti_device_release(struct device *dev) if (drvdata->csdev_release) drvdata->csdev_release(dev); } +static int __exit cti_remove(struct amba_device *adev) +{ + struct cti_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + mutex_lock(&ect_mutex); + cti_remove_conn_xrefs(drvdata); + mutex_unlock(&ect_mutex); + + coresight_unregister(drvdata->csdev); + + return 0; +}
static int cti_probe(struct amba_device *adev, const struct amba_id *id) { @@ -972,6 +983,7 @@ static const struct amba_id cti_ids[] = { CS_AMBA_UCI_ID(0x000bb9ed, uci_id_cti), /* Coresight CTI (SoC 600) */ { 0, 0}, }; +MODULE_DEVICE_TABLE(amba, cti_ids);
static struct amba_driver cti_driver = { .drv = { @@ -980,6 +992,7 @@ static struct amba_driver cti_driver = { .suppress_bind_attrs = true, }, .probe = cti_probe, + .remove = cti_remove, .id_table = cti_ids, };
@@ -1002,3 +1015,7 @@ static void __exit cti_exit(void)
module_init(cti_init); module_exit(cti_exit); + +MODULE_AUTHOR("Mike Leach mike.leach@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight CTI Driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c index bfd44231d7ad..635d55c1d580 100644 --- a/drivers/hwtracing/coresight/coresight-platform.c +++ b/drivers/hwtracing/coresight/coresight-platform.c @@ -75,6 +75,7 @@ coresight_find_csdev_by_fwnode(struct fwnode_handle *r_fwnode) } return csdev; } +EXPORT_SYMBOL_GPL(coresight_find_csdev_by_fwnode);
#ifdef CONFIG_OF static inline bool of_coresight_legacy_ep_is_input(struct device_node *ep) diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 78439b7cd8ba..3b4abdda1f72 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -289,6 +289,7 @@ void coresight_set_assoc_ectdev_mutex(struct coresight_device *csdev, csdev->ect_dev = ect_csdev; mutex_unlock(&coresight_mutex); } +EXPORT_SYMBOL_GPL(coresight_set_assoc_ectdev_mutex);
static int coresight_enable_sink(struct coresight_device *csdev, u32 mode, void *data)
From: Mian Yousaf Kaukab ykaukab@suse.de
Make etr_catu_buf_ops static. Instead of directly accessing it in etr_buf_ops[], add a function to let catu driver register the ops at runtime. Break circular dependency between tmc-etr and catu drivers.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org --- drivers/hwtracing/coresight/coresight-catu.c | 22 +++++++++++++++++-- drivers/hwtracing/coresight/coresight-catu.h | 2 -- .../hwtracing/coresight/coresight-tmc-etr.c | 15 +++++++++++-- drivers/hwtracing/coresight/coresight-tmc.h | 3 +++ 4 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 1801804a7762..47696a7d24a7 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -358,7 +358,7 @@ static int catu_alloc_etr_buf(struct tmc_drvdata *tmc_drvdata, return 0; }
-const struct etr_buf_operations etr_catu_buf_ops = { +static const struct etr_buf_operations etr_catu_buf_ops = { .alloc = catu_alloc_etr_buf, .free = catu_free_etr_buf, .sync = catu_sync_etr_buf, @@ -582,4 +582,22 @@ static struct amba_driver catu_driver = { .id_table = catu_ids, };
-builtin_amba_driver(catu_driver); +static int __init catu_init(void) +{ + int ret; + + ret = amba_driver_register(&catu_driver); + if (ret) + pr_info("Error registering catu driver\n"); + tmc_etr_set_catu_ops(&etr_catu_buf_ops); + return ret; +} + +static void __exit catu_exit(void) +{ + tmc_etr_remove_catu_ops(); + amba_driver_unregister(&catu_driver); +} + +module_init(catu_init); +module_exit(catu_exit); diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h index 80ceee3c739c..6160c2d75a56 100644 --- a/drivers/hwtracing/coresight/coresight-catu.h +++ b/drivers/hwtracing/coresight/coresight-catu.h @@ -108,6 +108,4 @@ static inline bool coresight_is_catu_device(struct coresight_device *csdev) return true; }
-extern const struct etr_buf_operations etr_catu_buf_ops; - #endif diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index ad991a37e2d2..714f9e867e5f 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -794,10 +794,21 @@ static inline void tmc_etr_disable_catu(struct tmc_drvdata *drvdata) static const struct etr_buf_operations *etr_buf_ops[] = { [ETR_MODE_FLAT] = &etr_flat_buf_ops, [ETR_MODE_ETR_SG] = &etr_sg_buf_ops, - [ETR_MODE_CATU] = IS_ENABLED(CONFIG_CORESIGHT_CATU) - ? &etr_catu_buf_ops : NULL, + [ETR_MODE_CATU] = NULL, };
+void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu) +{ + etr_buf_ops[ETR_MODE_CATU] = catu; +} +EXPORT_SYMBOL_GPL(tmc_etr_set_catu_ops); + +void tmc_etr_remove_catu_ops(void) +{ + etr_buf_ops[ETR_MODE_CATU] = NULL; +} +EXPORT_SYMBOL_GPL(tmc_etr_remove_catu_ops); + static inline int tmc_etr_mode_alloc_buf(int mode, struct tmc_drvdata *drvdata, struct etr_buf *etr_buf, int node, diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 6e8d2dc33d17..b91ec7dde7bc 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -326,4 +326,7 @@ tmc_sg_table_buf_size(struct tmc_sg_table *sg_table)
struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata);
+void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu); +void tmc_etr_remove_catu_ops(void); + #endif
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Mian Yousaf Kaukab ykaukab@suse.de
Make etr_catu_buf_ops static. Instead of directly accessing it in etr_buf_ops[], add a function to let catu driver register the ops at runtime. Break circular dependency between tmc-etr and catu drivers.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org
drivers/hwtracing/coresight/coresight-catu.c | 22 +++++++++++++++++-- drivers/hwtracing/coresight/coresight-catu.h | 2 -- .../hwtracing/coresight/coresight-tmc-etr.c | 15 +++++++++++-- drivers/hwtracing/coresight/coresight-tmc.h | 3 +++ 4 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 1801804a7762..47696a7d24a7 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -358,7 +358,7 @@ static int catu_alloc_etr_buf(struct tmc_drvdata *tmc_drvdata, return 0; } -const struct etr_buf_operations etr_catu_buf_ops = { +static const struct etr_buf_operations etr_catu_buf_ops = { .alloc = catu_alloc_etr_buf, .free = catu_free_etr_buf, .sync = catu_sync_etr_buf, @@ -582,4 +582,22 @@ static struct amba_driver catu_driver = { .id_table = catu_ids, }; -builtin_amba_driver(catu_driver); +static int __init catu_init(void) +{
- int ret;
- ret = amba_driver_register(&catu_driver);
- if (ret)
pr_info("Error registering catu driver\n");
- tmc_etr_set_catu_ops(&etr_catu_buf_ops);
- return ret;
+}
+static void __exit catu_exit(void) +{
- tmc_etr_remove_catu_ops();
- amba_driver_unregister(&catu_driver);
+}
+module_init(catu_init); +module_exit(catu_exit);
Please add :
MODULE_AUTHOR("Suzuki K Poulose suzuki.poulose@arm.com"); MODULE_DESCRIPTION("Arm CoreSight Address Translation Unit driver"); MODULE_LICENSE("GPL v2");
With that
Reviewed-by : Suzuki K Poulose suzuki.poulose@arm.com
On 08/05/2020 05:09 PM, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
From: Mian Yousaf Kaukab ykaukab@suse.de
Make etr_catu_buf_ops static. Instead of directly accessing it in etr_buf_ops[], add a function to let catu driver register the ops at runtime. Break circular dependency between tmc-etr and catu drivers.
Signed-off-by: Mian Yousaf Kaukab ykaukab@suse.de Signed-off-by: Tingwei Zhang tingwei@codeaurora.org
drivers/hwtracing/coresight/coresight-catu.c | 22 +++++++++++++++++-- drivers/hwtracing/coresight/coresight-catu.h | 2 -- .../hwtracing/coresight/coresight-tmc-etr.c | 15 +++++++++++-- drivers/hwtracing/coresight/coresight-tmc.h | 3 +++ 4 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 1801804a7762..47696a7d24a7 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -358,7 +358,7 @@ static int catu_alloc_etr_buf(struct tmc_drvdata *tmc_drvdata, return 0; } -const struct etr_buf_operations etr_catu_buf_ops = { +static const struct etr_buf_operations etr_catu_buf_ops = { .alloc = catu_alloc_etr_buf, .free = catu_free_etr_buf, .sync = catu_sync_etr_buf, @@ -582,4 +582,22 @@ static struct amba_driver catu_driver = { .id_table = catu_ids, }; -builtin_amba_driver(catu_driver); +static int __init catu_init(void) +{ + int ret;
+ ret = amba_driver_register(&catu_driver); + if (ret) + pr_info("Error registering catu driver\n"); + tmc_etr_set_catu_ops(&etr_catu_buf_ops); + return ret; +}
+static void __exit catu_exit(void) +{ + tmc_etr_remove_catu_ops(); + amba_driver_unregister(&catu_driver); +}
+module_init(catu_init); +module_exit(catu_exit);
Please add :
MODULE_AUTHOR("Suzuki K Poulose suzuki.poulose@arm.com"); MODULE_DESCRIPTION("Arm CoreSight Address Translation Unit driver"); MODULE_LICENSE("GPL v2");
Please ignore that, I see this in the next patch.
Reviewed-by : Suzuki K Poulose suzuki.poulose@arm.com
Suzuki
Allow to build coresight-catu as modules, for ease of development. - Kconfig becomes a tristate, to allow =m - add catu_remove functions, for module unload - add a MODULE_DEVICE_TABLE for autoloading on boot
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 ++++- drivers/hwtracing/coresight/coresight-catu.c | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index b04aae2ceecc..dfe407cde262 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -42,7 +42,7 @@ config CORESIGHT_LINK_AND_SINK_TMC module will be called coresight-tmc.
config CORESIGHT_CATU - bool "Coresight Address Translation Unit (CATU) driver" + tristate "Coresight Address Translation Unit (CATU) driver" depends on CORESIGHT_LINK_AND_SINK_TMC help Enable support for the Coresight Address Translation Unit (CATU). @@ -52,6 +52,9 @@ config CORESIGHT_CATU by looking up the provided table. CATU can also be used in pass-through mode where the address is not translated.
+ To compile this driver as a module, choose M here: the + module will be called coresight-catu. + config CORESIGHT_SINK_TPIU tristate "Coresight generic TPIU driver" depends on CORESIGHT_LINKS_AND_SINKS diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index 47696a7d24a7..99430f6cf5a5 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -567,11 +567,21 @@ static int catu_probe(struct amba_device *adev, const struct amba_id *id) return ret; }
+static int __exit catu_remove(struct amba_device *adev) +{ + struct catu_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + coresight_unregister(drvdata->csdev); + return 0; +} + static struct amba_id catu_ids[] = { CS_AMBA_ID(0x000bb9ee), {}, };
+MODULE_DEVICE_TABLE(amba, catu_ids); + static struct amba_driver catu_driver = { .drv = { .name = "coresight-catu", @@ -579,6 +589,7 @@ static struct amba_driver catu_driver = { .suppress_bind_attrs = true, }, .probe = catu_probe, + .remove = catu_remove, .id_table = catu_ids, };
@@ -601,3 +612,7 @@ static void __exit catu_exit(void)
module_init(catu_init); module_exit(catu_exit); + +MODULE_AUTHOR("Suzuki K Poulose suzuki.poulose@arm.com"); +MODULE_DESCRIPTION("Arm CoreSight Address Translation Unit (CATU) Driver"); +MODULE_LICENSE("GPL v2");
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Allow to build coresight-catu as modules, for ease of development.
- Kconfig becomes a tristate, to allow =m
- add catu_remove functions, for module unload
- add a MODULE_DEVICE_TABLE for autoloading on boot
Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Reviewed-by: Mike Leach mike.leach@linaro.org
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
Enhance coresight developer's efficiency to debug coresight drivers. - Kconfig becomes a tristate, to allow =m - append -core to source file name to allow module to be called coresight by the Makefile - modules can have only one init/exit, so we add the etm_perf register/unregister function calls to the core init/exit functions. - add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org --- drivers/hwtracing/coresight/Kconfig | 5 ++- drivers/hwtracing/coresight/Makefile | 5 ++- .../{coresight.c => coresight-core.c} | 42 ++++++++++++++----- .../hwtracing/coresight/coresight-etm-perf.c | 8 +++- .../hwtracing/coresight/coresight-etm-perf.h | 3 ++ 5 files changed, 48 insertions(+), 15 deletions(-) rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (98%)
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index dfe407cde262..c1198245461d 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -3,7 +3,7 @@ # Coresight configuration # menuconfig CORESIGHT - bool "CoreSight Tracing Support" + tristate "CoreSight Tracing Support" depends on ARM || ARM64 depends on OF || ACPI select ARM_AMBA @@ -15,6 +15,9 @@ menuconfig CORESIGHT specification and configure the right series of components when a trace source gets enabled.
+ To compile this driver as a module, choose M here: the + module will be called coresight. + if CORESIGHT config CORESIGHT_LINKS_AND_SINKS tristate "CoreSight Link and Sink drivers" diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 0359d5a1588f..1b35b55bd420 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -2,8 +2,9 @@ # # Makefile for CoreSight drivers. # -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o \ - coresight-platform.o coresight-sysfs.o +obj-$(CONFIG_CORESIGHT) += coresight.o +coresight-y := coresight-core.o coresight-etm-perf.o coresight-platform.o \ + coresight-sysfs.o obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \ coresight-tmc-etr.o diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight-core.c similarity index 98% rename from drivers/hwtracing/coresight/coresight.c rename to drivers/hwtracing/coresight/coresight-core.c index 3b4abdda1f72..5cd200031613 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1444,16 +1444,6 @@ int coresight_timeout(void __iomem *addr, u32 offset, int position, int value) } EXPORT_SYMBOL_GPL(coresight_timeout);
-struct bus_type coresight_bustype = { - .name = "coresight", -}; - -static int __init coresight_init(void) -{ - return bus_register(&coresight_bustype); -} -postcore_initcall(coresight_init); - /* * coresight_release_platform_data: Release references to the devices connected * to the output port of this device. @@ -1662,3 +1652,35 @@ char *coresight_alloc_device_name(struct coresight_dev_list *dict, return name; } EXPORT_SYMBOL_GPL(coresight_alloc_device_name); + +struct bus_type coresight_bustype = { + .name = "coresight", +}; + +static int __init coresight_init(void) +{ + int ret; + + ret = bus_register(&coresight_bustype); + if (ret) + return ret; + + ret = etm_perf_init(); + if (ret) + bus_unregister(&coresight_bustype); + + return ret; +} + +static void __exit coresight_exit(void) +{ + etm_perf_exit(); + bus_unregister(&coresight_bustype); +} + +module_init(coresight_init); +module_exit(coresight_exit); + +MODULE_AUTHOR("Mathieu Poirier mathieu.poirier@linaro.org"); +MODULE_DESCRIPTION("Arm CoreSight tracer driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 3728c44e5763..668b3ff11576 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -591,7 +591,7 @@ void etm_perf_del_symlink_sink(struct coresight_device *csdev) csdev->ea = NULL; }
-static int __init etm_perf_init(void) +int __init etm_perf_init(void) { int ret;
@@ -618,4 +618,8 @@ static int __init etm_perf_init(void)
return ret; } -device_initcall(etm_perf_init); + +void __exit etm_perf_exit(void) +{ + perf_pmu_unregister(&etm_pmu); +} diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 05f89723e282..3e4f2ad5e193 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -82,4 +82,7 @@ static inline void *etm_perf_sink_config(struct perf_output_handle *handle)
#endif /* CONFIG_CORESIGHT */
+int __init etm_perf_init(void); +void __exit etm_perf_exit(void); + #endif
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Enhance coresight developer's efficiency to debug coresight drivers.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to be called coresight by the Makefile
- modules can have only one init/exit, so we add the etm_perf register/unregister function calls to the core init/exit functions.
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++- drivers/hwtracing/coresight/Makefile | 5 ++- .../{coresight.c => coresight-core.c} | 42 ++++++++++++++----- .../hwtracing/coresight/coresight-etm-perf.c | 8 +++- .../hwtracing/coresight/coresight-etm-perf.h | 3 ++ 5 files changed, 48 insertions(+), 15 deletions(-) rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (98%)
Personally, I would like to rename this to core.c dropping the "coresight-" prefix here (now that we have to do a rename). And we should do that ideally for all the other files (but not proposing it to be part of this series, and could be something that we could pursue if everyone agrees to it).
We are inside the coresight directory anyways and having a prefix doesn't help with anything.
The patch as such looks good to me.
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
On 08/05/2020 05:29 PM, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Enhance coresight developer's efficiency to debug coresight drivers.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to
be called coresight by the Makefile
- modules can have only one init/exit, so we add the etm_perf
register/unregister function calls to the core init/exit functions.
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++- drivers/hwtracing/coresight/Makefile | 5 ++- .../{coresight.c => coresight-core.c} | 42 ++++++++++++++----- .../hwtracing/coresight/coresight-etm-perf.c | 8 +++- .../hwtracing/coresight/coresight-etm-perf.h | 3 ++ 5 files changed, 48 insertions(+), 15 deletions(-) rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (98%)
Personally, I would like to rename this to core.c dropping the "coresight-" prefix here (now that we have to do a rename). And we should do that ideally for all the other files (but not proposing it to be part of this series, and could be something that we could pursue if everyone agrees to it).
We are inside the coresight directory anyways and having a prefix doesn't help with anything.
The patch as such looks good to me.
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
On a second look, I believe for the sake of completion, we should set the "owner" of the etm, now that we are a module. The question is, which one should that be. It could be the "coresight" or the "coresight-etm{3,4}x".
I believe the "coresight" is the better choice.
Cheers Suzuki
On 2020-08-06 17:33, Suzuki K Poulose wrote:
On 08/05/2020 05:29 PM, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Enhance coresight developer's efficiency to debug coresight drivers.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to
be called coresight by the Makefile
- modules can have only one init/exit, so we add the etm_perf
register/unregister function calls to the core init/exit functions.
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++- drivers/hwtracing/coresight/Makefile | 5 ++- .../{coresight.c => coresight-core.c} | 42 ++++++++++++++----- .../hwtracing/coresight/coresight-etm-perf.c | 8 +++- .../hwtracing/coresight/coresight-etm-perf.h | 3 ++ 5 files changed, 48 insertions(+), 15 deletions(-) rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (98%)
Personally, I would like to rename this to core.c dropping the "coresight-" prefix here (now that we have to do a rename). And we should do that ideally for all the other files (but not proposing it to be part of this series, and could be something that we could pursue if everyone agrees to it).
We are inside the coresight directory anyways and having a prefix doesn't help with anything.
The patch as such looks good to me.
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
On a second look, I believe for the sake of completion, we should set the "owner" of the etm, now that we are a module. The question is, which one should that be. It could be the "coresight" or the "coresight-etm{3,4}x".
I believe the "coresight" is the better choice.
If you mean pmu->owner, you shouldn't really have much of a choice - it should be the module containing the actual PMU callbacks, such that they can't suddenly disappear while the PMU is in use. Allowing perf to take a reference to some other module and not actually protect itself would not be good. It should be pretty rare that the correct owner is anything other than THIS_MODULE ;)
Hopefully the dependencies are such that the core module automatically holds its own reference to the individual ETM driver module(s) by the time it registers the PMU.
Robin.
On 2020-08-06 18:25, Robin Murphy wrote:
On 2020-08-06 17:33, Suzuki K Poulose wrote:
On 08/05/2020 05:29 PM, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Enhance coresight developer's efficiency to debug coresight drivers.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to
be called coresight by the Makefile
- modules can have only one init/exit, so we add the etm_perf
register/unregister function calls to the core init/exit functions.
- add a MODULE_DEVICE_TABLE for autoloading on boot
Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Leo Yan leo.yan@linaro.org Cc: Alexander Shishkin alexander.shishkin@linux.intel.com Cc: Randy Dunlap rdunlap@infradead.org Cc: Suzuki K Poulose Suzuki.Poulose@arm.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Russell King linux@armlinux.org.uk Signed-off-by: Kim Phillips kim.phillips@arm.com Signed-off-by: Tingwei Zhang tingwei@codeaurora.org Tested-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/Kconfig | 5 ++- drivers/hwtracing/coresight/Makefile | 5 ++- .../{coresight.c => coresight-core.c} | 42 ++++++++++++++----- .../hwtracing/coresight/coresight-etm-perf.c | 8 +++- .../hwtracing/coresight/coresight-etm-perf.h | 3 ++ 5 files changed, 48 insertions(+), 15 deletions(-) rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (98%)
Personally, I would like to rename this to core.c dropping the "coresight-" prefix here (now that we have to do a rename). And we should do that ideally for all the other files (but not proposing it to be part of this series, and could be something that we could pursue if everyone agrees to it).
We are inside the coresight directory anyways and having a prefix doesn't help with anything.
The patch as such looks good to me.
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
On a second look, I believe for the sake of completion, we should set the "owner" of the etm, now that we are a module. The question is, which one should that be. It could be the "coresight" or the "coresight-etm{3,4}x".
I believe the "coresight" is the better choice.
If you mean pmu->owner, you shouldn't really have much of a choice - it
...by which I meant pmu->module, obviously. Oops :)
Anyway, that should certainly be set not just for completeness but for correctness, since there do exist users who are adventurous enough to try unloading modules while perf is running.
Robin.
should be the module containing the actual PMU callbacks, such that they can't suddenly disappear while the PMU is in use. Allowing perf to take a reference to some other module and not actually protect itself would not be good. It should be pretty rare that the correct owner is anything other than THIS_MODULE ;)
Hopefully the dependencies are such that the core module automatically holds its own reference to the individual ETM driver module(s) by the time it registers the PMU.
Robin.
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 08/06/2020 06:39 PM, Robin Murphy wrote:
On 2020-08-06 18:25, Robin Murphy wrote:
On 2020-08-06 17:33, Suzuki K Poulose wrote:
On 08/05/2020 05:29 PM, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Enhance coresight developer's efficiency to debug coresight drivers.
- Kconfig becomes a tristate, to allow =m
- append -core to source file name to allow module to
be called coresight by the Makefile
- modules can have only one init/exit, so we add the etm_perf
register/unregister function calls to the core init/exit functions.
- add a MODULE_DEVICE_TABLE for autoloading on boot
drivers/hwtracing/coresight/Kconfig | 5 ++- drivers/hwtracing/coresight/Makefile | 5 ++- .../{coresight.c => coresight-core.c} | 42 ++++++++++++++----- .../hwtracing/coresight/coresight-etm-perf.c | 8 +++- .../hwtracing/coresight/coresight-etm-perf.h | 3 ++ 5 files changed, 48 insertions(+), 15 deletions(-) rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} (98%)
Personally, I would like to rename this to core.c dropping the "coresight-" prefix here (now that we have to do a rename). And we should do that ideally for all the other files (but not proposing it to be part of this series, and could be something that we could pursue if everyone agrees to it).
We are inside the coresight directory anyways and having a prefix doesn't help with anything.
The patch as such looks good to me.
Reviewed-by: Suzuki K Poulose suzuki.poulose@arm.com
On a second look, I believe for the sake of completion, we should set the "owner" of the etm, now that we are a module. The question is, which one should that be. It could be the "coresight" or the "coresight-etm{3,4}x".
I believe the "coresight" is the better choice.
If you mean pmu->owner, you shouldn't really have much of a choice - it
...by which I meant pmu->module, obviously. Oops :)
Anyway, that should certainly be set not just for completeness but for correctness, since there do exist users who are adventurous enough to try unloading modules while perf is running.
Right. It should be the coresight module, as it holds the PMU code and THIS_MODULE is sufficient.
Robin.
should be the module containing the actual PMU callbacks, such that they can't suddenly disappear while the PMU is in use. Allowing perf to take a reference to some other module and not actually protect itself would not be good. It should be pretty rare that the correct owner is anything other than THIS_MODULE ;)
Hopefully the dependencies are such that the core module automatically holds its own reference to the individual ETM driver module(s) by the time it registers the PMU.
This is taken care of by holding module reference whenever we prepare a perf session.
Suzuki
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Allow to build coresight as modules. This gives developers the feasibility to test their code without reboot.
I ran this on my Juno and modules are auto loaded. However:
root@juno-server:~# lsmod Module Size Used by coresight_tmc 40960 -2 coresight_etm4x 61440 -2 coresight_funnel 20480 -2 coresight_stm 24576 -2 coresight_replicator 20480 -2 coresight_cpu_debug 20480 -2 coresight_tpiu 20480 -2 coresight 45056 -2
The used by count is -2 for all of them. Does anyone hit this ? I am unable to remove the modules.
Suzuki
I had no issues on Juno, though I was manually insmod / rmmod them Mike
On Wed, 5 Aug 2020 at 18:12, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Allow to build coresight as modules. This gives developers the feasibility to test their code without reboot.
I ran this on my Juno and modules are auto loaded. However:
root@juno-server:~# lsmod Module Size Used by coresight_tmc 40960 -2 coresight_etm4x 61440 -2 coresight_funnel 20480 -2 coresight_stm 24576 -2 coresight_replicator 20480 -2 coresight_cpu_debug 20480 -2 coresight_tpiu 20480 -2 coresight 45056 -2
The used by count is -2 for all of them. Does anyone hit this ? I am unable to remove the modules.
Suzuki
On Thu, Aug 06, 2020 at 01:17:23AM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Allow to build coresight as modules. This gives developers the
feasibility to
test their code without reboot.
I ran this on my Juno and modules are auto loaded. However:
root@juno-server:~# lsmod Module Size Used by coresight_tmc 40960 -2 coresight_etm4x 61440 -2 coresight_funnel 20480 -2 coresight_stm 24576 -2 coresight_replicator 20480 -2 coresight_cpu_debug 20480 -2 coresight_tpiu 20480 -2 coresight 45056 -2
The used by count is -2 for all of them. Does anyone hit this ? I am unable to remove the modules.
I tested on db845c and modules are auto loaded. I didn't see this issue. Module can be removed.
Thanks, Tingwei
Suzuki
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Just tried autoload on DB410 - all good.
Mike
On Thu, 6 Aug 2020 at 04:06, Tingwei Zhang tingweiz@codeaurora.org wrote:
On Thu, Aug 06, 2020 at 01:17:23AM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Allow to build coresight as modules. This gives developers the
feasibility to
test their code without reboot.
I ran this on my Juno and modules are auto loaded. However:
root@juno-server:~# lsmod Module Size Used by coresight_tmc 40960 -2 coresight_etm4x 61440 -2 coresight_funnel 20480 -2 coresight_stm 24576 -2 coresight_replicator 20480 -2 coresight_cpu_debug 20480 -2 coresight_tpiu 20480 -2 coresight 45056 -2
The used by count is -2 for all of them. Does anyone hit this ? I am unable to remove the modules.
I tested on db845c and modules are auto loaded. I didn't see this issue. Module can be removed.
Thanks, Tingwei
Suzuki
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 08/06/2020 10:38 AM, Mike Leach wrote:
Just tried autoload on DB410 - all good.
Mike
On Thu, 6 Aug 2020 at 04:06, Tingwei Zhang tingweiz@codeaurora.org wrote:
On Thu, Aug 06, 2020 at 01:17:23AM +0800, Suzuki K Poulose wrote:
On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
Allow to build coresight as modules. This gives developers the
feasibility to
test their code without reboot.
I ran this on my Juno and modules are auto loaded. However:
root@juno-server:~# lsmod Module Size Used by coresight_tmc 40960 -2 coresight_etm4x 61440 -2 coresight_funnel 20480 -2 coresight_stm 24576 -2 coresight_replicator 20480 -2 coresight_cpu_debug 20480 -2 coresight_tpiu 20480 -2 coresight 45056 -2
The used by count is -2 for all of them. Does anyone hit this ? I am unable to remove the modules.
Apologies, it was my mistake. It had the module_unload disabled in config. It is all normal now. I will test the misc_device race
Cheers Suzuki