On Fri, May 22, 2020 at 09:42:22PM +0530, Jagan Teki wrote: > On Wed, May 20, 2020 at 7:40 PM Tom Rini <tr...@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 <tr...@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....@gdsys.cc> > > > > > Cc: Tom Rini <tr...@konsulko.com> > > > > > Cc: Simon Glass <s...@chromium.org> > > > > > Cc: Jean-Jacques Hiblot <jjhib...@ti.com> > > > > > Signed-off-by: Jagan Teki <ja...@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
signature.asc
Description: PGP signature