From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-10 21:07:20
../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 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
@@ -221,8 +221,9 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,;/* Leave ea as is */elseif(prefix_r&&!ra)ea+=regs->nip;-elseif(prefix_r&&ra)+elseif(prefix_r&&ra){;/* Invalid form. Should already be checked for by caller! */+}returnea;}
From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-10 21:09:47
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(-)
From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-10 21:11:59
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 | 51 +++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 25 deletions(-)
@@ -622,11 +622,6 @@ static void __init early_reserve_mem_dt(void)staticvoid__initearly_reserve_mem(void){-__be64*reserve_map;--reserve_map=(__be64*)(((unsignedlong)initial_boot_params)+-fdt_off_mem_rsvmap(initial_boot_params));-/* Look for the new "reserved-regions" property in the DT */early_reserve_mem_dt();
From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-10 21:16:47
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(-)
From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-10 21:20:16
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>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
arch/powerpc/perf/imc-pmu.c | 3 ---
1 file changed, 3 deletions(-)
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>
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.
@@ -622,11 +622,6 @@ static void __init early_reserve_mem_dt(void)staticvoid__initearly_reserve_mem(void){-__be64*reserve_map;--reserve_map=(__be64*)(((unsignedlong)initial_boot_params)+-fdt_off_mem_rsvmap(initial_boot_params));-/* Look for the new "reserved-regions" property in the DT */early_reserve_mem_dt();
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
../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.
@@ -221,8 +221,9 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,;/* Leave ea as is */elseif(prefix_r&&!ra)ea+=regs->nip;-elseif(prefix_r&&ra)+elseif(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
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.
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>
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 <redacted>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
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 <redacted>
Reviewed-by: Christophe Leroy <redacted>
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 +
From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-11 06:02:06
On 9/11/20 7:38 AM, Christophe Leroy wrote:
Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
quoted
../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.
@@ -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)'
From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-11 06:04:18
On 9/11/20 7:33 AM, Christophe Leroy wrote:
Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
quoted
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.
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.
quoted
- /*
- * 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
../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.
@@ -221,8 +221,9 @@ static nokprobe_inline unsigned long mlsd_8lsd_ea(unsigned int instr,;/* Leave ea as is */elseif(prefix_r&&!ra)ea+=regs->nip;-elseif(prefix_r&&ra)+elseif(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.
On Fri, Sep 11, 2020 at 7:02 AM Cédric Le Goater [off-list ref] 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>
From: Cédric Le Goater <clg@kaod.org> Date: 2020-09-11 09:12:31
On 9/11/20 7:26 AM, Christophe Leroy wrote:
Le 10/09/2020 à 23:02, Cédric Le Goater a écrit :
quoted
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.