From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 13 Apr 2016 00:26:43 +0200
Subject: [U-Boot] [PATCH v3 2/2] arm: add initial support for Amlogic
 Meson and ODROID-C2
In-Reply-To: <20160412214945.GA3006@gmail.com>
References: <1460302242-16420-1-git-send-email-b.galvani@gmail.com>
 <1460302242-16420-3-git-send-email-b.galvani@gmail.com>
 <570AEB18.5010901@denx.de> <20160411210211.GB22294@gmail.com>
 <570C1232.1070908@denx.de> <20160412214945.GA3006@gmail.com>
Message-ID: <570D7623.8020603@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.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