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

Hello,

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

Thanks,

C.

Changes in v2:

 - Better commit logs
 - Reworked early_reserve_mem() in prom
 - Remove if statement in sstep

Cédric Le Goater (7):
  powerpc/sysfs: Remove unused 'err' variable in
    sysfs_create_dscr_default()
  powerpc/prom: Introduce early_reserve_mem_old()
  powerpc/sstep: Remove empty if statement checking for invalid form
  powerpc/xive: Make debug routines static
  powerpc/powernv/pci: Remove unused variable 'parent' in
    pnv_ioda_configure_pe()
  powerpc/perf: Remove unused variable 'target' in
    trace_imc_event_init()
  powerpc/32: Declare stack_overflow_exception() prototype

 arch/powerpc/include/asm/asm-prototypes.h |  1 +
 arch/powerpc/kernel/prom.c                | 37 ++++++++++++-----------
 arch/powerpc/kernel/sysfs.c               |  3 +-
 arch/powerpc/lib/sstep.c                  |  9 ++++--
 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, 30 insertions(+), 35 deletions(-)

-- 
2.25.4

This fixes a compile error with W=1.

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)
      ^~~~~~~~~~~~~~~~~~~

Fixes: 930914b7d528 ("powerpc/xive: Add a debugfs file to dump internal XIVE state")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Christophe Leroy <redacted>
---
 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

and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
compile error with W=1.

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 <redacted>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kernel/prom.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index d8a2fb87ba0c..c958b67cf1a5 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c

@@ -620,27 +620,14 @@ static void __init early_reserve_mem_dt(void)
 	}
 }
 
-static void __init early_reserve_mem(void)
+static void __init early_reserve_mem_old(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();
-
-#ifdef CONFIG_BLK_DEV_INITRD
-	/* Then reserve the initrd, if any */
-	if (initrd_start && (initrd_end > initrd_start)) {
-		memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
-			ALIGN(initrd_end, PAGE_SIZE) -
-			ALIGN_DOWN(initrd_start, PAGE_SIZE));
-	}
-#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
 	 */

@@ -658,9 +645,25 @@ static void __init early_reserve_mem(void)
 			DBG("reserving: %x -> %x\n", base_32, size_32);
 			memblock_reserve(base_32, size_32);
 		}
-		return;
 	}
-#endif
+}
+
+static void __init early_reserve_mem(void)
+{
+	/* Look for the new "reserved-regions" property in the DT */
+	early_reserve_mem_dt();
+
+#ifdef CONFIG_BLK_DEV_INITRD
+	/* Then reserve the initrd, if any */
+	if (initrd_start && (initrd_end > initrd_start)) {
+		memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
+			ALIGN(initrd_end, PAGE_SIZE) -
+			ALIGN_DOWN(initrd_start, PAGE_SIZE));
+	}
+#endif /* CONFIG_BLK_DEV_INITRD */
+
+	if (IS_ENABLED(CONFIG_PPC32))
+		early_reserve_mem_old();
 }
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM

-- 
2.25.4

The check should be performed by the caller. This fixes a compile
error with W=1.

../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 <redacted>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/lib/sstep.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index caee8cc77e19..e9dcaba9a4f8 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c

@@ -219,10 +219,13 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,
 		ea += regs->gpr[ra];
 	else if (!prefix_r && !ra)
 		; /* Leave ea as is */
-	else if (prefix_r && !ra)
+	else if (prefix_r)
 		ea += regs->nip;
-	else if (prefix_r && ra)
-		; /* Invalid form. Should already be checked for by caller! */
+
+	/*
+	 * (prefix_r && ra) is an invalid form. Should already be
+	 * checked for by caller!
+	 */
 
 	return ea;
 }

-- 
2.25.4

This fixes a compile error with W=1.

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>
Reviewed-by: Christophe Leroy <redacted>
Reviewed-by: 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

This fixes a compile error with W=1.

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>
Reviewed-by: Christophe Leroy <redacted>
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

This fixes a compile error with W=1.

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 <redacted>
Reviewed-by: Christophe Leroy <redacted>
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

This fixes a compile error with W=1.

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 <redacted>
Fixes: 3978eb78517c ("powerpc/32: Add early stack overflow detection with VMAP stack.")
Reviewed-by: Christophe Leroy <redacted>
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

Cédric Le Goater [off-list ref] a écrit :

and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
compile error with W=1.

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 <redacted>

@csgroup.eu instead of @c-s.fr please

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kernel/prom.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

That's a lot of changes for a tiny warning.

You could make it easy by just replacing the #ifdef by:

         if (!IS_ENABLED(CONFIG_PPC32))
                 return;

Why _old ? Do you mean ppc32 are old ? Modern ADSL boxes like for  
instance the famous French freebox have powerpc32 microcontroller.
Eventually you could name it _ppc32, but I don't think that's the good  
way, see above.

Christophe

On 9/15/20 6:46 PM, Christophe Leroy wrote:

Cédric Le Goater [off-list ref] a écrit :

quoted

and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
compile error with W=1.

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 <redacted>

@csgroup.eu instead of @c-s.fr please

quoted

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/kernel/prom.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

That's a lot of changes for a tiny warning.

You could make it easy by just replacing the #ifdef by:

        if (!IS_ENABLED(CONFIG_PPC32))
                return;

It's equivalent and it moves out the reserve_map variable of the main routine
which I think is better.

quoted

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index d8a2fb87ba0c..c958b67cf1a5 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c

@@ -620,27 +620,14 @@ static void __init early_reserve_mem_dt(void)

     }
 }

-static void __init early_reserve_mem(void)
+static void __init early_reserve_mem_old(void)

Why _old ? Do you mean ppc32 are old ? Modern ADSL boxes like for instance the famous French freebox have powerpc32 microcontroller.
Eventually you could name it _ppc32, but I don't think that's the good way, see above.

I choose old because of the comment ' ... booting from an old kexec ... ', 
but I agree _ppc32 might be a better choice.

Thanks,

C. 

Christophe

quoted

 {
     __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();
-
-#ifdef CONFIG_BLK_DEV_INITRD
-    /* Then reserve the initrd, if any */
-    if (initrd_start && (initrd_end > initrd_start)) {
-        memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
-            ALIGN(initrd_end, PAGE_SIZE) -
-            ALIGN_DOWN(initrd_start, PAGE_SIZE));
-    }
-#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
      */

@@ -658,9 +645,25 @@ static void __init early_reserve_mem(void)

             DBG("reserving: %x -> %x\n", base_32, size_32);
             memblock_reserve(base_32, size_32);
         }
-        return;
     }
-#endif
+}
+
+static void __init early_reserve_mem(void)
+{
+    /* Look for the new "reserved-regions" property in the DT */
+    early_reserve_mem_dt();
+
+#ifdef CONFIG_BLK_DEV_INITRD
+    /* Then reserve the initrd, if any */
+    if (initrd_start && (initrd_end > initrd_start)) {
+        memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
+            ALIGN(initrd_end, PAGE_SIZE) -
+            ALIGN_DOWN(initrd_start, PAGE_SIZE));
+    }
+#endif /* CONFIG_BLK_DEV_INITRD */
+
+    if (IS_ENABLED(CONFIG_PPC32))
+        early_reserve_mem_old();
 }

 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-- 
2.25.4

Reviewed-by: Athira Rajeev <redacted>

Thanks
Athira

	event->pmu->task_ctx_nr = perf_hw_context;
	event->destroy = reset_global_refc;
-- 
2.25.4

On Mon, 14 Sep 2020 23:10:00 +0200, C��dric Le Goater wrote:

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

Thanks,

C.

Changes in v2:

[...]

Patches 1, 3, 4 and 7 applied to powerpc/next.

[1/7] powerpc/sysfs: Remove unused 'err' variable in sysfs_create_dscr_default()
      https://git.kernel.org/powerpc/c/7b2aab5f22f0f7cc9e2f8672c9e65e2e88d30102
[3/7] powerpc/sstep: Remove empty if statement checking for invalid form
      https://git.kernel.org/powerpc/c/5ab187e01a5310e1f9cd2f6b192b2343b8bd14cb
[4/7] powerpc/xive: Make debug routines static
      https://git.kernel.org/powerpc/c/2228f19cf90ef796c8d84f54f3a5db2dcc85c83f
[7/7] powerpc/32: Declare stack_overflow_exception() prototype
      https://git.kernel.org/powerpc/c/ebbfeef0d8093a06ff39c60105b6650be3344cbe

cheers