[PATCH v2 1/1] spl: initialize PCI before booting

[Simon Glass]

Hi Tom,

On Tue, 25 Jul 2023 at 15:39, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Jul 25, 2023 at 03:28:37PM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 25 Jul 2023 at 10:37, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Tue, Jul 25, 2023 at 08:51:55AM -0600, Simon Glass wrote:
> > > > On Mon, 24 Jul 2023 at 14:19, Heinrich Schuchardt
> > > > <heinrich.schuchardt at canonical.com> wrote:
> > > > >
> > > > > MMC, SATA, and USB may be using PCI based controllers.
> > > > > Initialize the PCI sub-system before trying to boot.
> > > > >
> > > > > Remove the initialization for NVMe that is now redundant.
> > > > >
> > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> > > > > ---
> > > > > v2:
> > > > >         Centralize the PCI initialization
> > > > > ---
> > > > >  common/spl/spl.c      | 7 +++++++
> > > > >  common/spl/spl_nvme.c | 5 -----
> > > > >  2 files changed, 7 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > > > > index f09bb97781..0062f3f45d 100644
> > > > > --- a/common/spl/spl.c
> > > > > +++ b/common/spl/spl.c
> > > > > @@ -800,6 +800,13 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> > > > >             IS_ENABLED(CONFIG_SPL_ATF))
> > > > >                 dram_init_banksize();
> > > > >
> > > > > +       if (CONFIG_IS_ENABLED(PCI)) {
> > > > > +               ret = pci_init();
> > > > > +               if (ret)
> > > > > +                       puts(SPL_TPL_PROMPT "Cannot initialize PCI\n");
> > > > > +               /* Don't fail. We still can try other boot methods. */
> > > >
> > > > But which ones? This seems dubious to me, since there is no check (for
> > > > each loader) as to whether PCI is needed or not. If PCI cannot be set
> > > > up, we should probably return an error.
> > >
> > > I think it's reasonable here to say that we failed to initialize PCI.
> > > It's still up to the specific loader to say it couldn't find a
> > > controller.  This in turn will provide reasonable hints if someone has a
> > > problem.  The flip side is "Oh, PCI failed but I was trying to boot from
> > > SD card anyhow which doesn't need it" (think bring-up) and having to go
> > > hack things up.
> >
> > OK, so perhaps it should say 'warning' so it is clear that it might
> > not be fatal?
>
> That's more bytes and we're already in what should obviously be Not
> Great shape.
>
> > I cannot imagine a failure to init PCI though. It seems pretty fatal to me.
>
> Working on PCI SPL support (NVMe, etc) with fall back to SD boot (so the
> board is easy to recover while working).

OK, I've said my piece.

Regards,
Simon