On Wed, 8 Dec 2010 10:59:44 -0800 Deepak Saxena deepak_saxena@mentor.com wrote:
On 12/07/2010 01:22 PM, Scott Wood wrote:
On Mon, 6 Dec 2010 16:56:26 -0800 Deepak Saxena deepak_saxena@mentor.com wrote:
+/*
- Check to see if an valid memory/reg property exists
- in the fdt. If so, we do not overwrite it with what's
- been scanned.
- Valid mean all the following:
- Memory node has a device-type of "memory"
- A reg property exists which:
- has exactly as many cells as #address-cells + #size-cells
- provides a range that is within [bi_memstart, bi_memstart +
bi_memsize]
- */
This will get false positives -- a lot of existing device tree templates have something like this:
ACK. The code in the patch actually checks for this, just didn't point it out in the comments.
Ah, I looked for that and missed it. But there are also some boards that have socketed RAM, but for some reason have a real memory size in the device tree (corresponding to what the board ships with, probably). I think this is something that should have to be selectable by the board config (for image size reasons as well).
Also some nits:
+static int ft_validate_memory(void *blob, bd_t *bd) +{
- int nodeoffset;
- u32 *addrcell = (u32*)fdt_getprop(blob, 0, "#address-cells", NULL);
- u32 *sizecell = (u32*)fdt_getprop(blob, 0, "#size-cells", NULL);
const u32 *, and eliminate the cast.
- u64 reg_base, reg_size;
- void *reg, *dtype;
- int len;
- if ((nodeoffset = fdt_path_offset(blob, "/memory")) >= 0)
- {
Brace on same line as if
Don't put the assignment inside the if statement.
dtype = fdt_getprop(blob, nodeoffset, "device_type", &len);if (!dtype || (strcmp(dtype, "memory") != 0))return 0;reg = fdt_getprop(blob, nodeoffset, "reg", &len);if (reg && len == ((*addrcell + *sizecell) * 4)) {if (*addrcell == 2) {reg_base = ((u64*)reg)[0];reg_size = ((u64*)reg)[1];} else {reg_base = ((u32*)reg)[0];reg_size = ((u32*)reg)[1];}
This only works on big-endian.
if ((reg_size) &&(reg_base >= (u64)bd->bi_memstart) &&((reg_size + reg_base)<= ((u64)bd->bi_memstart + (u64)bd->bi_memsize)))return 1;}
Probably want to complain to the user if reg is invalid and not zero/missing.
-Scott