[U-Boot] [PATCH v5 7/8] board: intel: Add new slimbootloader board

[Bin Meng]

Hi Andy,

On Mon, Jul 22, 2019 at 11:49 PM Andy Shevchenko
<andy.shevchenko at gmail.com> wrote:
>
> On Wed, Jul 17, 2019 at 7:42 AM Park, Aiden <aiden.park at intel.com> wrote:
> >
> > Add slimbootloader board to run U-boot as a Slim Bootloader payload
> > - Add new board/intel/slimbootloader directory with minimum codes
> > - Add slimbootloader configuration files
> > - Add README in board/intel/slimbootloader
>
> >  board/intel/slimbootloader/README           | 133 ++++++++++++++++++++
>
> Shouldn't become reST one?

I think this will need be converted to reST after my doc series are applied.

>
> > +Stitch IFWI:
> > +  Refer to https://slimbootloader.github.io/supported-hardware/apollo-lake-crb.html#stitching.
> > +  > python Platform/ApollolakeBoardPkg/Script/StitchLoader.py -i <Existing IFWI> -s Outputs/apl/Stitch_Components.zip -o <Output IFWI>
>
> It reminds me that DnX protocol.
>
> > +Flash IFWI:
> > +  Use DediProg to flash IFWI.
> > +  Now, you should reach at U-Boot serial console.
>
> Can't it be done via DFU?
>
> > +int board_early_init_r(void)
> > +{
> > +       /*
> > +        * Make sure PCI bus is enumerated so that peripherals on the PCI bus
> > +        * can be discovered by their drivers
> > +        */
> > +       pci_init();
>
> I'm not sure this is how U-Boot is designed with DM.
> At least my expectations that bus gets initialized followed by the
> certain driver in a lazy way.
> Isn't it the case? Bin?

For most x86 board, yes, PCI gets enumerated automatically if some PCI
APIs are called in the early initialization codes: eg: pci_{read,
write}_config().

But for boards like coreboot/slimbootloader, if there is no touch to
any PCI config register on that board in the early phase, PCI bus
remains not probed.

>
> > +
> > +       return 0;
> > +}
>
>
> > +++ b/board/intel/slimbootloader/start.S
> > @@ -0,0 +1,9 @@
>
> > +/* board early initialization */
> > +.globl early_board_init
> > +early_board_init:
> > +       jmp     early_board_init_ret
>
> Do you need this stub at all? How other CPUs work without it?

This hook is currently needed by every board.

>
> > +++ b/include/configs/slimbootloader.h
> > @@ -0,0 +1,59 @@
>
> > +#undef CONFIG_NFSBOOTCOMMAND
> > +#undef CONFIG_RAMBOOTCOMMAND
> > +#undef CONFIG_EXTRA_ENV_SETTINGS
> > +#undef CONFIG_BOOTCOMMAND
>
> Do we really need all these in the header?
> Can't some at least be done via configuration?

I suspect not all of these are converted to Kconfig yet.

Regards,
Bin