diff mbox

[v2,1/2] ARM: Availability of psci_smp_available depends on CONFIG_SMP

Message ID 8b94f7869972d64f442f97208707a5856cbc8b14.1442990383.git.jan.kiszka@siemens.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka Sept. 23, 2015, 6:39 a.m. UTC
Ensure that we can use psci_smp_available without checking for
CONFIG_SMP first.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/arm/include/asm/psci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thierry Reding Oct. 6, 2015, 8:11 a.m. UTC | #1
On Wed, Sep 23, 2015 at 08:39:43AM +0200, Jan Kiszka wrote:
> Ensure that we can use psci_smp_available without checking for
> CONFIG_SMP first.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  arch/arm/include/asm/psci.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> index 68ee3ce..ff956f4 100644
> --- a/arch/arm/include/asm/psci.h
> +++ b/arch/arm/include/asm/psci.h
> @@ -16,7 +16,7 @@
>  
>  extern struct smp_operations psci_smp_ops;
>  
> -#ifdef CONFIG_ARM_PSCI
> +#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_SMP)
>  bool psci_smp_available(void);
>  #else
>  static inline bool psci_smp_available(void) { return false; }

Hi Will,

you had questions about this when this was first submitted back in
May[0], but discussion stalled. Can you take another look, please?

I think irrespective of what the series is trying to do this is a
correct fix. The arch/arm/kernel/psci_smp.c file is only compiled if
both ARM_PSCI and SMP are selected. Builds break if we don't mirror
that conditional in the header because for ARM_PSCI && !SMP no dummy
will be defined, but the implementation for the prototype won't be
available either, leading to a linker error.

Also Cc'ing Sebastian who sent this same patch yesterday.

[0]: http://patchwork.ozlabs.org/patch/469878/
Will Deacon Oct. 6, 2015, 3:18 p.m. UTC | #2
On Tue, Oct 06, 2015 at 10:11:24AM +0200, Thierry Reding wrote:
> On Wed, Sep 23, 2015 at 08:39:43AM +0200, Jan Kiszka wrote:
> > Ensure that we can use psci_smp_available without checking for
> > CONFIG_SMP first.
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> >  arch/arm/include/asm/psci.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> > index 68ee3ce..ff956f4 100644
> > --- a/arch/arm/include/asm/psci.h
> > +++ b/arch/arm/include/asm/psci.h
> > @@ -16,7 +16,7 @@
> >  
> >  extern struct smp_operations psci_smp_ops;
> >  
> > -#ifdef CONFIG_ARM_PSCI
> > +#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_SMP)
> >  bool psci_smp_available(void);
> >  #else
> >  static inline bool psci_smp_available(void) { return false; }
> 
> Hi Will,

Hi Thierry,

> you had questions about this when this was first submitted back in
> May[0], but discussion stalled. Can you take another look, please?
> 
> I think irrespective of what the series is trying to do this is a
> correct fix. The arch/arm/kernel/psci_smp.c file is only compiled if
> both ARM_PSCI and SMP are selected. Builds break if we don't mirror
> that conditional in the header because for ARM_PSCI && !SMP no dummy
> will be defined, but the implementation for the prototype won't be
> available either, leading to a linker error.

Sure, I'm fine with this patch in isolation, I just didn't (don't) fully
grok what the series is trying to achieve.

Will
Thierry Reding Oct. 8, 2015, 4:04 p.m. UTC | #3
On Tue, Oct 06, 2015 at 04:18:58PM +0100, Will Deacon wrote:
> On Tue, Oct 06, 2015 at 10:11:24AM +0200, Thierry Reding wrote:
> > On Wed, Sep 23, 2015 at 08:39:43AM +0200, Jan Kiszka wrote:
> > > Ensure that we can use psci_smp_available without checking for
> > > CONFIG_SMP first.
> > > 
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > ---
> > >  arch/arm/include/asm/psci.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> > > index 68ee3ce..ff956f4 100644
> > > --- a/arch/arm/include/asm/psci.h
> > > +++ b/arch/arm/include/asm/psci.h
> > > @@ -16,7 +16,7 @@
> > >  
> > >  extern struct smp_operations psci_smp_ops;
> > >  
> > > -#ifdef CONFIG_ARM_PSCI
> > > +#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_SMP)
> > >  bool psci_smp_available(void);
> > >  #else
> > >  static inline bool psci_smp_available(void) { return false; }
> > 
> > Hi Will,
> 
> Hi Thierry,
> 
> > you had questions about this when this was first submitted back in
> > May[0], but discussion stalled. Can you take another look, please?
> > 
> > I think irrespective of what the series is trying to do this is a
> > correct fix. The arch/arm/kernel/psci_smp.c file is only compiled if
> > both ARM_PSCI and SMP are selected. Builds break if we don't mirror
> > that conditional in the header because for ARM_PSCI && !SMP no dummy
> > will be defined, but the implementation for the prototype won't be
> > available either, leading to a linker error.
> 
> Sure, I'm fine with this patch in isolation, I just didn't (don't) fully
> grok what the series is trying to achieve.

The goal is to prevent the kernel from registering a CPU idle driver if
PSCI is going to be used for SMP. This is necessary because both the CPU
idle driver and the PSCI implementation (provided by U-Boot in this
case) access the same resources. The effect of having both enabled is
usually that the system will simply hang sometime during boot.

Thierry
Sebastian Andrzej Siewior Nov. 12, 2015, 9:43 a.m. UTC | #4
On 2015-10-06 16:18:58 [+0100], Will Deacon wrote:
> On Tue, Oct 06, 2015 at 10:11:24AM +0200, Thierry Reding wrote:
> > On Wed, Sep 23, 2015 at 08:39:43AM +0200, Jan Kiszka wrote:
> > > Ensure that we can use psci_smp_available without checking for
> > > CONFIG_SMP first.
> > > 
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > ---
> > >  arch/arm/include/asm/psci.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> > > index 68ee3ce..ff956f4 100644
> > > --- a/arch/arm/include/asm/psci.h
> > > +++ b/arch/arm/include/asm/psci.h
> > > @@ -16,7 +16,7 @@
> > >  
> > >  extern struct smp_operations psci_smp_ops;
> > >  
> > > -#ifdef CONFIG_ARM_PSCI
> > > +#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_SMP)
> > >  bool psci_smp_available(void);
> > >  #else
> > >  static inline bool psci_smp_available(void) { return false; }
> 
> Sure, I'm fine with this patch in isolation, I just didn't (don't) fully
> grok what the series is trying to achieve.

Will, do you want this patch re-sent or do you just need some time to pick
it up?

> Will

Sebastian
Will Deacon Nov. 12, 2015, 9:49 a.m. UTC | #5
On Thu, Nov 12, 2015 at 10:43:33AM +0100, Sebastian Andrzej Siewior wrote:
> On 2015-10-06 16:18:58 [+0100], Will Deacon wrote:
> > On Tue, Oct 06, 2015 at 10:11:24AM +0200, Thierry Reding wrote:
> > > On Wed, Sep 23, 2015 at 08:39:43AM +0200, Jan Kiszka wrote:
> > > > Ensure that we can use psci_smp_available without checking for
> > > > CONFIG_SMP first.
> > > > 
> > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > > ---
> > > >  arch/arm/include/asm/psci.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> > > > index 68ee3ce..ff956f4 100644
> > > > --- a/arch/arm/include/asm/psci.h
> > > > +++ b/arch/arm/include/asm/psci.h
> > > > @@ -16,7 +16,7 @@
> > > >  
> > > >  extern struct smp_operations psci_smp_ops;
> > > >  
> > > > -#ifdef CONFIG_ARM_PSCI
> > > > +#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_SMP)
> > > >  bool psci_smp_available(void);
> > > >  #else
> > > >  static inline bool psci_smp_available(void) { return false; }
> > 
> > Sure, I'm fine with this patch in isolation, I just didn't (don't) fully
> > grok what the series is trying to achieve.
> 
> Will, do you want this patch re-sent or do you just need some time to pick
> it up?

I wasn't planning to pick this one up as I assumed you'd be taking it
via arm-soc along with the second patch that depends on it. I just
stopped moaning about not understanding it :)

Will
Sebastian Andrzej Siewior Nov. 12, 2015, 10:07 a.m. UTC | #6
On 2015-11-12 09:49:20 [+0000], Will Deacon wrote:
> I wasn't planning to pick this one up as I assumed you'd be taking it
> via arm-soc along with the second patch that depends on it. I just
> stopped moaning about not understanding it :)

Ah, okay, thanks for the update. I just reposted it and hope for the best.

> Will

Sebastian
diff mbox

Patch

diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
index 68ee3ce..ff956f4 100644
--- a/arch/arm/include/asm/psci.h
+++ b/arch/arm/include/asm/psci.h
@@ -16,7 +16,7 @@ 
 
 extern struct smp_operations psci_smp_ops;
 
-#ifdef CONFIG_ARM_PSCI
+#if defined(CONFIG_ARM_PSCI) && defined(CONFIG_SMP)
 bool psci_smp_available(void);
 #else
 static inline bool psci_smp_available(void) { return false; }