diff mbox series

[v2,2/4] dm: core: fix signedness in debug messages

Message ID 20240611-misc-20240610-v2-2-028e82b0b620@cherry.de
State Accepted
Commit 29010cd31be2bc6f674459137b5dcc054b77c042
Delegated to: Simon Glass
Headers show
Series dm: core: fix several debug messages and migrate debug() to dm_warn | expand

Commit Message

Quentin Schulz June 11, 2024, 1:04 p.m. UTC 
From: Quentin Schulz <quentin.schulz@cherry.de>

outp always point to an unsigned type in ofnode_read_u* functions but
the format specifier is currently always using signed type.

This is an issue since the signed type can only contain half of the
unsigned type values above 0.

However, this now breaks another usecase. Indeed,
ofnode_read_s32_default is actually passing an s32 but it'll be printed
as a u32 instead. But since the function is called u32, it makes more
sense to have it print an unsigned value.

This was discovered because arm,smc-id = <0x82000010>; on RK3588S is
above the max signed value and therefore would return a negative signed
decimal value instead of its proper unsigned one.

Fixes: fa12dfa08a7b ("dm: core: support reading a single indexed u64 value")
Fixes: 4bb7075c830c ("dm: core: support reading a single indexed u32 value")
Fixes: 7e5196c409f1 ("dm: core: Add ofnode function to read a 64-bit int")
Fixes: 9e51204527dc ("dm: core: Add operations on device tree references")
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 drivers/core/ofnode.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Simon Glass June 11, 2024, 6:51 p.m. UTC | #1 
On Tue, 11 Jun 2024 at 07:04, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@cherry.de>
>
> outp always point to an unsigned type in ofnode_read_u* functions but
> the format specifier is currently always using signed type.
>
> This is an issue since the signed type can only contain half of the
> unsigned type values above 0.
>
> However, this now breaks another usecase. Indeed,
> ofnode_read_s32_default is actually passing an s32 but it'll be printed
> as a u32 instead. But since the function is called u32, it makes more
> sense to have it print an unsigned value.
>
> This was discovered because arm,smc-id = <0x82000010>; on RK3588S is
> above the max signed value and therefore would return a negative signed
> decimal value instead of its proper unsigned one.
>
> Fixes: fa12dfa08a7b ("dm: core: support reading a single indexed u64 value")
> Fixes: 4bb7075c830c ("dm: core: support reading a single indexed u32 value")
> Fixes: 7e5196c409f1 ("dm: core: Add ofnode function to read a 64-bit int")
> Fixes: 9e51204527dc ("dm: core: Add operations on device tree references")
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  drivers/core/ofnode.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
diff mbox series

Patch

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 9a5eaaa4d13..9ff46460e7d 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -326,7 +326,7 @@  int ofnode_read_u8(ofnode node, const char *propname, u8 *outp)
 		return -EINVAL;
 	}
 	*outp = *cell;
-	debug("%#x (%d)\n", *outp, *outp);
+	debug("%#x (%u)\n", *outp, *outp);
 
 	return 0;
 }
@@ -357,7 +357,7 @@  int ofnode_read_u16(ofnode node, const char *propname, u16 *outp)
 		return -EINVAL;
 	}
 	*outp = be16_to_cpup(cell);
-	debug("%#x (%d)\n", *outp, *outp);
+	debug("%#x (%u)\n", *outp, *outp);
 
 	return 0;
 }
@@ -409,7 +409,7 @@  int ofnode_read_u32_index(ofnode node, const char *propname, int index,
 	}
 
 	*outp = fdt32_to_cpu(cell[index]);
-	debug("%#x (%d)\n", *outp, *outp);
+	debug("%#x (%u)\n", *outp, *outp);
 
 	return 0;
 }
@@ -439,7 +439,7 @@  int ofnode_read_u64_index(ofnode node, const char *propname, int index,
 	}
 
 	*outp = fdt64_to_cpu(cell[index]);
-	debug("%#llx (%lld)\n", *outp, *outp);
+	debug("%#llx (%llu)\n", *outp, *outp);
 
 	return 0;
 }
@@ -479,7 +479,7 @@  int ofnode_read_u64(ofnode node, const char *propname, u64 *outp)
 		return -EINVAL;
 	}
 	*outp = fdt64_to_cpu(cell[0]);
-	debug("%#llx (%lld)\n", (unsigned long long)*outp,
+	debug("%#llx (%llu)\n", (unsigned long long)*outp,
 	      (unsigned long long)*outp);
 
 	return 0;