| Message ID | 20250331230034.806124-6-willmcvicker@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add module support for Arm64 Exynos MCT driver | expand |
On Mon, Mar 31, 2025 at 04:00:27PM -0700, Will McVicker wrote: > From: Donghoon Yu <hoony.yu@samsung.com> > > On Arm64 platforms the Exynos MCT driver can be built as a module. On > boot (and even after boot) the arch_timer is used as the clocksource and > tick timer. Once the MCT driver is loaded, it can be used as the wakeup > source for the arch_timer. > > Signed-off-by: Donghoon Yu <hoony.yu@samsung.com> > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com> > [Original commit from https://android.googlesource.com/kernel/gs/+/8a52a8288ec7d88ff78f0b37480dbb0e9c65bbfd] > Signed-off-by: Will McVicker <willmcvicker@google.com> > --- > drivers/clocksource/Kconfig | 3 +- > drivers/clocksource/exynos_mct.c | 47 +++++++++++++++++++++++++++----- > 2 files changed, 42 insertions(+), 8 deletions(-) [...] > +#ifdef MODULE > +static int exynos4_mct_probe(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + > + if (of_machine_is_compatible("samsung,exynos4412-mct")) Your root node compatible has "samsung,exynos4412-mct"!? In any case, add a data ptr to of_device_id table and then use the match data rather than comparing compatible strings again. > + return mct_init_ppi(np); > + > + return mct_init_spi(np); > +} > + > +static const struct of_device_id exynos4_mct_match_table[] = { > + { .compatible = "samsung,exynos4210-mct" }, > + { .compatible = "samsung,exynos4412-mct" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, exynos4_mct_match_table);
On 01/04/2025 01:00, Will McVicker wrote: > From: Donghoon Yu <hoony.yu@samsung.com> > > On Arm64 platforms the Exynos MCT driver can be built as a module. On > boot (and even after boot) the arch_timer is used as the clocksource and > tick timer. Once the MCT driver is loaded, it can be used as the wakeup > source for the arch_timer. > > Signed-off-by: Donghoon Yu <hoony.yu@samsung.com> > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com> > [Original commit from https://android.googlesource.com/kernel/gs/+/8a52a8288ec7d88ff78f0b37480dbb0e9c65bbfd] > Signed-off-by: Will McVicker <willmcvicker@google.com> > --- > drivers/clocksource/Kconfig | 3 +- > drivers/clocksource/exynos_mct.c | 47 +++++++++++++++++++++++++++----- > 2 files changed, 42 insertions(+), 8 deletions(-) > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > index 487c85259967..e5d9d8383607 100644 > --- a/drivers/clocksource/Kconfig > +++ b/drivers/clocksource/Kconfig > @@ -443,7 +443,8 @@ config ATMEL_TCB_CLKSRC > Support for Timer Counter Blocks on Atmel SoCs. > > config CLKSRC_EXYNOS_MCT > - bool "Exynos multi core timer driver" if COMPILE_TEST > + tristate "Exynos multi core timer driver" > + default y if ARCH_EXYNOS > depends on ARM || ARM64 > depends on ARCH_ARTPEC || ARCH_EXYNOS || COMPILE_TEST I am not sure if you actually tested it as module. On arm I cannot build it even: ERROR: modpost: "register_current_timer_delay" [drivers/clocksource/exynos_mct.ko] undefined! ERROR: modpost: "sched_clock_register" [drivers/clocksource/exynos_mct.ko] undefined! Best regards, Krzysztof
On 04/01/2025, Krzysztof Kozlowski wrote: > On 01/04/2025 01:00, Will McVicker wrote: > > From: Donghoon Yu <hoony.yu@samsung.com> > > > > On Arm64 platforms the Exynos MCT driver can be built as a module. On > > boot (and even after boot) the arch_timer is used as the clocksource and > > tick timer. Once the MCT driver is loaded, it can be used as the wakeup > > source for the arch_timer. > > > > Signed-off-by: Donghoon Yu <hoony.yu@samsung.com> > > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com> > > [Original commit from https://android.googlesource.com/kernel/gs/+/8a52a8288ec7d88ff78f0b37480dbb0e9c65bbfd] > > Signed-off-by: Will McVicker <willmcvicker@google.com> > > --- > > drivers/clocksource/Kconfig | 3 +- > > drivers/clocksource/exynos_mct.c | 47 +++++++++++++++++++++++++++----- > > 2 files changed, 42 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > index 487c85259967..e5d9d8383607 100644 > > --- a/drivers/clocksource/Kconfig > > +++ b/drivers/clocksource/Kconfig > > @@ -443,7 +443,8 @@ config ATMEL_TCB_CLKSRC > > Support for Timer Counter Blocks on Atmel SoCs. > > > > config CLKSRC_EXYNOS_MCT > > - bool "Exynos multi core timer driver" if COMPILE_TEST > > + tristate "Exynos multi core timer driver" > > + default y if ARCH_EXYNOS > > depends on ARM || ARM64 > > depends on ARCH_ARTPEC || ARCH_EXYNOS || COMPILE_TEST > I am not sure if you actually tested it as module. On arm I cannot build > it even: > > ERROR: modpost: "register_current_timer_delay" > [drivers/clocksource/exynos_mct.ko] undefined! > ERROR: modpost: "sched_clock_register" > [drivers/clocksource/exynos_mct.ko] undefined! I tested with the gs101 ARM64 configuration. You're right it won't work with ARM32. Thanks for catching this! Since ARM32 architectures don't have the arch_timer, I'm not sure if we can actually support Exynos MCT as a module as you wouldn't have any available clocksource during boot. I'll update the Kconfig for v2 to handle this and make sure it works for ARM32. I'm guessing it'll work with something like: config CLKSRC_EXYNOS_MCT tristate "Exynos multi core timer driver" if ARM64 Regards, Will [...]
On 03/31/2025, Rob Herring wrote: > On Mon, Mar 31, 2025 at 04:00:27PM -0700, Will McVicker wrote: > > From: Donghoon Yu <hoony.yu@samsung.com> > > > > On Arm64 platforms the Exynos MCT driver can be built as a module. On > > boot (and even after boot) the arch_timer is used as the clocksource and > > tick timer. Once the MCT driver is loaded, it can be used as the wakeup > > source for the arch_timer. > > > > Signed-off-by: Donghoon Yu <hoony.yu@samsung.com> > > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com> > > [Original commit from https://android.googlesource.com/kernel/gs/+/8a52a8288ec7d88ff78f0b37480dbb0e9c65bbfd] > > Signed-off-by: Will McVicker <willmcvicker@google.com> > > --- > > drivers/clocksource/Kconfig | 3 +- > > drivers/clocksource/exynos_mct.c | 47 +++++++++++++++++++++++++++----- > > 2 files changed, 42 insertions(+), 8 deletions(-) > > [...] > > > +#ifdef MODULE > > +static int exynos4_mct_probe(struct platform_device *pdev) > > +{ > > + struct device_node *np = pdev->dev.of_node; > > + > > + if (of_machine_is_compatible("samsung,exynos4412-mct")) > > Your root node compatible has "samsung,exynos4412-mct"!? > > In any case, add a data ptr to of_device_id table and then use the match > data rather than comparing compatible strings again. Ah yes, you're right. Thanks for the suggestion! I'll update on v2. Regards, Will [...]
On Tue, Apr 01, 2025 at 09:27:09AM -0700, William McVicker wrote: > On 04/01/2025, Krzysztof Kozlowski wrote: > > On 01/04/2025 01:00, Will McVicker wrote: > > > From: Donghoon Yu <hoony.yu@samsung.com> > > > > > > On Arm64 platforms the Exynos MCT driver can be built as a module. On > > > boot (and even after boot) the arch_timer is used as the clocksource and > > > tick timer. Once the MCT driver is loaded, it can be used as the wakeup > > > source for the arch_timer. > > > > > > Signed-off-by: Donghoon Yu <hoony.yu@samsung.com> > > > Signed-off-by: Youngmin Nam <youngmin.nam@samsung.com> > > > [Original commit from https://android.googlesource.com/kernel/gs/+/8a52a8288ec7d88ff78f0b37480dbb0e9c65bbfd] > > > Signed-off-by: Will McVicker <willmcvicker@google.com> > > > --- > > > drivers/clocksource/Kconfig | 3 +- > > > drivers/clocksource/exynos_mct.c | 47 +++++++++++++++++++++++++++----- > > > 2 files changed, 42 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig > > > index 487c85259967..e5d9d8383607 100644 > > > --- a/drivers/clocksource/Kconfig > > > +++ b/drivers/clocksource/Kconfig > > > @@ -443,7 +443,8 @@ config ATMEL_TCB_CLKSRC > > > Support for Timer Counter Blocks on Atmel SoCs. > > > > > > config CLKSRC_EXYNOS_MCT > > > - bool "Exynos multi core timer driver" if COMPILE_TEST > > > + tristate "Exynos multi core timer driver" > > > + default y if ARCH_EXYNOS > > > depends on ARM || ARM64 > > > depends on ARCH_ARTPEC || ARCH_EXYNOS || COMPILE_TEST > > I am not sure if you actually tested it as module. On arm I cannot build > > it even: > > > > ERROR: modpost: "register_current_timer_delay" > > [drivers/clocksource/exynos_mct.ko] undefined! > > ERROR: modpost: "sched_clock_register" > > [drivers/clocksource/exynos_mct.ko] undefined! > > I tested with the gs101 ARM64 configuration. You're right it won't work with > ARM32. Thanks for catching this! Since ARM32 architectures don't have the > arch_timer, I'm not sure if we can actually support Exynos MCT as a module as > you wouldn't have any available clocksource during boot. I'll update the > Kconfig for v2 to handle this and make sure it works for ARM32. I'm guessing > it'll work with something like: > > config CLKSRC_EXYNOS_MCT > tristate "Exynos multi core timer driver" if ARM64 > > > Regards, > Will > > [...] > Thanks for working on upstreaming the MCT driver modularization. I'll take another look at your v2 patch Thanks. Youngmin
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 487c85259967..e5d9d8383607 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -443,7 +443,8 @@ config ATMEL_TCB_CLKSRC Support for Timer Counter Blocks on Atmel SoCs. config CLKSRC_EXYNOS_MCT - bool "Exynos multi core timer driver" if COMPILE_TEST + tristate "Exynos multi core timer driver" + default y if ARCH_EXYNOS depends on ARM || ARM64 depends on ARCH_ARTPEC || ARCH_EXYNOS || COMPILE_TEST help diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c index 21ded37137d7..da4460d8a0ba 100644 --- a/drivers/clocksource/exynos_mct.c +++ b/drivers/clocksource/exynos_mct.c @@ -15,9 +15,11 @@ #include <linux/cpu.h> #include <linux/delay.h> #include <linux/percpu.h> +#include <linux/module.h> #include <linux/of.h> #include <linux/of_irq.h> #include <linux/of_address.h> +#include <linux/platform_device.h> #include <linux/clocksource.h> #include <linux/sched_clock.h> @@ -235,7 +237,7 @@ static cycles_t exynos4_read_current_timer(void) } #endif -static int __init exynos4_clocksource_init(bool frc_shared) +static int exynos4_clocksource_init(bool frc_shared) { /* * When the frc is shared, the main processor should have already @@ -507,7 +509,7 @@ static int exynos4_mct_dying_cpu(unsigned int cpu) return 0; } -static int __init exynos4_timer_resources(struct device_node *np) +static int exynos4_timer_resources(struct device_node *np) { struct clk *mct_clk, *tick_clk; @@ -535,7 +537,7 @@ static int __init exynos4_timer_resources(struct device_node *np) * @local_idx: array mapping CPU numbers to local timer indices * @nr_local: size of @local_idx array */ -static int __init exynos4_timer_interrupts(struct device_node *np, +static int exynos4_timer_interrupts(struct device_node *np, unsigned int int_type, const u32 *local_idx, size_t nr_local) @@ -640,7 +642,7 @@ static int __init exynos4_timer_interrupts(struct device_node *np, return err; } -static int __init mct_init_dt(struct device_node *np, unsigned int int_type) +static int mct_init_dt(struct device_node *np, unsigned int int_type) { bool frc_shared = of_property_read_bool(np, "samsung,frc-shared"); u32 local_idx[MCT_NR_LOCAL] = {0}; @@ -688,15 +690,46 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type) return exynos4_clockevent_init(); } - -static int __init mct_init_spi(struct device_node *np) +static int mct_init_spi(struct device_node *np) { return mct_init_dt(np, MCT_INT_SPI); } -static int __init mct_init_ppi(struct device_node *np) +static int mct_init_ppi(struct device_node *np) { return mct_init_dt(np, MCT_INT_PPI); } + +#ifdef MODULE +static int exynos4_mct_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + + if (of_machine_is_compatible("samsung,exynos4412-mct")) + return mct_init_ppi(np); + + return mct_init_spi(np); +} + +static const struct of_device_id exynos4_mct_match_table[] = { + { .compatible = "samsung,exynos4210-mct" }, + { .compatible = "samsung,exynos4412-mct" }, + {} +}; +MODULE_DEVICE_TABLE(of, exynos4_mct_match_table); + +static struct platform_driver exynos4_mct_driver = { + .probe = exynos4_mct_probe, + .driver = { + .name = "exynos-mct", + .of_match_table = exynos4_mct_match_table, + }, +}; +module_platform_driver(exynos4_mct_driver); +#else TIMER_OF_DECLARE(exynos4210, "samsung,exynos4210-mct", mct_init_spi); TIMER_OF_DECLARE(exynos4412, "samsung,exynos4412-mct", mct_init_ppi); +#endif + +MODULE_DESCRIPTION("Exynos Multi Core Timer Driver"); +MODULE_LICENSE("GPL");