[PATCH 0/7] powerpc: Fix a few W=1 compile warnings

[PATCH 0/7] powerpc: Fix a few W=1 compile warnings

[Cédric Le Goater]

Hello,

Here is a small contribution on improving compile with W=1.	

Thanks,

C.

Cédric Le Goater (7):
  powerpc/sysfs: Fix W=1 compile warning
  powerpc/prom: Fix W=1 compile warning
  powerpc/sstep: Fix W=1 compile warning
  powerpc/xive: Fix W=1 compile warning
  powerpc/powernv/pci: Fix W=1 compile warning
  powerpc/perf: Fix W=1 compile warning
  powerpc/32: Fix W=1 compile warning

 arch/powerpc/include/asm/asm-prototypes.h |  1 +
 arch/powerpc/kernel/prom.c                | 51 ++++++++++++-----------
 arch/powerpc/kernel/sysfs.c               |  3 +-
 arch/powerpc/lib/sstep.c                  |  3 +-
 arch/powerpc/perf/imc-pmu.c               |  3 --
 arch/powerpc/platforms/powernv/pci-ioda.c |  8 ----
 arch/powerpc/sysdev/xive/common.c         |  4 +-
 7 files changed, 32 insertions(+), 41 deletions(-)

-- 
2.25.4

[PATCH 1/7] powerpc/sysfs: Fix W=1 compile warning

[Cédric Le Goater]

arch/powerpc/kernel/sysfs.c: In function ‘sysfs_create_dscr_default’:
arch/powerpc/kernel/sysfs.c:228:7: error: variable ‘err’ set but not used [-Werror=unused-but-set-variable]
   int err = 0;
       ^~~
cc1: all warnings being treated as errors

Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kernel/sysfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 46b4ebc33db7..821a3dc4c924 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -225,14 +225,13 @@ static DEVICE_ATTR(dscr_default, 0600,
 static void sysfs_create_dscr_default(void)
 {
 	if (cpu_has_feature(CPU_FTR_DSCR)) {
-		int err = 0;
 		int cpu;
 
 		dscr_default = spr_default_dscr;
 		for_each_possible_cpu(cpu)
 			paca_ptrs[cpu]->dscr_default = dscr_default;
 
-		err = device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default);
+		device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default);
 	}
 }
 #endif /* CONFIG_PPC64 */
-- 
2.25.4

[PATCH 2/7] powerpc/prom: Fix W=1 compile warning

[Cédric Le Goater]

arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
  __be64 *reserve_map;
          ^~~~~~~~~~~
cc1: all warnings being treated as errors

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kernel/prom.c | 51 +++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index d8a2fb87ba0c..4bae9ebc7d0b 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -622,11 +622,6 @@ static void __init early_reserve_mem_dt(void)
 
 static void __init early_reserve_mem(void)
 {
-	__be64 *reserve_map;
-
-	reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
-			fdt_off_mem_rsvmap(initial_boot_params));
-
 	/* Look for the new "reserved-regions" property in the DT */
 	early_reserve_mem_dt();
 
@@ -639,28 +634,34 @@ static void __init early_reserve_mem(void)
 	}
 #endif /* CONFIG_BLK_DEV_INITRD */
 
-#ifdef CONFIG_PPC32
-	/* 
-	 * Handle the case where we might be booting from an old kexec
-	 * image that setup the mem_rsvmap as pairs of 32-bit values
-	 */
-	if (be64_to_cpup(reserve_map) > 0xffffffffull) {
-		u32 base_32, size_32;
-		__be32 *reserve_map_32 = (__be32 *)reserve_map;
-
-		DBG("Found old 32-bit reserve map\n");
-
-		while (1) {
-			base_32 = be32_to_cpup(reserve_map_32++);
-			size_32 = be32_to_cpup(reserve_map_32++);
-			if (size_32 == 0)
-				break;
-			DBG("reserving: %x -> %x\n", base_32, size_32);
-			memblock_reserve(base_32, size_32);
+	if (IS_ENABLED(CONFIG_PPC32)) {
+		__be64 *reserve_map;
+
+		reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
+				 fdt_off_mem_rsvmap(initial_boot_params));
+
+		/*
+		 * Handle the case where we might be booting from an
+		 * old kexec image that setup the mem_rsvmap as pairs
+		 * of 32-bit values
+		 */
+		if (be64_to_cpup(reserve_map) > 0xffffffffull) {
+			u32 base_32, size_32;
+			__be32 *reserve_map_32 = (__be32 *)reserve_map;
+
+			DBG("Found old 32-bit reserve map\n");
+
+			while (1) {
+				base_32 = be32_to_cpup(reserve_map_32++);
+				size_32 = be32_to_cpup(reserve_map_32++);
+				if (size_32 == 0)
+					break;
+				DBG("reserving: %x -> %x\n", base_32, size_32);
+				memblock_reserve(base_32, size_32);
+			}
+			return;
 		}
-		return;
 	}
-#endif
 }
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-- 
2.25.4

[PATCH 3/7] powerpc/sstep: Fix W=1 compile warning

[Cédric Le Goater]

../arch/powerpc/lib/sstep.c: In function ‘mlsd_8lsd_ea’:
../arch/powerpc/lib/sstep.c:225:3: error: suggest braces around empty body in an ‘if’ statement [-Werror=empty-body]
   ; /* Invalid form. Should already be checked for by caller! */
   ^

Cc: Jordan Niethe <jniethe5@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/lib/sstep.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index caee8cc77e19..14572af16e55 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -221,8 +221,9 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
 		; /* Leave ea as is */
 	else if (prefix_r && !ra)
 		ea += regs->nip;
-	else if (prefix_r && ra)
+	else if (prefix_r && ra) {
 		; /* Invalid form. Should already be checked for by caller! */
+	}
 
 	return ea;
 }
-- 
2.25.4

[PATCH 4/7] powerpc/xive: Fix W=1 compile warning

[Cédric Le Goater]

CC      arch/powerpc/sysdev/xive/common.o
../arch/powerpc/sysdev/xive/common.c:1568:6: error: no previous prototype for ‘xive_debug_show_cpu’ [-Werror=missing-prototypes]
 void xive_debug_show_cpu(struct seq_file *m, int cpu)
      ^~~~~~~~~~~~~~~~~~~
../arch/powerpc/sysdev/xive/common.c:1602:6: error: no previous prototype for ‘xive_debug_show_irq’ [-Werror=missing-prototypes]
 void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct irq_data *d)
      ^~~~~~~~~~~~~~~~~~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/sysdev/xive/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index f591be9f01f4..a80440af491a 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1565,7 +1565,7 @@ static int __init xive_off(char *arg)
 }
 __setup("xive=off", xive_off);
 
-void xive_debug_show_cpu(struct seq_file *m, int cpu)
+static void xive_debug_show_cpu(struct seq_file *m, int cpu)
 {
 	struct xive_cpu *xc = per_cpu(xive_cpu, cpu);
 
@@ -1599,7 +1599,7 @@ void xive_debug_show_cpu(struct seq_file *m, int cpu)
 	seq_puts(m, "\n");
 }
 
-void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct irq_data *d)
+static void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct irq_data *d)
 {
 	struct irq_chip *chip = irq_data_get_irq_chip(d);
 	int rc;
-- 
2.25.4

[PATCH 5/7] powerpc/powernv/pci: Fix W=1 compile warning

[Cédric Le Goater]

  CC      arch/powerpc/platforms/powernv/pci-ioda.o
../arch/powerpc/platforms/powernv/pci-ioda.c: In function ‘pnv_ioda_configure_pe’:
../arch/powerpc/platforms/powernv/pci-ioda.c:897:18: error: variable ‘parent’ set but not used [-Werror=unused-but-set-variable]
  struct pci_dev *parent;
                  ^~~~~~

Cc: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 023a4f987bb2..2b4ceb5e6ce4 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -894,7 +894,6 @@ int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 
 int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 {
-	struct pci_dev *parent;
 	uint8_t bcomp, dcomp, fcomp;
 	long rc, rid_end, rid;
 
@@ -904,7 +903,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 
 		dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
 		fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
-		parent = pe->pbus->self;
 		if (pe->flags & PNV_IODA_PE_BUS_ALL)
 			count = resource_size(&pe->pbus->busn_res);
 		else
@@ -925,12 +923,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
 		}
 		rid_end = pe->rid + (count << 8);
 	} else {
-#ifdef CONFIG_PCI_IOV
-		if (pe->flags & PNV_IODA_PE_VF)
-			parent = pe->parent_dev;
-		else
-#endif /* CONFIG_PCI_IOV */
-			parent = pe->pdev->bus->self;
 		bcomp = OpalPciBusAll;
 		dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
 		fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
-- 
2.25.4

[PATCH 6/7] powerpc/perf: Fix W=1 compile warning

[Cédric Le Goater]

CC      arch/powerpc/perf/imc-pmu.o
../arch/powerpc/perf/imc-pmu.c: In function ‘trace_imc_event_init’:
../arch/powerpc/perf/imc-pmu.c:1429:22: error: variable ‘target’ set but not used [-Werror=unused-but-set-variable]
  struct task_struct *target;
                      ^~~~~~

Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/perf/imc-pmu.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 62d0b54086f8..9ed4fcccf8a9 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1426,8 +1426,6 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
 
 static int trace_imc_event_init(struct perf_event *event)
 {
-	struct task_struct *target;
-
 	if (event->attr.type != event->pmu->type)
 		return -ENOENT;
 
@@ -1458,7 +1456,6 @@ static int trace_imc_event_init(struct perf_event *event)
 	mutex_unlock(&imc_global_refc.lock);
 
 	event->hw.idx = -1;
-	target = event->hw.target;
 
 	event->pmu->task_ctx_nr = perf_hw_context;
 	event->destroy = reset_global_refc;
-- 
2.25.4

[PATCH 7/7] powerpc/32: Fix W=1 compile warning

[Cédric Le Goater]

CC      arch/powerpc/kernel/traps.o
../arch/powerpc/kernel/traps.c:1663:6: error: no previous prototype for ‘stack_overflow_exception’ [-Werror=missing-prototypes]
 void stack_overflow_exception(struct pt_regs *regs)
      ^~~~~~~~~~~~~~~~~~~~~~~~

Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 3978eb78517c ("powerpc/32: Add early stack overflow detection with VMAP stack.")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/include/asm/asm-prototypes.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index de14b1a34d56..4957119604c7 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -67,6 +67,7 @@ void single_step_exception(struct pt_regs *regs);
 void program_check_exception(struct pt_regs *regs);
 void alignment_exception(struct pt_regs *regs);
 void StackOverflow(struct pt_regs *regs);
+void stack_overflow_exception(struct pt_regs *regs);
 void kernel_fp_unavailable_exception(struct pt_regs *regs);
 void altivec_unavailable_exception(struct pt_regs *regs);
 void vsx_unavailable_exception(struct pt_regs *regs);
-- 
2.25.4

Re: [PATCH 1/7] powerpc/sysfs: Fix W=1 compile warning

[Christophe Leroy]

Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
> arch/powerpc/kernel/sysfs.c: In function ‘sysfs_create_dscr_default’:
> arch/powerpc/kernel/sysfs.c:228:7: error: variable ‘err’ set but not used [-Werror=unused-but-set-variable]
>     int err = 0;
>         ^~~
> cc1: all warnings being treated as errors

A small sentence explaining how this is fixes would be welcome, so that 
you don't need to read the code the know what the commit does to fix the 
warning. Even the subject should be more explicite, rather than saying 
"Fix W=1 compile warning", I think it should say something like "remove 
unused err variable"


> 
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

> ---
>   arch/powerpc/kernel/sysfs.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 46b4ebc33db7..821a3dc4c924 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -225,14 +225,13 @@ static DEVICE_ATTR(dscr_default, 0600,
>   static void sysfs_create_dscr_default(void)
>   {
>   	if (cpu_has_feature(CPU_FTR_DSCR)) {
> -		int err = 0;
>   		int cpu;
>   
>   		dscr_default = spr_default_dscr;
>   		for_each_possible_cpu(cpu)
>   			paca_ptrs[cpu]->dscr_default = dscr_default;
>   
> -		err = device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default);
> +		device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default);
>   	}
>   }
>   #endif /* CONFIG_PPC64 */
> 

Re: [PATCH 2/7] powerpc/prom: Fix W=1 compile warning

[Christophe Leroy]

Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
> arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
> arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
>    __be64 *reserve_map;
>            ^~~~~~~~~~~
> cc1: all warnings being treated as errors

A small sentence explaining how this is fixes would be welcome, so that 
you don't need to read the code the know what the commit does to fix the 
warning. Also the subject should be more explicit.


> 
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   arch/powerpc/kernel/prom.c | 51 +++++++++++++++++++-------------------
>   1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index d8a2fb87ba0c..4bae9ebc7d0b 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -622,11 +622,6 @@ static void __init early_reserve_mem_dt(void)
>   
>   static void __init early_reserve_mem(void)
>   {
> -	__be64 *reserve_map;
> -
> -	reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
> -			fdt_off_mem_rsvmap(initial_boot_params));
> -
>   	/* Look for the new "reserved-regions" property in the DT */
>   	early_reserve_mem_dt();
>   
> @@ -639,28 +634,34 @@ static void __init early_reserve_mem(void)
>   	}
>   #endif /* CONFIG_BLK_DEV_INITRD */
>   
> -#ifdef CONFIG_PPC32

Instead of such a big change, you could simply do the following in 
addition to the move of reserve_map allocation after it.

	if (!IS_ENABLED(CONFIG_PPC32))
		return;

> -	/*
> -	 * Handle the case where we might be booting from an old kexec
> -	 * image that setup the mem_rsvmap as pairs of 32-bit values
> -	 */
> -	if (be64_to_cpup(reserve_map) > 0xffffffffull) {
> -		u32 base_32, size_32;
> -		__be32 *reserve_map_32 = (__be32 *)reserve_map;
> -
> -		DBG("Found old 32-bit reserve map\n");
> -
> -		while (1) {
> -			base_32 = be32_to_cpup(reserve_map_32++);
> -			size_32 = be32_to_cpup(reserve_map_32++);
> -			if (size_32 == 0)
> -				break;
> -			DBG("reserving: %x -> %x\n", base_32, size_32);
> -			memblock_reserve(base_32, size_32);
> +	if (IS_ENABLED(CONFIG_PPC32)) {
> +		__be64 *reserve_map;
> +
> +		reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
> +				 fdt_off_mem_rsvmap(initial_boot_params));
> +
> +		/*
> +		 * Handle the case where we might be booting from an
> +		 * old kexec image that setup the mem_rsvmap as pairs
> +		 * of 32-bit values
> +		 */
> +		if (be64_to_cpup(reserve_map) > 0xffffffffull) {
> +			u32 base_32, size_32;
> +			__be32 *reserve_map_32 = (__be32 *)reserve_map;
> +
> +			DBG("Found old 32-bit reserve map\n");
> +
> +			while (1) {
> +				base_32 = be32_to_cpup(reserve_map_32++);
> +				size_32 = be32_to_cpup(reserve_map_32++);
> +				if (size_32 == 0)
> +					break;
> +				DBG("reserving: %x -> %x\n", base_32, size_32);
> +				memblock_reserve(base_32, size_32);
> +			}
> +			return;
>   		}
> -		return;
>   	}
> -#endif
>   }
>   
>   #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> 


Christophe

Re: [PATCH 3/7] powerpc/sstep: Fix W=1 compile warning

[Christophe Leroy]

Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
> ../arch/powerpc/lib/sstep.c: In function ‘mlsd_8lsd_ea’:
> ../arch/powerpc/lib/sstep.c:225:3: error: suggest braces around empty body in an ‘if’ statement [-Werror=empty-body]
>     ; /* Invalid form. Should already be checked for by caller! */
>     ^

A small sentence explaining how this is fixed would be welcome, so that 
you don't need to read the code the know what the commit does to fix the 
warning. Also the subject should be more explicit.



> 
> Cc: Jordan Niethe <jniethe5@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   arch/powerpc/lib/sstep.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index caee8cc77e19..14572af16e55 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -221,8 +221,9 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
>   		; /* Leave ea as is */
>   	else if (prefix_r && !ra)
>   		ea += regs->nip;
> -	else if (prefix_r && ra)
> +	else if (prefix_r && ra) {
>   		; /* Invalid form. Should already be checked for by caller! */
> +	}

You can't do that. Now checkpatch will complain that you don't have 
braces on all legs of the if/else dance.

I think the last 'else if' should simply be removed entirely as it does 
nothing. Eventually, just leave the comment, something like:

	/* (prefix_r && ra) is Invalid form. Should already be checked for by 
caller! */

And if (prefix_r && ra) is not possible, then the previous if should 
just be 'if (prefx_r)'

Christophe


>   
>   	return ea;
>   }
> 

Re: [PATCH 4/7] powerpc/xive: Fix W=1 compile warning

[Christophe Leroy]

Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
> CC      arch/powerpc/sysdev/xive/common.o
> ../arch/powerpc/sysdev/xive/common.c:1568:6: error: no previous prototype for ‘xive_debug_show_cpu’ [-Werror=missing-prototypes]
>   void xive_debug_show_cpu(struct seq_file *m, int cpu)
>        ^~~~~~~~~~~~~~~~~~~
> ../arch/powerpc/sysdev/xive/common.c:1602:6: error: no previous prototype for ‘xive_debug_show_irq’ [-Werror=missing-prototypes]
>   void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct irq_data *d)
>        ^~~~~~~~~~~~~~~~~~~
> 

A small sentence explaining how this is fixed would be welcome, so that 
you don't need to read the code the know what the commit does to fix the 
warning. Also the subject should be more explicit.

There are two ways of fixing it:
- Add the missing prototype
- Make it static

You chose the second alternative, this needs to be told in the commit log.


> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/sysdev/xive/common.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index f591be9f01f4..a80440af491a 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1565,7 +1565,7 @@ static int __init xive_off(char *arg)
>   }
>   __setup("xive=off", xive_off);
>   
> -void xive_debug_show_cpu(struct seq_file *m, int cpu)
> +static void xive_debug_show_cpu(struct seq_file *m, int cpu)
>   {
>   	struct xive_cpu *xc = per_cpu(xive_cpu, cpu);
>   
> @@ -1599,7 +1599,7 @@ void xive_debug_show_cpu(struct seq_file *m, int cpu)
>   	seq_puts(m, "\n");
>   }
>   
> -void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct irq_data *d)
> +static void xive_debug_show_irq(struct seq_file *m, u32 hw_irq, struct irq_data *d)
>   {
>   	struct irq_chip *chip = irq_data_get_irq_chip(d);
>   	int rc;
> 

Re: [PATCH 5/7] powerpc/powernv/pci: Fix W=1 compile warning

[Christophe Leroy]

Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
>    CC      arch/powerpc/platforms/powernv/pci-ioda.o
> ../arch/powerpc/platforms/powernv/pci-ioda.c: In function ‘pnv_ioda_configure_pe’:
> ../arch/powerpc/platforms/powernv/pci-ioda.c:897:18: error: variable ‘parent’ set but not used [-Werror=unused-but-set-variable]
>    struct pci_dev *parent;
>                    ^~~~~~
> 

A small sentence explaining how this is fixed would be welcome, so that 
you don't need to read the code the know what the commit does to fix the 
warning. Also the subject should be more explicit.


> Cc: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/platforms/powernv/pci-ioda.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 023a4f987bb2..2b4ceb5e6ce4 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -894,7 +894,6 @@ int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
>   
>   int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
>   {
> -	struct pci_dev *parent;
>   	uint8_t bcomp, dcomp, fcomp;
>   	long rc, rid_end, rid;
>   
> @@ -904,7 +903,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
>   
>   		dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
>   		fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
> -		parent = pe->pbus->self;
>   		if (pe->flags & PNV_IODA_PE_BUS_ALL)
>   			count = resource_size(&pe->pbus->busn_res);
>   		else
> @@ -925,12 +923,6 @@ int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
>   		}
>   		rid_end = pe->rid + (count << 8);
>   	} else {
> -#ifdef CONFIG_PCI_IOV
> -		if (pe->flags & PNV_IODA_PE_VF)
> -			parent = pe->parent_dev;
> -		else
> -#endif /* CONFIG_PCI_IOV */
> -			parent = pe->pdev->bus->self;
>   		bcomp = OpalPciBusAll;
>   		dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
>   		fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
> 

Re: [PATCH 6/7] powerpc/perf: Fix W=1 compile warning

[Christophe Leroy]

Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
> CC      arch/powerpc/perf/imc-pmu.o
> ../arch/powerpc/perf/imc-pmu.c: In function ‘trace_imc_event_init’:
> ../arch/powerpc/perf/imc-pmu.c:1429:22: error: variable ‘target’ set but not used [-Werror=unused-but-set-variable]
>    struct task_struct *target;
>                        ^~~~~~

A small sentence explaining how this is fixed would be welcome, so that 
you don't need to read the code the know what the commit does to fix the 
warning. Also the subject should be more explicit.

> 
> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   arch/powerpc/perf/imc-pmu.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index 62d0b54086f8..9ed4fcccf8a9 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -1426,8 +1426,6 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
>   
>   static int trace_imc_event_init(struct perf_event *event)
>   {
> -	struct task_struct *target;
> -
>   	if (event->attr.type != event->pmu->type)
>   		return -ENOENT;
>   
> @@ -1458,7 +1456,6 @@ static int trace_imc_event_init(struct perf_event *event)
>   	mutex_unlock(&imc_global_refc.lock);
>   
>   	event->hw.idx = -1;
> -	target = event->hw.target;
>   
>   	event->pmu->task_ctx_nr = perf_hw_context;
>   	event->destroy = reset_global_refc;
> 

Re: [PATCH 7/7] powerpc/32: Fix W=1 compile warning

[Christophe Leroy]

Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
> CC      arch/powerpc/kernel/traps.o
> ../arch/powerpc/kernel/traps.c:1663:6: error: no previous prototype for ‘stack_overflow_exception’ [-Werror=missing-prototypes]
>   void stack_overflow_exception(struct pt_regs *regs)
>        ^~~~~~~~~~~~~~~~~~~~~~~~
> 

A small sentence explaining how this is fixed would be welcome, so that 
you don't need to read the code the know what the commit does to fix the 
warning. Also the subject should be more explicit.


> Cc: Christophe Leroy <christophe.leroy@c-s.fr>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>


> Fixes: 3978eb78517c ("powerpc/32: Add early stack overflow detection with VMAP stack.")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   arch/powerpc/include/asm/asm-prototypes.h | 1 +

Note that asm-prototypes.h is not the right place for such a prototype, 
but that's probably for another cleanup patch. See discussion at 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/1463534212-4879-2-git-send-email-dja@axtens.net/

Christophe


>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
> index de14b1a34d56..4957119604c7 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -67,6 +67,7 @@ void single_step_exception(struct pt_regs *regs);
>   void program_check_exception(struct pt_regs *regs);
>   void alignment_exception(struct pt_regs *regs);
>   void StackOverflow(struct pt_regs *regs);
> +void stack_overflow_exception(struct pt_regs *regs);
>   void kernel_fp_unavailable_exception(struct pt_regs *regs);
>   void altivec_unavailable_exception(struct pt_regs *regs);
>   void vsx_unavailable_exception(struct pt_regs *regs);
> 

Re: [PATCH 3/7] powerpc/sstep: Fix W=1 compile warning

[Cédric Le Goater]

On 9/11/20 7:38 AM, Christophe Leroy wrote:
> 
> 
> Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
>> ../arch/powerpc/lib/sstep.c: In function ‘mlsd_8lsd_ea’:
>> ../arch/powerpc/lib/sstep.c:225:3: error: suggest braces around empty body in an ‘if’ statement [-Werror=empty-body]
>>     ; /* Invalid form. Should already be checked for by caller! */
>>     ^
> 
> A small sentence explaining how this is fixed would be welcome, so that you don't need to read the code the know what the commit does to fix the warning. Also the subject should be more explicit.
> 
> 
> 
>>
>> Cc: Jordan Niethe <jniethe5@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   arch/powerpc/lib/sstep.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
>> index caee8cc77e19..14572af16e55 100644
>> --- a/arch/powerpc/lib/sstep.c
>> +++ b/arch/powerpc/lib/sstep.c
>> @@ -221,8 +221,9 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
>>           ; /* Leave ea as is */
>>       else if (prefix_r && !ra)
>>           ea += regs->nip;
>> -    else if (prefix_r && ra)
>> +    else if (prefix_r && ra) {
>>           ; /* Invalid form. Should already be checked for by caller! */
>> +    }
> 
> You can't do that. Now checkpatch will complain that you don't have braces on all legs of the if/else dance.

Should we fix checkpatch ?

> I think the last 'else if' should simply be removed entirely as it does nothing. Eventually, just leave the comment, something like:
> 
>     /* (prefix_r && ra) is Invalid form. Should already be checked for by caller! */
> 
> And if (prefix_r && ra) is not possible, then the previous if should just be 'if (prefx_r)'

Indeed. I will rework that one.

Thanks,

C. 


> Christophe
> 
> 
>>         return ea;
>>   }
>>

Re: [PATCH 2/7] powerpc/prom: Fix W=1 compile warning

[Cédric Le Goater]

On 9/11/20 7:33 AM, Christophe Leroy wrote:
> 
> 
> Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
>> arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
>> arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not used [-Werror=unused-but-set-variable]
>>    __be64 *reserve_map;
>>            ^~~~~~~~~~~
>> cc1: all warnings being treated as errors
> 
> A small sentence explaining how this is fixes would be welcome, so that you don't need to read the code the know what the commit does to fix the warning. Also the subject should be more explicit.
> 
> 
>>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   arch/powerpc/kernel/prom.c | 51 +++++++++++++++++++-------------------
>>   1 file changed, 26 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index d8a2fb87ba0c..4bae9ebc7d0b 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -622,11 +622,6 @@ static void __init early_reserve_mem_dt(void)
>>     static void __init early_reserve_mem(void)
>>   {
>> -    __be64 *reserve_map;
>> -
>> -    reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
>> -            fdt_off_mem_rsvmap(initial_boot_params));
>> -
>>       /* Look for the new "reserved-regions" property in the DT */
>>       early_reserve_mem_dt();
>>   @@ -639,28 +634,34 @@ static void __init early_reserve_mem(void)
>>       }
>>   #endif /* CONFIG_BLK_DEV_INITRD */
>>   -#ifdef CONFIG_PPC32
> 
> Instead of such a big change, you could simply do the following in addition to the move of reserve_map allocation after it.
> 
>     if (!IS_ENABLED(CONFIG_PPC32))
>         return;

yes. I will include a change for CONFIG_BLK_DEV_INITRD also.

Thanks,

C.  

> 
>> -    /*
>> -     * Handle the case where we might be booting from an old kexec
>> -     * image that setup the mem_rsvmap as pairs of 32-bit values
>> -     */
>> -    if (be64_to_cpup(reserve_map) > 0xffffffffull) {
>> -        u32 base_32, size_32;
>> -        __be32 *reserve_map_32 = (__be32 *)reserve_map;
>> -
>> -        DBG("Found old 32-bit reserve map\n");
>> -
>> -        while (1) {
>> -            base_32 = be32_to_cpup(reserve_map_32++);
>> -            size_32 = be32_to_cpup(reserve_map_32++);
>> -            if (size_32 == 0)
>> -                break;
>> -            DBG("reserving: %x -> %x\n", base_32, size_32);
>> -            memblock_reserve(base_32, size_32);
>> +    if (IS_ENABLED(CONFIG_PPC32)) {
>> +        __be64 *reserve_map;
>> +
>> +        reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
>> +                 fdt_off_mem_rsvmap(initial_boot_params));
>> +
>> +        /*
>> +         * Handle the case where we might be booting from an
>> +         * old kexec image that setup the mem_rsvmap as pairs
>> +         * of 32-bit values
>> +         */
>> +        if (be64_to_cpup(reserve_map) > 0xffffffffull) {
>> +            u32 base_32, size_32;
>> +            __be32 *reserve_map_32 = (__be32 *)reserve_map;
>> +
>> +            DBG("Found old 32-bit reserve map\n");
>> +
>> +            while (1) {
>> +                base_32 = be32_to_cpup(reserve_map_32++);
>> +                size_32 = be32_to_cpup(reserve_map_32++);
>> +                if (size_32 == 0)
>> +                    break;
>> +                DBG("reserving: %x -> %x\n", base_32, size_32);
>> +                memblock_reserve(base_32, size_32);
>> +            }
>> +            return;
>>           }
>> -        return;
>>       }
>> -#endif
>>   }
>>     #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>>
> 
> 
> Christophe

Re: [PATCH 3/7] powerpc/sstep: Fix W=1 compile warning

[Christophe Leroy]

Le 11/09/2020 à 07:59, Cédric Le Goater a écrit :
> On 9/11/20 7:38 AM, Christophe Leroy wrote:
>>
>>
>> Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
>>> ../arch/powerpc/lib/sstep.c: In function ‘mlsd_8lsd_ea’:
>>> ../arch/powerpc/lib/sstep.c:225:3: error: suggest braces around empty body in an ‘if’ statement [-Werror=empty-body]
>>>      ; /* Invalid form. Should already be checked for by caller! */
>>>      ^
>>
>> A small sentence explaining how this is fixed would be welcome, so that you don't need to read the code the know what the commit does to fix the warning. Also the subject should be more explicit.
>>
>>
>>
>>>
>>> Cc: Jordan Niethe <jniethe5@gmail.com>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>    arch/powerpc/lib/sstep.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
>>> index caee8cc77e19..14572af16e55 100644
>>> --- a/arch/powerpc/lib/sstep.c
>>> +++ b/arch/powerpc/lib/sstep.c
>>> @@ -221,8 +221,9 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
>>>            ; /* Leave ea as is */
>>>        else if (prefix_r && !ra)
>>>            ea += regs->nip;
>>> -    else if (prefix_r && ra)
>>> +    else if (prefix_r && ra) {
>>>            ; /* Invalid form. Should already be checked for by caller! */
>>> +    }
>>
>> You can't do that. Now checkpatch will complain that you don't have braces on all legs of the if/else dance.
> 
> Should we fix checkpatch ?

Why not, not then fix 
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces 
first :)

Christophe

Re: [PATCH 5/7] powerpc/powernv/pci: Fix W=1 compile warning

[Oliver O'Halloran]

On Fri, Sep 11, 2020 at 7:02 AM Cédric Le Goater <clg@kaod.org> wrote:
>
>   CC      arch/powerpc/platforms/powernv/pci-ioda.o
> ../arch/powerpc/platforms/powernv/pci-ioda.c: In function ‘pnv_ioda_configure_pe’:
> ../arch/powerpc/platforms/powernv/pci-ioda.c:897:18: error: variable ‘parent’ set but not used [-Werror=unused-but-set-variable]
>   struct pci_dev *parent;
>                   ^~~~~~
>
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Come to think of it a fix for this might already be in -next, see
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=193967&state=*

If not,

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>

Re: [PATCH 1/7] powerpc/sysfs: Fix W=1 compile warning

[Cédric Le Goater]

On 9/11/20 7:26 AM, Christophe Leroy wrote:
> 
> 
> Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
>> arch/powerpc/kernel/sysfs.c: In function ‘sysfs_create_dscr_default’:
>> arch/powerpc/kernel/sysfs.c:228:7: error: variable ‘err’ set but not used [-Werror=unused-but-set-variable]
>>     int err = 0;
>>         ^~~
>> cc1: all warnings being treated as errors
> 
> A small sentence explaining how this is fixes would be welcome, so that you don't need to read the code the know what the commit does to fix the warning. Even the subject should be more explicite, rather than saying "Fix W=1 compile warning", I think it should say something like "remove unused err variable"

Yes. I will respin a v2 with better commit logs for all.

Thanks,

C.