From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Fri, 28 Feb 2020 20:34:56 +0800 Subject: [RFC PATCH] serial: ns16550: Move PCI access from ofdata_to_platdata() to probe() In-Reply-To: References: <20200218123424.85618-1-wolfgang.wallner@br-automation.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Wolfgang, On Fri, Feb 28, 2020 at 3:50 PM Bin Meng wrote: > > On Thu, Feb 27, 2020 at 5:36 PM Bin Meng wrote: > > > > On Tue, Feb 18, 2020 at 8:36 PM Wolfgang Wallner > > 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 > > > > > > 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 > > > > This fixed the boot failure of Intel Galileo > > > > Tested on Intel Galileo > > Tested-by: Bin Meng > > 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