On 14 August 2015 at 10:10, Bin Meng bmeng.cn@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.
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.
Thanks
Michal