| Message ID | 1423770163-583064-2-git-send-email-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 12 Feb 2015, Arnd Bergmann wrote: > The vexpress tc2 power management code calls mcpm_loopback, which > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > get a link error: > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > This explicitly selects ARM_CPU_SUSPEND like other platforms that > need it. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 3592d7e002438 ("ARM: 8082/1: TC2: test the MCPM loopback during boot") > Cc: Nicolas Pitre <nico@linaro.org> > Cc: Kevin Hilman <khilman@linaro.org> > Cc: Liviu Dudau <liviu.dudau@arm.com> > Cc: Sudeep Holla <sudeep.holla@arm.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Acked-by: Nicolas Pitre <nico@linaro.org> > --- > arch/arm/mach-vexpress/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig > index d6b16d9a7838..3c2509b4b694 100644 > --- a/arch/arm/mach-vexpress/Kconfig > +++ b/arch/arm/mach-vexpress/Kconfig > @@ -73,6 +73,7 @@ config ARCH_VEXPRESS_TC2_PM > depends on MCPM > select ARM_CCI > select ARCH_VEXPRESS_SPC > + select ARM_CPU_SUSPEND > help > Support for CPU and cluster power management on Versatile Express > with a TC2 (A15x2 A7x3) big.LITTLE core tile. > -- > 2.1.0.rc2 > >
On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > The vexpress tc2 power management code calls mcpm_loopback, which > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > get a link error: > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' Arnd, We are having reports of TC2 not booting with MCPM enabled. Lorenzo was investigating this but then headed for Linaro Connect this week, so we don't have yet a resolution. Can this patch be postponed for a later -rc? Best regards, Liviu > > This explicitly selects ARM_CPU_SUSPEND like other platforms that > need it. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Fixes: 3592d7e002438 ("ARM: 8082/1: TC2: test the MCPM loopback during boot") > Cc: Nicolas Pitre <nico@linaro.org> > Cc: Kevin Hilman <khilman@linaro.org> > Cc: Liviu Dudau <liviu.dudau@arm.com> > Cc: Sudeep Holla <sudeep.holla@arm.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > --- > arch/arm/mach-vexpress/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig > index d6b16d9a7838..3c2509b4b694 100644 > --- a/arch/arm/mach-vexpress/Kconfig > +++ b/arch/arm/mach-vexpress/Kconfig > @@ -73,6 +73,7 @@ config ARCH_VEXPRESS_TC2_PM > depends on MCPM > select ARM_CCI > select ARCH_VEXPRESS_SPC > + select ARM_CPU_SUSPEND > help > Support for CPU and cluster power management on Versatile Express > with a TC2 (A15x2 A7x3) big.LITTLE core tile. > -- > 2.1.0.rc2 > >
On Fri, Feb 13, 2015 at 10:37:16AM +0000, Liviu Dudau wrote: > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > The vexpress tc2 power management code calls mcpm_loopback, which > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > get a link error: > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > Arnd, > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > was investigating this but then headed for Linaro Connect this week, > so we don't have yet a resolution. Can this patch be postponed for > a later -rc? Why? This patch isn't forcing MCPM to be enabled for Versatile Express. * In order to build tc2_pm.c, MCPM must already be enabled (the feature depends on MCPM in the Kconfig already) * When this feature is enabled, the above link error occurs if ARM_CPU_SUSPEND is not also enabled. So, it is merely fixing the requirement that tc2_pm.c needs the CPU suspend infrastructure, which in turn has no effect on whether MCPM is enabled or not.
On Fri, Feb 13, 2015 at 10:57:53AM +0000, Russell King - ARM Linux wrote: > On Fri, Feb 13, 2015 at 10:37:16AM +0000, Liviu Dudau wrote: > > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > > The vexpress tc2 power management code calls mcpm_loopback, which > > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > > get a link error: > > > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > > > Arnd, > > > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > > was investigating this but then headed for Linaro Connect this week, > > so we don't have yet a resolution. Can this patch be postponed for > > a later -rc? > > Why? > > This patch isn't forcing MCPM to be enabled for Versatile Express. > > * In order to build tc2_pm.c, MCPM must already be enabled (the > feature depends on MCPM in the Kconfig already) > * When this feature is enabled, the above link error occurs if > ARM_CPU_SUSPEND is not also enabled. > > So, it is merely fixing the requirement that tc2_pm.c needs the CPU > suspend infrastructure, which in turn has no effect on whether MCPM > is enabled or not. Because I think Lorenzo's plan was to disable MCPM for TC2. Which would make this patch moot. Best regards, Liviu > > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net. >
On Fri, Feb 13, 2015 at 11:01:52AM +0000, Liviu Dudau wrote: > On Fri, Feb 13, 2015 at 10:57:53AM +0000, Russell King - ARM Linux wrote: > > On Fri, Feb 13, 2015 at 10:37:16AM +0000, Liviu Dudau wrote: > > > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > > > The vexpress tc2 power management code calls mcpm_loopback, which > > > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > > > get a link error: > > > > > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > > > > > Arnd, > > > > > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > > > was investigating this but then headed for Linaro Connect this week, > > > so we don't have yet a resolution. Can this patch be postponed for > > > a later -rc? > > > > Why? > > > > This patch isn't forcing MCPM to be enabled for Versatile Express. > > > > * In order to build tc2_pm.c, MCPM must already be enabled (the > > feature depends on MCPM in the Kconfig already) > > * When this feature is enabled, the above link error occurs if > > ARM_CPU_SUSPEND is not also enabled. > > > > So, it is merely fixing the requirement that tc2_pm.c needs the CPU > > suspend infrastructure, which in turn has no effect on whether MCPM > > is enabled or not. > > Because I think Lorenzo's plan was to disable MCPM for TC2. Which would > make this patch moot. I don't think that makes a difference - the two issues are orthogonal. As the code stands today, it requires ARM_CPU_SUSPEND to be set, so merging this change makes total sense. If the code is temporarily disabled, then we still need this patch merged to fix the error when the feature is re-enabled. If the code is changed later such that it doesn't need mcpm_loopback, then that's the time to remove the select of this symbol. If the code is removed, that is something which is not going to happen during this merge window, so the patch still might as well be applied to prevent build errors.
On Fri, Feb 13, 2015 at 11:16:12AM +0000, Russell King - ARM Linux wrote: > On Fri, Feb 13, 2015 at 11:01:52AM +0000, Liviu Dudau wrote: > > On Fri, Feb 13, 2015 at 10:57:53AM +0000, Russell King - ARM Linux wrote: > > > On Fri, Feb 13, 2015 at 10:37:16AM +0000, Liviu Dudau wrote: > > > > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > > > > The vexpress tc2 power management code calls mcpm_loopback, which > > > > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > > > > get a link error: > > > > > > > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > > > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > > > > > > > Arnd, > > > > > > > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > > > > was investigating this but then headed for Linaro Connect this week, > > > > so we don't have yet a resolution. Can this patch be postponed for > > > > a later -rc? > > > > > > Why? > > > > > > This patch isn't forcing MCPM to be enabled for Versatile Express. > > > > > > * In order to build tc2_pm.c, MCPM must already be enabled (the > > > feature depends on MCPM in the Kconfig already) > > > * When this feature is enabled, the above link error occurs if > > > ARM_CPU_SUSPEND is not also enabled. > > > > > > So, it is merely fixing the requirement that tc2_pm.c needs the CPU > > > suspend infrastructure, which in turn has no effect on whether MCPM > > > is enabled or not. > > > > Because I think Lorenzo's plan was to disable MCPM for TC2. Which would > > make this patch moot. > > I don't think that makes a difference - the two issues are orthogonal. > > As the code stands today, it requires ARM_CPU_SUSPEND to be set, so > merging this change makes total sense. > > If the code is temporarily disabled, then we still need this patch > merged to fix the error when the feature is re-enabled. If the code > is changed later such that it doesn't need mcpm_loopback, then that's > the time to remove the select of this symbol. > > If the code is removed, that is something which is not going to happen > during this merge window, so the patch still might as well be applied > to prevent build errors. Russell, I'm not arguing either way, I was just asking for a delay until Lorenzo (one other maintainer of the code that is in the Cc) has a chance to have his say, because I thought it was relevant (and I have explained why). If Arnd feels he would rather push the patch now rather than (let say) late Monday, then I can give my Acked-by. Best regards, Liviu > > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net. >
On Fri, 13 Feb 2015, Liviu Dudau wrote: > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > The vexpress tc2 power management code calls mcpm_loopback, which > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > get a link error: > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > Arnd, > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > was investigating this but then headed for Linaro Connect this week, > so we don't have yet a resolution. Can this patch be postponed for > a later -rc? Can you share those reports? I did boot my TC2 with v3.19-rc7 before leaving for Linaro Connect and it just worked. Regardless, this patch is about a build fix and doesn't change any functionality, so I wouldn't postpone it. > > Best regards, > Liviu > > > > > > This explicitly selects ARM_CPU_SUSPEND like other platforms that > > need it. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Fixes: 3592d7e002438 ("ARM: 8082/1: TC2: test the MCPM loopback during boot") > > Cc: Nicolas Pitre <nico@linaro.org> > > Cc: Kevin Hilman <khilman@linaro.org> > > Cc: Liviu Dudau <liviu.dudau@arm.com> > > Cc: Sudeep Holla <sudeep.holla@arm.com> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > --- > > arch/arm/mach-vexpress/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig > > index d6b16d9a7838..3c2509b4b694 100644 > > --- a/arch/arm/mach-vexpress/Kconfig > > +++ b/arch/arm/mach-vexpress/Kconfig > > @@ -73,6 +73,7 @@ config ARCH_VEXPRESS_TC2_PM > > depends on MCPM > > select ARM_CCI > > select ARCH_VEXPRESS_SPC > > + select ARM_CPU_SUSPEND > > help > > Support for CPU and cluster power management on Versatile Express > > with a TC2 (A15x2 A7x3) big.LITTLE core tile. > > -- > > 2.1.0.rc2 > > > > > > -- > ==================== > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --------------- > ¯\_(?)_/¯ > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > >
On Fri, 13 Feb 2015, Liviu Dudau wrote: > On Fri, Feb 13, 2015 at 11:16:12AM +0000, Russell King - ARM Linux wrote: > > On Fri, Feb 13, 2015 at 11:01:52AM +0000, Liviu Dudau wrote: > > > On Fri, Feb 13, 2015 at 10:57:53AM +0000, Russell King - ARM Linux wrote: > > > > On Fri, Feb 13, 2015 at 10:37:16AM +0000, Liviu Dudau wrote: > > > > > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > > > > > The vexpress tc2 power management code calls mcpm_loopback, which > > > > > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > > > > > get a link error: > > > > > > > > > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > > > > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > > > > > > > > > Arnd, > > > > > > > > > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > > > > > was investigating this but then headed for Linaro Connect this week, > > > > > so we don't have yet a resolution. Can this patch be postponed for > > > > > a later -rc? > > > > > > > > Why? > > > > > > > > This patch isn't forcing MCPM to be enabled for Versatile Express. > > > > > > > > * In order to build tc2_pm.c, MCPM must already be enabled (the > > > > feature depends on MCPM in the Kconfig already) > > > > * When this feature is enabled, the above link error occurs if > > > > ARM_CPU_SUSPEND is not also enabled. > > > > > > > > So, it is merely fixing the requirement that tc2_pm.c needs the CPU > > > > suspend infrastructure, which in turn has no effect on whether MCPM > > > > is enabled or not. > > > > > > Because I think Lorenzo's plan was to disable MCPM for TC2. Which would > > > make this patch moot. > > > > I don't think that makes a difference - the two issues are orthogonal. > > > > As the code stands today, it requires ARM_CPU_SUSPEND to be set, so > > merging this change makes total sense. > > > > If the code is temporarily disabled, then we still need this patch > > merged to fix the error when the feature is re-enabled. If the code > > is changed later such that it doesn't need mcpm_loopback, then that's > > the time to remove the select of this symbol. > > > > If the code is removed, that is something which is not going to happen > > during this merge window, so the patch still might as well be applied > > to prevent build errors. > > Russell, > > I'm not arguing either way, I was just asking for a delay until Lorenzo > (one other maintainer of the code that is in the Cc) has a chance to > have his say, because I thought it was relevant (and I have explained why). > > If Arnd feels he would rather push the patch now rather than (let say) late > Monday, then I can give my Acked-by. Sorry but I must disagree. As the original author of that file _and_ actually being responsible for the bug fixed by that patch, I really don't understand why waiting for Lorenzo would be necessary here. Nicolas
On Sat, Feb 14, 2015 at 06:55:05AM +0800, Nicolas Pitre wrote: > On Fri, 13 Feb 2015, Liviu Dudau wrote: > > > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > > The vexpress tc2 power management code calls mcpm_loopback, which > > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > > get a link error: > > > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > > > Arnd, > > > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > > was investigating this but then headed for Linaro Connect this week, > > so we don't have yet a resolution. Can this patch be postponed for > > a later -rc? > > Can you share those reports? I did boot my TC2 with v3.19-rc7 before > leaving for Linaro Connect and it just worked. > > Regardless, this patch is about a build fix and doesn't change any > functionality, so I wouldn't postpone it. > OK. Acked-by: Liviu Dudau <Liviu.Dudau@arm.com> Best regards, Liviu > > > > > > Best regards, > > Liviu > > > > > > > > > > This explicitly selects ARM_CPU_SUSPEND like other platforms that > > > need it. > > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > Fixes: 3592d7e002438 ("ARM: 8082/1: TC2: test the MCPM loopback during boot") > > > Cc: Nicolas Pitre <nico@linaro.org> > > > Cc: Kevin Hilman <khilman@linaro.org> > > > Cc: Liviu Dudau <liviu.dudau@arm.com> > > > Cc: Sudeep Holla <sudeep.holla@arm.com> > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > --- > > > arch/arm/mach-vexpress/Kconfig | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig > > > index d6b16d9a7838..3c2509b4b694 100644 > > > --- a/arch/arm/mach-vexpress/Kconfig > > > +++ b/arch/arm/mach-vexpress/Kconfig > > > @@ -73,6 +73,7 @@ config ARCH_VEXPRESS_TC2_PM > > > depends on MCPM > > > select ARM_CCI > > > select ARCH_VEXPRESS_SPC > > > + select ARM_CPU_SUSPEND > > > help > > > Support for CPU and cluster power management on Versatile Express > > > with a TC2 (A15x2 A7x3) big.LITTLE core tile. > > > -- > > > 2.1.0.rc2 > > > > > > > > > > -- > > ==================== > > | I would like to | > > | fix the world, | > > | but they're not | > > | giving me the | > > \ source code! / > > --------------- > > ¯\_(?)_/¯ > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > >
On Sat, Feb 14, 2015 at 01:34:34AM +0000, Liviu Dudau wrote: > On Sat, Feb 14, 2015 at 06:55:05AM +0800, Nicolas Pitre wrote: > > On Fri, 13 Feb 2015, Liviu Dudau wrote: > > > > > On Thu, Feb 12, 2015 at 07:42:33PM +0000, Arnd Bergmann wrote: > > > > The vexpress tc2 power management code calls mcpm_loopback, which > > > > is only available if ARM_CPU_SUSPEND is enabled, otherwise we > > > > get a link error: > > > > > > > > arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': > > > > arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' > > > > > > Arnd, > > > > > > We are having reports of TC2 not booting with MCPM enabled. Lorenzo > > > was investigating this but then headed for Linaro Connect this week, > > > so we don't have yet a resolution. Can this patch be postponed for > > > a later -rc? > > > > Can you share those reports? I did boot my TC2 with v3.19-rc7 before > > leaving for Linaro Connect and it just worked. > > > > Regardless, this patch is about a build fix and doesn't change any > > functionality, so I wouldn't postpone it. > > > > OK. Acked-by: Liviu Dudau <Liviu.Dudau@arm.com> There is no reason to postpone it, even though TC2 builds with MCPM enabled and both bL switcher and bL CPUidle driver disabled are questionable but possible. There are other issues with PM on TC2 that are not related to this patch anyway, they can't be since this is a fix for a build issue on a config that does not enable any PM at all. Lorenzo
diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig index d6b16d9a7838..3c2509b4b694 100644 --- a/arch/arm/mach-vexpress/Kconfig +++ b/arch/arm/mach-vexpress/Kconfig @@ -73,6 +73,7 @@ config ARCH_VEXPRESS_TC2_PM depends on MCPM select ARM_CCI select ARCH_VEXPRESS_SPC + select ARM_CPU_SUSPEND help Support for CPU and cluster power management on Versatile Express with a TC2 (A15x2 A7x3) big.LITTLE core tile.
The vexpress tc2 power management code calls mcpm_loopback, which is only available if ARM_CPU_SUSPEND is enabled, otherwise we get a link error: arch/arm/mach-vexpress/built-in.o: In function `tc2_pm_init': arch/arm/mach-vexpress/tc2_pm.c:389: undefined reference to `mcpm_loopback' This explicitly selects ARM_CPU_SUSPEND like other platforms that need it. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Fixes: 3592d7e002438 ("ARM: 8082/1: TC2: test the MCPM loopback during boot") Cc: Nicolas Pitre <nico@linaro.org> Cc: Kevin Hilman <khilman@linaro.org> Cc: Liviu Dudau <liviu.dudau@arm.com> Cc: Sudeep Holla <sudeep.holla@arm.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> --- arch/arm/mach-vexpress/Kconfig | 1 + 1 file changed, 1 insertion(+)