Hi Tom, On Tue, 25 Jul 2023 at 15:39, Tom Rini <tr...@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 <tr...@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.schucha...@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.schucha...@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