[RFC PATCH] serial: ns16550: Move PCI access from ofdata_to_platdata() to probe()

[Bin Meng]

Hi Wolfgang,

On Fri, Feb 28, 2020 at 3:50 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> On Thu, Feb 27, 2020 at 5:36 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > On Tue, Feb 18, 2020 at 8:36 PM Wolfgang Wallner
> > <wolfgang.wallner at br-automation.com> wrote:
> > >
> > > Currently the ofdata_to_platdata() method calls dev_read_addr_pci(),
> > > which potentially accesses the parent PCI bus. If this happens before
> > > the parent PCI bus is probed the resulting address will be wrong.
> > >
> > > This behavior was triggered by commit 82de42fa1468 ("dm: core:
> > > Allocate parent data separate from probing parent").
> > >
> > > According to a comment in drivers/pci/pci-uclass.c [1] accessing
> > > the PCI parent bus in ofdata_to_platdata() is not allowed, and the
> > > access should be moved to the probe() function.
> > >
> > > Move the call to dev_read_addr_pci() and the related handling of the
> > > 'addr' value from the ofdata_to_platdata() to the probe() method.
> > >
> > > While moving the code, the comment /* try Processor Local Bus device
> > > first */ was dropped. It was initially added with commit 3db886a5bf38
> > > ("serial: ns16550: Support ns16550 compatible pci uart devices") and
> > > later made obsolete with commit 33c215af4b9d ("dm: pci: Add a function
> > > to read a PCI BAR").
> > >
> > > [1] Comment in drivers/pci/pci-uclass.c:
> > > "A common cause of this problem is that this function is called in the
> > > ofdata_to_platdata() method of @dev. Accessing the PCI bus in that
> > > method is not allowed, since it has not yet been probed. To fix this,
> > > move that access to the probe() method of @dev instead."
> > >
> > > Signed-off-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> > >
> > > Fixes: 82de42fa1468 ("dm: core: Allocate parent data separate from
> > > probing parent")
> > >
> > > ---
> > > The discussion leading to this patch is located at
> > > https://lists.denx.de/pipermail/u-boot/2020-February/399811.html
> > >
> > >  drivers/serial/ns16550.c | 26 +++++++++++++-------------
> > >  1 file changed, 13 insertions(+), 13 deletions(-)
> > >
> >
> > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> >
> > This fixed the boot failure of Intel Galileo
> >
> > Tested on Intel Galileo
> > Tested-by: Bin Meng <bmeng.cn at gmail.com>
>
> applied to u-boot-x86, thanks!

Unfortunately this patch breaks several ARM board:

+drivers/built-in.o: In function `ofnode_get_parent':
+drivers/core/ofnode.c:235: undefined reference to `fdt_parent_offset'
+drivers/built-in.o: In function `ofnode_read_simple_addr_cells':
+drivers/core/ofnode.c:717: undefined reference to `fdt_address_cells'
+drivers/built-in.o: In function `ofnode_read_simple_size_cells':
+drivers/core/ofnode.c:725: undefined reference to `fdt_size_cells'
+drivers/built-in.o: In function `ofnode_get_addr_size_index':
+drivers/core/ofnode.c:294: undefined reference to `fdtdec_get_addr_size_fixed'

See https://dev.azure.com/bmeng/GitHub/_build/results?buildId=158&view=logs&j=25fc9316-c681-5af2-a03f-b888fd665e84&t=3af932b6-18ad-575e-b0ea-0b31bb7c6c26

Would you please send a v2 to address this? thanks!

Regards,
Bin