dm: core: Board failing to boot since commit 82de42fa1468 (parent data/probe)
Hello Simon,
Since commit 82de42fa1468 ("dm: core: Allocate parent data separate from probing parent") I have trouble booting my board (a custom Apollo Lake design booted via Coreboot + U-Boot).
I think this is because the function ns16550_serial_ofdata_to_platdata() of the UART driver noew tries to access the PCI bus before it is probed. I'm aware of the comment which you have added to pci-uclass.c [1].
So the correct way to fix this would be to adapt the uart driver in ns16550.c?
regards, Wolfgang
Detail information:
As far as I understand the situation the following things happen: * My debug UART is probed * This triggers a call to ns16550_serial_ofdata_to_platdata() * This function tries to read BAR0 from the PCI devices * Reading BAR0 returns 0xffffffff, as the PCI bus has not been probed yet * The serial driver tries to read from a non-existing register based on this invalid BAR0 value and hangs indefinitely.
This is confirmed by the warning which you have introduced a few commits ealier in commit 4886287ee4f9 ("pci: Print a warning if the bus is accessed before probing"):
"dm_pci_get_bdf() PCI: Device 'uart@18,2' on unprobed bus 'pci'"
[1] "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."
+Bin
Hi Wolfgang,
On Tue, 11 Feb 2020 at 06:59, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hello Simon,
Since commit 82de42fa1468 ("dm: core: Allocate parent data separate from probing parent") I have trouble booting my board (a custom Apollo Lake design booted via Coreboot + U-Boot).
I think this is because the function ns16550_serial_ofdata_to_platdata() of the UART driver noew tries to access the PCI bus before it is probed. I'm aware of the comment which you have added to pci-uclass.c [1].
So the correct way to fix this would be to adapt the uart driver in ns16550.c?
Why is the UART being used so early? Also, if you are booting from coreboot you really shouldn't need to auto-config PCI at all, so perhaps just disable CONFIG_PCI_PNP? But I understand that that changes the build.
The way I fixed it is to allow the UART to stay at a fixed PCI address:
9e11293319 dm: pci: Allow disabling auto-config for a device
That's in u-boot-dm/coral-working
regards, Wolfgang
Detail information:
As far as I understand the situation the following things happen:
- My debug UART is probed
- This triggers a call to ns16550_serial_ofdata_to_platdata()
- This function tries to read BAR0 from the PCI devices
- Reading BAR0 returns 0xffffffff, as the PCI bus has not been probed yet
- The serial driver tries to read from a non-existing register based on this invalid BAR0 value and hangs indefinitely.
This is confirmed by the warning which you have introduced a few commits ealier in commit 4886287ee4f9 ("pci: Print a warning if the bus is accessed before probing"):
"dm_pci_get_bdf() PCI: Device 'uart@18,2' on unprobed bus 'pci'"
[1] "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."
Regards, Simon
Hi Wolfgang,
On Wed, Feb 12, 2020 at 1:14 AM Simon Glass sjg@chromium.org wrote:
+Bin
Hi Wolfgang,
On Tue, 11 Feb 2020 at 06:59, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hello Simon,
Since commit 82de42fa1468 ("dm: core: Allocate parent data separate from probing parent") I have trouble booting my board (a custom Apollo Lake design booted via Coreboot + U-Boot).
I think this is because the function ns16550_serial_ofdata_to_platdata() of the UART driver noew tries to access the PCI bus before it is probed. I'm aware of the comment which you have added to pci-uclass.c [1].
So the correct way to fix this would be to adapt the uart driver in ns16550.c?
Why is the UART being used so early? Also, if you are booting from coreboot you really shouldn't need to auto-config PCI at all, so perhaps just disable CONFIG_PCI_PNP? But I understand that that changes the build.
The way I fixed it is to allow the UART to stay at a fixed PCI address:
9e11293319 dm: pci: Allow disabling auto-config for a device
Please suggest whether I should get the above patch applied to fix the issue you saw on your board.
That's in u-boot-dm/coral-working
regards, Wolfgang
Detail information:
As far as I understand the situation the following things happen:
- My debug UART is probed
- This triggers a call to ns16550_serial_ofdata_to_platdata()
- This function tries to read BAR0 from the PCI devices
- Reading BAR0 returns 0xffffffff, as the PCI bus has not been probed yet
- The serial driver tries to read from a non-existing register based on this invalid BAR0 value and hangs indefinitely.
This is confirmed by the warning which you have introduced a few commits ealier in commit 4886287ee4f9 ("pci: Print a warning if the bus is accessed before probing"):
"dm_pci_get_bdf() PCI: Device 'uart@18,2' on unprobed bus 'pci'"
[1] "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."
Regards, Bin
Hi Bin, Simon,
-----"Bin Meng" bmeng.cn@gmail.com schrieb: -----
Hi Wolfgang,
On Wed, Feb 12, 2020 at 1:14 AM Simon Glass sjg@chromium.org wrote:
+Bin
Hi Wolfgang,
On Tue, 11 Feb 2020 at 06:59, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hello Simon,
Since commit 82de42fa1468 ("dm: core: Allocate parent data separate from probing parent") I have trouble booting my board (a custom Apollo Lake design booted via Coreboot + U-Boot).
I think this is because the function ns16550_serial_ofdata_to_platdata() of the UART driver noew tries to access the PCI bus before it is probed. I'm aware of the comment which you have added to pci-uclass.c [1].
So the correct way to fix this would be to adapt the uart driver in ns16550.c?
Why is the UART being used so early?
board_init_f() calls serial_init(), because it is part of init_sequence_f. This leads to device_probe() being called for my UART.
The detailed call stack looks as follows:
File Function --------------------------------------------------------------------- board_f.c board_init_f() serial-uclass.c serial_init() serial_find_console_or_panic() serial_check_stdout() // stdout-path is set to serial0:115200n8 // serial0 is an alias for the device pciuart2, // which is called uart@18,2 in my device tree // This device is similar to what is called // serial@18,2 in chromebook_coral.dts uclass.c uclass_get_device_by_of_offset() uclass_get_device_tail() device.c device_probe() // device_probe calls drv->ofdata_to_platdata(), // which points to // ns16550_serial_ofdata_to_platdata in my case ns16550.c ns16550_serial_ofdata_to_platdata() // This is where the PCI access to the // uninitialized bus 'pci' happens
Also, if you are booting from coreboot you really shouldn't need to auto-config PCI at all, so perhaps just disable CONFIG_PCI_PNP?
CONFIG_PCI_PNP is already disabled in my configuration.
But I understand that that changes the build.
The way I fixed it is to allow the UART to stay at a fixed PCI address:
9e11293319 dm: pci: Allow disabling auto-config for a device
I think commit 9e11293319 would be a solution if the problem would be CONFIG_PCI_PNP, but that is not the case here.
Just to be sure I have cherry-picked 9e11293319 and added "pci,no-autoconfig;" to my UART, but that does not solve my issue.
Please suggest whether I should get the above patch applied to fix the issue you saw on your board.
I think the issue I see is something different.
And I can't provide a proper review for commit 9e11293319 as I don't know the PCI subsystem in U-Boot well enough.
That's in u-boot-dm/coral-working
regards, Wolfgang
Detail information:
As far as I understand the situation the following things happen:
- My debug UART is probed
- This triggers a call to ns16550_serial_ofdata_to_platdata()
- This function tries to read BAR0 from the PCI devices
- Reading BAR0 returns 0xffffffff, as the PCI bus has not been probed yet
- The serial driver tries to read from a non-existing register based on this invalid BAR0 value and hangs indefinitely.
This is confirmed by the warning which you have introduced a few commits ealier in commit 4886287ee4f9 ("pci: Print a warning if the bus is accessed before probing"):
"dm_pci_get_bdf() PCI: Device 'uart@18,2' on unprobed bus 'pci'"
[1] "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."
Regards, Bin
regards, Wolfgang
Hi Wolfgang,
On Mon, 17 Feb 2020 at 03:39, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Bin, Simon,
-----"Bin Meng" bmeng.cn@gmail.com schrieb: -----
Hi Wolfgang,
On Wed, Feb 12, 2020 at 1:14 AM Simon Glass sjg@chromium.org wrote:
+Bin
Hi Wolfgang,
On Tue, 11 Feb 2020 at 06:59, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hello Simon,
Since commit 82de42fa1468 ("dm: core: Allocate parent data separate from probing parent") I have trouble booting my board (a custom Apollo Lake design booted via Coreboot + U-Boot).
I think this is because the function ns16550_serial_ofdata_to_platdata() of the UART driver noew tries to access the PCI bus before it is probed. I'm aware of the comment which you have added to pci-uclass.c [1].
So the correct way to fix this would be to adapt the uart driver in ns16550.c?
Why is the UART being used so early?
board_init_f() calls serial_init(), because it is part of init_sequence_f. This leads to device_probe() being called for my UART.
The detailed call stack looks as follows:
File Function
board_f.c board_init_f() serial-uclass.c serial_init() serial_find_console_or_panic() serial_check_stdout() // stdout-path is set to serial0:115200n8 // serial0 is an alias for the device pciuart2, // which is called uart@18,2 in my device tree // This device is similar to what is called // serial@18,2 in chromebook_coral.dts uclass.c uclass_get_device_by_of_offset() uclass_get_device_tail() device.c device_probe() // device_probe calls drv->ofdata_to_platdata(), // which points to // ns16550_serial_ofdata_to_platdata in my case ns16550.c ns16550_serial_ofdata_to_platdata() // This is where the PCI access to the // uninitialized bus 'pci' happens
Also, if you are booting from coreboot you really shouldn't need to auto-config PCI at all, so perhaps just disable CONFIG_PCI_PNP?
CONFIG_PCI_PNP is already disabled in my configuration.
But I understand that that changes the build.
The way I fixed it is to allow the UART to stay at a fixed PCI address:
9e11293319 dm: pci: Allow disabling auto-config for a device
I think commit 9e11293319 would be a solution if the problem would be CONFIG_PCI_PNP, but that is not the case here.
Just to be sure I have cherry-picked 9e11293319 and added "pci,no-autoconfig;" to my UART, but that does not solve my issue.
Please suggest whether I should get the above patch applied to fix the issue you saw on your board.
I think the issue I see is something different.
And I can't provide a proper review for commit 9e11293319 as I don't know the PCI subsystem in U-Boot well enough.
It looks like the dev_read_addr_pci() needs to move into a probe() function in that driver.
Regards, Simon
Hi Simon,
On Fri, Feb 28, 2020 at 7:41 AM Simon Glass sjg@chromium.org wrote:
Hi Wolfgang,
On Mon, 17 Feb 2020 at 03:39, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Bin, Simon,
-----"Bin Meng" bmeng.cn@gmail.com schrieb: -----
Hi Wolfgang,
On Wed, Feb 12, 2020 at 1:14 AM Simon Glass sjg@chromium.org wrote:
+Bin
Hi Wolfgang,
On Tue, 11 Feb 2020 at 06:59, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hello Simon,
Since commit 82de42fa1468 ("dm: core: Allocate parent data separate from probing parent") I have trouble booting my board (a custom Apollo Lake design booted via Coreboot + U-Boot).
I think this is because the function ns16550_serial_ofdata_to_platdata() of the UART driver noew tries to access the PCI bus before it is probed. I'm aware of the comment which you have added to pci-uclass.c [1].
So the correct way to fix this would be to adapt the uart driver in ns16550.c?
Why is the UART being used so early?
board_init_f() calls serial_init(), because it is part of init_sequence_f. This leads to device_probe() being called for my UART.
The detailed call stack looks as follows:
File Function
board_f.c board_init_f() serial-uclass.c serial_init() serial_find_console_or_panic() serial_check_stdout() // stdout-path is set to serial0:115200n8 // serial0 is an alias for the device pciuart2, // which is called uart@18,2 in my device tree // This device is similar to what is called // serial@18,2 in chromebook_coral.dts uclass.c uclass_get_device_by_of_offset() uclass_get_device_tail() device.c device_probe() // device_probe calls drv->ofdata_to_platdata(), // which points to // ns16550_serial_ofdata_to_platdata in my case ns16550.c ns16550_serial_ofdata_to_platdata() // This is where the PCI access to the // uninitialized bus 'pci' happens
Also, if you are booting from coreboot you really shouldn't need to auto-config PCI at all, so perhaps just disable CONFIG_PCI_PNP?
CONFIG_PCI_PNP is already disabled in my configuration.
But I understand that that changes the build.
The way I fixed it is to allow the UART to stay at a fixed PCI address:
9e11293319 dm: pci: Allow disabling auto-config for a device
I think commit 9e11293319 would be a solution if the problem would be CONFIG_PCI_PNP, but that is not the case here.
Just to be sure I have cherry-picked 9e11293319 and added "pci,no-autoconfig;" to my UART, but that does not solve my issue.
Please suggest whether I should get the above patch applied to fix the issue you saw on your board.
I think the issue I see is something different.
And I can't provide a proper review for commit 9e11293319 as I don't know the PCI subsystem in U-Boot well enough.
It looks like the dev_read_addr_pci() needs to move into a probe() function in that driver.
This is http://patchwork.ozlabs.org/patch/1239995/ already in the queue.
Regards, Bin
participants (3)
-
Bin Meng -
Simon Glass -
Wolfgang Wallner