[PATCH 1/1] powerpc: fix fdt_fixup_liodn_tbl_fman()

[Heinrich Schuchardt]

On 10/13/22 18:12, Marek Behún wrote:
> On Wed, 12 Oct 2022 19:13:11 +0200
> Heinrich Schuchardt <heinrich.schuchardt at canonical.com> wrote:
> 
>> Builiding with GCC 12.2 fails:
>>
>>      arch/powerpc/cpu/mpc85xx/liodn.c: In function 'fdt_fixup_liodn_tbl_fman':
>>      arch/powerpc/cpu/mpc85xx/liodn.c:340:35: error: the comparison will
>>      always evaluate as 'false' for the address of 'compat'
>>      will never be NULL [-Werror=address]
>>        340 |                 if (tbl[i].compat == NULL)
>>            |
>>
>> Remove the superfluous check.
>>
>> Fixes: 97a8d010e029 ("net/fman: Support both new and legacy FMan Compatibles")
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>>   arch/powerpc/cpu/mpc85xx/liodn.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/arch/powerpc/cpu/mpc85xx/liodn.c b/arch/powerpc/cpu/mpc85xx/liodn.c
>> index a084002494..2d55916841 100644
>> --- a/arch/powerpc/cpu/mpc85xx/liodn.c
>> +++ b/arch/powerpc/cpu/mpc85xx/liodn.c
>> @@ -337,9 +337,6 @@ static void fdt_fixup_liodn_tbl_fman(void *blob,
>>   	for (i = 0; i < sz; i++) {
>>   		int off;
>>   
>> -		if (tbl[i].compat == NULL)
>> -			continue;
>> -
>>   		/* Try the new compatible first.
>>   		 * If the node is missing, try the old.
>>   		 */
> 
> This is the wrong fix, IMO. Instead we should do something like
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/liodn.c b/arch/powerpc/cpu/mpc85xx/liodn.c
> index a084002494..41b7d53ec3 100644
> --- a/arch/powerpc/cpu/mpc85xx/liodn.c
> +++ b/arch/powerpc/cpu/mpc85xx/liodn.c
> @@ -337,7 +337,7 @@ static void fdt_fixup_liodn_tbl_fman(void *blob,
>   	for (i = 0; i < sz; i++) {
>   		int off;
>   
> -		if (tbl[i].compat == NULL)
> +		if (tbl[i].compat[0] == NULL)
>   			continue;
>   
>   		/* Try the new compatible first.
> @@ -345,7 +345,7 @@ static void fdt_fixup_liodn_tbl_fman(void *blob,
>   		  */
>   		off = fdt_node_offset_by_compat_reg(blob,
>   				tbl[i].compat[0], tbl[i].compat_offset);
> -		if (off < 0)
> +		if (off < 0 && tbl[i].compat[1] != NULL)
>   			off = fdt_node_offset_by_compat_reg(blob,
>   					tbl[i].compat[1], tbl[i].compat_offset);

There are two orthogonal changes here:

* removing a superfluous check.
* adding new ones

According to your review there seems to be nothing wrong in removing the 
old check.

But if you think that a check of compat[i] is needed and you prefer to 
create a patch combining both changes, please, go ahead.

Best regards

Heinrich