[PATCH v2 0/3] Add module build support for timer driver

[PATCH v2 0/3] Add module build support for timer driver

[Chunyan Zhang]

From: Chunyan Zhang <chunyan.zhang@unisoc.com>

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

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

[Chunyan Zhang]

From: Saravana Kannan <saravanak@google.com>

This allows timer drivers to be compiled as modules.

Signed-off-by: Saravana Kannan <saravanak@google.com>
Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
---
 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

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5775 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 19582 bytes --]

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

[Chunyan Zhang]

On Sun, 1 Aug 2021 at 14:18, kernel test robot <lkp@intel.com> 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 <lkp@intel.com>
>
> 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/kbuild-all@lists.01.org

Re: [kbuild-all] Re: [PATCH v2 1/3] drivers/clocksource/timer-of: Remove __init markings

[Chen, Rong A]

On 8/12/2021 2:39 PM, Chunyan Zhang wrote:
> On Sun, 1 Aug 2021 at 14:18, kernel test robot <lkp@intel.com> 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 <lkp@intel.com>
>>
>> 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?

Hi Chunyan,

Thanks for the feedback, the bot found the error was first found with 
this patch, sometimes it doesn't mean the patch brings the error.

Best Regards,
Rong Chen

> 
> 
>>     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/kbuild-all@lists.01.org
> _______________________________________________
> kbuild-all mailing list -- kbuild-all@lists.01.org
> To unsubscribe send an email to kbuild-all-leave@lists.01.org
> 

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

[Thomas Gleixner]

On Thu, Aug 12 2021 at 14:39, Chunyan Zhang wrote:
> On Sun, 1 Aug 2021 at 14:18, kernel test robot <lkp@intel.com> 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

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

[Chunyan Zhang]

On Thu, 12 Aug 2021 at 22:49, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Thu, Aug 12 2021 at 14:39, Chunyan Zhang wrote:
> > On Sun, 1 Aug 2021 at 14:18, kernel test robot <lkp@intel.com> 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

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

[Daniel Lezcano]

On 15/07/2021 08:54, Chunyan Zhang wrote:
> From: Saravana Kannan <saravanak@google.com>
> 
> 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 <saravanak@google.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> ---
>  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

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

[Chunyan Zhang]

On Fri, 13 Aug 2021 at 21:34, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 15/07/2021 08:54, Chunyan Zhang wrote:
> > From: Saravana Kannan <saravanak@google.com>
> >
> > 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 <saravanak@google.com>
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > ---
> >  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

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

[Chunyan Zhang]

From: Chunyan Zhang <chunyan.zhang@unisoc.com>

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 <chunyan.zhang@unisoc.com>
---
 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

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

[Chunyan Zhang]

From: Chunyan Zhang <chunyan.zhang@unisoc.com>

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 <baolin.wang7@gmail.com>
Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
---
 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

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

[Daniel Lezcano]

On 15/07/2021 08:54, Chunyan Zhang wrote:
> From: Chunyan Zhang <chunyan.zhang@unisoc.com>
> 
> 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 <baolin.wang7@gmail.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> ---
>  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

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

[Saravana Kannan]

On Fri, Aug 13, 2021 at 9:00 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 15/07/2021 08:54, Chunyan Zhang wrote:
> > From: Chunyan Zhang <chunyan.zhang@unisoc.com>
> >
> > 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 <baolin.wang7@gmail.com>
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > ---
> >  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

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

[Chunyan Zhang]

Hi Daniel,

On Sat, 14 Aug 2021 at 00:00, Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> On 15/07/2021 08:54, Chunyan Zhang wrote:
> > From: Chunyan Zhang <chunyan.zhang@unisoc.com>
> >
> > 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 <baolin.wang7@gmail.com>
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > ---
> >  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