Re: [PATCH] ARM: shmobile: apmu: silence build warnings

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 2488 bytes --]

On Mon, Jul 13, 2015 at 10:04:06AM +0900, Simon Horman wrote:
> Hi Wolfram,
> 
> On Fri, Jul 10, 2015 at 10:48:16PM +0200, Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > 
> > With shmobile_defconfig but SMP=n && SUSPEND=n, I get:
> > 
> > arch/arm/mach-shmobile/platsmp-apmu.c:49:12: warning: 'apmu_power_off' defined but not used [-Wunused-function]
> > arch/arm/mach-shmobile/platsmp-apmu.c:70:12: warning: 'apmu_wrap' defined but not used [-Wunused-function]
> > 
> > Annotate those functions like the functions around it.
> 
> thanks for noticing this.
> 
> I'm not familiar with when it is appropriate to use __maybe_unused but does
> anything speak against using #if as per the conditional compilation of the
> callers in platsmp-apmu.c of the above functions?

I did my approach for consistency reasons. It looked to me that the
*_on/off functions wanted to be grouped at the beginning.

Your approach works but looks fragile to me as soon as the ifdeffery of
the callers change. So, we could move the code into the callers ifdef
block, but we should do this consistently, even for the functions
currently marked __maybe_unused. That would scatter them a little,
though. apmu_wrap needs its own ifdeffery in any case.

Do you have a preference?

> 
> diff --git a/arch/arm/mach-shmobile/platsmp-apmu.c b/arch/arm/mach-shmobile/platsmp-apmu.c
> index b0790fc32282..54cf153b570f 100644
> --- a/arch/arm/mach-shmobile/platsmp-apmu.c
> +++ b/arch/arm/mach-shmobile/platsmp-apmu.c
> @@ -46,12 +46,14 @@ static int __maybe_unused apmu_power_on(void __iomem *p, int bit)
>  	return 0;
>  }
>  
> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_SUSPEND)
>  static int apmu_power_off(void __iomem *p, int bit)
>  {
>  	/* request Core Standby for next WFI */
>  	writel_relaxed(3, p + CPUNCR_OFFS(bit));
>  	return 0;
>  }
> +#endif
>  
>  static int __maybe_unused apmu_power_off_poll(void __iomem *p, int bit)
>  {
> @@ -67,12 +69,14 @@ static int __maybe_unused apmu_power_off_poll(void __iomem *p, int bit)
>  	return 0;
>  }
>  
> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_SUSPEND) || defined(CONFIG_SMP)
>  static int apmu_wrap(int cpu, int (*fn)(void __iomem *p, int cpu))
>  {
>  	void __iomem *p = apmu_cpus[cpu].iomem;
>  
>  	return p ? fn(p, apmu_cpus[cpu].bit) : -EINVAL;
>  }
> +#endif
>  
>  static void apmu_init_cpu(struct resource *res, int cpu, int bit)
>  {

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]