[U-Boot] [PATCH] Revert "fdt: Fix fdtdec_get_addr_size() for 64-bit"

[Bin Meng]

Hi Michal,

On Fri, Aug 14, 2015 at 5:01 PM, Michal Suchanek <hramrach at gmail.com> wrote:
> On 14 August 2015 at 10:10, Bin Meng <bmeng.cn at gmail.com> wrote:
>
>> Do we have any conclusion about commit 5b34436? Today I started to
>> check the pre-relocatoin DM PCI UART issue, but found it is now broken
>> due to this commit. The broken part is at
>> ns16550_serial_ofdata_to_platdata() in drivers/serial/ns16550.c, in
>> which it has:
>>
>>     addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
>> #ifdef CONFIG_PCI
>>     if (addr == FDT_ADDR_T_NONE) {
>> ...
>>
>> Before commit 5b34436, the old behavior is that the call to
>> fdtdec_get_addr() returns FDT_ADDR_T_NONE so that we can trap into the
>> PCI logic. But with commit 5b34436, addr is now zero which just bypass
>> this logic.
>>
>> As for why addr is now zero, this is because fdtdec_get_number() can
>> only handle a 64-bit number at most. However for PCI reg, it has 3
>> cells. So if I have the following encoding:
>>
>> reg = <0x00025100 0x0 0x0 0x0 0x0
>>            0x01025110 0x0 0x0 0x0 0x0>;
>>
>> The addr will be assigned to zero after two rounds of left shift by 32-bit.
>>
>> I can certainly change ns16550 driver to test the return value against
>> 0 now, but I think this fdtdec_get_addr() does not cover all cases.
>> Please advise.
>>
>
> Hello,
>
> What do you expect the fdtdec_get_addr to do?
>
> Any 32bit (or 64bit number on 64bit archs) is valid return value so
> there is no possibility to return an error. This is probably a problem
> with the interface. If there is more than can fit or there is an error
> you will never know.

Agreed.

>
> eg. Linux has this code for decoding numbers which can have 1-2 cells:
>
>                 reg = of_get_property(pp, "reg", &len);
>                 if (!reg) {
> blah
>                 }
>
>                 a_cells = of_n_addr_cells(pp); /* scan parents for
> #address-cells */
>                 s_cells = of_n_size_cells(pp);
>                 (*pparts)[i].offset = of_read_number(reg, a_cells);
>                 (*pparts)[i].size = of_read_number(reg + a_cells, s_cells);
>
>
> If you read an address that can be 3 cells then you need a function
> that returns cell array rather than a single integer. Or you can check
> the address length and decide if you can fit that into an integer on
> your platform.
>

Yep, we have an API fdtdec_get_pci_addr() to parse a PCI reg. I was
raising this because I see this broke the existing codes and there was
discussion on reverting this commit.

Regards,
Bin