[U-Boot] [PATCH 02/11] lib: fdtdec: fix size cell and address cell parse from DT

Mugunthan V N
2016-04-11T05:52:11Z

Stephen

On Sunday 10 April 2016 09:11 AM, Stephen Warren wrote:
> 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().

Will fix thos in v2 by using fdtdec_get_addr_size_auto_parent()

Regards
Mugunthan V N