On Thu, Aug 06, 2020 at 10:56:57PM +0800, Suzuki K Poulose wrote:

On 08/06/2020 12:19 PM, Tingwei Zhang wrote:

quoted

On Wed, Aug 05, 2020 at 11:43:36PM +0800, Suzuki K Poulose wrote:

quoted

On 08/05/2020 03:54 AM, Tingwei Zhang wrote:

quoted

From: Kim Phillips <redacted>

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 <redacted>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Randy Dunlap <redacted>
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 <redacted>
Signed-off-by: Tingwei Zhang <redacted>
Tested-by: Mike Leach <redacted>

quoted

---
 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel