* [PATCH linux-next v3 0/4] Support timer drivers as loadable modules
@ 2023-04-19 7:49 walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 1/4] time/sched_clock: Export sched_clock_register() walter.chang
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: walter.chang @ 2023-04-19 7:49 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
AngeloGioacchino Del Regno, Maciej W . Rozycki, John Stultz
Cc: wsd_upstream, stanley.chu, Chun-hung.Wu, Freddy.Hsin,
walter.chang, linux-kernel, linux-arm-kernel, linux-mediatek
From: Walter Chang <walter.chang@mediatek.com>
This set of patches aims to make SoC related timer drivers, such as
timer-mediatek.c become loadable modules for the Generic Kernel Image
(GKI).
This driver registers an always-on timer as tick_broadcast_device on
MediaTek SoCs. If the system does not load this module at startup,
system will also boot normally by using built-in `bc_hrtimer` instead.
Besides, the previous experiment [1] indicates that the SYST/GPT, in
combination with a loadable module, is fully operational.
The first three patches export functions and remove __init markings to
support loadable timer modules.
The fourth patch makes timer-mediatek.c become loadable module for GKI.
[1] https://lore.kernel.org/all/32777456f8e0f98e4cd5b950f421d21f71b149cf.camel@mediatek.com/#t
[v3]
- Rebase on linux-next
[v2]
- Convert timer-mediatek.c driver to loadable module
Chun-Hung Wu (4):
time/sched_clock: Export sched_clock_register()
clocksource/drivers/mmio: Export clocksource_mmio_init()
clocksource/drivers/timer-of: Remove __init markings
clocksource/drivers/timer-mediatek: Make timer-mediatek become
loadable module
drivers/clocksource/Kconfig | 2 +-
drivers/clocksource/mmio.c | 8 +++---
drivers/clocksource/timer-mediatek.c | 39 ++++++++++++++++++++++++++++
drivers/clocksource/timer-of.c | 23 ++++++++--------
drivers/clocksource/timer-of.h | 6 ++---
kernel/time/sched_clock.c | 4 +--
6 files changed, 62 insertions(+), 20 deletions(-)
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH linux-next v3 1/4] time/sched_clock: Export sched_clock_register()
2023-04-19 7:49 [PATCH linux-next v3 0/4] Support timer drivers as loadable modules walter.chang
@ 2023-04-19 7:49 ` walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 2/4] clocksource/drivers/mmio: Export clocksource_mmio_init() walter.chang
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: walter.chang @ 2023-04-19 7:49 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
AngeloGioacchino Del Regno, Maciej W . Rozycki, John Stultz
Cc: wsd_upstream, stanley.chu, Chun-hung.Wu, Freddy.Hsin,
walter.chang, Chun-Hung Wu, linux-kernel, linux-arm-kernel,
linux-mediatek
From: Chun-Hung Wu <chun-hung.wu@mediatek.com>
clocksource driver may use sched_clock_register()
to resigter itself as a sched_clock source.
Export it to support building such driver
as module, like timer-mediatek.c
Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
kernel/time/sched_clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index 8464c5acc913..8e49e87d1221 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -150,8 +150,7 @@ static enum hrtimer_restart sched_clock_poll(struct hrtimer *hrt)
return HRTIMER_RESTART;
}
-void __init
-sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
+void sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
{
u64 res, wrap, new_mask, new_epoch, cyc, ns;
u32 new_mult, new_shift;
@@ -223,6 +222,7 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
pr_debug("Registered %pS as sched_clock source\n", read);
}
+EXPORT_SYMBOL_GPL(sched_clock_register);
void __init generic_sched_clock_init(void)
{
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH linux-next v3 2/4] clocksource/drivers/mmio: Export clocksource_mmio_init()
2023-04-19 7:49 [PATCH linux-next v3 0/4] Support timer drivers as loadable modules walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 1/4] time/sched_clock: Export sched_clock_register() walter.chang
@ 2023-04-19 7:49 ` walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 3/4] clocksource/drivers/timer-of: Remove __init markings walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 4/4] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module walter.chang
3 siblings, 0 replies; 7+ messages in thread
From: walter.chang @ 2023-04-19 7:49 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
AngeloGioacchino Del Regno, Maciej W . Rozycki, John Stultz
Cc: wsd_upstream, stanley.chu, Chun-hung.Wu, Freddy.Hsin,
walter.chang, Chun-Hung Wu, linux-kernel, linux-arm-kernel,
linux-mediatek
From: Chun-Hung Wu <chun-hung.wu@mediatek.com>
Export clocksource_mmio_init() and clocksource_mmio_readl_up()
to support building clocksource driver as module,
such as timer-mediatek.c.
Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
drivers/clocksource/mmio.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
index 9de751531831..b08b2f9d7a8b 100644
--- a/drivers/clocksource/mmio.c
+++ b/drivers/clocksource/mmio.c
@@ -21,6 +21,7 @@ u64 clocksource_mmio_readl_up(struct clocksource *c)
{
return (u64)readl_relaxed(to_mmio_clksrc(c)->reg);
}
+EXPORT_SYMBOL_GPL(clocksource_mmio_readl_up);
u64 clocksource_mmio_readl_down(struct clocksource *c)
{
@@ -46,9 +47,9 @@ u64 clocksource_mmio_readw_down(struct clocksource *c)
* @bits: Number of valid bits
* @read: One of clocksource_mmio_read*() above
*/
-int __init clocksource_mmio_init(void __iomem *base, const char *name,
- unsigned long hz, int rating, unsigned bits,
- u64 (*read)(struct clocksource *))
+int clocksource_mmio_init(void __iomem *base, const char *name,
+ unsigned long hz, int rating, unsigned bits,
+ u64 (*read)(struct clocksource *))
{
struct clocksource_mmio *cs;
@@ -68,3 +69,4 @@ int __init clocksource_mmio_init(void __iomem *base, const char *name,
return clocksource_register_hz(&cs->clksrc, hz);
}
+EXPORT_SYMBOL_GPL(clocksource_mmio_init);
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH linux-next v3 3/4] clocksource/drivers/timer-of: Remove __init markings
2023-04-19 7:49 [PATCH linux-next v3 0/4] Support timer drivers as loadable modules walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 1/4] time/sched_clock: Export sched_clock_register() walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 2/4] clocksource/drivers/mmio: Export clocksource_mmio_init() walter.chang
@ 2023-04-19 7:49 ` walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 4/4] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module walter.chang
3 siblings, 0 replies; 7+ messages in thread
From: walter.chang @ 2023-04-19 7:49 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
AngeloGioacchino Del Regno, Maciej W . Rozycki, John Stultz
Cc: wsd_upstream, stanley.chu, Chun-hung.Wu, Freddy.Hsin,
walter.chang, Chun-Hung Wu, linux-kernel, linux-arm-kernel,
linux-mediatek
From: Chun-Hung Wu <chun-hung.wu@mediatek.com>
Remove __init markings to allow timer drivers
can be compiled as modules.
Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
drivers/clocksource/timer-of.c | 23 ++++++++++++-----------
drivers/clocksource/timer-of.h | 6 +++---
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index c3f54d9912be..59bc5921acad 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,8 +47,8 @@ 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,
- struct of_timer_irq *of_irq)
+static int timer_of_irq_init(struct device_node *np,
+ struct of_timer_irq *of_irq)
{
int ret;
struct timer_of *to = container_of(of_irq, struct timer_of, of_irq);
@@ -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,8 +107,8 @@ 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,
- struct of_timer_clk *of_clk)
+static int timer_of_clk_init(struct device_node *np,
+ struct of_timer_clk *of_clk)
{
int ret;
@@ -146,13 +146,13 @@ 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,
- struct of_timer_base *of_base)
+static int timer_of_base_init(struct device_node *np,
+ struct of_timer_base *of_base)
{
of_base->base = of_base->name ?
of_io_request_and_map(np, of_base->index, 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..5d1472846346 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,
- struct timer_of *to);
+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.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH linux-next v3 4/4] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module
2023-04-19 7:49 [PATCH linux-next v3 0/4] Support timer drivers as loadable modules walter.chang
` (2 preceding siblings ...)
2023-04-19 7:49 ` [PATCH linux-next v3 3/4] clocksource/drivers/timer-of: Remove __init markings walter.chang
@ 2023-04-19 7:49 ` walter.chang
2023-04-19 8:10 ` AngeloGioacchino Del Regno
3 siblings, 1 reply; 7+ messages in thread
From: walter.chang @ 2023-04-19 7:49 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
AngeloGioacchino Del Regno, Maciej W . Rozycki, John Stultz
Cc: wsd_upstream, stanley.chu, Chun-hung.Wu, Freddy.Hsin,
walter.chang, Chun-Hung Wu, linux-kernel, linux-arm-kernel,
linux-mediatek
From: Chun-Hung Wu <chun-hung.wu@mediatek.com>
Make the timer-mediatek driver which can register
an always-on timer as tick_broadcast_device on
MediaTek SoCs become loadable module in GKI.
Tested-by: Walter Chang <walter.chang@mediatek.com>
Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
drivers/clocksource/Kconfig | 2 +-
drivers/clocksource/timer-mediatek.c | 39 ++++++++++++++++++++++++++++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 526382dc7482..a7413ad7b6ad 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -472,7 +472,7 @@ config SYS_SUPPORTS_SH_CMT
bool
config MTK_TIMER
- bool "Mediatek timer driver" if COMPILE_TEST
+ tristate "Mediatek timer driver"
depends on HAS_IOMEM
select TIMER_OF
select CLKSRC_MMIO
diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c
index 7bcb4a3f26fb..3448848682c0 100644
--- a/drivers/clocksource/timer-mediatek.c
+++ b/drivers/clocksource/timer-mediatek.c
@@ -13,6 +13,9 @@
#include <linux/clocksource.h>
#include <linux/interrupt.h>
#include <linux/irqreturn.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
#include <linux/sched_clock.h>
#include <linux/slab.h>
#include "timer-of.h"
@@ -337,5 +340,41 @@ static int __init mtk_gpt_init(struct device_node *node)
return 0;
}
+
+#ifdef MODULE
+static int mtk_timer_probe(struct platform_device *pdev)
+{
+ int (*timer_init)(struct device_node *node);
+ struct device_node *np = pdev->dev.of_node;
+
+ timer_init = of_device_get_match_data(&pdev->dev);
+ return timer_init(np);
+}
+
+static const struct of_device_id mtk_timer_match_table[] = {
+ {
+ .compatible = "mediatek,mt6577-timer",
+ .data = mtk_gpt_init,
+ },
+ {
+ .compatible = "mediatek,mt6765-timer",
+ .data = mtk_syst_init,
+ },
+ {}
+};
+
+static struct platform_driver mtk_timer_driver = {
+ .probe = mtk_timer_probe,
+ .driver = {
+ .name = "mtk-timer",
+ .of_match_table = mtk_timer_match_table,
+ },
+};
+module_platform_driver(mtk_timer_driver);
+
+MODULE_DESCRIPTION("MediaTek Module Timer driver");
+MODULE_LICENSE("GPL v2");
+#else
TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
TIMER_OF_DECLARE(mtk_mt6765, "mediatek,mt6765-timer", mtk_syst_init);
+#endif
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH linux-next v3 4/4] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module
2023-04-19 7:49 ` [PATCH linux-next v3 4/4] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module walter.chang
@ 2023-04-19 8:10 ` AngeloGioacchino Del Regno
2023-04-20 1:46 ` Walter Chang (張維哲)
0 siblings, 1 reply; 7+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-04-19 8:10 UTC (permalink / raw)
To: walter.chang, Daniel Lezcano, Thomas Gleixner, Matthias Brugger,
Maciej W . Rozycki, John Stultz
Cc: wsd_upstream, stanley.chu, Chun-hung.Wu, Freddy.Hsin,
linux-kernel, linux-arm-kernel, linux-mediatek
Il 19/04/23 09:49, walter.chang@mediatek.com ha scritto:
> From: Chun-Hung Wu <chun-hung.wu@mediatek.com>
>
> Make the timer-mediatek driver which can register
> an always-on timer as tick_broadcast_device on
> MediaTek SoCs become loadable module in GKI.
>
> Tested-by: Walter Chang <walter.chang@mediatek.com>
> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
I think I typoed your email when sending the example patch for the
conversion to platform_device. Check [1], it may be better to just
iterate through that? (please ignore the pure_initcall() part, that's
a mistake, it's never gonna happen as it automatically becomes a
module_init() call).
It depends on what maintainers think about that clocksource.h addition,
the patch got zero comments, so if you're interested in that perhaps we
can explicitly ask what would be the best option between yours and mine;
that addition is done only to avoid the big ifdef party that this patch
proposes and makes things a bit shorter if this timer modularization
goes on with more drivers, but I don't have strong opinions anyway.
In the meanwhile, just to eventually speed up integrating this, or the
other patch - I'll still give you a review of this one.
[1]:
https://patchwork.kernel.org/project/linux-mediatek/patch/20230309132119.175650-1-angelogioacchino.delregno@collabora.com/
> ---
> drivers/clocksource/Kconfig | 2 +-
> drivers/clocksource/timer-mediatek.c | 39 ++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 526382dc7482..a7413ad7b6ad 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -472,7 +472,7 @@ config SYS_SUPPORTS_SH_CMT
> bool
>
> config MTK_TIMER
> - bool "Mediatek timer driver" if COMPILE_TEST
> + tristate "Mediatek timer driver"
While at it, you could also fix the text, Mediatek -> MediaTek
> depends on HAS_IOMEM
> select TIMER_OF
> select CLKSRC_MMIO
> diff --git a/drivers/clocksource/timer-mediatek.c b/drivers/clocksource/timer-mediatek.c
> index 7bcb4a3f26fb..3448848682c0 100644
> --- a/drivers/clocksource/timer-mediatek.c
> +++ b/drivers/clocksource/timer-mediatek.c
> @@ -13,6 +13,9 @@
> #include <linux/clocksource.h>
> #include <linux/interrupt.h>
> #include <linux/irqreturn.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> #include <linux/sched_clock.h>
> #include <linux/slab.h>
> #include "timer-of.h"
> @@ -337,5 +340,41 @@ static int __init mtk_gpt_init(struct device_node *node)
>
> return 0;
> }
> +
> +#ifdef MODULE
#ifndef MODULE
... two lines...
#else
... a bunch of lines ...
#endif
looks more readable. I'd go with that.
> +static int mtk_timer_probe(struct platform_device *pdev)
> +{
> + int (*timer_init)(struct device_node *node);
> + struct device_node *np = pdev->dev.of_node;
> +
> + timer_init = of_device_get_match_data(&pdev->dev);
> + return timer_init(np);
> +}
> +
> +static const struct of_device_id mtk_timer_match_table[] = {
> + {
> + .compatible = "mediatek,mt6577-timer",
> + .data = mtk_gpt_init,
Fits in one line!
> + },
> + {
> + .compatible = "mediatek,mt6765-timer",
> + .data = mtk_syst_init,
ditto.
> + },
> + {}
Always end with { /* sentinel */ }
> +};
> +
> +static struct platform_driver mtk_timer_driver = {
> + .probe = mtk_timer_probe,
> + .driver = {
> + .name = "mtk-timer",
"mediatek-timer" looks nicer :-)
> + .of_match_table = mtk_timer_match_table,
> + },
> +};
> +module_platform_driver(mtk_timer_driver);
> +
> +MODULE_DESCRIPTION("MediaTek Module Timer driver");
"MediaTek Timer driver" is enough, "Module" gets misleading if this gets compiled
as built in platform driver (instead of built in timer_of).
> +MODULE_LICENSE("GPL v2");
> +#else
> TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
> TIMER_OF_DECLARE(mtk_mt6765, "mediatek,mt6765-timer", mtk_syst_init);
> +#endif
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH linux-next v3 4/4] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module
2023-04-19 8:10 ` AngeloGioacchino Del Regno
@ 2023-04-20 1:46 ` Walter Chang (張維哲)
0 siblings, 0 replies; 7+ messages in thread
From: Walter Chang (張維哲) @ 2023-04-20 1:46 UTC (permalink / raw)
To: tglx@linutronix.de, daniel.lezcano@linaro.org,
matthias.bgg@gmail.com, jstultz@google.com,
angelogioacchino.delregno@collabora.com, macro@orcam.me.uk
Cc: linux-arm-kernel@lists.infradead.org, wsd_upstream,
linux-kernel@vger.kernel.org,
Stanley Chu (朱原陞),
Freddy Hsin (辛恒豐),
linux-mediatek@lists.infradead.org,
Chun-Hung Wu (巫駿宏)
On Wed, 2023-04-19 at 10:10 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 19/04/23 09:49, walter.chang@mediatek.com ha scritto:
> > From: Chun-Hung Wu <chun-hung.wu@mediatek.com>
> >
> > Make the timer-mediatek driver which can register
> > an always-on timer as tick_broadcast_device on
> > MediaTek SoCs become loadable module in GKI.
> >
> > Tested-by: Walter Chang <walter.chang@mediatek.com>
> > Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
>
> I think I typoed your email when sending the example patch for the
> conversion to platform_device. Check [1], it may be better to just
> iterate through that? (please ignore the pure_initcall() part, that's
> a mistake, it's never gonna happen as it automatically becomes a
> module_init() call).
>
> It depends on what maintainers think about that clocksource.h
> addition,
> the patch got zero comments, so if you're interested in that perhaps
> we
> can explicitly ask what would be the best option between yours and
> mine;
> that addition is done only to avoid the big ifdef party that this
> patch
> proposes and makes things a bit shorter if this timer modularization
> goes on with more drivers, but I don't have strong opinions anyway.
>
> In the meanwhile, just to eventually speed up integrating this, or
> the
> other patch - I'll still give you a review of this one.
>
> [1]:
>
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20230309132119.175650-1-angelogioacchino.delregno@collabora.com/__;!!CTRNKA9wMg0ARbw!kellIP9qWQwZCUsbK9-s9WForDbz_5-CAGdMIzYbGSXgvXjVGnyGmlmGHeKEbXr_URQeKVHnb6eFAh-fBshGD1KTERh-mzSAlQ$
Thank you for providing an alternative implementation to make the timer
driver loadable. We will study whether this solution is feasible.
>
> > ---
> > drivers/clocksource/Kconfig | 2 +-
> > drivers/clocksource/timer-mediatek.c | 39
> > ++++++++++++++++++++++++++++
> > 2 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clocksource/Kconfig
> > b/drivers/clocksource/Kconfig
> > index 526382dc7482..a7413ad7b6ad 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -472,7 +472,7 @@ config SYS_SUPPORTS_SH_CMT
> > bool
> >
> > config MTK_TIMER
> > - bool "Mediatek timer driver" if COMPILE_TEST
> > + tristate "Mediatek timer driver"
>
> While at it, you could also fix the text, Mediatek -> MediaTek
>
> > depends on HAS_IOMEM
> > select TIMER_OF
> > select CLKSRC_MMIO
> > diff --git a/drivers/clocksource/timer-mediatek.c
> > b/drivers/clocksource/timer-mediatek.c
> > index 7bcb4a3f26fb..3448848682c0 100644
> > --- a/drivers/clocksource/timer-mediatek.c
> > +++ b/drivers/clocksource/timer-mediatek.c
> > @@ -13,6 +13,9 @@
> > #include <linux/clocksource.h>
> > #include <linux/interrupt.h>
> > #include <linux/irqreturn.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > #include <linux/sched_clock.h>
> > #include <linux/slab.h>
> > #include "timer-of.h"
> > @@ -337,5 +340,41 @@ static int __init mtk_gpt_init(struct
> > device_node *node)
> >
> > return 0;
> > }
> > +
> > +#ifdef MODULE
>
> #ifndef MODULE
> ... two lines...
> #else
> ... a bunch of lines ...
> #endif
>
> looks more readable. I'd go with that.
>
> > +static int mtk_timer_probe(struct platform_device *pdev)
> > +{
> > + int (*timer_init)(struct device_node *node);
> > + struct device_node *np = pdev->dev.of_node;
> > +
> > + timer_init = of_device_get_match_data(&pdev->dev);
> > + return timer_init(np);
> > +}
> > +
> > +static const struct of_device_id mtk_timer_match_table[] = {
> > + {
> > + .compatible = "mediatek,mt6577-timer",
> > + .data = mtk_gpt_init,
>
> Fits in one line!
>
> > + },
> > + {
> > + .compatible = "mediatek,mt6765-timer",
> > + .data = mtk_syst_init,
>
> ditto.
>
> > + },
> > + {}
>
> Always end with { /* sentinel */ }
>
> > +};
> > +
> > +static struct platform_driver mtk_timer_driver = {
> > + .probe = mtk_timer_probe,
> > + .driver = {
> > + .name = "mtk-timer",
>
> "mediatek-timer" looks nicer :-)
>
> > + .of_match_table = mtk_timer_match_table,
> > + },
> > +};
> > +module_platform_driver(mtk_timer_driver);
> > +
> > +MODULE_DESCRIPTION("MediaTek Module Timer driver");
>
> "MediaTek Timer driver" is enough, "Module" gets misleading if this
> gets compiled
> as built in platform driver (instead of built in timer_of).
>
Thanks for your review, I will fix these in next patch.
Best regards,
Walter Chang
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-04-20 1:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-19 7:49 [PATCH linux-next v3 0/4] Support timer drivers as loadable modules walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 1/4] time/sched_clock: Export sched_clock_register() walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 2/4] clocksource/drivers/mmio: Export clocksource_mmio_init() walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 3/4] clocksource/drivers/timer-of: Remove __init markings walter.chang
2023-04-19 7:49 ` [PATCH linux-next v3 4/4] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module walter.chang
2023-04-19 8:10 ` AngeloGioacchino Del Regno
2023-04-20 1:46 ` Walter Chang (張維哲)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).