Message ID | 20230210100058.19861-1-walter.chang@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module | expand |
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
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