| Message ID | 20200910210250.1962595-3-clg@kaod.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Fix a few W=1 compile warnings | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (4b552a4cbf286ff9dcdab19153f3c1c7d1680fab) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 65 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
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
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
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
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(-)