| Message ID | 20180403061836.3926-9-mylene.josserand@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | Sunxi: Add SMP support on A83T | expand |
On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote: > Add the initialization of CNTVOFF for sun8i-a83t. > > For boot CPU, Create a new machine that handles this > function's call in an "init_early" callback. > For secondary CPUs, add this function into secondary_startup > assembly entry. > > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com> > --- > arch/arm/mach-sunxi/headsmp.S | 1 + > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++- > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S > index 79890fbe5613..b586b7cf803a 100644 > --- a/arch/arm/mach-sunxi/headsmp.S > +++ b/arch/arm/mach-sunxi/headsmp.S > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable) > > ENTRY(sunxi_mc_smp_secondary_startup) > bl sunxi_mc_smp_cluster_cache_enable > + bl smp_init_cntvoff > b secondary_startup > ENDPROC(sunxi_mc_smp_secondary_startup) > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > index 5e9602ce1573..090784108c0a 100644 > --- a/arch/arm/mach-sunxi/sunxi.c > +++ b/arch/arm/mach-sunxi/sunxi.c > @@ -16,6 +16,7 @@ > #include <linux/platform_device.h> > > #include <asm/mach/arch.h> > +#include <asm/smp_cntvoff.h> > > static const char * const sunxi_board_dt_compat[] = { > "allwinner,sun4i-a10", > @@ -62,7 +63,6 @@ MACHINE_END > static const char * const sun8i_board_dt_compat[] = { > "allwinner,sun8i-a23", > "allwinner,sun8i-a33", > - "allwinner,sun8i-a83t", > "allwinner,sun8i-h2-plus", > "allwinner,sun8i-h3", > "allwinner,sun8i-r40", > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family") > .dt_compat = sun8i_board_dt_compat, > MACHINE_END > > +void __init sun8i_cntvoff_init(void) > +{ > + smp_init_cntvoff(); Can't this be moved to the SMP setup code? > +} > + > +static const char * const sun8i_cntvoff_board_dt_compat[] = { > + "allwinner,sun8i-a83t", > + NULL, > +}; > + > +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff") All of the SoCs need CNTVOFF, so that doesn't really make sense. Why not just calling it for what it is: an A83t? Maxime
Hi Mylène,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on arm-soc/for-next]
[also build test ERROR on next-20180403]
[cannot apply to v4.16]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Myl-ne-Josserand/Sunxi-Add-SMP-support-on-A83T/20180403-143751
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git for-next
config: arm-arm67 (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
arch/arm/mach-sunxi/sunxi.o: In function `sun8i_cntvoff_init':
>> sunxi.c:(.init.text+0x3c): undefined reference to `smp_init_cntvoff'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hello, Thank you for the review. On Tue, 3 Apr 2018 11:12:18 +0200 Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote: > > Add the initialization of CNTVOFF for sun8i-a83t. > > > > For boot CPU, Create a new machine that handles this > > function's call in an "init_early" callback. > > For secondary CPUs, add this function into secondary_startup > > assembly entry. > > > > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com> > > --- > > arch/arm/mach-sunxi/headsmp.S | 1 + > > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++- > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S > > index 79890fbe5613..b586b7cf803a 100644 > > --- a/arch/arm/mach-sunxi/headsmp.S > > +++ b/arch/arm/mach-sunxi/headsmp.S > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable) > > > > ENTRY(sunxi_mc_smp_secondary_startup) > > bl sunxi_mc_smp_cluster_cache_enable > > + bl smp_init_cntvoff > > b secondary_startup > > ENDPROC(sunxi_mc_smp_secondary_startup) > > > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > > index 5e9602ce1573..090784108c0a 100644 > > --- a/arch/arm/mach-sunxi/sunxi.c > > +++ b/arch/arm/mach-sunxi/sunxi.c > > @@ -16,6 +16,7 @@ > > #include <linux/platform_device.h> > > > > #include <asm/mach/arch.h> > > +#include <asm/smp_cntvoff.h> > > > > static const char * const sunxi_board_dt_compat[] = { > > "allwinner,sun4i-a10", > > @@ -62,7 +63,6 @@ MACHINE_END > > static const char * const sun8i_board_dt_compat[] = { > > "allwinner,sun8i-a23", > > "allwinner,sun8i-a33", > > - "allwinner,sun8i-a83t", > > "allwinner,sun8i-h2-plus", > > "allwinner,sun8i-h3", > > "allwinner,sun8i-r40", > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family") > > .dt_compat = sun8i_board_dt_compat, > > MACHINE_END > > > > +void __init sun8i_cntvoff_init(void) > > +{ > > + smp_init_cntvoff(); > > Can't this be moved to the SMP setup code? I tried to put it in the first lines of "sunxi_mc_smp_init" function but it did not work. I tried to find some callbacks to have an early "init" and I only found the "init_early"'s one. There is probably another way to handle that so do not hesitate to tell me any ideas. > > > +} > > + > > +static const char * const sun8i_cntvoff_board_dt_compat[] = { > > + "allwinner,sun8i-a83t", > > + NULL, > > +}; > > + > > +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff") > > All of the SoCs need CNTVOFF, so that doesn't really make sense. Why > not just calling it for what it is: an A83t? Sure, I will update it. Best regards,
On Tue, Apr 03, 2018 at 10:06:28PM +0200, Mylène Josserand wrote: > Hello, > > Thank you for the review. > > On Tue, 3 Apr 2018 11:12:18 +0200 > Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote: > > > Add the initialization of CNTVOFF for sun8i-a83t. > > > > > > For boot CPU, Create a new machine that handles this > > > function's call in an "init_early" callback. > > > For secondary CPUs, add this function into secondary_startup > > > assembly entry. > > > > > > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com> > > > --- > > > arch/arm/mach-sunxi/headsmp.S | 1 + > > > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++- > > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S > > > index 79890fbe5613..b586b7cf803a 100644 > > > --- a/arch/arm/mach-sunxi/headsmp.S > > > +++ b/arch/arm/mach-sunxi/headsmp.S > > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable) > > > > > > ENTRY(sunxi_mc_smp_secondary_startup) > > > bl sunxi_mc_smp_cluster_cache_enable > > > + bl smp_init_cntvoff > > > b secondary_startup > > > ENDPROC(sunxi_mc_smp_secondary_startup) > > > > > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > > > index 5e9602ce1573..090784108c0a 100644 > > > --- a/arch/arm/mach-sunxi/sunxi.c > > > +++ b/arch/arm/mach-sunxi/sunxi.c > > > @@ -16,6 +16,7 @@ > > > #include <linux/platform_device.h> > > > > > > #include <asm/mach/arch.h> > > > +#include <asm/smp_cntvoff.h> > > > > > > static const char * const sunxi_board_dt_compat[] = { > > > "allwinner,sun4i-a10", > > > @@ -62,7 +63,6 @@ MACHINE_END > > > static const char * const sun8i_board_dt_compat[] = { > > > "allwinner,sun8i-a23", > > > "allwinner,sun8i-a33", > > > - "allwinner,sun8i-a83t", > > > "allwinner,sun8i-h2-plus", > > > "allwinner,sun8i-h3", > > > "allwinner,sun8i-r40", > > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family") > > > .dt_compat = sun8i_board_dt_compat, > > > MACHINE_END > > > > > > +void __init sun8i_cntvoff_init(void) > > > +{ > > > + smp_init_cntvoff(); > > > > Can't this be moved to the SMP setup code? > > I tried to put it in the first lines of "sunxi_mc_smp_init" function > but it did not work. I tried to find some callbacks to have an > early "init" and I only found the "init_early"'s one. There is probably > another way to handle that so do not hesitate to tell me any ideas. It's hard to say without more context about why it doesn't work. Have you checked the order between early_initcall, the timer initialization function and init_early? > > > +} > > > + > > > +static const char * const sun8i_cntvoff_board_dt_compat[] = { > > > + "allwinner,sun8i-a83t", > > > + NULL, > > > +}; > > > + > > > +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff") > > > > All of the SoCs need CNTVOFF, so that doesn't really make sense. Why > > not just calling it for what it is: an A83t? > > Sure, I will update it. Looking back at that code, I guess you want to change also the smp_init_cntvoff function. It's not really related to SMP either. Maxime
Hello Maxime, On Wed, 4 Apr 2018 09:45:15 +0200 Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Tue, Apr 03, 2018 at 10:06:28PM +0200, Mylène Josserand wrote: > > Hello, > > > > Thank you for the review. > > > > On Tue, 3 Apr 2018 11:12:18 +0200 > > Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote: > > > > Add the initialization of CNTVOFF for sun8i-a83t. > > > > > > > > For boot CPU, Create a new machine that handles this > > > > function's call in an "init_early" callback. > > > > For secondary CPUs, add this function into secondary_startup > > > > assembly entry. > > > > > > > > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com> > > > > --- > > > > arch/arm/mach-sunxi/headsmp.S | 1 + > > > > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++- > > > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S > > > > index 79890fbe5613..b586b7cf803a 100644 > > > > --- a/arch/arm/mach-sunxi/headsmp.S > > > > +++ b/arch/arm/mach-sunxi/headsmp.S > > > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable) > > > > > > > > ENTRY(sunxi_mc_smp_secondary_startup) > > > > bl sunxi_mc_smp_cluster_cache_enable > > > > + bl smp_init_cntvoff > > > > b secondary_startup > > > > ENDPROC(sunxi_mc_smp_secondary_startup) > > > > > > > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > > > > index 5e9602ce1573..090784108c0a 100644 > > > > --- a/arch/arm/mach-sunxi/sunxi.c > > > > +++ b/arch/arm/mach-sunxi/sunxi.c > > > > @@ -16,6 +16,7 @@ > > > > #include <linux/platform_device.h> > > > > > > > > #include <asm/mach/arch.h> > > > > +#include <asm/smp_cntvoff.h> > > > > > > > > static const char * const sunxi_board_dt_compat[] = { > > > > "allwinner,sun4i-a10", > > > > @@ -62,7 +63,6 @@ MACHINE_END > > > > static const char * const sun8i_board_dt_compat[] = { > > > > "allwinner,sun8i-a23", > > > > "allwinner,sun8i-a33", > > > > - "allwinner,sun8i-a83t", > > > > "allwinner,sun8i-h2-plus", > > > > "allwinner,sun8i-h3", > > > > "allwinner,sun8i-r40", > > > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family") > > > > .dt_compat = sun8i_board_dt_compat, > > > > MACHINE_END > > > > > > > > +void __init sun8i_cntvoff_init(void) > > > > +{ > > > > + smp_init_cntvoff(); > > > > > > Can't this be moved to the SMP setup code? > > > > I tried to put it in the first lines of "sunxi_mc_smp_init" function > > but it did not work. I tried to find some callbacks to have an > > early "init" and I only found the "init_early"'s one. There is probably > > another way to handle that so do not hesitate to tell me any ideas. > > It's hard to say without more context about why it doesn't work. Have > you checked the order between early_initcall, the timer initialization > function and init_early? > Yes, I tested it. I wanted to test it again to give more context. Here is the boot log: http://code.bulix.org/n1x864-315948?raw I added printk in the 3 functions (with "===========>"). The "init_early" and "sun6i_timer" are executed before "arch_timer_of_init" (which is the function that parses the DT property that we used previously "arm,cpu-registers-not-fw-configured"). The "sunxi_mc_smp_init" function is called later so I guess that is why it is not working in that case. > > > > +} > > > > + > > > > +static const char * const sun8i_cntvoff_board_dt_compat[] = { > > > > + "allwinner,sun8i-a83t", > > > > + NULL, > > > > +}; > > > > + > > > > +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff") > > > > > > All of the SoCs need CNTVOFF, so that doesn't really make sense. Why > > > not just calling it for what it is: an A83t? > > > > Sure, I will update it. > > Looking back at that code, I guess you want to change also the > smp_init_cntvoff function. It's not really related to SMP either. Yep, I will update it in next version. Thanks, Best regards,
On Sun, Apr 08, 2018 at 11:09:32AM +0200, Mylène Josserand wrote: > Hello Maxime, > > On Wed, 4 Apr 2018 09:45:15 +0200 > Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > On Tue, Apr 03, 2018 at 10:06:28PM +0200, Mylène Josserand wrote: > > > Hello, > > > > > > Thank you for the review. > > > > > > On Tue, 3 Apr 2018 11:12:18 +0200 > > > Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > > > > On Tue, Apr 03, 2018 at 08:18:31AM +0200, Mylène Josserand wrote: > > > > > Add the initialization of CNTVOFF for sun8i-a83t. > > > > > > > > > > For boot CPU, Create a new machine that handles this > > > > > function's call in an "init_early" callback. > > > > > For secondary CPUs, add this function into secondary_startup > > > > > assembly entry. > > > > > > > > > > Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com> > > > > > --- > > > > > arch/arm/mach-sunxi/headsmp.S | 1 + > > > > > arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++- > > > > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S > > > > > index 79890fbe5613..b586b7cf803a 100644 > > > > > --- a/arch/arm/mach-sunxi/headsmp.S > > > > > +++ b/arch/arm/mach-sunxi/headsmp.S > > > > > @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable) > > > > > > > > > > ENTRY(sunxi_mc_smp_secondary_startup) > > > > > bl sunxi_mc_smp_cluster_cache_enable > > > > > + bl smp_init_cntvoff > > > > > b secondary_startup > > > > > ENDPROC(sunxi_mc_smp_secondary_startup) > > > > > > > > > > diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c > > > > > index 5e9602ce1573..090784108c0a 100644 > > > > > --- a/arch/arm/mach-sunxi/sunxi.c > > > > > +++ b/arch/arm/mach-sunxi/sunxi.c > > > > > @@ -16,6 +16,7 @@ > > > > > #include <linux/platform_device.h> > > > > > > > > > > #include <asm/mach/arch.h> > > > > > +#include <asm/smp_cntvoff.h> > > > > > > > > > > static const char * const sunxi_board_dt_compat[] = { > > > > > "allwinner,sun4i-a10", > > > > > @@ -62,7 +63,6 @@ MACHINE_END > > > > > static const char * const sun8i_board_dt_compat[] = { > > > > > "allwinner,sun8i-a23", > > > > > "allwinner,sun8i-a33", > > > > > - "allwinner,sun8i-a83t", > > > > > "allwinner,sun8i-h2-plus", > > > > > "allwinner,sun8i-h3", > > > > > "allwinner,sun8i-r40", > > > > > @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family") > > > > > .dt_compat = sun8i_board_dt_compat, > > > > > MACHINE_END > > > > > > > > > > +void __init sun8i_cntvoff_init(void) > > > > > +{ > > > > > + smp_init_cntvoff(); > > > > > > > > Can't this be moved to the SMP setup code? > > > > > > I tried to put it in the first lines of "sunxi_mc_smp_init" function > > > but it did not work. I tried to find some callbacks to have an > > > early "init" and I only found the "init_early"'s one. There is probably > > > another way to handle that so do not hesitate to tell me any ideas. > > > > It's hard to say without more context about why it doesn't work. Have > > you checked the order between early_initcall, the timer initialization > > function and init_early? > > > > Yes, I tested it. I wanted to test it again to give more context. Here > is the boot log: http://code.bulix.org/n1x864-315948?raw > I added printk in the 3 functions (with "===========>"). > > The "init_early" and "sun6i_timer" are executed before > "arch_timer_of_init" (which is the function that parses the DT property > that we used previously "arm,cpu-registers-not-fw-configured"). > The "sunxi_mc_smp_init" function is called later so I guess that is why > it is not working in that case. Ok. Make sure to mention that in your commit log. Maxime
diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S index 79890fbe5613..b586b7cf803a 100644 --- a/arch/arm/mach-sunxi/headsmp.S +++ b/arch/arm/mach-sunxi/headsmp.S @@ -71,6 +71,7 @@ ENDPROC(sunxi_mc_smp_cluster_cache_enable) ENTRY(sunxi_mc_smp_secondary_startup) bl sunxi_mc_smp_cluster_cache_enable + bl smp_init_cntvoff b secondary_startup ENDPROC(sunxi_mc_smp_secondary_startup) diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c index 5e9602ce1573..090784108c0a 100644 --- a/arch/arm/mach-sunxi/sunxi.c +++ b/arch/arm/mach-sunxi/sunxi.c @@ -16,6 +16,7 @@ #include <linux/platform_device.h> #include <asm/mach/arch.h> +#include <asm/smp_cntvoff.h> static const char * const sunxi_board_dt_compat[] = { "allwinner,sun4i-a10", @@ -62,7 +63,6 @@ MACHINE_END static const char * const sun8i_board_dt_compat[] = { "allwinner,sun8i-a23", "allwinner,sun8i-a33", - "allwinner,sun8i-a83t", "allwinner,sun8i-h2-plus", "allwinner,sun8i-h3", "allwinner,sun8i-r40", @@ -75,6 +75,22 @@ DT_MACHINE_START(SUN8I_DT, "Allwinner sun8i Family") .dt_compat = sun8i_board_dt_compat, MACHINE_END +void __init sun8i_cntvoff_init(void) +{ + smp_init_cntvoff(); +} + +static const char * const sun8i_cntvoff_board_dt_compat[] = { + "allwinner,sun8i-a83t", + NULL, +}; + +DT_MACHINE_START(SUN8I_CNTVOFF_DT, "Allwinner sun8i boards needing cntvoff") + .init_early = sun8i_cntvoff_init, + .init_time = sun6i_timer_init, + .dt_compat = sun8i_cntvoff_board_dt_compat, +MACHINE_END + static const char * const sun9i_board_dt_compat[] = { "allwinner,sun9i-a80", NULL,
Add the initialization of CNTVOFF for sun8i-a83t. For boot CPU, Create a new machine that handles this function's call in an "init_early" callback. For secondary CPUs, add this function into secondary_startup assembly entry. Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com> --- arch/arm/mach-sunxi/headsmp.S | 1 + arch/arm/mach-sunxi/sunxi.c | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-)