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

Marek Vasut
2016-04-12T22:26:43Z

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