[PATCH v2 1/9] spl: Try to get SPL boot device via board_get_int

[Tom Rini <trini@konsulko.com>]

On Fri, May 22, 2020 at 09:42:22PM +0530, Jagan Teki wrote:
> On Wed, May 20, 2020 at 7:40 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, May 20, 2020 at 06:46:55PM +0530, Jagan Teki wrote:
> > > On Wed, May 20, 2020 at 6:32 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, May 20, 2020 at 12:53:32AM +0530, Jagan Teki wrote:
> > > >
> > > > > Usually, the associated board would supply spl boot device
> > > > > using spl_boot_device() but some boards have board driver
> > > > > that are possible to supply boot device via board_get_int
> > > > > with BOARD_SPL_BOOT_DEVICE id.
> > > > >
> > > > > This patch add support for those.
> > > > >
> > > > > Cc: Mario Six <mario.six@gdsys.cc>
> > > > > Cc: Tom Rini <trini@konsulko.com>
> > > > > Cc: Simon Glass <sjg@chromium.org>
> > > > > Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
> > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > ---
> > > > > Changes for v2:
> > > > > - new patch
> > > > >
> > > > >  common/spl/spl.c | 14 +++++++++++++-
> > > > >  include/board.h  |  9 +++++++++
> > > > >  2 files changed, 22 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > > > > index fc5cbbbeba..a07b71b3c1 100644
> > > > > --- a/common/spl/spl.c
> > > > > +++ b/common/spl/spl.c
> > > > > @@ -9,6 +9,7 @@
> > > > >  #include <common.h>
> > > > >  #include <bloblist.h>
> > > > >  #include <binman_sym.h>
> > > > > +#include <board.h>
> > > > >  #include <dm.h>
> > > > >  #include <handoff.h>
> > > > >  #include <hang.h>
> > > > > @@ -483,9 +484,20 @@ int spl_init(void)
> > > > >  #define BOOT_DEVICE_NONE 0xdeadbeef
> > > > >  #endif
> > > > >
> > > > > +__weak u32 spl_boot_device(void)
> > > > > +{
> > > > > +     return 0;
> > > > > +}
> > > > > +
> > > > >  __weak void board_boot_order(u32 *spl_boot_list)
> > > > >  {
> > > > > -     spl_boot_list[0] = spl_boot_device();
> > > > > +     struct udevice *board;
> > > > > +
> > > > > +     if (!board_get(&board))
> > > > > +             board_get_int(board, BOARD_SPL_BOOT_DEVICE,
> > > > > +                           (int *)&spl_boot_list[0]);
> > > > > +     else
> > > > > +             spl_boot_list[0] = spl_boot_device();
> > > > >  }
> > > > >
> > > > >  static struct spl_image_loader *spl_ll_find_loader(uint boot_device)
> > > > > diff --git a/include/board.h b/include/board.h
> > > > > index 678b652b0a..ce4eaba38d 100644
> > > > > --- a/include/board.h
> > > > > +++ b/include/board.h
> > > > > @@ -211,3 +211,12 @@ static inline int board_get_fit_loadable(struct udevice *dev, int index,
> > > > >  }
> > > > >
> > > > >  #endif
> > > > > +
> > > > > +/**
> > > > > + * Common board unique identifier
> > > > > + *
> > > > > + * @BOARD_SPL_BOOT_DEVICE:   id to get SPL boot device.
> > > > > + */
> > > > > +enum common_ids {
> > > > > +     BOARD_SPL_BOOT_DEVICE,
> > > > > +};
> > > >
> > > > I don't understand why we need this abstraction.  The intention of what
> > > > we have today is that the generic SPL framework calls out to something
> > > > to ask "what are we booted from?".  Why can the board driver not just
> > > > supply that information?  Thanks!
> > >
> > > Yes, we can update boot-device on respective areas by probing board
> > > driver and assign spl_boot_list[0] by explicitly define
> > > spl_boot_device function, but this change bypass all these codes. Just
> > > like how we did on SPL fit to load the concerned image via board
> > > driver.
> >
> > I still don't get it, sorry.  Why is spl_boot_device() not provided by
> > the "board" driver to say what to boot in this case?
> 
> That means, we have to add spl_boot_device in board-uclass.c ? so-that
> respective board driver shall use?

Yes, or perhaps a board driver doesn't even make sense in this case and
the existing abstraction should be used as is?  This isn't a unique
problem, it's something we've been handling in SPL since the beginning.
In so far as we can now try and solve this problem with something
DM-based instead of not, it should still I believe just be the same
function call.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200522/4c67de72/attachment.sig>