dm: core: Board failing to boot since commit 82de42fa1468 (parent data/probe)

[Bin Meng]

Hi Simon,

On Fri, Feb 28, 2020 at 7:41 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Wolfgang,
>
> On Mon, 17 Feb 2020 at 03:39, Wolfgang Wallner
> <wolfgang.wallner at br-automation.com> wrote:
> >
> > Hi Bin, Simon,
> >
> > -----"Bin Meng" <bmeng.cn at gmail.com> schrieb: -----
> > > Hi Wolfgang,
> > >
> > > On Wed, Feb 12, 2020 at 1:14 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > +Bin
> > > >
> > > > Hi Wolfgang,
> > > >
> > > > On Tue, 11 Feb 2020 at 06:59, Wolfgang Wallner
> > > > <wolfgang.wallner at 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 at 18,2 in my device tree
> >                         // This device is similar to what is called
> >                         // serial at 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