[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[Kumar Gala]

Add the function fdt_fixup_memory() to fixup the /memory node of the fdt

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---

Had a copy/paste error with start & size

(I've update reset & updated my libfdt_testing branch)

 common/fdt_support.c  |   78 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/fdt_support.h |    1 +
 2 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 69f4dd5..808ec70 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -438,6 +438,84 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat,
 	do_fixup_by_compat(fdt, compat, prop, &val, 4, create);
 }

+int fdt_fixup_memory(void *blob, u64 start, u64 size)
+{
+	int err, nodeoffset, len = 0;
+	u8 tmp[16];
+	const u32 *addrcell, *sizecell;
+
+	err = fdt_check_header(blob);
+	if (err < 0) {
+		printf("%s: %s\n", __FUNCTION__, fdt_strerror(err));
+		return err;
+	}
+
+	/* update, or add and update /memory node */
+	nodeoffset = fdt_path_offset(blob, "/memory");
+	if (nodeoffset < 0) {
+		nodeoffset = fdt_add_subnode(blob, 0, "memory");
+		if (nodeoffset < 0)
+			printf("WARNING: could not create /memory: %s.\n",
+					fdt_strerror(nodeoffset));
+		return nodeoffset;
+	}
+	err = fdt_setprop(blob, nodeoffset, "device_type", "memory",
+			sizeof("memory"));
+	if (err < 0) {
+		printf("WARNING: could not set %s %s.\n", "device_type",
+				fdt_strerror(err));
+		return err;
+	}
+
+	addrcell = fdt_getprop(blob, 0, "#address-cells", NULL);
+	/* use shifts and mask to ensure endianness */
+	if ((addrcell) && (*addrcell == 2)) {
+		tmp[0] = (start >> 56) & 0xff;
+		tmp[1] = (start >> 48) & 0xff;
+		tmp[2] = (start >> 40) & 0xff;
+		tmp[3] = (start >> 32) & 0xff;
+		tmp[4] = (start >> 24) & 0xff;
+		tmp[5] = (start >> 16) & 0xff;
+		tmp[6] = (start >>  8) & 0xff;
+		tmp[7] = (start      ) & 0xff;
+		len = 8;
+	} else {
+		tmp[0] = (start >> 24) & 0xff;
+		tmp[1] = (start >> 16) & 0xff;
+		tmp[2] = (start >>  8) & 0xff;
+		tmp[3] = (start      ) & 0xff;
+		len = 4;
+	}
+
+	sizecell = fdt_getprop(blob, 0, "#size-cells", NULL);
+	/* use shifts and mask to ensure endianness */
+	if ((sizecell) && (*sizecell == 2)) {
+		tmp[0+len] = (size >> 56) & 0xff;
+		tmp[1+len] = (size >> 48) & 0xff;
+		tmp[2+len] = (size >> 40) & 0xff;
+		tmp[3+len] = (size >> 32) & 0xff;
+		tmp[4+len] = (size >> 24) & 0xff;
+		tmp[5+len] = (size >> 16) & 0xff;
+		tmp[6+len] = (size >>  8) & 0xff;
+		tmp[7+len] = (size      ) & 0xff;
+		len += 8;
+	} else {
+		tmp[0+len] = (size >> 24) & 0xff;
+		tmp[1+len] = (size >> 16) & 0xff;
+		tmp[2+len] = (size >>  8) & 0xff;
+		tmp[3+len] = (size      ) & 0xff;
+		len += 4;
+	}
+
+	err = fdt_setprop(blob, nodeoffset, "reg", tmp, len);
+	if (err < 0) {
+		printf("WARNING: could not set %s %s.\n",
+				"reg", fdt_strerror(err));
+		return err;
+	}
+	return 0;
+}
+
 void fdt_fixup_ethernet(void *fdt, bd_t *bd)
 {
 	int node;
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 8f781d4..3d6c1a8 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -44,6 +44,7 @@ void do_fixup_by_compat(void *fdt, const char *compat,
 			const char *prop, const void *val, int len, int create);
 void do_fixup_by_compat_u32(void *fdt, const char *compat,
 			    const char *prop, u32 val, int create);
+int fdt_fixup_memory(void *blob, u64 start, u64 size);
 void fdt_fixup_ethernet(void *fdt, bd_t *bd);

 #ifdef CONFIG_OF_HAS_UBOOT_ENV
-- 
1.5.3.4

[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[Scott Wood]

Kumar Gala wrote:
> +	addrcell = fdt_getprop(blob, 0, "#address-cells", NULL);
> +	/* use shifts and mask to ensure endianness */
> +	if ((addrcell) && (*addrcell == 2)) {
> +		tmp[0] = (start >> 56) & 0xff;
> +		tmp[1] = (start >> 48) & 0xff;
> +		tmp[2] = (start >> 40) & 0xff;
> +		tmp[3] = (start >> 32) & 0xff;
> +		tmp[4] = (start >> 24) & 0xff;
> +		tmp[5] = (start >> 16) & 0xff;
> +		tmp[6] = (start >>  8) & 0xff;
> +		tmp[7] = (start      ) & 0xff;
> +		len = 8;
> +	} else {
> +		tmp[0] = (start >> 24) & 0xff;
> +		tmp[1] = (start >> 16) & 0xff;
> +		tmp[2] = (start >>  8) & 0xff;
> +		tmp[3] = (start      ) & 0xff;
> +		len = 4;
> +	}

Could we please use the existing byteswapping functions?

-Scott

[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[Kumar Gala]

On Nov 28, 2007, at 4:16 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> +	addrcell = fdt_getprop(blob, 0, "#address-cells", NULL);
>> +	/* use shifts and mask to ensure endianness */
>> +	if ((addrcell) && (*addrcell == 2)) {
>> +		tmp[0] = (start >> 56) & 0xff;
>> +		tmp[1] = (start >> 48) & 0xff;
>> +		tmp[2] = (start >> 40) & 0xff;
>> +		tmp[3] = (start >> 32) & 0xff;
>> +		tmp[4] = (start >> 24) & 0xff;
>> +		tmp[5] = (start >> 16) & 0xff;
>> +		tmp[6] = (start >>  8) & 0xff;
>> +		tmp[7] = (start      ) & 0xff;
>> +		len = 8;
>> +	} else {
>> +		tmp[0] = (start >> 24) & 0xff;
>> +		tmp[1] = (start >> 16) & 0xff;
>> +		tmp[2] = (start >>  8) & 0xff;
>> +		tmp[3] = (start      ) & 0xff;
>> +		len = 4;
>> +	}
>
> Could we please use the existing byteswapping functions?

I don't see how to use them w/o going through an additional memcpy.

If you can provide a better solution I'm all for it.  But I don't see  
any reason to hold up the patch at this point.

- k

[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[Scott Wood]

Kumar Gala wrote:
> 
> On Nov 28, 2007, at 4:16 PM, Scott Wood wrote:
> 
>> Kumar Gala wrote:
>>> +    addrcell = fdt_getprop(blob, 0, "#address-cells", NULL);
>>> +    /* use shifts and mask to ensure endianness */
>>> +    if ((addrcell) && (*addrcell == 2)) {
>>> +        tmp[0] = (start >> 56) & 0xff;
>>> +        tmp[1] = (start >> 48) & 0xff;
>>> +        tmp[2] = (start >> 40) & 0xff;
>>> +        tmp[3] = (start >> 32) & 0xff;
>>> +        tmp[4] = (start >> 24) & 0xff;
>>> +        tmp[5] = (start >> 16) & 0xff;
>>> +        tmp[6] = (start >>  8) & 0xff;
>>> +        tmp[7] = (start      ) & 0xff;
>>> +        len = 8;
>>> +    } else {
>>> +        tmp[0] = (start >> 24) & 0xff;
>>> +        tmp[1] = (start >> 16) & 0xff;
>>> +        tmp[2] = (start >>  8) & 0xff;
>>> +        tmp[3] = (start      ) & 0xff;
>>> +        len = 4;
>>> +    }
>>
>> Could we please use the existing byteswapping functions?
> 
> I don't see how to use them w/o going through an additional memcpy.
> 
> If you can provide a better solution I'm all for it.  But I don't see 
> any reason to hold up the patch at this point.

union {
	uint8_t tmp[8];
	uint64_t mem64;
	uint32_t mem32;
}

int len;

if (addrcell && *addrcell == 2) {
	mem64 = cpu_to_fdt64(memsize);
	len = 8;
} else {
	mem32 = cpu_to_fdt32(memsize);
	len = 4;
}

fdt_setprop(..., tmp, len);

-Scott

[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[Kumar Gala]

On Nov 28, 2007, at 4:38 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> On Nov 28, 2007, at 4:16 PM, Scott Wood wrote:
>>> Kumar Gala wrote:
>>>> +    addrcell = fdt_getprop(blob, 0, "#address-cells", NULL);
>>>> +    /* use shifts and mask to ensure endianness */
>>>> +    if ((addrcell) && (*addrcell == 2)) {
>>>> +        tmp[0] = (start >> 56) & 0xff;
>>>> +        tmp[1] = (start >> 48) & 0xff;
>>>> +        tmp[2] = (start >> 40) & 0xff;
>>>> +        tmp[3] = (start >> 32) & 0xff;
>>>> +        tmp[4] = (start >> 24) & 0xff;
>>>> +        tmp[5] = (start >> 16) & 0xff;
>>>> +        tmp[6] = (start >>  8) & 0xff;
>>>> +        tmp[7] = (start      ) & 0xff;
>>>> +        len = 8;
>>>> +    } else {
>>>> +        tmp[0] = (start >> 24) & 0xff;
>>>> +        tmp[1] = (start >> 16) & 0xff;
>>>> +        tmp[2] = (start >>  8) & 0xff;
>>>> +        tmp[3] = (start      ) & 0xff;
>>>> +        len = 4;
>>>> +    }
>>>
>>> Could we please use the existing byteswapping functions?
>> I don't see how to use them w/o going through an additional memcpy.
>> If you can provide a better solution I'm all for it.  But I don't  
>> see any reason to hold up the patch at this point.
>
> union {
> 	uint8_t tmp[8];
> 	uint64_t mem64;
> 	uint32_t mem32;
> }
>
> int len;
>
> if (addrcell && *addrcell == 2) {
> 	mem64 = cpu_to_fdt64(memsize);
> 	len = 8;
> } else {
> 	mem32 = cpu_to_fdt32(memsize);
> 	len = 4;
> }

This gets more complicated with finishing off the rest of temp since  
you have to use len to figure out an index.

I'm keeping the code as is since its the simplest version.

- k

[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[Scott Wood]

Kumar Gala wrote:
> On Nov 28, 2007, at 4:38 PM, Scott Wood wrote:
>> Kumar Gala wrote:
>>> I don't see how to use them w/o going through an additional memcpy.
>>> If you can provide a better solution I'm all for it.  But I don't see 
>>> any reason to hold up the patch at this point.
>>
>> union {
>>     uint8_t tmp[8];
>>     uint64_t mem64;
>>     uint32_t mem32;
>> }
>>
>> int len;
>>
>> if (addrcell && *addrcell == 2) {
>>     mem64 = cpu_to_fdt64(memsize);
>>     len = 8;
>> } else {
>>     mem32 = cpu_to_fdt32(memsize);
>>     len = 4;
>> }
> 
> This gets more complicated with finishing off the rest of temp since you 
> have to use len to figure out an index.

OK, so make mem64/mem32 arrays. :-P

> I'm keeping the code as is since its the simplest version.

Fine, if you don't mind making people's eyes bleed...

Maybe a generic function that takes a buffer pointer, a uint64_t, and a 
numcells?  This is hardly going to be the only place that needs to write 
out a quantity that might be one or two cells.

-Scott

[U-Boot-Users] [PATCH][FDT][UPDATE] Add common memory fixup function

[Kumar Gala]

On Nov 28, 2007, at 5:04 PM, Scott Wood wrote:

> Kumar Gala wrote:
>> On Nov 28, 2007, at 4:38 PM, Scott Wood wrote:
>>> Kumar Gala wrote:
>>>> I don't see how to use them w/o going through an additional memcpy.
>>>> If you can provide a better solution I'm all for it.  But I don't  
>>>> see any reason to hold up the patch at this point.
>>>
>>> union {
>>>    uint8_t tmp[8];
>>>    uint64_t mem64;
>>>    uint32_t mem32;
>>> }
>>>
>>> int len;
>>>
>>> if (addrcell && *addrcell == 2) {
>>>    mem64 = cpu_to_fdt64(memsize);
>>>    len = 8;
>>> } else {
>>>    mem32 = cpu_to_fdt32(memsize);
>>>    len = 4;
>>> }
>> This gets more complicated with finishing off the rest of temp  
>> since you have to use len to figure out an index.
>
> OK, so make mem64/mem32 arrays. :-P
>
>> I'm keeping the code as is since its the simplest version.
>
> Fine, if you don't mind making people's eyes bleed...

:)

> Maybe a generic function that takes a buffer pointer, a uint64_t,  
> and a numcells?  This is hardly going to be the only place that  
> needs to write out a quantity that might be one or two cells.

Agreed, but I leave that as an exercise for the reader... when we get  
the second case of this we can figure out a better helper.

- k