From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Sat, 9 Apr 2016 21:41:38 -0600
Subject: [U-Boot] [PATCH 02/11] lib: fdtdec: fix size cell and address
 cell parse from DT
In-Reply-To: <CAPnjgZ1NKBhqNrY5ybhmpCViOskR4MrCBhvdtL4ZbEWr8fa96A@mail.gmail.com>
References: <1460042230-15205-1-git-send-email-mugunthanvnm@ti.com>
 <1460042230-15205-3-git-send-email-mugunthanvnm@ti.com>
 <CAPnjgZ1NKBhqNrY5ybhmpCViOskR4MrCBhvdtL4ZbEWr8fa96A@mail.gmail.com>
Message-ID: <5709CB72.1000503@wwwdotorg.org>
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/09/2016 12:35 PM, Simon Glass wrote:
> +Stephen
>
> Hi Mugunthan,
>
> On 7 April 2016 at 09:17, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> Size cell and address cell should be read from the parent node
>> and should not assume with data structures as an example
>> TI DRA7xx SoC is enabled as 64bit as there is LPAE support
>> but the addresses specified in DT are all 32 bit sizes. So
>> changing the code to read from parent node instead of
>> calculations.
>
> I don't understand this. Can you please reword it and shorten the sentences?

>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index 70acc29..8a5fb8c 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -88,15 +88,20 @@ fdt_addr_t fdtdec_get_addr_size_fixed(const void *blob, int node,
>>          const fdt32_t *prop_addr, *prop_size, *prop_after_size;
>>          int len;
>>          fdt_addr_t addr;
>> +       int parent;
>>
>>          debug("%s: %s: ", __func__, prop_name);
>>
>> -       if (na > (sizeof(fdt_addr_t) / sizeof(fdt32_t))) {
>> +       parent = fdt_parent_offset(blob, node);
>
> This is a very slow function. I hope this changes is not needed.
>
>> +
>> +       na = fdt_address_cells(blob, parent);
>> +       if (na < 1) {
>>                  debug("(na too large for fdt_addr_t type)\n");
>>                  return FDT_ADDR_T_NONE;
>>          }
>>
>> -       if (ns > (sizeof(fdt_size_t) / sizeof(fdt32_t))) {
>> +       ns = fdt_size_cells(blob, parent);
>> +       if (ns < 0) {
>>                  debug("(ns too large for fdt_size_t type)\n");
>>                  return FDT_ADDR_T_NONE;
>>          }

The entire point of fdtdec_get_addr_size_fixed() is for use-cases where 
na and ns are known and fixed ahead of time, or have already been 
retrieved from the parent node by the caller. This patch is incorrect. I 
expect the correct fix is to call e.g. 
fdtdec_get_addr_size_auto_parent() or 
fdtdec_get_addr_size_auto_noparent() from somewhere, rather than calling 
fdtdec_get_addr_size_fixed().