[U-Boot] [PATCH v3 2/2] arm: add initial support for Amlogic Meson and ODROID-C2

[Marek Vasut <marex@denx.de>]

On 04/12/2016 11:50 PM, Beniamino Galvani wrote:
> On Mon, Apr 11, 2016 at 11:08:02PM +0200, Marek Vasut wrote:
>>>>> +	val = fdt_getprop(gd->fdt_blob, offset, "reg", &len);
>>>>> +	if (len < sizeof(*val) * 4)
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	/* Don't use fdt64_t to avoid unaligned access */
>>>>
>>>> This looks iffy, can you elaborate on this issue ?
>>>
>>> I was getting a "Synchronous Abort handler, esr 0x96000021" which
>>> seemed to indicate a alignment fault, but thinking again about it I'm
>>> not sure anymore of the real cause. fdt64_t and fdt64_to_cpu() don't
>>> work here, I will try to investigate better why. Suggestions are
>>> welcome :)
>>
>> Toolchain issues ? Stack alignment issue ?
> 
> So, after some investigation, the reason is that the code runs when
> caches are still disabled and thus all the memory is treated as
> Device-nGnRnE, requiring aligned accesses.

You mean 8-byte aligned accesses, correct ?

> The return value of
> fdt_getprop() is guaranteed to be aligned to a 4 byte boundary (but
> not 8)

The return value of fdt_getprop() is a pointer, thus 8byte long on
aarch64 and thus aligned to 8 bytes on the stack unless there is
some real problem.

> and therefore a 32-bit type must be used to avoid alignment
> faults. Probably the comment should be updated to explain this better.

Take a look at what uniphier does : arch/arm/mach-uniphier/dram_init.c
Does that approach with fdt64_t work for you?

> Beniamino
> 
Code in question:

+int dram_init(void)
+{
+	const fdt32_t *val;
+	int offset;
+	int len;
+
+	offset = fdt_path_offset(gd->fdt_blob, "/memory");
+	if (offset < 0)
+		return -EINVAL;
+
+	val = fdt_getprop(gd->fdt_blob, offset, "reg", &len);
+	if (len < sizeof(*val) * 4)
+		return -EINVAL;
+
+	/* Don't use fdt64_t to avoid unaligned access */
+	gd->ram_size = (uint64_t)fdt32_to_cpu(val[2]) << 32;
+	gd->ram_size |= fdt32_to_cpu(val[3]);
+
+	return 0;
+}

-- 
Best regards,
Marek Vasut