Re: [PATCH 1/3] powerpc/boot: Fix simpleboot CPU node lookup check

[IBM]

Thorsten Blum <thorsten.blum@xxxxxxxxx> writes:

> fdt_node_offset_by_prop_value() returns a negative error code on
> failure - fix the check accordingly.
>
> Fixes: d2477b5cc8ca ("[POWERPC] bootwrapper: Add a firmware-independent simpleboot target.")
> Signed-off-by: Thorsten Blum <thorsten.blum@xxxxxxxxx>

yup, if it cannot find the node, then it should return a negative error
code, since cpu node is never the root node. The same thing was anyway
properly done for memory device node in the same function few lines
before...

	/* Find the memory range */
	node = fdt_node_offset_by_prop_value(_dtb_start, -1, "device_type",
					     "memory", sizeof("memory"));
	if (node < 0)
		fatal("Cannot find memory node\n");

...but for cpu node, it uses a wrong comparison check.

Looks good to me. Please feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>

> ---
>  arch/powerpc/boot/simpleboot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/simpleboot.c b/arch/powerpc/boot/simpleboot.c
> index c80691d83880..27591df41e9e 100644
> --- a/arch/powerpc/boot/simpleboot.c
> +++ b/arch/powerpc/boot/simpleboot.c
> @@ -68,7 +68,7 @@ void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
>  	/* finally, setup the timebase */
>  	node = fdt_node_offset_by_prop_value(_dtb_start, -1, "device_type",
>  					     "cpu", sizeof("cpu"));
> -	if (!node)
> +	if (node < 0)
>  		fatal("Cannot find cpu node\n");
>  	timebase = fdt_getprop(_dtb_start, node, "timebase-frequency", &size);
>  	if (timebase && (size == 4))