Messages in this thread | | | Date | Fri, 10 Feb 2023 11:07:58 +0100 | Subject | Re: [PATCH] clocksource/drivers/timer-mediatek: Make timer-mediatek become loadable module | From | Krzysztof Kozlowski <> |
| |
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
| |