2021-07-01 08:54:07

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH 0/3] Add module build support for timer driver

From: Chunyan Zhang <[email protected]>

This patchset was based on the previous one [1], and add a few
boilerplate macros for module build purpose according to comments
from Thomas Gleixner on the patch [2].

Also switch sprd timer driver to use the help macros for support
module build.

[1] https://lkml.org/lkml/2020/3/24/72
[2] https://www.spinics.net/lists/arm-kernel/msg826631.html

Chunyan Zhang (2):
clocksource/drivers/timer-of: Add boilerplate macros for timer module
driver
clocksource/drivers/sprd: Add module support to Unisoc timer

Saravana Kannan (1):
drivers/clocksource/timer-of: Remove __init markings

drivers/clocksource/Kconfig | 2 +-
drivers/clocksource/timer-of.c | 30 ++++++++++++++++++++++--------
drivers/clocksource/timer-of.h | 24 ++++++++++++++++++++++--
drivers/clocksource/timer-sprd.c | 15 +++++++++++++--
4 files changed, 58 insertions(+), 13 deletions(-)

--
2.25.1


2021-07-01 08:54:54

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH 3/3] clocksource/drivers/sprd: Add module support to Unisoc timer

From: Chunyan Zhang <[email protected]>

Timers still have devices created for them. So, when compiling a timer
driver as a module, implement it as a normal platform device driver.

Original-by: Baolin Wang <[email protected]>
Signed-off-by: Chunyan Zhang <[email protected]>
---
drivers/clocksource/Kconfig | 2 +-
drivers/clocksource/timer-sprd.c | 15 +++++++++++++--
2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 39aa21d01e05..9f16c2779edb 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -447,7 +447,7 @@ config MTK_TIMER
Support for Mediatek timer driver.

config SPRD_TIMER
- bool "Spreadtrum timer driver" if EXPERT
+ tristate "Spreadtrum timer driver" if EXPERT
depends on HAS_IOMEM
depends on (ARCH_SPRD || COMPILE_TEST)
default ARCH_SPRD
diff --git a/drivers/clocksource/timer-sprd.c b/drivers/clocksource/timer-sprd.c
index 430cb99d8d79..73c7b3f8c901 100644
--- a/drivers/clocksource/timer-sprd.c
+++ b/drivers/clocksource/timer-sprd.c
@@ -5,6 +5,8 @@

#include <linux/init.h>
#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>

#include "timer-of.h"

@@ -141,7 +143,7 @@ static struct timer_of to = {
},
};

-static int __init sprd_timer_init(struct device_node *np)
+static int sprd_timer_init(struct device_node *np)
{
int ret;

@@ -190,7 +192,7 @@ static struct clocksource suspend_clocksource = {
.flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
};

-static int __init sprd_suspend_timer_init(struct device_node *np)
+static int sprd_suspend_timer_init(struct device_node *np)
{
int ret;

@@ -204,6 +206,15 @@ static int __init sprd_suspend_timer_init(struct device_node *np)
return 0;
}

+#ifdef MODULE
+TIMER_PLATFORM_DRIVER_BEGIN(sprd_timer)
+TIMER_MATCH("sprd,sc9860-timer", sprd_timer_init)
+TIMER_MATCH("sprd,sc9860-suspend-timer", sprd_suspend_timer_init)
+TIMER_PLATFORM_DRIVER_END(sprd_timer);
+MODULE_DESCRIPTION("Unisoc broadcast timer module");
+MODULE_LICENSE("GPL");
+#else
TIMER_OF_DECLARE(sc9860_timer, "sprd,sc9860-timer", sprd_timer_init);
TIMER_OF_DECLARE(sc9860_persistent_timer, "sprd,sc9860-suspend-timer",
sprd_suspend_timer_init);
+#endif
--
2.25.1

2021-07-01 08:55:32

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH 2/3] clocksource/drivers/timer-of: Add boilerplate macros for timer module driver

From: Chunyan Zhang <[email protected]>

To support module build, platform driver structs, .probe(), match table and
module macros need to be added to the timer driver. So this patch provides
a few macros to take care of these things, and that would reduce the repeat
code lines in every sigle driver.

Signed-off-by: Chunyan Zhang <[email protected]>
---
drivers/clocksource/timer-of.c | 13 +++++++++++++
drivers/clocksource/timer-of.h | 20 ++++++++++++++++++++
2 files changed, 33 insertions(+)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 7f108978fd51..ecd7f7379400 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -8,7 +8,9 @@
#include <linux/interrupt.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/of_irq.h>
+#include <linux/platform_device.h>
#include <linux/slab.h>

#include "timer-of.h"
@@ -229,3 +231,14 @@ void timer_of_cleanup(struct timer_of *to)
if (to->flags & TIMER_OF_BASE)
timer_of_base_exit(&to->of_base);
}
+
+int platform_timer_probe(struct platform_device *pdev)
+{
+ int (*init_cb)(struct device_node *node);
+ struct device_node *np = pdev->dev.of_node;
+
+ init_cb = of_device_get_match_data(&pdev->dev);
+
+ return init_cb(np);
+}
+EXPORT_SYMBOL_GPL(platform_timer_probe);
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index 1b8cfac5900a..129f539d5f54 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -3,6 +3,7 @@
#define __TIMER_OF_H__

#include <linux/clockchips.h>
+#include <linux/platform_device.h>

#define TIMER_OF_BASE 0x1
#define TIMER_OF_CLOCK 0x2
@@ -71,4 +72,23 @@ extern int timer_of_init(struct device_node *np,

extern void timer_of_cleanup(struct timer_of *to);

+extern int platform_timer_probe(struct platform_device *pdev);
+
+#define TIMER_PLATFORM_DRIVER_BEGIN(drv_name) \
+static const struct of_device_id drv_name##_timer_match_table[] = {
+
+#define TIMER_MATCH(compat, _data) { .compatible = compat, .data = _data },
+
+#define TIMER_PLATFORM_DRIVER_END(drv_name) \
+ {}, \
+}; \
+MODULE_DEVICE_TABLE(of, drv_name##_timer_match_table); \
+static struct platform_driver drv_name##_driver = { \
+ .probe = platform_timer_probe, \
+ .driver = { \
+ .name = #drv_name, \
+ .of_match_table = drv_name##_timer_match_table, \
+ }, \
+}; \
+module_platform_driver(drv_name##_driver)
#endif
--
2.25.1

2021-07-01 19:21:27

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH 3/3] clocksource/drivers/sprd: Add module support to Unisoc timer

On Thu, Jul 1, 2021 at 1:52 AM Chunyan Zhang <[email protected]> wrote:
>
> From: Chunyan Zhang <[email protected]>
>
> Timers still have devices created for them. So, when compiling a timer
> driver as a module, implement it as a normal platform device driver.
>
> Original-by: Baolin Wang <[email protected]>
> Signed-off-by: Chunyan Zhang <[email protected]>
> ---
> drivers/clocksource/Kconfig | 2 +-
> drivers/clocksource/timer-sprd.c | 15 +++++++++++++--
> 2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 39aa21d01e05..9f16c2779edb 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -447,7 +447,7 @@ config MTK_TIMER
> Support for Mediatek timer driver.
>
> config SPRD_TIMER
> - bool "Spreadtrum timer driver" if EXPERT
> + tristate "Spreadtrum timer driver" if EXPERT
> depends on HAS_IOMEM
> depends on (ARCH_SPRD || COMPILE_TEST)
> default ARCH_SPRD
> diff --git a/drivers/clocksource/timer-sprd.c b/drivers/clocksource/timer-sprd.c
> index 430cb99d8d79..73c7b3f8c901 100644
> --- a/drivers/clocksource/timer-sprd.c
> +++ b/drivers/clocksource/timer-sprd.c
> @@ -5,6 +5,8 @@
>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
>
> #include "timer-of.h"
>
> @@ -141,7 +143,7 @@ static struct timer_of to = {
> },
> };
>
> -static int __init sprd_timer_init(struct device_node *np)
> +static int sprd_timer_init(struct device_node *np)
> {
> int ret;
>
> @@ -190,7 +192,7 @@ static struct clocksource suspend_clocksource = {
> .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
> };
>
> -static int __init sprd_suspend_timer_init(struct device_node *np)
> +static int sprd_suspend_timer_init(struct device_node *np)
> {
> int ret;
>
> @@ -204,6 +206,15 @@ static int __init sprd_suspend_timer_init(struct device_node *np)
> return 0;
> }
>
> +#ifdef MODULE
> +TIMER_PLATFORM_DRIVER_BEGIN(sprd_timer)
> +TIMER_MATCH("sprd,sc9860-timer", sprd_timer_init)
> +TIMER_MATCH("sprd,sc9860-suspend-timer", sprd_suspend_timer_init)
> +TIMER_PLATFORM_DRIVER_END(sprd_timer);
> +MODULE_DESCRIPTION("Unisoc broadcast timer module");
> +MODULE_LICENSE("GPL");
> +#else
> TIMER_OF_DECLARE(sc9860_timer, "sprd,sc9860-timer", sprd_timer_init);
> TIMER_OF_DECLARE(sc9860_persistent_timer, "sprd,sc9860-suspend-timer",
> sprd_suspend_timer_init);
> +#endif

Would it be a problem if you removed the ifdef and dropped these
TIMER_OF_DECLARE? Doesn't look like either of these timers are needed
for the early scheduler timer.

-Saravana

2021-07-02 02:31:22

by Chunyan Zhang

[permalink] [raw]
Subject: Re: [PATCH 3/3] clocksource/drivers/sprd: Add module support to Unisoc timer

On Fri, 2 Jul 2021 at 03:18, Saravana Kannan <[email protected]> wrote:
>
> On Thu, Jul 1, 2021 at 1:52 AM Chunyan Zhang <[email protected]> wrote:
> >
> > From: Chunyan Zhang <[email protected]>
> >
> > Timers still have devices created for them. So, when compiling a timer
> > driver as a module, implement it as a normal platform device driver.
> >
> > Original-by: Baolin Wang <[email protected]>
> > Signed-off-by: Chunyan Zhang <[email protected]>
> > ---
> > drivers/clocksource/Kconfig | 2 +-
> > drivers/clocksource/timer-sprd.c | 15 +++++++++++++--
> > 2 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 39aa21d01e05..9f16c2779edb 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -447,7 +447,7 @@ config MTK_TIMER
> > Support for Mediatek timer driver.
> >
> > config SPRD_TIMER
> > - bool "Spreadtrum timer driver" if EXPERT
> > + tristate "Spreadtrum timer driver" if EXPERT
> > depends on HAS_IOMEM
> > depends on (ARCH_SPRD || COMPILE_TEST)
> > default ARCH_SPRD
> > diff --git a/drivers/clocksource/timer-sprd.c b/drivers/clocksource/timer-sprd.c
> > index 430cb99d8d79..73c7b3f8c901 100644
> > --- a/drivers/clocksource/timer-sprd.c
> > +++ b/drivers/clocksource/timer-sprd.c
> > @@ -5,6 +5,8 @@
> >
> > #include <linux/init.h>
> > #include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> >
> > #include "timer-of.h"
> >
> > @@ -141,7 +143,7 @@ static struct timer_of to = {
> > },
> > };
> >
> > -static int __init sprd_timer_init(struct device_node *np)
> > +static int sprd_timer_init(struct device_node *np)
> > {
> > int ret;
> >
> > @@ -190,7 +192,7 @@ static struct clocksource suspend_clocksource = {
> > .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
> > };
> >
> > -static int __init sprd_suspend_timer_init(struct device_node *np)
> > +static int sprd_suspend_timer_init(struct device_node *np)
> > {
> > int ret;
> >
> > @@ -204,6 +206,15 @@ static int __init sprd_suspend_timer_init(struct device_node *np)
> > return 0;
> > }
> >
> > +#ifdef MODULE
> > +TIMER_PLATFORM_DRIVER_BEGIN(sprd_timer)
> > +TIMER_MATCH("sprd,sc9860-timer", sprd_timer_init)
> > +TIMER_MATCH("sprd,sc9860-suspend-timer", sprd_suspend_timer_init)
> > +TIMER_PLATFORM_DRIVER_END(sprd_timer);
> > +MODULE_DESCRIPTION("Unisoc broadcast timer module");
> > +MODULE_LICENSE("GPL");
> > +#else
> > TIMER_OF_DECLARE(sc9860_timer, "sprd,sc9860-timer", sprd_timer_init);
> > TIMER_OF_DECLARE(sc9860_persistent_timer, "sprd,sc9860-suspend-timer",
> > sprd_suspend_timer_init);
> > +#endif
>
> Would it be a problem if you removed the ifdef and dropped these
> TIMER_OF_DECLARE? Doesn't look like either of these timers are needed
> for the early scheduler timer.

Yes, there seems no problem indeed for now, I will drop them.

Thanks,
Chunyan

>
> -Saravana