2021-07-15 10:12:54

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH v2 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.

* Changes from v1:
- Removed TIMER_OF_DECLARE() from timer-sprd.c, and removed ifdef;
- Rebased on v5.14-rc1.

[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, 55 insertions(+), 16 deletions(-)

--
2.25.1


2021-07-15 10:16:58

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

From: Saravana Kannan <[email protected]>

This allows timer drivers to be compiled as modules.

Signed-off-by: Saravana Kannan <[email protected]>
Signed-off-by: Chunyan Zhang <[email protected]>
---
drivers/clocksource/timer-of.c | 17 +++++++++--------
drivers/clocksource/timer-of.h | 4 ++--
2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 529cc6a51cdb..7f108978fd51 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -19,7 +19,7 @@
*
* Free the irq resource
*/
-static __init void timer_of_irq_exit(struct of_timer_irq *of_irq)
+static void timer_of_irq_exit(struct of_timer_irq *of_irq)
{
struct timer_of *to = container_of(of_irq, struct timer_of, of_irq);

@@ -47,7 +47,7 @@ static __init void timer_of_irq_exit(struct of_timer_irq *of_irq)
*
* Returns 0 on success, < 0 otherwise
*/
-static __init int timer_of_irq_init(struct device_node *np,
+static int timer_of_irq_init(struct device_node *np,
struct of_timer_irq *of_irq)
{
int ret;
@@ -91,7 +91,7 @@ static __init int timer_of_irq_init(struct device_node *np,
*
* Disables and releases the refcount on the clk
*/
-static __init void timer_of_clk_exit(struct of_timer_clk *of_clk)
+static void timer_of_clk_exit(struct of_timer_clk *of_clk)
{
of_clk->rate = 0;
clk_disable_unprepare(of_clk->clk);
@@ -107,7 +107,7 @@ static __init void timer_of_clk_exit(struct of_timer_clk *of_clk)
*
* Returns 0 on success, < 0 otherwise
*/
-static __init int timer_of_clk_init(struct device_node *np,
+static int timer_of_clk_init(struct device_node *np,
struct of_timer_clk *of_clk)
{
int ret;
@@ -146,12 +146,12 @@ static __init int timer_of_clk_init(struct device_node *np,
goto out;
}

-static __init void timer_of_base_exit(struct of_timer_base *of_base)
+static void timer_of_base_exit(struct of_timer_base *of_base)
{
iounmap(of_base->base);
}

-static __init int timer_of_base_init(struct device_node *np,
+static int timer_of_base_init(struct device_node *np,
struct of_timer_base *of_base)
{
of_base->base = of_base->name ?
@@ -165,7 +165,7 @@ static __init int timer_of_base_init(struct device_node *np,
return 0;
}

-int __init timer_of_init(struct device_node *np, struct timer_of *to)
+int timer_of_init(struct device_node *np, struct timer_of *to)
{
int ret = -EINVAL;
int flags = 0;
@@ -209,6 +209,7 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
timer_of_base_exit(&to->of_base);
return ret;
}
+EXPORT_SYMBOL_GPL(timer_of_init);

/**
* timer_of_cleanup - release timer_of resources
@@ -217,7 +218,7 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
* Release the resources that has been used in timer_of_init().
* This function should be called in init error cases
*/
-void __init timer_of_cleanup(struct timer_of *to)
+void timer_of_cleanup(struct timer_of *to)
{
if (to->flags & TIMER_OF_IRQ)
timer_of_irq_exit(&to->of_irq);
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index a5478f3e8589..1b8cfac5900a 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -66,9 +66,9 @@ static inline unsigned long timer_of_period(struct timer_of *to)
return to->of_clk.period;
}

-extern int __init timer_of_init(struct device_node *np,
+extern int timer_of_init(struct device_node *np,
struct timer_of *to);

-extern void __init timer_of_cleanup(struct timer_of *to);
+extern void timer_of_cleanup(struct timer_of *to);

#endif
--
2.25.1

2021-07-15 10:17:23

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH v2 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-15 10:17:27

by Chunyan Zhang

[permalink] [raw]
Subject: [PATCH v2 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, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index eb661b539a3e..a5a5b7c883ec 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -461,7 +461,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..a8a7d3ea3464 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,9 @@ static int __init sprd_suspend_timer_init(struct device_node *np)
return 0;
}

-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);
+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");
--
2.25.1

2021-08-01 06:22:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

Hi Chunyan,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/timers/core]
[also build test ERROR on linux/master linus/master v5.14-rc3 next-20210730]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Chunyan-Zhang/Add-module-build-support-for-timer-driver/20210715-145711
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 2d0a9eb23ccfdf11308bec6db0bc007585d919d2
config: s390-buildonly-randconfig-r003-20210728 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c49df15c278857adecd12db6bb1cdc96885f7079)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/8e3c2c4da32affdbca933979110050e564351c84
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Chunyan-Zhang/Add-module-build-support-for-timer-driver/20210715-145711
git checkout 8e3c2c4da32affdbca933979110050e564351c84
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

s390x-linux-gnu-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_attach':
main.c:(.text+0x21a): undefined reference to `iounmap'
s390x-linux-gnu-ld: main.c:(.text+0x270): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_detach':
main.c:(.text+0x478): undefined reference to `iounmap'
s390x-linux-gnu-ld: main.c:(.text+0x4d4): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_probe':
main.c:(.text+0x70c): undefined reference to `ioremap'
s390x-linux-gnu-ld: main.c:(.text+0x83e): undefined reference to `iounmap'
s390x-linux-gnu-ld: main.c:(.text+0x8b6): undefined reference to `ioremap'
s390x-linux-gnu-ld: main.c:(.text+0x93a): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/char/xillybus/xillybus_of.o: in function `xilly_drv_probe':
xillybus_of.c:(.text+0x9a): undefined reference to `devm_platform_ioremap_resource'
s390x-linux-gnu-ld: drivers/net/arcnet/arc-rimi.o: in function `check_mirror':
arc-rimi.c:(.text+0x5c): undefined reference to `ioremap'
s390x-linux-gnu-ld: arc-rimi.c:(.text+0xc2): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/net/arcnet/arc-rimi.o: in function `arc_rimi_exit':
arc-rimi.c:(.exit.text+0x44): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/net/arcnet/arc-rimi.o: in function `arcrimi_found':
arc-rimi.c:(.init.text+0x37c): undefined reference to `ioremap'
s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x3c8): undefined reference to `iounmap'
s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x614): undefined reference to `iounmap'
s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x674): undefined reference to `ioremap'
s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x6de): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/net/ethernet/fujitsu/fmvj18x_cs.o: in function `fmvj18x_probe':
fmvj18x_cs.c:(.text+0x756): undefined reference to `ioremap'
s390x-linux-gnu-ld: fmvj18x_cs.c:(.text+0x788): undefined reference to `iounmap'
s390x-linux-gnu-ld: fmvj18x_cs.c:(.text+0x7e0): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/net/ethernet/fujitsu/fmvj18x_cs.o: in function `fmvj18x_detach':
fmvj18x_cs.c:(.text+0xce0): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/net/ethernet/fujitsu/fmvj18x_cs.o: in function `fmvj18x_get_hwinfo':
fmvj18x_cs.c:(.text+0x27d4): undefined reference to `ioremap'
s390x-linux-gnu-ld: fmvj18x_cs.c:(.text+0x2940): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/pcmcia/cistpl.o: in function `release_cis_mem':
cistpl.c:(.text+0x9c): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/pcmcia/cistpl.o: in function `set_cis_map':
cistpl.c:(.text+0x46c): undefined reference to `ioremap'
s390x-linux-gnu-ld: cistpl.c:(.text+0x4a8): undefined reference to `iounmap'
s390x-linux-gnu-ld: cistpl.c:(.text+0x4e6): undefined reference to `iounmap'
s390x-linux-gnu-ld: cistpl.c:(.text+0x4f8): undefined reference to `ioremap'
s390x-linux-gnu-ld: drivers/crypto/ccree/cc_driver.o: in function `ccree_probe':
cc_driver.c:(.text+0x5a8): undefined reference to `devm_ioremap_resource'
s390x-linux-gnu-ld: drivers/crypto/ccree/cc_debugfs.o: in function `cc_debugfs_init':
cc_debugfs.c:(.text+0xac): undefined reference to `debugfs_create_regset32'
s390x-linux-gnu-ld: cc_debugfs.c:(.text+0x190): undefined reference to `debugfs_create_regset32'
s390x-linux-gnu-ld: drivers/clocksource/timer-of.o: in function `timer_of_init':
timer-of.c:(.text+0x104): undefined reference to `of_iomap'
>> s390x-linux-gnu-ld: timer-of.c:(.text+0x306): undefined reference to `iounmap'
s390x-linux-gnu-ld: drivers/clocksource/timer-of.o: in function `timer_of_cleanup':
timer-of.c:(.text+0x5f2): undefined reference to `iounmap'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (5.72 kB)
.config.gz (19.12 kB)
Download all attachments

2021-08-12 08:22:26

by Chunyan Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

On Sun, 1 Aug 2021 at 14:18, kernel test robot <[email protected]> wrote:
>
> Hi Chunyan,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on tip/timers/core]
> [also build test ERROR on linux/master linus/master v5.14-rc3 next-20210730]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Chunyan-Zhang/Add-module-build-support-for-timer-driver/20210715-145711
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 2d0a9eb23ccfdf11308bec6db0bc007585d919d2
> config: s390-buildonly-randconfig-r003-20210728 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project c49df15c278857adecd12db6bb1cdc96885f7079)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install s390 cross compiling tool for clang build
> # apt-get install binutils-s390x-linux-gnu
> # https://github.com/0day-ci/linux/commit/8e3c2c4da32affdbca933979110050e564351c84
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Chunyan-Zhang/Add-module-build-support-for-timer-driver/20210715-145711
> git checkout 8e3c2c4da32affdbca933979110050e564351c84
> # save the attached .config to linux build tree
> mkdir build_dir
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=s390 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> s390x-linux-gnu-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_attach':
> main.c:(.text+0x21a): undefined reference to `iounmap'
> s390x-linux-gnu-ld: main.c:(.text+0x270): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_detach':
> main.c:(.text+0x478): undefined reference to `iounmap'
> s390x-linux-gnu-ld: main.c:(.text+0x4d4): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/tty/ipwireless/main.o: in function `ipwireless_probe':
> main.c:(.text+0x70c): undefined reference to `ioremap'
> s390x-linux-gnu-ld: main.c:(.text+0x83e): undefined reference to `iounmap'
> s390x-linux-gnu-ld: main.c:(.text+0x8b6): undefined reference to `ioremap'
> s390x-linux-gnu-ld: main.c:(.text+0x93a): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/char/xillybus/xillybus_of.o: in function `xilly_drv_probe':
> xillybus_of.c:(.text+0x9a): undefined reference to `devm_platform_ioremap_resource'
> s390x-linux-gnu-ld: drivers/net/arcnet/arc-rimi.o: in function `check_mirror':
> arc-rimi.c:(.text+0x5c): undefined reference to `ioremap'
> s390x-linux-gnu-ld: arc-rimi.c:(.text+0xc2): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/net/arcnet/arc-rimi.o: in function `arc_rimi_exit':
> arc-rimi.c:(.exit.text+0x44): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/net/arcnet/arc-rimi.o: in function `arcrimi_found':
> arc-rimi.c:(.init.text+0x37c): undefined reference to `ioremap'
> s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x3c8): undefined reference to `iounmap'
> s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x614): undefined reference to `iounmap'
> s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x674): undefined reference to `ioremap'
> s390x-linux-gnu-ld: arc-rimi.c:(.init.text+0x6de): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/net/ethernet/fujitsu/fmvj18x_cs.o: in function `fmvj18x_probe':
> fmvj18x_cs.c:(.text+0x756): undefined reference to `ioremap'
> s390x-linux-gnu-ld: fmvj18x_cs.c:(.text+0x788): undefined reference to `iounmap'
> s390x-linux-gnu-ld: fmvj18x_cs.c:(.text+0x7e0): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/net/ethernet/fujitsu/fmvj18x_cs.o: in function `fmvj18x_detach':
> fmvj18x_cs.c:(.text+0xce0): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/net/ethernet/fujitsu/fmvj18x_cs.o: in function `fmvj18x_get_hwinfo':
> fmvj18x_cs.c:(.text+0x27d4): undefined reference to `ioremap'
> s390x-linux-gnu-ld: fmvj18x_cs.c:(.text+0x2940): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/pcmcia/cistpl.o: in function `release_cis_mem':
> cistpl.c:(.text+0x9c): undefined reference to `iounmap'
> s390x-linux-gnu-ld: drivers/pcmcia/cistpl.o: in function `set_cis_map':
> cistpl.c:(.text+0x46c): undefined reference to `ioremap'
> s390x-linux-gnu-ld: cistpl.c:(.text+0x4a8): undefined reference to `iounmap'
> s390x-linux-gnu-ld: cistpl.c:(.text+0x4e6): undefined reference to `iounmap'
> s390x-linux-gnu-ld: cistpl.c:(.text+0x4f8): undefined reference to `ioremap'
> s390x-linux-gnu-ld: drivers/crypto/ccree/cc_driver.o: in function `ccree_probe':
> cc_driver.c:(.text+0x5a8): undefined reference to `devm_ioremap_resource'
> s390x-linux-gnu-ld: drivers/crypto/ccree/cc_debugfs.o: in function `cc_debugfs_init':
> cc_debugfs.c:(.text+0xac): undefined reference to `debugfs_create_regset32'
> s390x-linux-gnu-ld: cc_debugfs.c:(.text+0x190): undefined reference to `debugfs_create_regset32'
> s390x-linux-gnu-ld: drivers/clocksource/timer-of.o: in function `timer_of_init':
> timer-of.c:(.text+0x104): undefined reference to `of_iomap'
> >> s390x-linux-gnu-ld: timer-of.c:(.text+0x306): undefined reference to `iounmap'

Seems TIMER_OF should depend on HAS_IOMEM, but this error is not
related with changes in the above patch?


> s390x-linux-gnu-ld: drivers/clocksource/timer-of.o: in function `timer_of_cleanup':
> timer-of.c:(.text+0x5f2): undefined reference to `iounmap'
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]

2021-08-12 15:21:09

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

On Thu, Aug 12 2021 at 14:39, Chunyan Zhang wrote:
> On Sun, 1 Aug 2021 at 14:18, kernel test robot <[email protected]> wrote:
>> >> s390x-linux-gnu-ld: timer-of.c:(.text+0x306): undefined reference to `iounmap'
>
> Seems TIMER_OF should depend on HAS_IOMEM, but this error is not
> related with changes in the above patch?

Right, it's not caused by your patch, but if you already analyzed the
problem, then why are you not sending a fix for this?

Thanks,

tglx

2021-08-13 02:33:00

by Chunyan Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

On Thu, 12 Aug 2021 at 22:49, Thomas Gleixner <[email protected]> wrote:
>
> On Thu, Aug 12 2021 at 14:39, Chunyan Zhang wrote:
> > On Sun, 1 Aug 2021 at 14:18, kernel test robot <[email protected]> wrote:
> >> >> s390x-linux-gnu-ld: timer-of.c:(.text+0x306): undefined reference to `iounmap'
> >
> > Seems TIMER_OF should depend on HAS_IOMEM, but this error is not
> > related with changes in the above patch?
>
> Right, it's not caused by your patch, but if you already analyzed the
> problem, then why are you not sending a fix for this?

Ok, I will send a fix.
BTW, if no more comments on this patchset, could you or Daniel please
apply the patch-set to your tree?

Thanks,
Chunyan

2021-08-13 14:16:56

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

On 15/07/2021 08:54, Chunyan Zhang wrote:
> From: Saravana Kannan <[email protected]>
>
> This allows timer drivers to be compiled as modules.

Why ?

These changes will create a precedence with the timers being loaded as
modules. A longer description is important.

Also, loading the timers may be fine but unloading them is not supported
AFAICT from the time framework. That should be described also and the
code should make sure the unloading will be never supported in any
module conversion.

> Signed-off-by: Saravana Kannan <[email protected]>
> Signed-off-by: Chunyan Zhang <[email protected]>
> ---
> drivers/clocksource/timer-of.c | 17 +++++++++--------
> drivers/clocksource/timer-of.h | 4 ++--
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
> index 529cc6a51cdb..7f108978fd51 100644
> --- a/drivers/clocksource/timer-of.c
> +++ b/drivers/clocksource/timer-of.c
> @@ -19,7 +19,7 @@
> *
> * Free the irq resource
> */
> -static __init void timer_of_irq_exit(struct of_timer_irq *of_irq)
> +static void timer_of_irq_exit(struct of_timer_irq *of_irq)
> {
> struct timer_of *to = container_of(of_irq, struct timer_of, of_irq);
>
> @@ -47,7 +47,7 @@ static __init void timer_of_irq_exit(struct of_timer_irq *of_irq)
> *
> * Returns 0 on success, < 0 otherwise
> */
> -static __init int timer_of_irq_init(struct device_node *np,
> +static int timer_of_irq_init(struct device_node *np,
> struct of_timer_irq *of_irq)
> {
> int ret;
> @@ -91,7 +91,7 @@ static __init int timer_of_irq_init(struct device_node *np,
> *
> * Disables and releases the refcount on the clk
> */
> -static __init void timer_of_clk_exit(struct of_timer_clk *of_clk)
> +static void timer_of_clk_exit(struct of_timer_clk *of_clk)
> {
> of_clk->rate = 0;
> clk_disable_unprepare(of_clk->clk);
> @@ -107,7 +107,7 @@ static __init void timer_of_clk_exit(struct of_timer_clk *of_clk)
> *
> * Returns 0 on success, < 0 otherwise
> */
> -static __init int timer_of_clk_init(struct device_node *np,
> +static int timer_of_clk_init(struct device_node *np,
> struct of_timer_clk *of_clk)
> {
> int ret;
> @@ -146,12 +146,12 @@ static __init int timer_of_clk_init(struct device_node *np,
> goto out;
> }
>
> -static __init void timer_of_base_exit(struct of_timer_base *of_base)
> +static void timer_of_base_exit(struct of_timer_base *of_base)
> {
> iounmap(of_base->base);
> }
>
> -static __init int timer_of_base_init(struct device_node *np,
> +static int timer_of_base_init(struct device_node *np,
> struct of_timer_base *of_base)
> {
> of_base->base = of_base->name ?
> @@ -165,7 +165,7 @@ static __init int timer_of_base_init(struct device_node *np,
> return 0;
> }
>
> -int __init timer_of_init(struct device_node *np, struct timer_of *to)
> +int timer_of_init(struct device_node *np, struct timer_of *to)
> {
> int ret = -EINVAL;
> int flags = 0;
> @@ -209,6 +209,7 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
> timer_of_base_exit(&to->of_base);
> return ret;
> }
> +EXPORT_SYMBOL_GPL(timer_of_init);
>
> /**
> * timer_of_cleanup - release timer_of resources
> @@ -217,7 +218,7 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
> * Release the resources that has been used in timer_of_init().
> * This function should be called in init error cases
> */
> -void __init timer_of_cleanup(struct timer_of *to)
> +void timer_of_cleanup(struct timer_of *to)
> {
> if (to->flags & TIMER_OF_IRQ)
> timer_of_irq_exit(&to->of_irq);
> diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
> index a5478f3e8589..1b8cfac5900a 100644
> --- a/drivers/clocksource/timer-of.h
> +++ b/drivers/clocksource/timer-of.h
> @@ -66,9 +66,9 @@ static inline unsigned long timer_of_period(struct timer_of *to)
> return to->of_clk.period;
> }
>
> -extern int __init timer_of_init(struct device_node *np,
> +extern int timer_of_init(struct device_node *np,
> struct timer_of *to);
>
> -extern void __init timer_of_cleanup(struct timer_of *to);
> +extern void timer_of_cleanup(struct timer_of *to);
>
> #endif
>


--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2021-08-13 16:02:07

by Daniel Lezcano

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

On 15/07/2021 08:54, Chunyan Zhang 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, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index eb661b539a3e..a5a5b7c883ec 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -461,7 +461,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..a8a7d3ea3464 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)

Does the __init annotation really need to be removed ?

> {
> 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,9 @@ static int __init sprd_suspend_timer_init(struct device_node *np)
> return 0;
> }
>
> -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);
> +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);

Please replace the above by something like:

TIMER_PLATFORM_DECLARE(sc9860_timer,
"sprd,sc9860-timer",
sprd_timer_init);

TIMER_PLATFORM_DECLARE(sc9860_persistent_timer,
"sprd,sc9860-suspend-timer",
sprd_suspend_timer_init);

Without TIMER_PLATFORM_DRIVER_BEGIN/END, and if possible the
MODULE_DESCRIPTION/LICENSE in the TIMER_PLATFORM_DECLARE macro itself.
The module description could be the first argument of the timer platform
declaration.

> +MODULE_DESCRIPTION("Unisoc broadcast timer module");
> +MODULE_LICENSE("GPL");



--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2021-08-13 17:49:23

by Saravana Kannan

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

On Fri, Aug 13, 2021 at 9:00 AM Daniel Lezcano
<[email protected]> wrote:
>
> On 15/07/2021 08:54, Chunyan Zhang 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, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index eb661b539a3e..a5a5b7c883ec 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -461,7 +461,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..a8a7d3ea3464 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)
>
> Does the __init annotation really need to be removed ?
>
> > {
> > 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,9 @@ static int __init sprd_suspend_timer_init(struct device_node *np)
> > return 0;
> > }
> >
> > -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);
> > +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);
>
> Please replace the above by something like:
>
> TIMER_PLATFORM_DECLARE(sc9860_timer,
> "sprd,sc9860-timer",
> sprd_timer_init);
>
> TIMER_PLATFORM_DECLARE(sc9860_persistent_timer,
> "sprd,sc9860-suspend-timer",
> sprd_suspend_timer_init);
>
> Without TIMER_PLATFORM_DRIVER_BEGIN/END, and if possible the
> MODULE_DESCRIPTION/LICENSE in the TIMER_PLATFORM_DECLARE macro itself.

Wrt BEGIN/END macros, Chunyan is just trying to be consistent with
what was done for similar macros for IRQ. If you want two separate
drivers being registered for this case, that's ok, but the macros
should support having multiple compatible strings handled by the same
driver. And to do that, you'd need the BEGIN/END variants.

-Saravana

> The module description could be the first argument of the timer platform
> declaration.
>
> > +MODULE_DESCRIPTION("Unisoc broadcast timer module");
> > +MODULE_LICENSE("GPL");
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

2021-08-20 07:47:31

by Chunyan Zhang

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

On Fri, 13 Aug 2021 at 21:34, Daniel Lezcano <[email protected]> wrote:
>
> On 15/07/2021 08:54, Chunyan Zhang wrote:
> > From: Saravana Kannan <[email protected]>
> >
> > This allows timer drivers to be compiled as modules.
>
> Why ?
>
> These changes will create a precedence with the timers being loaded as
> modules. A longer description is important.
>
> Also, loading the timers may be fine but unloading them is not supported
> AFAICT from the time framework. That should be described also and the
> code should make sure the unloading will be never supported in any
> module conversion.

Ok, I will change to use builtin_platform_driver() to replace
module_platform_driver(), that can make sure to support loading only.
And will add a description for that in the next version patch-2.

Thanks,
Chunyan

>
> > Signed-off-by: Saravana Kannan <[email protected]>
> > Signed-off-by: Chunyan Zhang <[email protected]>
> > ---
> > drivers/clocksource/timer-of.c | 17 +++++++++--------
> > drivers/clocksource/timer-of.h | 4 ++--
> > 2 files changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
> > index 529cc6a51cdb..7f108978fd51 100644
> > --- a/drivers/clocksource/timer-of.c
> > +++ b/drivers/clocksource/timer-of.c
> > @@ -19,7 +19,7 @@
> > *
> > * Free the irq resource
> > */
> > -static __init void timer_of_irq_exit(struct of_timer_irq *of_irq)
> > +static void timer_of_irq_exit(struct of_timer_irq *of_irq)
> > {
> > struct timer_of *to = container_of(of_irq, struct timer_of, of_irq);
> >
> > @@ -47,7 +47,7 @@ static __init void timer_of_irq_exit(struct of_timer_irq *of_irq)
> > *
> > * Returns 0 on success, < 0 otherwise
> > */
> > -static __init int timer_of_irq_init(struct device_node *np,
> > +static int timer_of_irq_init(struct device_node *np,
> > struct of_timer_irq *of_irq)
> > {
> > int ret;
> > @@ -91,7 +91,7 @@ static __init int timer_of_irq_init(struct device_node *np,
> > *
> > * Disables and releases the refcount on the clk
> > */
> > -static __init void timer_of_clk_exit(struct of_timer_clk *of_clk)
> > +static void timer_of_clk_exit(struct of_timer_clk *of_clk)
> > {
> > of_clk->rate = 0;
> > clk_disable_unprepare(of_clk->clk);
> > @@ -107,7 +107,7 @@ static __init void timer_of_clk_exit(struct of_timer_clk *of_clk)
> > *
> > * Returns 0 on success, < 0 otherwise
> > */
> > -static __init int timer_of_clk_init(struct device_node *np,
> > +static int timer_of_clk_init(struct device_node *np,
> > struct of_timer_clk *of_clk)
> > {
> > int ret;
> > @@ -146,12 +146,12 @@ static __init int timer_of_clk_init(struct device_node *np,
> > goto out;
> > }
> >
> > -static __init void timer_of_base_exit(struct of_timer_base *of_base)
> > +static void timer_of_base_exit(struct of_timer_base *of_base)
> > {
> > iounmap(of_base->base);
> > }
> >
> > -static __init int timer_of_base_init(struct device_node *np,
> > +static int timer_of_base_init(struct device_node *np,
> > struct of_timer_base *of_base)
> > {
> > of_base->base = of_base->name ?
> > @@ -165,7 +165,7 @@ static __init int timer_of_base_init(struct device_node *np,
> > return 0;
> > }
> >
> > -int __init timer_of_init(struct device_node *np, struct timer_of *to)
> > +int timer_of_init(struct device_node *np, struct timer_of *to)
> > {
> > int ret = -EINVAL;
> > int flags = 0;
> > @@ -209,6 +209,7 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
> > timer_of_base_exit(&to->of_base);
> > return ret;
> > }
> > +EXPORT_SYMBOL_GPL(timer_of_init);
> >
> > /**
> > * timer_of_cleanup - release timer_of resources
> > @@ -217,7 +218,7 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
> > * Release the resources that has been used in timer_of_init().
> > * This function should be called in init error cases
> > */
> > -void __init timer_of_cleanup(struct timer_of *to)
> > +void timer_of_cleanup(struct timer_of *to)
> > {
> > if (to->flags & TIMER_OF_IRQ)
> > timer_of_irq_exit(&to->of_irq);
> > diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
> > index a5478f3e8589..1b8cfac5900a 100644
> > --- a/drivers/clocksource/timer-of.h
> > +++ b/drivers/clocksource/timer-of.h
> > @@ -66,9 +66,9 @@ static inline unsigned long timer_of_period(struct timer_of *to)
> > return to->of_clk.period;
> > }
> >
> > -extern int __init timer_of_init(struct device_node *np,
> > +extern int timer_of_init(struct device_node *np,
> > struct timer_of *to);
> >
> > -extern void __init timer_of_cleanup(struct timer_of *to);
> > +extern void timer_of_cleanup(struct timer_of *to);
> >
> > #endif
> >
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog

2021-08-20 07:48:03

by Chunyan Zhang

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

Hi Daniel,

On Sat, 14 Aug 2021 at 00:00, Daniel Lezcano <[email protected]> wrote:
>
> On 15/07/2021 08:54, Chunyan Zhang 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, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index eb661b539a3e..a5a5b7c883ec 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -461,7 +461,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..a8a7d3ea3464 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)
>
> Does the __init annotation really need to be removed ?

Yes, since sprd_timer_init() would be invoked by
platform_timer_probe() which seems not able to be marked as __init.

>
> > {
> > 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,9 @@ static int __init sprd_suspend_timer_init(struct device_node *np)
> > return 0;
> > }
> >
> > -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);
> > +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);
>
> Please replace the above by something like:
>
> TIMER_PLATFORM_DECLARE(sc9860_timer,
> "sprd,sc9860-timer",
> sprd_timer_init);
>
> TIMER_PLATFORM_DECLARE(sc9860_persistent_timer,
> "sprd,sc9860-suspend-timer",
> sprd_suspend_timer_init);
>
> Without TIMER_PLATFORM_DRIVER_BEGIN/END, and if possible the
> MODULE_DESCRIPTION/LICENSE in the TIMER_PLATFORM_DECLARE macro itself.
> The module description could be the first argument of the timer platform
> declaration.

I hope I got your point, will address this in the next version.
Let's see if changes are what you want then.

Thanks for your review,
Chunyan


>
> > +MODULE_DESCRIPTION("Unisoc broadcast timer module");
> > +MODULE_LICENSE("GPL");
>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog