| Message ID | 20180518012024.22645-6-kim.phillips@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 17, 2018 at 08:20:24PM -0500, Kim Phillips wrote: > Allow to build coresight as modules. This greatly enhances developer > efficiency by allowing the development to take place exclusively on the > target, and without needing to reboot in between changes. > > - Kconfig bools become tristates, to allow =m > > - use -objs to denote merge object directives in Makefile, adds a > coresight-core nomenclature for the base module. > > - Export core functions so as to be able to be used by > non-core modules. > > - add a coresight_exit() that unregisters the coresight bus, add > remove fns for most others. > > - fix up modules with ID tables for autoloading on boot > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Kim Phillips <kim.phillips@arm.com> > --- > drivers/hwtracing/coresight/Kconfig | 48 +++++++++++++++---- > drivers/hwtracing/coresight/Makefile | 28 +++++++---- > .../hwtracing/coresight/coresight-cpu-debug.c | 2 + > .../coresight/coresight-dynamic-replicator.c | 26 ++++++++-- > drivers/hwtracing/coresight/coresight-etb10.c | 27 +++++++++-- > .../hwtracing/coresight/coresight-etm-perf.c | 9 +++- > .../coresight/coresight-etm3x-sysfs.c | 1 + > drivers/hwtracing/coresight/coresight-etm3x.c | 32 +++++++++++-- > .../coresight/coresight-etm4x-sysfs.c | 1 + > drivers/hwtracing/coresight/coresight-etm4x.c | 33 +++++++++++-- > .../hwtracing/coresight/coresight-funnel.c | 26 ++++++++-- > drivers/hwtracing/coresight/coresight-priv.h | 1 - > .../coresight/coresight-replicator.c | 28 +++++++++-- > drivers/hwtracing/coresight/coresight-stm.c | 23 ++++++++- > drivers/hwtracing/coresight/coresight-tmc.c | 18 ++++++- > drivers/hwtracing/coresight/coresight-tpiu.c | 26 ++++++++-- > drivers/hwtracing/coresight/coresight.c | 14 ++++++ > 17 files changed, 299 insertions(+), 44 deletions(-) For the next revision please split the work based on files. > > diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig > index f9abdef5b0d9..4512885f7a3e 100644 > --- a/drivers/hwtracing/coresight/Kconfig > +++ b/drivers/hwtracing/coresight/Kconfig > @@ -2,7 +2,7 @@ > # Coresight configuration > # > menuconfig CORESIGHT > - bool "CoreSight Tracing Support" > + tristate "CoreSight Tracing Support" > select ARM_AMBA > select PERF_EVENTS > help > @@ -12,17 +12,23 @@ menuconfig CORESIGHT > specification and configure the right series of components when a > trace source gets enabled. > > + To compile this code as a module, choose M here: the > + module will be called coresight-core. > + > 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 this code as modules, choose M here: the > + modules will be called coresight-funnel and coresight-replicator. > + > config CORESIGHT_LINK_AND_SINK_TMC > - bool "Coresight generic TMC driver" > + tristate "Coresight generic TMC driver" > help > This enables support for the Trace Memory Controller driver. > Depending on its configuration the device can act as a link (embedded > @@ -30,8 +36,11 @@ config CORESIGHT_LINK_AND_SINK_TMC > complies with the generic implementation of the component without > special enhancement or added features. > > + To compile this code as a module, choose M here: the > + module will be called coresight-tmc-core. > + > config CORESIGHT_SINK_TPIU > - bool "Coresight generic TPIU driver" > + tristate "Coresight generic TPIU driver" > help > This enables support for the Trace Port Interface Unit driver, > responsible for bridging the gap between the on-chip coresight > @@ -40,15 +49,21 @@ 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 code as a module, choose M here: the > + module will be called coresight-tpiu. > + > config CORESIGHT_SINK_ETBV10 > - bool "Coresight ETBv1.0 driver" > + tristate "Coresight ETBv1.0 driver" > 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 code as a module, choose M here: the > + module will be called coresight-etb10. > + > config CORESIGHT_SOURCE_ETM3X > - bool "CoreSight Embedded Trace Macrocell 3.x driver" > + tristate "CoreSight Embedded Trace Macrocell 3.x driver" > depends on !ARM64 > help > This driver provides support for processor ETM3.x and PTM1.x modules, > @@ -56,8 +71,11 @@ 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 code as a module, choose M here: the > + module will be called coresight-etm3x-core. > + > config CORESIGHT_SOURCE_ETM4X > - bool "CoreSight Embedded Trace Macrocell 4.x driver" > + tristate "CoreSight Embedded Trace Macrocell 4.x driver" > depends on ARM64 > help > This driver provides support for the ETM4.x tracer module, tracing the > @@ -65,15 +83,21 @@ config CORESIGHT_SOURCE_ETM4X > for instruction level tracing. Depending on the implemented version > data tracing may also be available. > > + To compile this code as a module, choose M here: the > + module will be called coresight-etm4x-core. > + > config CORESIGHT_DYNAMIC_REPLICATOR > - bool "CoreSight Programmable Replicator driver" > + tristate "CoreSight Programmable Replicator driver" > help > This enables support for dynamic CoreSight replicator link driver. > The programmable ATB replicator allows independent filtering of the > trace data based on the traceid. > > + To compile this code as a module, choose M here: the > + module will be called coresight-dynamic-replicator. > + > config CORESIGHT_STM > - bool "CoreSight System Trace Macrocell driver" > + tristate "CoreSight System Trace Macrocell driver" > depends on STM && ((ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64) > help > This driver provides support for hardware assisted software > @@ -81,6 +105,9 @@ config CORESIGHT_STM > logging useful software events or data coming from various entities > in the system, possibly running different OSs > > + To compile this code 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 > @@ -95,4 +122,7 @@ config CORESIGHT_CPU_DEBUG > properly, please refer Documentation/trace/coresight-cpu-debug.txt > for detailed description and the example for usage. > > + To compile this code as a module, choose M here: the > + module will be called coresight-cpu-debug. > + > endif > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > index 61db9dd0d571..5990710289c2 100644 > --- a/drivers/hwtracing/coresight/Makefile > +++ b/drivers/hwtracing/coresight/Makefile > @@ -2,19 +2,29 @@ > # > # Makefile for CoreSight drivers. > # > -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o > -obj-$(CONFIG_OF) += of_coresight.o > -obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \ > - coresight-tmc-etf.o \ > - coresight-tmc-etr.o > +obj-$(CONFIG_CORESIGHT) += coresight-core.o > +coresight-core-objs := coresight.o \ > + of_coresight.o > + > +obj-$(CONFIG_CORESIGHT) += coresight-etm-perf.o > + > +obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc-core.o > +coresight-tmc-core-objs := coresight-tmc.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 \ > coresight-replicator.o > -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.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_ETM3X) += coresight-etm3x-core.o > +coresight-etm3x-core-objs := coresight-etm3x.o \ > + coresight-etm-cp14.o \ > + coresight-etm3x-sysfs.o > + > +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x-core.o > +coresight-etm4x-core-objs := coresight-etm4x.o coresight-etm4x-sysfs.o > + > obj-$(CONFIG_CORESIGHT_DYNAMIC_REPLICATOR) += coresight-dynamic-replicator.o > obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o > obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o > diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c > index 45b2460f3166..1efe9626eb6c 100644 > --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c > +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c > @@ -671,6 +671,8 @@ static const struct amba_id debug_ids[] = { > { 0, 0 }, > }; > > +MODULE_DEVICE_TABLE(amba, debug_ids); > + > static struct amba_driver debug_driver = { > .drv = { > .name = "coresight-cpu-debug", > diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c > index fc742215ab05..bc42b8022556 100644 > --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c > +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c > @@ -37,7 +37,12 @@ struct replicator_state { > static int replicator_enable(struct coresight_device *csdev, int inport, > int outport) > { > - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct replicator_state *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > CS_UNLOCK(drvdata->base); > > @@ -63,7 +68,9 @@ static int replicator_enable(struct coresight_device *csdev, int inport, > static void replicator_disable(struct coresight_device *csdev, int inport, > int outport) > { > - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct replicator_state *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > > CS_UNLOCK(drvdata->base); > > @@ -75,6 +82,7 @@ static void replicator_disable(struct coresight_device *csdev, int inport, > > CS_LOCK(drvdata->base); > > + module_put(module); > dev_info(drvdata->dev, "REPLICATOR disabled\n"); > } > > @@ -159,6 +167,15 @@ static int replicator_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR_OR_ZERO(drvdata->csdev); > } > > +static int __exit replicator_remove(struct amba_device *adev) > +{ > + struct replicator_state *drvdata = dev_get_drvdata(&adev->dev); > + > + coresight_unregister(drvdata->csdev); > + > + return 0; > +} > + > #ifdef CONFIG_PM > static int replicator_runtime_suspend(struct device *dev) > { > @@ -200,6 +217,8 @@ static const struct amba_id replicator_ids[] = { > { 0, 0 }, > }; > > +MODULE_DEVICE_TABLE(amba, replicator_ids); > + > static struct amba_driver replicator_driver = { > .drv = { > .name = "coresight-dynamic-replicator", > @@ -207,9 +226,10 @@ static struct amba_driver replicator_driver = { > .suppress_bind_attrs = true, > }, > .probe = replicator_probe, > + .remove = replicator_remove, > .id_table = replicator_ids, > }; > -builtin_amba_driver(replicator_driver); > +module_amba_driver(replicator_driver); > > MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>"); > MODULE_DESCRIPTION("ARM Coresight Dynamic Replicator Driver"); > diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c > index a3dac5a8b37c..8825a3e4e47a 100644 > --- a/drivers/hwtracing/coresight/coresight-etb10.c > +++ b/drivers/hwtracing/coresight/coresight-etb10.c > @@ -135,7 +135,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) > { > u32 val; > unsigned long flags; > - struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > val = local_cmpxchg(&drvdata->mode, > CS_MODE_DISABLED, mode); > @@ -256,7 +261,9 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) > > static void etb_disable(struct coresight_device *csdev) > { > - struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > unsigned long flags; > > spin_lock_irqsave(&drvdata->spinlock, flags); > @@ -266,6 +273,7 @@ static void etb_disable(struct coresight_device *csdev) > > local_set(&drvdata->mode, CS_MODE_DISABLED); > > + module_put(module); > dev_info(drvdata->dev, "ETB disabled\n"); > } > > @@ -712,6 +720,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) > { > @@ -746,6 +764,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", > @@ -755,9 +775,10 @@ 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>"); > diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c > index ad0ef8d27111..feb287083ba5 100644 > --- a/drivers/hwtracing/coresight/coresight-etm-perf.c > +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c > @@ -466,6 +466,7 @@ int etm_perf_symlink(struct coresight_device *csdev, bool link) > > return 0; > } > +EXPORT_SYMBOL_GPL(etm_perf_symlink); > > static int __init etm_perf_init(void) > { > @@ -493,7 +494,13 @@ static int __init etm_perf_init(void) > > return ret; > } > -device_initcall(etm_perf_init); > +module_init(etm_perf_init); > + > +static void __exit etm_perf_exit(void) > +{ > + perf_pmu_unregister(&etm_pmu); > +} > +module_exit(etm_perf_exit); > > MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); > MODULE_DESCRIPTION("Arm CoreSight tracer perf driver"); > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > index 91a2a23143d8..84fa5e0fe07b 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > @@ -7,6 +7,7 @@ > #include <linux/pid_namespace.h> > #include <linux/pm_runtime.h> > #include <linux/sysfs.h> > +#include <linux/coresight.h> Why do we need this? > #include "coresight-etm.h" > #include "coresight-priv.h" > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c > index 7ca73a15c735..a2357b26b3a2 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x.c > @@ -514,7 +514,12 @@ static int etm_enable(struct coresight_device *csdev, > { > int ret; > u32 val; > - struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode); > > @@ -611,7 +616,9 @@ static void etm_disable(struct coresight_device *csdev, > struct perf_event *event) > { > u32 mode; > - struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > > /* > * For as long as the tracer isn't disabled another entity can't > @@ -636,6 +643,8 @@ static void etm_disable(struct coresight_device *csdev, > > if (mode) > local_set(&drvdata->mode, CS_MODE_DISABLED); > + > + module_put(module); > } > > static const struct coresight_ops_source etm_source_ops = { > @@ -864,6 +873,20 @@ 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); > + > + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); > + cpuhp_remove_state_nocalls(hp_online); > + > + coresight_unregister(drvdata->csdev); > + > + return 0; > +} > + > #ifdef CONFIG_PM > static int etm_runtime_suspend(struct device *dev) > { > @@ -924,6 +947,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", > @@ -932,9 +957,10 @@ 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>"); > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > index 577a38673444..ee0cbada45d6 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c > @@ -2173,6 +2173,7 @@ const struct attribute_group *coresight_etmv4_groups[] = { > &coresight_etmv4_trcidr_group, > NULL, > }; > +EXPORT_SYMBOL_GPL(coresight_etmv4_groups); From where I stand this is not needed. > > MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); > MODULE_DESCRIPTION("Arm CoreSight Program Flow Trace v4 sysfs driver"); > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c > index ba10f5302a55..a6ff152ab61d 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c > @@ -280,7 +280,12 @@ static int etm4_enable(struct coresight_device *csdev, > { > int ret; > u32 val; > - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode); > > @@ -387,7 +392,9 @@ static void etm4_disable(struct coresight_device *csdev, > struct perf_event *event) > { > u32 mode; > - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > > /* > * For as long as the tracer isn't disabled another entity can't > @@ -409,6 +416,8 @@ static void etm4_disable(struct coresight_device *csdev, > > if (mode) > local_set(&drvdata->mode, CS_MODE_DISABLED); > + > + module_put(module); > } > > static const struct coresight_ops_source etm4_source_ops = { > @@ -1045,6 +1054,20 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) > return ret; > } > > +static int __exit etm4_remove(struct amba_device *adev) > +{ > + struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev); > + > + etm_perf_symlink(drvdata->csdev, false); > + > + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); > + cpuhp_remove_state_nocalls(hp_online); > + > + coresight_unregister(drvdata->csdev); > + > + return 0; > +} > + > static const struct amba_id etm4_ids[] = { > { /* ETM 4.0 - Cortex-A53 */ > .id = 0x000bb95d, > @@ -1064,15 +1087,19 @@ static const struct amba_id etm4_ids[] = { > { 0, 0}, > }; > > +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>"); > diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c > index 1e497a75b956..c355a66bcc51 100644 > --- a/drivers/hwtracing/coresight/coresight-funnel.c > +++ b/drivers/hwtracing/coresight/coresight-funnel.c > @@ -61,7 +61,12 @@ static void funnel_enable_hw(struct funnel_drvdata *drvdata, int port) > static int funnel_enable(struct coresight_device *csdev, int inport, > int outport) > { > - struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > funnel_enable_hw(drvdata, inport); > > @@ -85,10 +90,13 @@ static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport) > static void funnel_disable(struct coresight_device *csdev, int inport, > int outport) > { > - struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > > funnel_disable_hw(drvdata, inport); > > + module_put(module); > dev_info(drvdata->dev, "FUNNEL inport %d disabled\n", inport); > } > > @@ -211,6 +219,15 @@ static int funnel_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR_OR_ZERO(drvdata->csdev); > } > > +static int __exit funnel_remove(struct amba_device *adev) > +{ > + struct funnel_drvdata *drvdata = dev_get_drvdata(&adev->dev); > + > + coresight_unregister(drvdata->csdev); > + > + return 0; > +} > + > #ifdef CONFIG_PM > static int funnel_runtime_suspend(struct device *dev) > { > @@ -250,6 +267,8 @@ static const struct amba_id funnel_ids[] = { > { 0, 0}, > }; > > +MODULE_DEVICE_TABLE(amba, funnel_ids); > + > static struct amba_driver funnel_driver = { > .drv = { > .name = "coresight-funnel", > @@ -258,9 +277,10 @@ static struct amba_driver funnel_driver = { > .suppress_bind_attrs = true, > }, > .probe = funnel_probe, > + .remove = funnel_remove, > .id_table = funnel_ids, > }; > -builtin_amba_driver(funnel_driver); > +module_amba_driver(funnel_driver); > > MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); > MODULE_DESCRIPTION("ARM Coresight Funnel Driver"); > diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h > index 45de8c15b687..896958c2dd44 100644 > --- a/drivers/hwtracing/coresight/coresight-priv.h > +++ b/drivers/hwtracing/coresight/coresight-priv.h > @@ -65,7 +65,6 @@ static DEVICE_ATTR_RO(name) > static const u32 barrier_pkt[5] = {0x7fffffff, 0x7fffffff, > 0x7fffffff, 0x7fffffff, 0x0}; > > - No need for that. > enum etm_addr_type { > ETM_ADDR_TYPE_NONE, > ETM_ADDR_TYPE_SINGLE, > diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c > index 9ef539893eaa..6f16dcd7e107 100644 > --- a/drivers/hwtracing/coresight/coresight-replicator.c > +++ b/drivers/hwtracing/coresight/coresight-replicator.c > @@ -33,7 +33,12 @@ struct replicator_drvdata { > static int replicator_enable(struct coresight_device *csdev, int inport, > int outport) > { > - struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > dev_info(drvdata->dev, "REPLICATOR enabled\n"); > return 0; > @@ -42,8 +47,11 @@ static int replicator_enable(struct coresight_device *csdev, int inport, > static void replicator_disable(struct coresight_device *csdev, int inport, > int outport) > { > - struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > > + module_put(module); > dev_info(drvdata->dev, "REPLICATOR disabled\n"); > } > > @@ -112,6 +120,17 @@ static int replicator_probe(struct platform_device *pdev) > return ret; > } > > +static int __exit replicator_remove(struct platform_device *pdev) > +{ > + struct replicator_drvdata *drvdata = dev_get_drvdata(&pdev->dev); > + > + coresight_unregister(drvdata->csdev); > + > + pm_runtime_disable(&pdev->dev); > + > + return 0; > +} > + > #ifdef CONFIG_PM > static int replicator_runtime_suspend(struct device *dev) > { > @@ -144,8 +163,11 @@ static const struct of_device_id replicator_match[] = { > {} > }; > > +MODULE_DEVICE_TABLE(of, replicator_match); > + > static struct platform_driver replicator_driver = { > .probe = replicator_probe, > + .remove = replicator_remove, > .driver = { > .name = "coresight-replicator", > .of_match_table = replicator_match, > @@ -153,7 +175,7 @@ static struct platform_driver replicator_driver = { > .suppress_bind_attrs = true, > }, > }; > -builtin_platform_driver(replicator_driver); > +module_platform_driver(replicator_driver); > > MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>"); > MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); > diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c > index 30eae52a8757..9997ba0dbd54 100644 > --- a/drivers/hwtracing/coresight/coresight-stm.c > +++ b/drivers/hwtracing/coresight/coresight-stm.c > @@ -194,7 +194,12 @@ static int stm_enable(struct coresight_device *csdev, > struct perf_event *event, u32 mode) > { > u32 val; > - struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct stm_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > if (mode != CS_MODE_SYSFS) > return -EINVAL; Function stm_disable() would likely benefit from a module_put(). > @@ -882,6 +887,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) > { > @@ -922,6 +938,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", > @@ -930,10 +948,11 @@ 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"); > diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c > index 176a5aeab20e..eb3cdb832f84 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.c > +++ b/drivers/hwtracing/coresight/coresight-tmc.c > @@ -429,6 +429,19 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) > return ret; > } > > +static int __exit tmc_remove(struct amba_device *adev) > +{ > + struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); > + > + /* free ETB/ETF or ETR memory */ > + tmc_read_unprepare(drvdata); > + > + misc_deregister(&drvdata->miscdev); > + coresight_unregister(drvdata->csdev); > + > + return 0; > +} > + Right now I can remove the module for a TMC link or sink when part of an active session, something I pointed out during an earlier revision. I also think we need to deal with driver removal cases when the TMC buffer (ETR or ETF) is being read from sysFS. > static const struct amba_id tmc_ids[] = { > { > .id = 0x000bb961, > @@ -453,6 +466,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", > @@ -460,9 +475,10 @@ static struct amba_driver tmc_driver = { > .suppress_bind_attrs = true, > }, > .probe = tmc_probe, > + .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"); > diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c > index f3b154e150b3..9622f2a5a451 100644 > --- a/drivers/hwtracing/coresight/coresight-tpiu.c > +++ b/drivers/hwtracing/coresight/coresight-tpiu.c > @@ -69,7 +69,12 @@ static void tpiu_enable_hw(struct tpiu_drvdata *drvdata) > > static int tpiu_enable(struct coresight_device *csdev, u32 mode) > { > - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > tpiu_enable_hw(drvdata); > > @@ -95,10 +100,13 @@ static void tpiu_disable_hw(struct tpiu_drvdata *drvdata) > > static void tpiu_disable(struct coresight_device *csdev) > { > - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > > tpiu_disable_hw(drvdata); > > + module_put(module); > dev_info(drvdata->dev, "TPIU disabled\n"); > } > > @@ -164,6 +172,15 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) > return PTR_ERR_OR_ZERO(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) > { > @@ -207,6 +224,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", > @@ -215,9 +234,10 @@ 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>"); > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index 406899f316e4..c00229b0db52 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -302,6 +302,7 @@ void coresight_disable_path(struct list_head *path) > } > } > } > +EXPORT_SYMBOL_GPL(coresight_disable_path); > > int coresight_enable_path(struct list_head *path, u32 mode) > { > @@ -353,6 +354,7 @@ int coresight_enable_path(struct list_head *path, u32 mode) > coresight_disable_path(path); > goto out; > } > +EXPORT_SYMBOL_GPL(coresight_enable_path); > > struct coresight_device *coresight_get_sink(struct list_head *path) > { > @@ -368,6 +370,7 @@ struct coresight_device *coresight_get_sink(struct list_head *path) > > return csdev; > } > +EXPORT_SYMBOL_GPL(coresight_get_sink); > > static int coresight_enabled_sink(struct device *dev, void *data) > { > @@ -392,6 +395,7 @@ static int coresight_enabled_sink(struct device *dev, void *data) > > return 0; > } > +EXPORT_SYMBOL_GPL(coresight_enabled_sink); > > /** > * coresight_get_enabled_sink - returns the first enabled sink found on the bus > @@ -414,6 +418,7 @@ struct coresight_device *coresight_get_enabled_sink(bool deactivate) > > return dev ? to_coresight_device(dev) : NULL; > } > +EXPORT_SYMBOL_GPL(coresight_get_enabled_sink); > > /** > * _coresight_build_path - recursively build a path from a @csdev to a sink. > @@ -493,6 +498,7 @@ struct list_head *coresight_build_path(struct coresight_device *source, > > return path; > } > +EXPORT_SYMBOL_GPL(coresight_build_path); > > /** > * coresight_release_path - release a previously built path. > @@ -517,6 +523,7 @@ void coresight_release_path(struct list_head *path) > kfree(path); > path = NULL; > } > +EXPORT_SYMBOL_GPL(coresight_release_path); > > /** coresight_validate_source - make sure a source has the right credentials > * @csdev: the device structure for a source. > @@ -933,6 +940,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", > @@ -944,6 +952,12 @@ static int __init coresight_init(void) > } > postcore_initcall(coresight_init); > > +static void __exit coresight_exit(void) > +{ > + bus_unregister(&coresight_bustype); > +} > +module_exit(coresight_exit); > + > struct coresight_device *coresight_register(struct coresight_desc *desc) > { > int i; > -- > 2.17.0 >
On Tue, 22 May 2018 15:39:06 -0600 Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On Thu, May 17, 2018 at 08:20:24PM -0500, Kim Phillips wrote: > > Allow to build coresight as modules. This greatly enhances developer > > efficiency by allowing the development to take place exclusively on the > > target, and without needing to reboot in between changes. > > > > - Kconfig bools become tristates, to allow =m > > > > - use -objs to denote merge object directives in Makefile, adds a > > coresight-core nomenclature for the base module. > > > > - Export core functions so as to be able to be used by > > non-core modules. > > > > - add a coresight_exit() that unregisters the coresight bus, add > > remove fns for most others. > > > > - fix up modules with ID tables for autoloading on boot > > > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > > Cc: Randy Dunlap <rdunlap@infradead.org> > > Signed-off-by: Kim Phillips <kim.phillips@arm.com> > > --- > > drivers/hwtracing/coresight/Kconfig | 48 +++++++++++++++---- > > drivers/hwtracing/coresight/Makefile | 28 +++++++---- > > .../hwtracing/coresight/coresight-cpu-debug.c | 2 + > > .../coresight/coresight-dynamic-replicator.c | 26 ++++++++-- > > drivers/hwtracing/coresight/coresight-etb10.c | 27 +++++++++-- > > .../hwtracing/coresight/coresight-etm-perf.c | 9 +++- > > .../coresight/coresight-etm3x-sysfs.c | 1 + > > drivers/hwtracing/coresight/coresight-etm3x.c | 32 +++++++++++-- > > .../coresight/coresight-etm4x-sysfs.c | 1 + > > drivers/hwtracing/coresight/coresight-etm4x.c | 33 +++++++++++-- > > .../hwtracing/coresight/coresight-funnel.c | 26 ++++++++-- > > drivers/hwtracing/coresight/coresight-priv.h | 1 - > > .../coresight/coresight-replicator.c | 28 +++++++++-- > > drivers/hwtracing/coresight/coresight-stm.c | 23 ++++++++- > > drivers/hwtracing/coresight/coresight-tmc.c | 18 ++++++- > > drivers/hwtracing/coresight/coresight-tpiu.c | 26 ++++++++-- > > drivers/hwtracing/coresight/coresight.c | 14 ++++++ > > 17 files changed, 299 insertions(+), 44 deletions(-) > > For the next revision please split the work based on files. If I read that literally, one file-by-one would break build bisectability. Do you mean split by source files depending on the logical modules they belong to, e.g., etm3x, etm4x, etb10, etc.? If so, I think it would look like the coresight-core would be first, followed by the rest, but I also think there are cross-dependencies. Hmm, OK, I'll have a look, but there's also one more thing: I think the Makefile obj '-core' nomenclature was to change the name of the module to not be the same as the core source file, so what do you think about renaming the core source file instead of the module name? e.g.: Instead of this: obj-$(CONFIG_CORESIGHT) += coresight-core.o coresight-core-objs := coresight.o \ of_coresight.o we have this: obj-$(CONFIG_CORESIGHT) += coresight.o coresight-objs := coresight-core.o \ of_coresight.o and e.g., instead of this: obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x-core.o coresight-etm3x-core-objs := coresight-etm3x.o \ coresight-etm-cp14.o \ coresight-etm3x-sysfs.o we have this: obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm3x-objs := coresight-etm3x-core.o \ coresight-etm-cp14.o \ coresight-etm3x-sysfs.o ? > > +static int __exit tmc_remove(struct amba_device *adev) > > +{ > > + struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > + > > + /* free ETB/ETF or ETR memory */ > > + tmc_read_unprepare(drvdata); > > + > > + misc_deregister(&drvdata->miscdev); > > + coresight_unregister(drvdata->csdev); > > + > > + return 0; > > +} > > + > > Right now I can remove the module for a TMC link or sink when part of an active > session, something I pointed out during an earlier revision. Right, I missed that :( So would the first thing tmc_remove does is this: if (drvdata->reading) return -EBUSY; work, or would we need to introduce a new sentinel? > I also think we need to deal with driver removal cases when the TMC buffer > (ETR or ETF) is being read from sysFS. OK, I thought the: struct file_operations tmc_fops = { .owner = THIS_MODULE, would prevent module unload whilst sysfs access was being performed, but I'll double check. Thanks, Kim
On 18/05/18 02:20, Kim Phillips wrote: > Allow to build coresight as modules. This greatly enhances developer > efficiency by allowing the development to take place exclusively on the > target, and without needing to reboot in between changes. > > - Kconfig bools become tristates, to allow =m > > - use -objs to denote merge object directives in Makefile, adds a > coresight-core nomenclature for the base module. > > - Export core functions so as to be able to be used by > non-core modules. > > - add a coresight_exit() that unregisters the coresight bus, add > remove fns for most others. > > - fix up modules with ID tables for autoloading on boot > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Kim Phillips <kim.phillips@arm.com> Kim, I see that you have addressed my comments on a previous version of this series posted in April. But I don't see the version number increased for this new version. Please add versioning to make it easier to make it more obvious. Also, generally it is a good idea to keep the people who reviewed and commented on your previous versions in the newer versions. Some comments below : > diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c > index fc742215ab05..bc42b8022556 100644 > --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c > +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c > @@ -37,7 +37,12 @@ struct replicator_state { > static int replicator_enable(struct coresight_device *csdev, int inport, > int outport) > { > - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct device *parent_dev = csdev->dev.parent; > + struct replicator_state *drvdata = dev_get_drvdata(parent_dev); > + struct module *module = parent_dev->driver->owner; > + > + if (!try_module_get(module)) > + return -ENODEV; > > CS_UNLOCK(drvdata->base); What is the guarantee that the "csdev" is still available when we reach here ? A module could be unloaded "after the component was added to the path" (via coresight_build_path) and before we invoke the "enable" on each component in the path ? Also, it is tedious to do module_get in "enable" and module_put in the disable call backs for each component. Instead, if we do a module_get() in build_path and module_put() in release path, we could solve all these problems and keep it the module refcount in a central place. > +MODULE_DEVICE_TABLE(amba, replicator_ids); > + > static struct amba_driver replicator_driver = { > .drv = { > .name = "coresight-dynamic-replicator", > @@ -207,9 +226,10 @@ static struct amba_driver replicator_driver = { > .suppress_bind_attrs = true, > }, > .probe = replicator_probe, > + .remove = replicator_remove, > .id_table = replicator_ids, > }; Do we have the owner field set here for this driver ? I see that you added it for some components and not others. e.g, you have added it for etm4x, while not for replicator and others. > +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); Suzuki
On 24 May 2018 at 17:49, Kim Phillips <kim.phillips@arm.com> wrote: > On Tue, 22 May 2018 15:39:06 -0600 > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >> On Thu, May 17, 2018 at 08:20:24PM -0500, Kim Phillips wrote: >> > Allow to build coresight as modules. This greatly enhances developer >> > efficiency by allowing the development to take place exclusively on the >> > target, and without needing to reboot in between changes. >> > >> > - Kconfig bools become tristates, to allow =m >> > >> > - use -objs to denote merge object directives in Makefile, adds a >> > coresight-core nomenclature for the base module. >> > >> > - Export core functions so as to be able to be used by >> > non-core modules. >> > >> > - add a coresight_exit() that unregisters the coresight bus, add >> > remove fns for most others. >> > >> > - fix up modules with ID tables for autoloading on boot >> > >> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> >> > Cc: Randy Dunlap <rdunlap@infradead.org> >> > Signed-off-by: Kim Phillips <kim.phillips@arm.com> >> > --- >> > drivers/hwtracing/coresight/Kconfig | 48 +++++++++++++++---- >> > drivers/hwtracing/coresight/Makefile | 28 +++++++---- >> > .../hwtracing/coresight/coresight-cpu-debug.c | 2 + >> > .../coresight/coresight-dynamic-replicator.c | 26 ++++++++-- >> > drivers/hwtracing/coresight/coresight-etb10.c | 27 +++++++++-- >> > .../hwtracing/coresight/coresight-etm-perf.c | 9 +++- >> > .../coresight/coresight-etm3x-sysfs.c | 1 + >> > drivers/hwtracing/coresight/coresight-etm3x.c | 32 +++++++++++-- >> > .../coresight/coresight-etm4x-sysfs.c | 1 + >> > drivers/hwtracing/coresight/coresight-etm4x.c | 33 +++++++++++-- >> > .../hwtracing/coresight/coresight-funnel.c | 26 ++++++++-- >> > drivers/hwtracing/coresight/coresight-priv.h | 1 - >> > .../coresight/coresight-replicator.c | 28 +++++++++-- >> > drivers/hwtracing/coresight/coresight-stm.c | 23 ++++++++- >> > drivers/hwtracing/coresight/coresight-tmc.c | 18 ++++++- >> > drivers/hwtracing/coresight/coresight-tpiu.c | 26 ++++++++-- >> > drivers/hwtracing/coresight/coresight.c | 14 ++++++ >> > 17 files changed, 299 insertions(+), 44 deletions(-) >> >> For the next revision please split the work based on files. > > If I read that literally, one file-by-one would break build > bisectability. Do you mean split by source files depending on the > logical modules they belong to, e.g., etm3x, etm4x, etb10, etc.? If I meant to introduce all the needed changes - including the module author, description and licences - per module. That will break up this patch considerably and allow us to concentrate on individual component. As a final step do the changes to Kconfig and the Makefile. > so, I think it would look like the coresight-core would be first, > followed by the rest, but I also think there are cross-dependencies. > Hmm, OK, I'll have a look, but there's also one more thing: I think > the Makefile obj '-core' nomenclature was to change the name of the > module to not be the same as the core source file, so what do you think > about renaming the core source file instead of the module name? e.g.: > > Instead of this: > > obj-$(CONFIG_CORESIGHT) += coresight-core.o > coresight-core-objs := coresight.o \ > of_coresight.o > > we have this: > > obj-$(CONFIG_CORESIGHT) += coresight.o > coresight-objs := coresight-core.o \ > of_coresight.o > > and e.g., instead of this: > > obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x-core.o > coresight-etm3x-core-objs := coresight-etm3x.o \ > coresight-etm-cp14.o \ > coresight-etm3x-sysfs.o > > we have this: > > obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o > coresight-etm3x-objs := coresight-etm3x-core.o \ > coresight-etm-cp14.o \ > coresight-etm3x-sysfs.o > > ? I think that is much better and avoid carrying the heave "-core" appendage. Renaming needs to be a patch on its own (but still part of this set). > >> > +static int __exit tmc_remove(struct amba_device *adev) >> > +{ >> > + struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); >> > + >> > + /* free ETB/ETF or ETR memory */ >> > + tmc_read_unprepare(drvdata); >> > + >> > + misc_deregister(&drvdata->miscdev); >> > + coresight_unregister(drvdata->csdev); >> > + >> > + return 0; >> > +} >> > + >> >> Right now I can remove the module for a TMC link or sink when part of an active >> session, something I pointed out during an earlier revision. > > Right, I missed that :( > > So would the first thing tmc_remove does is this: > > if (drvdata->reading) > return -EBUSY; > > work, or would we need to introduce a new sentinel? The ->reading flag is to prevent concurrent access to the buffer from sysFS and to avoid clobbering said buffer with new trace data. The TMC driver shouldn't be different from the other ones with regards to the usage of the try_module_get()/module_put() functions. > >> I also think we need to deal with driver removal cases when the TMC buffer >> (ETR or ETF) is being read from sysFS. > > OK, I thought the: > > struct file_operations tmc_fops = { > .owner = THIS_MODULE, > > would prevent module unload whilst sysfs access was being performed, > but I'll double check. Right, just check that it works as advertised. > > Thanks, > > Kim
On 25 May 2018 at 11:12, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote: > On 18/05/18 02:20, Kim Phillips wrote: >> >> Allow to build coresight as modules. This greatly enhances developer >> efficiency by allowing the development to take place exclusively on the >> target, and without needing to reboot in between changes. >> >> - Kconfig bools become tristates, to allow =m >> >> - use -objs to denote merge object directives in Makefile, adds a >> coresight-core nomenclature for the base module. >> >> - Export core functions so as to be able to be used by >> non-core modules. >> >> - add a coresight_exit() that unregisters the coresight bus, add >> remove fns for most others. >> >> - fix up modules with ID tables for autoloading on boot >> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org> >> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> >> Cc: Randy Dunlap <rdunlap@infradead.org> >> Signed-off-by: Kim Phillips <kim.phillips@arm.com> > > > Kim, > > I see that you have addressed my comments on a previous version > of this series posted in April. But I don't see the version number > increased for this new version. Please add versioning to make it > easier to make it more obvious. > > Also, generally it is a good idea to keep the people who reviewed > and commented on your previous versions in the newer versions. > > Some comments below : > >> diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c >> b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c >> index fc742215ab05..bc42b8022556 100644 >> --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c >> +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c >> @@ -37,7 +37,12 @@ struct replicator_state { >> static int replicator_enable(struct coresight_device *csdev, int inport, >> int outport) >> { >> - struct replicator_state *drvdata = >> dev_get_drvdata(csdev->dev.parent); >> + struct device *parent_dev = csdev->dev.parent; >> + struct replicator_state *drvdata = dev_get_drvdata(parent_dev); >> + struct module *module = parent_dev->driver->owner; >> + >> + if (!try_module_get(module)) >> + return -ENODEV; >> CS_UNLOCK(drvdata->base); > > > What is the guarantee that the "csdev" is still available when we reach > here ? > > A module could be unloaded "after the component was added to the path" > (via coresight_build_path) and before we invoke the "enable" on each > component in the path ? Very good point - this is invariably racy. > > Also, it is tedious to do module_get in "enable" and module_put in the > disable call backs for each component. > > Instead, if we do a module_get() in build_path and module_put() in > release path, we could solve all these problems and keep it the module > refcount in a central place. Good idea, it does streamline things a lot. > >> +MODULE_DEVICE_TABLE(amba, replicator_ids); >> + >> static struct amba_driver replicator_driver = { >> .drv = { >> .name = "coresight-dynamic-replicator", >> @@ -207,9 +226,10 @@ static struct amba_driver replicator_driver = { >> .suppress_bind_attrs = true, >> }, >> .probe = replicator_probe, >> + .remove = replicator_remove, >> .id_table = replicator_ids, >> }; > > > Do we have the owner field set here for this driver ? I see that you > added it for some components and not others. e.g, you have added it for > etm4x, while not for replicator and others. > > >> +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); > > > > Suzuki
diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index f9abdef5b0d9..4512885f7a3e 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -2,7 +2,7 @@ # Coresight configuration # menuconfig CORESIGHT - bool "CoreSight Tracing Support" + tristate "CoreSight Tracing Support" select ARM_AMBA select PERF_EVENTS help @@ -12,17 +12,23 @@ menuconfig CORESIGHT specification and configure the right series of components when a trace source gets enabled. + To compile this code as a module, choose M here: the + module will be called coresight-core. + 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 this code as modules, choose M here: the + modules will be called coresight-funnel and coresight-replicator. + config CORESIGHT_LINK_AND_SINK_TMC - bool "Coresight generic TMC driver" + tristate "Coresight generic TMC driver" help This enables support for the Trace Memory Controller driver. Depending on its configuration the device can act as a link (embedded @@ -30,8 +36,11 @@ config CORESIGHT_LINK_AND_SINK_TMC complies with the generic implementation of the component without special enhancement or added features. + To compile this code as a module, choose M here: the + module will be called coresight-tmc-core. + config CORESIGHT_SINK_TPIU - bool "Coresight generic TPIU driver" + tristate "Coresight generic TPIU driver" help This enables support for the Trace Port Interface Unit driver, responsible for bridging the gap between the on-chip coresight @@ -40,15 +49,21 @@ 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 code as a module, choose M here: the + module will be called coresight-tpiu. + config CORESIGHT_SINK_ETBV10 - bool "Coresight ETBv1.0 driver" + tristate "Coresight ETBv1.0 driver" 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 code as a module, choose M here: the + module will be called coresight-etb10. + config CORESIGHT_SOURCE_ETM3X - bool "CoreSight Embedded Trace Macrocell 3.x driver" + tristate "CoreSight Embedded Trace Macrocell 3.x driver" depends on !ARM64 help This driver provides support for processor ETM3.x and PTM1.x modules, @@ -56,8 +71,11 @@ 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 code as a module, choose M here: the + module will be called coresight-etm3x-core. + config CORESIGHT_SOURCE_ETM4X - bool "CoreSight Embedded Trace Macrocell 4.x driver" + tristate "CoreSight Embedded Trace Macrocell 4.x driver" depends on ARM64 help This driver provides support for the ETM4.x tracer module, tracing the @@ -65,15 +83,21 @@ config CORESIGHT_SOURCE_ETM4X for instruction level tracing. Depending on the implemented version data tracing may also be available. + To compile this code as a module, choose M here: the + module will be called coresight-etm4x-core. + config CORESIGHT_DYNAMIC_REPLICATOR - bool "CoreSight Programmable Replicator driver" + tristate "CoreSight Programmable Replicator driver" help This enables support for dynamic CoreSight replicator link driver. The programmable ATB replicator allows independent filtering of the trace data based on the traceid. + To compile this code as a module, choose M here: the + module will be called coresight-dynamic-replicator. + config CORESIGHT_STM - bool "CoreSight System Trace Macrocell driver" + tristate "CoreSight System Trace Macrocell driver" depends on STM && ((ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64) help This driver provides support for hardware assisted software @@ -81,6 +105,9 @@ config CORESIGHT_STM logging useful software events or data coming from various entities in the system, possibly running different OSs + To compile this code 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 @@ -95,4 +122,7 @@ config CORESIGHT_CPU_DEBUG properly, please refer Documentation/trace/coresight-cpu-debug.txt for detailed description and the example for usage. + To compile this code as a module, choose M here: the + module will be called coresight-cpu-debug. + endif diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 61db9dd0d571..5990710289c2 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -2,19 +2,29 @@ # # Makefile for CoreSight drivers. # -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o -obj-$(CONFIG_OF) += of_coresight.o -obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \ - coresight-tmc-etf.o \ - coresight-tmc-etr.o +obj-$(CONFIG_CORESIGHT) += coresight-core.o +coresight-core-objs := coresight.o \ + of_coresight.o + +obj-$(CONFIG_CORESIGHT) += coresight-etm-perf.o + +obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc-core.o +coresight-tmc-core-objs := coresight-tmc.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 \ coresight-replicator.o -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.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_ETM3X) += coresight-etm3x-core.o +coresight-etm3x-core-objs := coresight-etm3x.o \ + coresight-etm-cp14.o \ + coresight-etm3x-sysfs.o + +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x-core.o +coresight-etm4x-core-objs := coresight-etm4x.o coresight-etm4x-sysfs.o + obj-$(CONFIG_CORESIGHT_DYNAMIC_REPLICATOR) += coresight-dynamic-replicator.o obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c index 45b2460f3166..1efe9626eb6c 100644 --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c @@ -671,6 +671,8 @@ static const struct amba_id debug_ids[] = { { 0, 0 }, }; +MODULE_DEVICE_TABLE(amba, debug_ids); + static struct amba_driver debug_driver = { .drv = { .name = "coresight-cpu-debug", diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c index fc742215ab05..bc42b8022556 100644 --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c @@ -37,7 +37,12 @@ struct replicator_state { static int replicator_enable(struct coresight_device *csdev, int inport, int outport) { - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct replicator_state *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; CS_UNLOCK(drvdata->base); @@ -63,7 +68,9 @@ static int replicator_enable(struct coresight_device *csdev, int inport, static void replicator_disable(struct coresight_device *csdev, int inport, int outport) { - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct replicator_state *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; CS_UNLOCK(drvdata->base); @@ -75,6 +82,7 @@ static void replicator_disable(struct coresight_device *csdev, int inport, CS_LOCK(drvdata->base); + module_put(module); dev_info(drvdata->dev, "REPLICATOR disabled\n"); } @@ -159,6 +167,15 @@ static int replicator_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR_OR_ZERO(drvdata->csdev); } +static int __exit replicator_remove(struct amba_device *adev) +{ + struct replicator_state *drvdata = dev_get_drvdata(&adev->dev); + + coresight_unregister(drvdata->csdev); + + return 0; +} + #ifdef CONFIG_PM static int replicator_runtime_suspend(struct device *dev) { @@ -200,6 +217,8 @@ static const struct amba_id replicator_ids[] = { { 0, 0 }, }; +MODULE_DEVICE_TABLE(amba, replicator_ids); + static struct amba_driver replicator_driver = { .drv = { .name = "coresight-dynamic-replicator", @@ -207,9 +226,10 @@ static struct amba_driver replicator_driver = { .suppress_bind_attrs = true, }, .probe = replicator_probe, + .remove = replicator_remove, .id_table = replicator_ids, }; -builtin_amba_driver(replicator_driver); +module_amba_driver(replicator_driver); MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>"); MODULE_DESCRIPTION("ARM Coresight Dynamic Replicator Driver"); diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index a3dac5a8b37c..8825a3e4e47a 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -135,7 +135,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode) { u32 val; unsigned long flags; - struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode); @@ -256,7 +261,9 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) static void etb_disable(struct coresight_device *csdev) { - struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; unsigned long flags; spin_lock_irqsave(&drvdata->spinlock, flags); @@ -266,6 +273,7 @@ static void etb_disable(struct coresight_device *csdev) local_set(&drvdata->mode, CS_MODE_DISABLED); + module_put(module); dev_info(drvdata->dev, "ETB disabled\n"); } @@ -712,6 +720,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) { @@ -746,6 +764,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", @@ -755,9 +775,10 @@ 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>"); diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index ad0ef8d27111..feb287083ba5 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -466,6 +466,7 @@ int etm_perf_symlink(struct coresight_device *csdev, bool link) return 0; } +EXPORT_SYMBOL_GPL(etm_perf_symlink); static int __init etm_perf_init(void) { @@ -493,7 +494,13 @@ static int __init etm_perf_init(void) return ret; } -device_initcall(etm_perf_init); +module_init(etm_perf_init); + +static void __exit etm_perf_exit(void) +{ + perf_pmu_unregister(&etm_pmu); +} +module_exit(etm_perf_exit); MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); MODULE_DESCRIPTION("Arm CoreSight tracer perf driver"); diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c index 91a2a23143d8..84fa5e0fe07b 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c @@ -7,6 +7,7 @@ #include <linux/pid_namespace.h> #include <linux/pm_runtime.h> #include <linux/sysfs.h> +#include <linux/coresight.h> #include "coresight-etm.h" #include "coresight-priv.h" diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c index 7ca73a15c735..a2357b26b3a2 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x.c +++ b/drivers/hwtracing/coresight/coresight-etm3x.c @@ -514,7 +514,12 @@ static int etm_enable(struct coresight_device *csdev, { int ret; u32 val; - struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode); @@ -611,7 +616,9 @@ static void etm_disable(struct coresight_device *csdev, struct perf_event *event) { u32 mode; - struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; /* * For as long as the tracer isn't disabled another entity can't @@ -636,6 +643,8 @@ static void etm_disable(struct coresight_device *csdev, if (mode) local_set(&drvdata->mode, CS_MODE_DISABLED); + + module_put(module); } static const struct coresight_ops_source etm_source_ops = { @@ -864,6 +873,20 @@ 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); + + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); + cpuhp_remove_state_nocalls(hp_online); + + coresight_unregister(drvdata->csdev); + + return 0; +} + #ifdef CONFIG_PM static int etm_runtime_suspend(struct device *dev) { @@ -924,6 +947,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", @@ -932,9 +957,10 @@ 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>"); diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 577a38673444..ee0cbada45d6 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2173,6 +2173,7 @@ const struct attribute_group *coresight_etmv4_groups[] = { &coresight_etmv4_trcidr_group, NULL, }; +EXPORT_SYMBOL_GPL(coresight_etmv4_groups); MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); MODULE_DESCRIPTION("Arm CoreSight Program Flow Trace v4 sysfs driver"); diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index ba10f5302a55..a6ff152ab61d 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -280,7 +280,12 @@ static int etm4_enable(struct coresight_device *csdev, { int ret; u32 val; - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode); @@ -387,7 +392,9 @@ static void etm4_disable(struct coresight_device *csdev, struct perf_event *event) { u32 mode; - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; /* * For as long as the tracer isn't disabled another entity can't @@ -409,6 +416,8 @@ static void etm4_disable(struct coresight_device *csdev, if (mode) local_set(&drvdata->mode, CS_MODE_DISABLED); + + module_put(module); } static const struct coresight_ops_source etm4_source_ops = { @@ -1045,6 +1054,20 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id) return ret; } +static int __exit etm4_remove(struct amba_device *adev) +{ + struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + etm_perf_symlink(drvdata->csdev, false); + + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); + cpuhp_remove_state_nocalls(hp_online); + + coresight_unregister(drvdata->csdev); + + return 0; +} + static const struct amba_id etm4_ids[] = { { /* ETM 4.0 - Cortex-A53 */ .id = 0x000bb95d, @@ -1064,15 +1087,19 @@ static const struct amba_id etm4_ids[] = { { 0, 0}, }; +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>"); diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 1e497a75b956..c355a66bcc51 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -61,7 +61,12 @@ static void funnel_enable_hw(struct funnel_drvdata *drvdata, int port) static int funnel_enable(struct coresight_device *csdev, int inport, int outport) { - struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; funnel_enable_hw(drvdata, inport); @@ -85,10 +90,13 @@ static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport) static void funnel_disable(struct coresight_device *csdev, int inport, int outport) { - struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; funnel_disable_hw(drvdata, inport); + module_put(module); dev_info(drvdata->dev, "FUNNEL inport %d disabled\n", inport); } @@ -211,6 +219,15 @@ static int funnel_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR_OR_ZERO(drvdata->csdev); } +static int __exit funnel_remove(struct amba_device *adev) +{ + struct funnel_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + coresight_unregister(drvdata->csdev); + + return 0; +} + #ifdef CONFIG_PM static int funnel_runtime_suspend(struct device *dev) { @@ -250,6 +267,8 @@ static const struct amba_id funnel_ids[] = { { 0, 0}, }; +MODULE_DEVICE_TABLE(amba, funnel_ids); + static struct amba_driver funnel_driver = { .drv = { .name = "coresight-funnel", @@ -258,9 +277,10 @@ static struct amba_driver funnel_driver = { .suppress_bind_attrs = true, }, .probe = funnel_probe, + .remove = funnel_remove, .id_table = funnel_ids, }; -builtin_amba_driver(funnel_driver); +module_amba_driver(funnel_driver); MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); MODULE_DESCRIPTION("ARM Coresight Funnel Driver"); diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index 45de8c15b687..896958c2dd44 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -65,7 +65,6 @@ static DEVICE_ATTR_RO(name) static const u32 barrier_pkt[5] = {0x7fffffff, 0x7fffffff, 0x7fffffff, 0x7fffffff, 0x0}; - enum etm_addr_type { ETM_ADDR_TYPE_NONE, ETM_ADDR_TYPE_SINGLE, diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c index 9ef539893eaa..6f16dcd7e107 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -33,7 +33,12 @@ struct replicator_drvdata { static int replicator_enable(struct coresight_device *csdev, int inport, int outport) { - struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; dev_info(drvdata->dev, "REPLICATOR enabled\n"); return 0; @@ -42,8 +47,11 @@ static int replicator_enable(struct coresight_device *csdev, int inport, static void replicator_disable(struct coresight_device *csdev, int inport, int outport) { - struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + module_put(module); dev_info(drvdata->dev, "REPLICATOR disabled\n"); } @@ -112,6 +120,17 @@ static int replicator_probe(struct platform_device *pdev) return ret; } +static int __exit replicator_remove(struct platform_device *pdev) +{ + struct replicator_drvdata *drvdata = dev_get_drvdata(&pdev->dev); + + coresight_unregister(drvdata->csdev); + + pm_runtime_disable(&pdev->dev); + + return 0; +} + #ifdef CONFIG_PM static int replicator_runtime_suspend(struct device *dev) { @@ -144,8 +163,11 @@ static const struct of_device_id replicator_match[] = { {} }; +MODULE_DEVICE_TABLE(of, replicator_match); + static struct platform_driver replicator_driver = { .probe = replicator_probe, + .remove = replicator_remove, .driver = { .name = "coresight-replicator", .of_match_table = replicator_match, @@ -153,7 +175,7 @@ static struct platform_driver replicator_driver = { .suppress_bind_attrs = true, }, }; -builtin_platform_driver(replicator_driver); +module_platform_driver(replicator_driver); MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>"); MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>"); diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index 30eae52a8757..9997ba0dbd54 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -194,7 +194,12 @@ static int stm_enable(struct coresight_device *csdev, struct perf_event *event, u32 mode) { u32 val; - struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct stm_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; if (mode != CS_MODE_SYSFS) return -EINVAL; @@ -882,6 +887,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) { @@ -922,6 +938,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", @@ -930,10 +948,11 @@ 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"); diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index 176a5aeab20e..eb3cdb832f84 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -429,6 +429,19 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) return ret; } +static int __exit tmc_remove(struct amba_device *adev) +{ + struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev); + + /* free ETB/ETF or ETR memory */ + tmc_read_unprepare(drvdata); + + misc_deregister(&drvdata->miscdev); + coresight_unregister(drvdata->csdev); + + return 0; +} + static const struct amba_id tmc_ids[] = { { .id = 0x000bb961, @@ -453,6 +466,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", @@ -460,9 +475,10 @@ static struct amba_driver tmc_driver = { .suppress_bind_attrs = true, }, .probe = tmc_probe, + .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"); diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c index f3b154e150b3..9622f2a5a451 100644 --- a/drivers/hwtracing/coresight/coresight-tpiu.c +++ b/drivers/hwtracing/coresight/coresight-tpiu.c @@ -69,7 +69,12 @@ static void tpiu_enable_hw(struct tpiu_drvdata *drvdata) static int tpiu_enable(struct coresight_device *csdev, u32 mode) { - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; + + if (!try_module_get(module)) + return -ENODEV; tpiu_enable_hw(drvdata); @@ -95,10 +100,13 @@ static void tpiu_disable_hw(struct tpiu_drvdata *drvdata) static void tpiu_disable(struct coresight_device *csdev) { - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct device *parent_dev = csdev->dev.parent; + struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev); + struct module *module = parent_dev->driver->owner; tpiu_disable_hw(drvdata); + module_put(module); dev_info(drvdata->dev, "TPIU disabled\n"); } @@ -164,6 +172,15 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id) return PTR_ERR_OR_ZERO(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) { @@ -207,6 +224,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", @@ -215,9 +234,10 @@ 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>"); diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c index 406899f316e4..c00229b0db52 100644 --- a/drivers/hwtracing/coresight/coresight.c +++ b/drivers/hwtracing/coresight/coresight.c @@ -302,6 +302,7 @@ void coresight_disable_path(struct list_head *path) } } } +EXPORT_SYMBOL_GPL(coresight_disable_path); int coresight_enable_path(struct list_head *path, u32 mode) { @@ -353,6 +354,7 @@ int coresight_enable_path(struct list_head *path, u32 mode) coresight_disable_path(path); goto out; } +EXPORT_SYMBOL_GPL(coresight_enable_path); struct coresight_device *coresight_get_sink(struct list_head *path) { @@ -368,6 +370,7 @@ struct coresight_device *coresight_get_sink(struct list_head *path) return csdev; } +EXPORT_SYMBOL_GPL(coresight_get_sink); static int coresight_enabled_sink(struct device *dev, void *data) { @@ -392,6 +395,7 @@ static int coresight_enabled_sink(struct device *dev, void *data) return 0; } +EXPORT_SYMBOL_GPL(coresight_enabled_sink); /** * coresight_get_enabled_sink - returns the first enabled sink found on the bus @@ -414,6 +418,7 @@ struct coresight_device *coresight_get_enabled_sink(bool deactivate) return dev ? to_coresight_device(dev) : NULL; } +EXPORT_SYMBOL_GPL(coresight_get_enabled_sink); /** * _coresight_build_path - recursively build a path from a @csdev to a sink. @@ -493,6 +498,7 @@ struct list_head *coresight_build_path(struct coresight_device *source, return path; } +EXPORT_SYMBOL_GPL(coresight_build_path); /** * coresight_release_path - release a previously built path. @@ -517,6 +523,7 @@ void coresight_release_path(struct list_head *path) kfree(path); path = NULL; } +EXPORT_SYMBOL_GPL(coresight_release_path); /** coresight_validate_source - make sure a source has the right credentials * @csdev: the device structure for a source. @@ -933,6 +940,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", @@ -944,6 +952,12 @@ static int __init coresight_init(void) } postcore_initcall(coresight_init); +static void __exit coresight_exit(void) +{ + bus_unregister(&coresight_bustype); +} +module_exit(coresight_exit); + struct coresight_device *coresight_register(struct coresight_desc *desc) { int i;
Allow to build coresight as modules. This greatly enhances developer efficiency by allowing the development to take place exclusively on the target, and without needing to reboot in between changes. - Kconfig bools become tristates, to allow =m - use -objs to denote merge object directives in Makefile, adds a coresight-core nomenclature for the base module. - Export core functions so as to be able to be used by non-core modules. - add a coresight_exit() that unregisters the coresight bus, add remove fns for most others. - fix up modules with ID tables for autoloading on boot Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Kim Phillips <kim.phillips@arm.com> --- drivers/hwtracing/coresight/Kconfig | 48 +++++++++++++++---- drivers/hwtracing/coresight/Makefile | 28 +++++++---- .../hwtracing/coresight/coresight-cpu-debug.c | 2 + .../coresight/coresight-dynamic-replicator.c | 26 ++++++++-- drivers/hwtracing/coresight/coresight-etb10.c | 27 +++++++++-- .../hwtracing/coresight/coresight-etm-perf.c | 9 +++- .../coresight/coresight-etm3x-sysfs.c | 1 + drivers/hwtracing/coresight/coresight-etm3x.c | 32 +++++++++++-- .../coresight/coresight-etm4x-sysfs.c | 1 + drivers/hwtracing/coresight/coresight-etm4x.c | 33 +++++++++++-- .../hwtracing/coresight/coresight-funnel.c | 26 ++++++++-- drivers/hwtracing/coresight/coresight-priv.h | 1 - .../coresight/coresight-replicator.c | 28 +++++++++-- drivers/hwtracing/coresight/coresight-stm.c | 23 ++++++++- drivers/hwtracing/coresight/coresight-tmc.c | 18 ++++++- drivers/hwtracing/coresight/coresight-tpiu.c | 26 ++++++++-- drivers/hwtracing/coresight/coresight.c | 14 ++++++ 17 files changed, 299 insertions(+), 44 deletions(-)