drivers/clocksource/Kconfig | 2 +- drivers/clocksource/timer-mediatek.c | 43 ++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-)
From: Chun-Hung Wu <chun-hung.wu@mediatek.com>
This patch makes the timer-mediatek driver which can
register an always-on timer as tick_broadcast_device
on MediaTek SoCs become loadable module in GKI.
This patch depends on the previous patch.
https://lore.kernel.org/lkml/20230208094813.20874-1-walter.chang@mediatek.com/T/#t
Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
---
drivers/clocksource/Kconfig | 2 +-
drivers/clocksource/timer-mediatek.c | 43 ++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4469e7f555e9..41345827055b 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 d5b29fd03ca2..806044ef391c 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"
@@ -450,6 +453,46 @@ 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,
+ },
+ {
+ .compatible = "mediatek,mt6795-systimer",
+ .data = mtk_cpux_init,
+ },
+ {}
+};
+
+static struct platform_driver mtk_timer_driver = {
+ .probe = mtk_timer_probe,
+ .driver = {
+ .name = "mtk-timer",
+ .of_match_table = mtk_timer_match_table,
+ },
+};
+MODULE_DESCRIPTION("MEDIATEK Module Timer driver");
+MODULE_LICENSE("GPL v2");
+
+module_platform_driver(mtk_timer_driver);
+#else
TIMER_OF_DECLARE(mtk_mt6577, "mediatek,mt6577-timer", mtk_gpt_init);
TIMER_OF_DECLARE(mtk_mt6765, "mediatek,mt6765-timer", mtk_syst_init);
TIMER_OF_DECLARE(mtk_mt6795, "mediatek,mt6795-systimer", mtk_cpux_init);
+#endif
--
2.18.0
On 10/02/2023 11:00, walter.chang@mediatek.com wrote: > From: Chun-Hung Wu <chun-hung.wu@mediatek.com> > > This patch makes the timer-mediatek driver which can Do not use "This commit/patch". https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > register an always-on timer as tick_broadcast_device > on MediaTek SoCs become loadable module in GKI. Are you planning to answer other parts of that discussion? IOW, does the system boot fine? What's the impact of this being a module? > > This patch depends on the previous patch. > https://lore.kernel.org/lkml/20230208094813.20874-1-walter.chang@mediatek.com/T/#t This does not belong to commit msg. What's the point of keeping it in commit history forever? > > Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com> > --- > drivers/clocksource/Kconfig | 2 +- > drivers/clocksource/timer-mediatek.c | 43 ++++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+), 1 deletion(-) > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index 4469e7f555e9..41345827055b 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 d5b29fd03ca2..806044ef391c 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" > @@ -450,6 +453,46 @@ 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, > + }, > + { > + .compatible = "mediatek,mt6795-systimer", > + .data = mtk_cpux_init, > + }, > + {} > +}; > + > +static struct platform_driver mtk_timer_driver = { > + .probe = mtk_timer_probe, > + .driver = { > + .name = "mtk-timer", > + .of_match_table = mtk_timer_match_table, > + }, > +}; > +MODULE_DESCRIPTION("MEDIATEK Module Timer driver"); > +MODULE_LICENSE("GPL v2"); I don't think you run checkpatch before sending... please do not use humans for review which is done by automatic tools. > + > +module_platform_driver(mtk_timer_driver); Follow coding convention like in very other driver, so this goes immediately after definition of driver structure. > +#else Best regards, Krzysztof
On Fri, 2023-02-10 at 11:07 +0100, Krzysztof Kozlowski wrote: > On 10/02/2023 11:00, walter.chang@mediatek.com wrote: > > From: Chun-Hung Wu <chun-hung.wu@mediatek.com> > > > > This patch makes the timer-mediatek driver which can > > Do not use "This commit/patch". > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g1yigOO6iIpjlr82Ud0dn1yVP_1yqSfLWJ-1GFC7O88n7l7lrb9SlYAw5KHzA3339zyiV-s72Wn_OZrARjlaY0RMmdnOUyQ$ > > > > register an always-on timer as tick_broadcast_device > > on MediaTek SoCs become loadable module in GKI. > > Are you planning to answer other parts of that discussion? IOW, does > the > system boot fine? What's the impact of this being a module? > > > > > This patch depends on the previous patch. > > https://lore.kernel.org/lkml/20230208094813.20874-1-walter.chang@mediatek.com/T/#t > > This does not belong to commit msg. What's the point of keeping it in > commit history forever? > > > > > Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com> > > --- > > drivers/clocksource/Kconfig | 2 +- > > drivers/clocksource/timer-mediatek.c | 43 > > ++++++++++++++++++++++++++++ > > 2 files changed, 44 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clocksource/Kconfig > > b/drivers/clocksource/Kconfig > > index 4469e7f555e9..41345827055b 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 d5b29fd03ca2..806044ef391c 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" > > @@ -450,6 +453,46 @@ 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, > > + }, > > + { > > + .compatible = "mediatek,mt6795-systimer", > > + .data = mtk_cpux_init, > > + }, > > + {} > > +}; > > + > > +static struct platform_driver mtk_timer_driver = { > > + .probe = mtk_timer_probe, > > + .driver = { > > + .name = "mtk-timer", > > + .of_match_table = mtk_timer_match_table, > > + }, > > +}; > > +MODULE_DESCRIPTION("MEDIATEK Module Timer driver"); > > +MODULE_LICENSE("GPL v2"); > > I don't think you run checkpatch before sending... please do not use > humans for review which is done by automatic tools. > > > + > > +module_platform_driver(mtk_timer_driver); > > Follow coding convention like in very other driver, so this goes > immediately after definition of driver structure. > > > +#else > > > Best regards, > Krzysztof > Thanks for pointing out the mistake. I have fixed it and submitted patch v2, which merged the driver and export functions in the same patch. https://lore.kernel.org/lkml/20230214105412.5856-1-walter.chang@mediatek.com/T/#t Thanks, Walter Chang
On 14/02/2023 12:06, Walter Chang (張維哲) wrote: > On Fri, 2023-02-10 at 11:07 +0100, Krzysztof Kozlowski wrote: >> On 10/02/2023 11:00, walter.chang@mediatek.com wrote: >>> From: Chun-Hung Wu <chun-hung.wu@mediatek.com> >>> >>> This patch makes the timer-mediatek driver which can >> >> Do not use "This commit/patch". >> > https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;Iw!!CTRNKA9wMg0ARbw!g1yigOO6iIpjlr82Ud0dn1yVP_1yqSfLWJ-1GFC7O88n7l7lrb9SlYAw5KHzA3339zyiV-s72Wn_OZrARjlaY0RMmdnOUyQ$ >> >> >>> register an always-on timer as tick_broadcast_device >>> on MediaTek SoCs become loadable module in GKI. >> >> Are you planning to answer other parts of that discussion? IOW, does >> the >> system boot fine? What's the impact of this being a module? >> >>> >>> This patch depends on the previous patch. >>> > https://lore.kernel.org/lkml/20230208094813.20874-1-walter.chang@mediatek.com/T/#t >> >> This does not belong to commit msg. What's the point of keeping it in >> commit history forever? >> >>> >>> Signed-off-by: Chun-Hung Wu <chun-hung.wu@mediatek.com> >>> --- >>> drivers/clocksource/Kconfig | 2 +- >>> drivers/clocksource/timer-mediatek.c | 43 >>> ++++++++++++++++++++++++++++ >>> 2 files changed, 44 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/clocksource/Kconfig >>> b/drivers/clocksource/Kconfig >>> index 4469e7f555e9..41345827055b 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 d5b29fd03ca2..806044ef391c 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" >>> @@ -450,6 +453,46 @@ 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, >>> + }, >>> + { >>> + .compatible = "mediatek,mt6795-systimer", >>> + .data = mtk_cpux_init, >>> + }, >>> + {} >>> +}; >>> + >>> +static struct platform_driver mtk_timer_driver = { >>> + .probe = mtk_timer_probe, >>> + .driver = { >>> + .name = "mtk-timer", >>> + .of_match_table = mtk_timer_match_table, >>> + }, >>> +}; >>> +MODULE_DESCRIPTION("MEDIATEK Module Timer driver"); >>> +MODULE_LICENSE("GPL v2"); >> >> I don't think you run checkpatch before sending... please do not use >> humans for review which is done by automatic tools. >> >>> + >>> +module_platform_driver(mtk_timer_driver); >> >> Follow coding convention like in very other driver, so this goes >> immediately after definition of driver structure. >> >>> +#else >> >> >> Best regards, >> Krzysztof >> > > Thanks for pointing out the mistake. I have fixed it and submitted > patch v2, which merged the driver and export functions in the same > patch. > > > https://lore.kernel.org/lkml/20230214105412.5856-1-walter.chang@mediatek.com/T/#t My other questions are unanswered. Best regards, Krzysztof
© 2016 - 2024 Red Hat, Inc.