Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Michael Ellerman]

On Tue, 4 Aug 2020 10:54:05 +1000, Oliver O'Halloran wrote:
> When building with W=1 we get the following warning:
> 
>  arch/powerpc/platforms/powernv/smp.c: In function 
> ‘pnv_smp_cpu_kill_self’:
>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
>   empty body in an ‘if’ statement [-Werror=empty-body]
>276 |  cpu, srr1);
>|^
>  cc1: all warnings being treated as errors
> 
> [...]

Applied to powerpc/next.

[1/6] powerpc/powernv/smp: Fix spurious DBG() warning
  https://git.kernel.org/powerpc/c/f6bac19cf65c5be21d14a0c9684c8f560f2096dd
[2/6] powerpc/powernv: Include asm/powernv.h from the local powernv.h
  https://git.kernel.org/powerpc/c/8471c1dd93de9a2278d41c527b76291e4ace8f1c
[3/6] powerpc/powernv: Staticify functions without prototypes
  https://git.kernel.org/powerpc/c/3b70464aa78917e88c1d4bfc2100c344c0eda8e0
[4/6] powerpc/powernv: Fix spurious kerneldoc warnings in opal-prd.c
  https://git.kernel.org/powerpc/c/fb248c3121af713f31736af359608491544cfc23
[5/6] powerpc/powernv: Remove set but not used variable 'parent'
  https://git.kernel.org/powerpc/c/18102e4bcc47f5b5ac70e2e4461d022c1ee6df24
[6/6] powerpc/nx: Don't pack struct coprocessor_request_block
  https://git.kernel.org/powerpc/c/3ced132a055c4e5046d21732393ae6848ff309e0

cheers

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Michael Ellerman]

Joel Stanley  writes:
> On Tue, 4 Aug 2020 at 00:57, Oliver O'Halloran  wrote:
>>
>> When building with W=1 we get the following warning:
>>
>>  arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
>>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
>> empty body in an ‘if’ statement [-Werror=empty-body]
>>276 |  cpu, srr1);
>>|^
>>  cc1: all warnings being treated as errors
>>
>> The full context is this block:
>>
>>  if (srr1 && !generic_check_cpu_restart(cpu))
>> DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
>> cpu, srr1);
>>
>> When building with DEBUG undefined DBG() expands to nothing and GCC emits
>> the warning due to the lack of braces around an empty statement.
>>
>> Signed-off-by: Oliver O'Halloran 
>> ---
>> We could add the braces too. That might even be better since it's a 
>> multi-line
>> if block even though it's only a single statement.
>
> Or you could put it all on one line, now that our 120 line overlords
> have taken over.

Yeah I think that one may as well be one line.

> Reviewed-by: Joel Stanley 
>
> Messy:
>
> $ git grep "define DBG(" arch/powerpc/ |grep -v print
> arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
> arch/powerpc/kernel/iommu.c:#define DBG(...)
> arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/kernel/prom.c:#define DBG(fmt...)
..

Yeah, gross old cruft.

The vast majority of those should just be replaced with pr_devel()
and/or pr_debug().

The pnv_smp_cpu_kill_self() case is one where we probably do want to
stick with udbg_printf(), because I don't think it's kosher to call
printk() from an offline CPU.

cheers

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Oliver O'Halloran]

On Tue, Aug 4, 2020 at 12:07 PM Joel Stanley  wrote:
>
> Messy:
>
> $ git grep "define DBG(" arch/powerpc/ |grep -v print
> arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
> arch/powerpc/kernel/iommu.c:#define DBG(...)
> arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/kernel/prom.c:#define DBG(fmt...)
> arch/powerpc/kernel/setup-common.c:#define DBG(fmt...)
> arch/powerpc/kernel/setup_32.c:#define DBG(fmt...)
> arch/powerpc/kernel/smp.c:#define DBG(fmt...)
> arch/powerpc/kernel/vdso.c:#define DBG(fmt...)
> arch/powerpc/kvm/book3s_hv_rm_xive.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/mm/book3s64/hash_utils.c:#define DBG(fmt...)
> arch/powerpc/platforms/83xx/mpc832x_mds.c:#define DBG(fmt...)
> arch/powerpc/platforms/83xx/mpc832x_rdb.c:#define DBG(fmt...)
> arch/powerpc/platforms/83xx/mpc836x_mds.c:#define DBG(fmt...)
> arch/powerpc/platforms/85xx/mpc85xx_ds.c:#define DBG(fmt, args...)
> arch/powerpc/platforms/85xx/mpc85xx_mds.c:#define DBG(fmt...)
> arch/powerpc/platforms/85xx/mpc85xx_rdb.c:#define DBG(fmt, args...)
> arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/platforms/cell/setup.c:#define DBG(fmt...)
> arch/powerpc/platforms/cell/smp.c:#define DBG(fmt...)
> arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c:#define DBG(fmt...)
> do { } while(0)
> arch/powerpc/platforms/maple/pci.c:#define DBG(x...)
> arch/powerpc/platforms/maple/setup.c:#define DBG(fmt...)
> arch/powerpc/platforms/maple/time.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/bootx_init.c:#define DBG(fmt...) do { } 
> while(0)
> arch/powerpc/platforms/powermac/feature.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...) do {\
> arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/nvram.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/pci.c:#define DBG(x...)
> arch/powerpc/platforms/powermac/pfunc_base.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/pfunc_core.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/smp.c:#define DBG(fmt...)
> arch/powerpc/platforms/powermac/time.c:#define DBG(x...)
> arch/powerpc/platforms/powernv/smp.c:#define DBG(fmt...)
> arch/powerpc/sysdev/dart_iommu.c:#define DBG(...)
> arch/powerpc/sysdev/ge/ge_pic.c:#define DBG(fmt...) do { } while (0)
> arch/powerpc/sysdev/mpic.c:#define DBG(fmt...)
> arch/powerpc/sysdev/tsi108_dev.c:#define DBG(fmt...) do { } while(0)
> arch/powerpc/sysdev/tsi108_pci.c:#define DBG(x...)

I started off writing a patch that fixed all these too. When I went to
test it I discovered there's a giant pile of other W=1 warnings from
other parts of arch/powerpc/ so I figured I'd start with something
less ambitious.

Re: [PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Joel Stanley]

On Tue, 4 Aug 2020 at 00:57, Oliver O'Halloran  wrote:
>
> When building with W=1 we get the following warning:
>
>  arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
>  arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
> empty body in an ‘if’ statement [-Werror=empty-body]
>276 |  cpu, srr1);
>|^
>  cc1: all warnings being treated as errors
>
> The full context is this block:
>
>  if (srr1 && !generic_check_cpu_restart(cpu))
> DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
> cpu, srr1);
>
> When building with DEBUG undefined DBG() expands to nothing and GCC emits
> the warning due to the lack of braces around an empty statement.
>
> Signed-off-by: Oliver O'Halloran 
> ---
> We could add the braces too. That might even be better since it's a multi-line
> if block even though it's only a single statement.

Or you could put it all on one line, now that our 120 line overlords
have taken over.

Reviewed-by: Joel Stanley 

Messy:

$ git grep "define DBG(" arch/powerpc/ |grep -v print
arch/powerpc/kernel/crash_dump.c:#define DBG(fmt...)
arch/powerpc/kernel/iommu.c:#define DBG(...)
arch/powerpc/kernel/legacy_serial.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/kernel/prom.c:#define DBG(fmt...)
arch/powerpc/kernel/setup-common.c:#define DBG(fmt...)
arch/powerpc/kernel/setup_32.c:#define DBG(fmt...)
arch/powerpc/kernel/smp.c:#define DBG(fmt...)
arch/powerpc/kernel/vdso.c:#define DBG(fmt...)
arch/powerpc/kvm/book3s_hv_rm_xive.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/mm/book3s64/hash_utils.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc832x_rdb.c:#define DBG(fmt...)
arch/powerpc/platforms/83xx/mpc836x_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_ds.c:#define DBG(fmt, args...)
arch/powerpc/platforms/85xx/mpc85xx_mds.c:#define DBG(fmt...)
arch/powerpc/platforms/85xx/mpc85xx_rdb.c:#define DBG(fmt, args...)
arch/powerpc/platforms/86xx/mpc86xx_hpcn.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/cell/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/cell/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/embedded6xx/mpc7448_hpc2.c:#define DBG(fmt...)
do { } while(0)
arch/powerpc/platforms/maple/pci.c:#define DBG(x...)
arch/powerpc/platforms/maple/setup.c:#define DBG(fmt...)
arch/powerpc/platforms/maple/time.c:#define DBG(x...)
arch/powerpc/platforms/powermac/bootx_init.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/platforms/powermac/feature.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...) do {\
arch/powerpc/platforms/powermac/low_i2c.c:#define DBG(x...)
arch/powerpc/platforms/powermac/nvram.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pci.c:#define DBG(x...)
arch/powerpc/platforms/powermac/pfunc_base.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/pfunc_core.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/smp.c:#define DBG(fmt...)
arch/powerpc/platforms/powermac/time.c:#define DBG(x...)
arch/powerpc/platforms/powernv/smp.c:#define DBG(fmt...)
arch/powerpc/sysdev/dart_iommu.c:#define DBG(...)
arch/powerpc/sysdev/ge/ge_pic.c:#define DBG(fmt...) do { } while (0)
arch/powerpc/sysdev/mpic.c:#define DBG(fmt...)
arch/powerpc/sysdev/tsi108_dev.c:#define DBG(fmt...) do { } while(0)
arch/powerpc/sysdev/tsi108_pci.c:#define DBG(x...)


> ---
>  arch/powerpc/platforms/powernv/smp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/smp.c 
> b/arch/powerpc/platforms/powernv/smp.c
> index b2ba3e95bda7..bbf361f23ae8 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -43,7 +43,7 @@
>  #include 
>  #define DBG(fmt...) udbg_printf(fmt)
>  #else
> -#define DBG(fmt...)
> +#define DBG(fmt...) do { } while (0)
>  #endif
>
>  static void pnv_smp_setup_cpu(int cpu)
> --
> 2.26.2
>

[PATCH 1/6] powerpc/powernv/smp: Fix spurious DBG() warning

[Oliver O'Halloran]

When building with W=1 we get the following warning:

 arch/powerpc/platforms/powernv/smp.c: In function ‘pnv_smp_cpu_kill_self’:
 arch/powerpc/platforms/powernv/smp.c:276:16: error: suggest braces around
empty body in an ‘if’ statement [-Werror=empty-body]
   276 |  cpu, srr1);
   |^
 cc1: all warnings being treated as errors

The full context is this block:

 if (srr1 && !generic_check_cpu_restart(cpu))
DBG("CPU%d Unexpected exit while offline srr1=%lx!\n",
cpu, srr1);

When building with DEBUG undefined DBG() expands to nothing and GCC emits
the warning due to the lack of braces around an empty statement.

Signed-off-by: Oliver O'Halloran 
---
We could add the braces too. That might even be better since it's a multi-line
if block even though it's only a single statement.
---
 arch/powerpc/platforms/powernv/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/smp.c 
b/arch/powerpc/platforms/powernv/smp.c
index b2ba3e95bda7..bbf361f23ae8 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -43,7 +43,7 @@
 #include 
 #define DBG(fmt...) udbg_printf(fmt)
 #else
-#define DBG(fmt...)
+#define DBG(fmt...) do { } while (0)
 #endif
 
 static void pnv_smp_setup_cpu(int cpu)
-- 
2.26.2