diff mbox series

[v8,15/24] coresight: allow replicator driver to be built as module

Message ID 20200807111153.7784-16-tingwei@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series coresight: allow to build coresight as modules | expand

Commit Message

Tingwei Zhang Aug. 7, 2020, 11:11 a.m. UTC
From: Kim Phillips <kim.phillips@arm.com>

Allow to build coresight-replicator as modules, for ease of development.

- Kconfig becomes a tristate, to allow =m
- combine static and dynamic replicator init into single
  module_init/exit call
- add 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>
Co-developed-by: Mian Yousaf Kaukab <ykaukab@suse.de>
Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
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 +-
 .../coresight/coresight-replicator.c          | 63 ++++++++++++++++++-
 2 files changed, 65 insertions(+), 3 deletions(-)

Comments

Mathieu Poirier Aug. 13, 2020, 8:35 p.m. UTC | #1
On Fri, Aug 07, 2020 at 07:11:44PM +0800, Tingwei Zhang wrote:
> From: Kim Phillips <kim.phillips@arm.com>
> 
> Allow to build coresight-replicator as modules, for ease of development.
> 
> - Kconfig becomes a tristate, to allow =m
> - combine static and dynamic replicator init into single
>   module_init/exit call
> - add 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>
> Co-developed-by: Mian Yousaf Kaukab <ykaukab@suse.de>
> Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
> 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 +-
>  .../coresight/coresight-replicator.c          | 63 ++++++++++++++++++-
>  2 files changed, 65 insertions(+), 3 deletions(-)
> 
> 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.

With the passage of time and the refinement of the supported components, the
CONFIG_ and description are no longer valid.  I'll leave it up to you to decide
if you want to fix it as part of this set, the choice is entirely up to you.

>  
> +	  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-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 78acf29c49ca..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,25 +357,28 @@ static const struct of_device_id static_replicator_match[] = {
>  	{.compatible = "arm,coresight-static-replicator"},
>  	{}
>  };
> +MODULE_DEVICE_TABLE(of, static_replicator_match);

Pick a way to do this, a newline or not, but stick with it for _all_ the drivers
in this set.

>  
>  #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);

Same as above

>  #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,
>  		.suppress_bind_attrs = true,
>  	},
>  };
> -builtin_platform_driver(static_replicator_driver);
>  
>  static int dynamic_replicator_probe(struct amba_device *adev,
>  				    const struct amba_id *id)
> @@ -368,19 +386,60 @@ 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,
>  };
> -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);
> +}
> +

Same here, choose to add a newline or not but stay consistent for all the
drives.

> +module_init(replicator_init);
> +module_exit(replicator_exit);
> +
> +MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");

Pratik should also be mentioned in the funnel driver.

> +MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> +MODULE_DESCRIPTION("Arm CoreSight Replicator Driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Tingwei Zhang Aug. 14, 2020, 12:08 a.m. UTC | #2
On Fri, Aug 14, 2020 at 04:35:11AM +0800, Mathieu Poirier wrote:
> On Fri, Aug 07, 2020 at 07:11:44PM +0800, Tingwei Zhang wrote:
> > From: Kim Phillips <kim.phillips@arm.com>
> > 
> > Allow to build coresight-replicator as modules, for ease of development.
> > 
> > - Kconfig becomes a tristate, to allow =m
> > - combine static and dynamic replicator init into single
> >   module_init/exit call
> > - add 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>
> > Co-developed-by: Mian Yousaf Kaukab <ykaukab@suse.de>
> > Signed-off-by: Mian Yousaf Kaukab <ykaukab@suse.de>
> > 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 +-
> >  .../coresight/coresight-replicator.c          | 63 ++++++++++++++++++-
> >  2 files changed, 65 insertions(+), 3 deletions(-)
> > 
> > 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.
> 
> With the passage of time and the refinement of the supported components,
> the
> CONFIG_ and description are no longer valid.  I'll leave it up to you to
> decide
> if you want to fix it as part of this set, the choice is entirely up to
> you.
> 

I'll leave that to another patch and focus this patch set to make driver
as module.

> >  
> > +	  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-replicator.c
> b/drivers/hwtracing/coresight/coresight-replicator.c
> > index 78acf29c49ca..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,25 +357,28 @@ static const struct of_device_id
> static_replicator_match[] = {
> >  	{.compatible = "arm,coresight-static-replicator"},
> >  	{}
> >  };
> > +MODULE_DEVICE_TABLE(of, static_replicator_match);
> 
> Pick a way to do this, a newline or not, but stick with it for _all_ the
> drivers
> in this set.
>
 
I will add a new line and check all the coresight drivers.

> >  
> >  #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);
> 
> Same as above
> 

I will add a new line.

> >  #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,
> >  		.suppress_bind_attrs = true,
> >  	},
> >  };
> > -builtin_platform_driver(static_replicator_driver);
> >  
> >  static int dynamic_replicator_probe(struct amba_device *adev,
> >  				    const struct amba_id *id)
> > @@ -368,19 +386,60 @@ 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,
> >  };
> > -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);
> > +}
> > +
> 
> Same here, choose to add a newline or not but stay consistent for all the
> drives.
>

Sure. 

> > +module_init(replicator_init);
> > +module_exit(replicator_exit);
> > +
> > +MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> 
> Pratik should also be mentioned in the funnel driver.
>
 
I'll add Pratik in v9.

> > +MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> > +MODULE_DESCRIPTION("Arm CoreSight Replicator Driver");
> > +MODULE_LICENSE("GPL v2");
> > -- 
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> > a Linux Foundation Collaborative Project
> >
diff mbox series

Patch

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-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 78acf29c49ca..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,25 +357,28 @@  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,
 		.suppress_bind_attrs = true,
 	},
 };
-builtin_platform_driver(static_replicator_driver);
 
 static int dynamic_replicator_probe(struct amba_device *adev,
 				    const struct amba_id *id)
@@ -368,19 +386,60 @@  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,
 };
-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);
+
+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");