[U-Boot] [PATCH 21/32] arch/powerpc/cpu/mpc85xx/fdt.c: sparse fixes

Kim Phillips kim.phillips at freescale.com
Wed Oct 17 02:28:37 CEST 2012


this is mostly about using the proper __be32 type for 32-bit device
tree property content variables.

includes uncasting fdt_getprop() callsites, enforcing callsite code to
use __be32 (identified by casting a 32-bit entity).  This allows sparse
to continue to find endianness type mismatches at the callsite.

__be-ifying QE/FMAN firmware struct entries enables sparse to detect
a couple of endian bugs when handling firmware length.  It is assumed
QE/FMAN firmware is defined big-endian.

fdt.c:62:29: warning: incorrect type in assignment (different base types)
fdt.c:62:29:    expected unsigned long long [unsigned] [usertype] val
fdt.c:62:29:    got restricted __be64 [usertype] <noident>
fdt.c:65:65: warning: Using plain integer as NULL pointer
fdt.c:47:6: warning: symbol 'ft_fixup_cpu' was not declared. Should it be static?
fdt.c:57:65: warning: Using plain integer as NULL pointer
fdt.c:112:29: warning: incorrect type in argument 1 (different address spaces)
fdt.c:112:29:    expected unsigned int const volatile [noderef] <asn:2>*addr
fdt.c:112:29:    got unsigned int *<noident>
fdt.c:182:64: warning: Using plain integer as NULL pointer
fdt.c:244:72: warning: Using plain integer as NULL pointer
fdt.c:259:73: warning: Using plain integer as NULL pointer
fdt.c:275:83: warning: Using plain integer as NULL pointer
fdt.c:289:5: warning: symbol 'fdt_fixup_phy_connection' was not declared. Should it be static?
fdt.c:324:73: warning: Using plain integer as NULL pointer
fdt.c:349:6: warning: symbol 'fdt_add_enet_stashing' was not declared. Should it be static?
fdt.c:428:6: warning: symbol 'fdt_fixup_fman_firmware' was not declared. Should it be static?
fdt.c:580:65: warning: Using plain integer as NULL pointer
fdt.c:484:52: warning: incorrect type in argument 2 (different base types)
fdt.c:484:52:    expected int [signed] add_len
fdt.c:484:52:    got restricted __be32 [usertype] length
fdt.c:516:78: warning: incorrect type in argument 5 (different base types)
fdt.c:516:78:    expected int [signed] len
fdt.c:516:78:    got restricted __be32 [usertype] length
fdt.c:582:21: warning: incorrect type in assignment (different base types)
fdt.c:582:21:    expected int [signed] val
fdt.c:582:21:    got restricted __be32 [usertype] <noident>

ft_verify_fdt is a weak symbol, so this still gets emitted:

fdt.c:708:5: warning: symbol 'ft_verify_fdt' was not declared. Should it be static?

perhaps we should stop using weak symbols.

Signed-off-by: Kim Phillips <kim.phillips at freescale.com>
---
 arch/powerpc/cpu/mpc85xx/fdt.c | 55 +++++++++++++++++++++++-------------------
 arch/powerpc/cpu/mpc8xxx/fdt.c |  3 ++-
 drivers/qe/qe.h                | 34 +++++++++++++-------------
 3 files changed, 49 insertions(+), 43 deletions(-)

diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
index e57d69c..0013e42 100644
--- a/arch/powerpc/cpu/mpc85xx/fdt.c
+++ b/arch/powerpc/cpu/mpc85xx/fdt.c
@@ -44,7 +44,7 @@ extern void ft_srio_setup(void *blob);
 #ifdef CONFIG_MP
 #include "mp.h"
 
-void ft_fixup_cpu(void *blob, u64 memory_limit)
+static void ft_fixup_cpu(void *blob, u64 memory_limit)
 {
 	int off;
 	ulong spin_tbl_addr = get_spin_phys_addr();
@@ -54,13 +54,15 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
 
 	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4);
 	while (off != -FDT_ERR_NOTFOUND) {
-		u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", 0);
+		const __be32 *reg = fdt_getprop(blob, off, "reg", NULL);
 
 		if (reg) {
-			u32 phys_cpu_id = thread_to_core(*reg);
-			u64 val = phys_cpu_id * SIZE_BOOT_ENTRY + spin_tbl_addr;
-			val = cpu_to_fdt64(val);
-			if (*reg == id) {
+			u32 phys_cpu_id = thread_to_core(fdt32_to_cpu(*reg));
+			__be64 val;
+
+			val = cpu_to_fdt64(phys_cpu_id * SIZE_BOOT_ENTRY +
+					   spin_tbl_addr);
+			if (fdt32_to_cpu(*reg) == id) {
 				fdt_setprop_string(blob, off, "status",
 								"okay");
 			} else {
@@ -106,7 +108,7 @@ void ft_fixup_cpu(void *blob, u64 memory_limit)
 static inline void ft_fixup_l3cache(void *blob, int off)
 {
 	u32 line_size, num_ways, size, num_sets;
-	cpc_corenet_t *cpc = (void *)CONFIG_SYS_FSL_CPC_ADDR;
+	cpc_corenet_t __iomem *cpc = (void __iomem *)CONFIG_SYS_FSL_CPC_ADDR;
 	u32 cfg0 = in_be32(&cpc->cpccfg0);
 
 	size = CPC_CFG0_SZ_K(cfg0) * 1024 * CONFIG_SYS_NUM_CPC;
@@ -163,7 +165,7 @@ static inline u32 l2cache_size(void)
 static inline void ft_fixup_l2cache(void *blob)
 {
 	int len, off;
-	u32 *ph;
+	const __be32 *ph;
 	struct cpu_type *cpu = identify_cpu(SVR_SOC_VER(get_svr()));
 
 	const u32 line_size = 32;
@@ -177,14 +179,14 @@ static inline void ft_fixup_l2cache(void *blob)
 		return;
 	}
 
-	ph = (u32 *)fdt_getprop(blob, off, "next-level-cache", 0);
+	ph = fdt_getprop(blob, off, "next-level-cache", NULL);
 
 	if (ph == NULL) {
 		debug("no next-level-cache property\n");
 		return ;
 	}
 
-	off = fdt_node_offset_by_phandle(blob, *ph);
+	off = fdt_node_offset_by_phandle(blob, fdt32_to_cpu(*ph));
 	if (off < 0) {
 		printf("%s: %s\n", __func__, fdt_strerror(off));
 		return ;
@@ -224,7 +226,7 @@ static inline void ft_fixup_l2cache(void *blob)
 static inline void ft_fixup_l2cache(void *blob)
 {
 	int off, l2_off, l3_off = -1;
-	u32 *ph;
+	const __be32 *ph;
 	u32 l2cfg0 = mfspr(SPRN_L2CFG0);
 	u32 size, line_size, num_ways, num_sets;
 	int has_l2 = 1;
@@ -241,14 +243,14 @@ static inline void ft_fixup_l2cache(void *blob)
 	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4);
 
 	while (off != -FDT_ERR_NOTFOUND) {
-		ph = (u32 *)fdt_getprop(blob, off, "next-level-cache", 0);
+		ph = fdt_getprop(blob, off, "next-level-cache", NULL);
 
 		if (ph == NULL) {
 			debug("no next-level-cache property\n");
 			goto next;
 		}
 
-		l2_off = fdt_node_offset_by_phandle(blob, *ph);
+		l2_off = fdt_node_offset_by_phandle(blob, fdt32_to_cpu(*ph));
 		if (l2_off < 0) {
 			printf("%s: %s\n", __func__, fdt_strerror(off));
 			goto next;
@@ -256,7 +258,7 @@ static inline void ft_fixup_l2cache(void *blob)
 
 		if (has_l2) {
 #ifdef CONFIG_SYS_CACHE_STASHING
-			u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", 0);
+			u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", NULL);
 			if (reg)
 				fdt_setprop_cell(blob, l2_off, "cache-stash-id",
 					 (*reg * 2) + 32 + 1);
@@ -272,13 +274,14 @@ static inline void ft_fixup_l2cache(void *blob)
 		}
 
 		if (l3_off < 0) {
-			ph = (u32 *)fdt_getprop(blob, l2_off, "next-level-cache", 0);
+			ph = fdt_getprop(blob, l2_off, "next-level-cache",
+					 NULL);
 
 			if (ph == NULL) {
 				debug("no next-level-cache property\n");
 				goto next;
 			}
-			l3_off = *ph;
+			l3_off = fdt32_to_cpu(*ph);
 		}
 next:
 		off = fdt_node_offset_by_prop_value(blob, off,
@@ -321,7 +324,7 @@ static inline void ft_fixup_cache(void *blob)
 
 #ifdef CONFIG_SYS_CACHE_STASHING
 		{
-			u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", 0);
+			u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", NULL);
 			if (reg)
 				fdt_setprop_cell(blob, off, "cache-stash-id",
 					 (*reg * 2) + 32 + 0);
@@ -346,7 +349,7 @@ static inline void ft_fixup_cache(void *blob)
 }
 
 
-void fdt_add_enet_stashing(void *fdt)
+static void fdt_add_enet_stashing(void *fdt)
 {
 	do_fixup_by_compat(fdt, "gianfar", "bd-stash", NULL, 0, 1);
 
@@ -425,13 +428,13 @@ static void ft_fixup_qe_snum(void *blob)
  * be able to parse the binary data to determine any attributes it needs.
  */
 #ifdef CONFIG_SYS_DPAA_FMAN
-void fdt_fixup_fman_firmware(void *blob)
+static void fdt_fixup_fman_firmware(void *blob)
 {
 	int rc, fmnode, fwnode = -1;
 	uint32_t phandle;
 	struct qe_firmware *fmanfw;
 	const struct qe_header *hdr;
-	unsigned int length;
+	unsigned int length, fw_length;
 	uint32_t crc;
 	const char *p;
 
@@ -471,14 +474,16 @@ void fdt_fixup_fman_firmware(void *blob)
 	}
 
 	length -= sizeof(u32);	/* Subtract the size of the CRC */
-	crc = be32_to_cpu(*(u32 *)((void *)fmanfw + length));
+	crc = be32_to_cpu(*(__be32 *)((void *)fmanfw + length));
 	if (crc != crc32_no_comp(0, (void *)fmanfw, length)) {
 		printf("Fman firmware at %p has invalid CRC\n", fmanfw);
 		return;
 	}
 
+	fw_length = be32_to_cpu(fmanfw->header.length);
+
 	/* Increase the size of the fdt to make room for the node. */
-	rc = fdt_increase_size(blob, fmanfw->header.length);
+	rc = fdt_increase_size(blob, fw_length);
 	if (rc < 0) {
 		printf("Unable to make room for Fman firmware: %s\n",
 			fdt_strerror(rc));
@@ -510,7 +515,7 @@ void fdt_fixup_fman_firmware(void *blob)
 		       fdt_strerror(rc));
 		return;
 	}
-	rc = fdt_setprop(blob, fwnode, "fsl,firmware", fmanfw, fmanfw->header.length);
+	rc = fdt_setprop(blob, fwnode, "fsl,firmware", fmanfw, fw_length);
 	if (rc < 0) {
 		char s[64];
 		fdt_get_path(blob, fwnode, s, sizeof(s));
@@ -559,7 +564,7 @@ static void fdt_fixup_usb(void *fdt)
 void ft_cpu_setup(void *blob, bd_t *bd)
 {
 	int off;
-	int val;
+	__be32 val;
 	sys_info_t sysinfo;
 
 	/* delete crypto node if not on an E-processor */
@@ -577,7 +582,7 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 	get_sys_info(&sysinfo);
 	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4);
 	while (off != -FDT_ERR_NOTFOUND) {
-		u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", 0);
+		u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", NULL);
 		val = cpu_to_fdt32(sysinfo.freqProcessor[*reg]);
 		fdt_setprop(blob, off, "clock-frequency", &val, 4);
 		off = fdt_node_offset_by_prop_value(blob, off, "device_type",
diff --git a/arch/powerpc/cpu/mpc8xxx/fdt.c b/arch/powerpc/cpu/mpc8xxx/fdt.c
index 32ab050..af2545d 100644
--- a/arch/powerpc/cpu/mpc8xxx/fdt.c
+++ b/arch/powerpc/cpu/mpc8xxx/fdt.c
@@ -27,6 +27,7 @@
 #include <libfdt.h>
 #include <fdt_support.h>
 #include <asm/mp.h>
+#include <asm/fsl_enet.h>
 #include <asm/fsl_serdes.h>
 #include <phy.h>
 #include <hwconfig.h>
@@ -61,7 +62,7 @@ void ft_fixup_num_cores(void *blob) {
 
 	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4);
 	while (off != -FDT_ERR_NOTFOUND) {
-		u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", 0);
+		u32 *reg = (u32 *)fdt_getprop(blob, off, "reg", NULL);
 		u32 phys_cpu_id = thread_to_core(*reg);
 
 		if (!is_core_valid(phys_cpu_id) || is_core_disabled(phys_cpu_id)) {
diff --git a/drivers/qe/qe.h b/drivers/qe/qe.h
index faad43c..3914fab 100644
--- a/drivers/qe/qe.h
+++ b/drivers/qe/qe.h
@@ -242,7 +242,7 @@ typedef enum qe_clock {
  */
 struct qe_firmware {
 	struct qe_header {
-		u32 length;	/* Length of the entire structure, in bytes */
+		__be32 length;	/* Length of the entire structure, in bytes */
 		u8 magic[3];	/* Set to { 'Q', 'E', 'F' } */
 		u8 version;	/* Version of this layout. First ver is '1' */
 	} header;
@@ -250,26 +250,26 @@ struct qe_firmware {
 	u8 split;		/* 0 = shared I-RAM, 1 = split I-RAM */
 	u8 count;		/* Number of microcode[] structures */
 	struct {
-		u16 model;	/* The SOC model  */
+		__be16 model;	/* The SOC model  */
 		u8 major;	/* The SOC revision major */
 		u8 minor;	/* The SOC revision minor */
 	} __attribute__ ((packed)) soc;
 	u8 padding[4];		/* Reserved, for alignment */
-	u64 extended_modes;	/* Extended modes */
-	u32 vtraps[8];		/* Virtual trap addresses */
+	__be64 extended_modes;	/* Extended modes */
+	__be32 vtraps[8];	/* Virtual trap addresses */
 	u8 reserved[4];		/* Reserved, for future expansion */
 	struct qe_microcode {
-		u8 id[32];	/* Null-terminated identifier */
-		u32 traps[16];	/* Trap addresses, 0 == ignore */
-		u32 eccr;	/* The value for the ECCR register */
-		u32 iram_offset;/* Offset into I-RAM for the code */
-		u32 count;	/* Number of 32-bit words of the code */
-		u32 code_offset;/* Offset of the actual microcode */
-		u8 major;	/* The microcode version major */
-		u8 minor;	/* The microcode version minor */
-		u8 revision;	/* The microcode version revision */
-		u8 padding;	/* Reserved, for alignment */
-		u8 reserved[4];	/* Reserved, for future expansion */
+		u8 id[32];		/* Null-terminated identifier */
+		__be32 traps[16];	/* Trap addresses, 0 == ignore */
+		__be32 eccr;		/* The value for the ECCR register */
+		__be32 iram_offset;	/* Offset into I-RAM for the code */
+		__be32 count;		/* Number of 32-bit words of the code */
+		__be32 code_offset;	/* Offset of the actual microcode */
+		u8 major;		/* The microcode version major */
+		u8 minor;		/* The microcode version minor */
+		u8 revision;		/* The microcode version revision */
+		u8 padding;		/* Reserved, for alignment */
+		u8 reserved[4];		/* Reserved, for future expansion */
 	} __attribute__ ((packed)) microcode[1];
 	/* All microcode binaries should be located here */
 	/* CRC32 should be located here, after the microcode binaries */
@@ -277,8 +277,8 @@ struct qe_firmware {
 
 struct qe_firmware_info {
 	char id[64];		/* Firmware name */
-	u32 vtraps[8];		/* Virtual trap addresses */
-	u64 extended_modes;	/* Extended modes */
+	__be32 vtraps[8];		/* Virtual trap addresses */
+	__be64 extended_modes;	/* Extended modes */
 };
 
 void qe_config_iopin(u8 port, u8 pin, int dir, int open_drain, int assign);
-- 
1.7.12.3




More information about the U-Boot mailing list