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; +}