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

[Christophe Leroy <christophe.leroy@csgroup.eu>]

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