| Message ID | 20200519192340.16624-2-jagan@amarulasolutions.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-amarula+bncBD7MFH7A7EEBB3HESD3AKGQEWMR2TNA@amarulasolutions.com> X-Original-To: linux-amarula@patchwork.amarulasolutions.com Delivered-To: linux-amarula@patchwork.amarulasolutions.com Received: from mail-pj1-f69.google.com (mail-pj1-f69.google.com [209.85.216.69]) by ganimede.amarulasolutions.com (Postfix) with ESMTPS id 0EA043F086 for <linux-amarula@patchwork.amarulasolutions.com>; Tue, 19 May 2020 21:24:30 +0200 (CEST) Received: by mail-pj1-f69.google.com with SMTP id a69sf747506pje.0 for <linux-amarula@patchwork.amarulasolutions.com>; Tue, 19 May 2020 12:24:29 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1589916268; cv=pass; d=google.com; s=arc-20160816; b=ziHuJJGbou0273GmQ3qTg92WAuTBU0D5SZ+MvL9496Kj1e+JXtT7Cu1ePBbMfvOyqf a0j8pmdXEzmYr3+09SEBXUmOWsOPdFLWIWVQBEC990PodkndvAyUqUQAmFEVJAV7oF8J 9XKNpSGCsknKw4YMzqeICPmFKb0FtajBFZxKHy4PDAVzRjHcv4QUyoe2CGjMp2Pin1+q VaWPS/mAfaQUm+6tgCIdoC8yVJZOd02+CKNAsuNwKiDfFMYYl86L75cnseZh/Fqpk42b g3l4U6s1U/pyWF5oZL0JLNLjeCw0vEs6F4GBRUW5ZXCnpKrSSzZZgTEFZnEb1Md3IaT6 yjSA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-unsubscribe:list-archive:list-help:list-post:list-id :mailing-list:precedence:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:dkim-signature; bh=Ty5Kt23hfphwHQ0iauCYsFInkYsQT7ZaYYE9gyM5jqM=; b=q1A7VC65B47L/pN/muyEw0UnjWasosAk818DvuavXyAmEYqD0IF14ULiUJdhp30X1/ NdFKM0Z1q28W/mgf2RVQyvzD2/22PyHjID7L0AxihQmyDSsulwwDSgoPG7pRoFDfkurL Yoio07aDJkjLFCHFa9qkUbxPAuUCZ/tEypXX6/RFzXpkByfR6zmQGwQn8s3Xn7qutee6 fGVwuoA1GehXDeo7E0K8W7VhEgGSCV75hpRrrsWuBs4911AdTJiiUB3Qkx5uubtpOR94 6Ww9ebzpRBsZN0piTep+N5uIN+tAtD1Uj6k0x2El973DdNjQAoL0xjBeWpkH2prnDRq+ 0q3Q== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@amarulasolutions.com header.s=google header.b=cvm8jLjP; spf=pass (google.com: domain of jagan@amarulasolutions.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=jagan@amarulasolutions.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amarulasolutions.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=Ty5Kt23hfphwHQ0iauCYsFInkYsQT7ZaYYE9gyM5jqM=; b=faSiNHV3+C+25H57HQOucE53ZRqv6tBBKSwoN/rR+y8N8lGjLlPBZaIVHpC7Mot/TQ MlivSwDLh8rx22/jyUkvy0bdxxCipBdFX+9jUuvm27CcUoD6CTUxYITc+p11xZB+5ycm RIebs9a5H6TqtgC1ef8r7bipkIjvMXwOAPwZA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:x-original-sender :x-original-authentication-results:precedence:mailing-list:list-id :x-spam-checked-in-group:list-post:list-help:list-archive :list-unsubscribe; bh=Ty5Kt23hfphwHQ0iauCYsFInkYsQT7ZaYYE9gyM5jqM=; b=rH9J1aO4dEJkErDmcMkTNMiRJhFPd1gIJ/UEE+kNA/8jBmOf1kxjvcTn40U90YYGyo QBobuE6wWLO8EeLo/DsceLt/DEzHh1wd0fegmSdpjVrhMGhOGb0OUv2BnZs7GcarOIYJ MxIfvvOjD38Oj4RMym9ivsYcM0axCKLbk+2PvXZVY8Jx379Hb9zUkOGiFOqd2VzzPs86 faVhVbTY2ts9boXXL8bGfRsJlpaivveADyuQOfUXHGMEnXZ58D+ZCG84m6TwUNCJz7bf qA2oJjjmOvzvP/N73dLzfOtp2eZU8IM6icZN12b2J3rnMWjeL+Sn9pDYbZ9fqx3t47iN X4gw== X-Gm-Message-State: AOAM533gwjkRtXt/8E6ubANng+XmVT0q5YtUI4Vc02Q1giZFCqOBYTSf HJXn5Zm78QBzg1IahHSEwSboH9Ak X-Google-Smtp-Source: ABdhPJx9kTJgVOpSDjR8u9N/0VEIxKz9pomHqDCH8WX8UIAuGNAT06iBVkoiDOy7CVAbJmtXiahkzQ== X-Received: by 2002:a17:90a:aa8d:: with SMTP id l13mr1212673pjq.92.1589916268685; Tue, 19 May 2020 12:24:28 -0700 (PDT) X-BeenThere: linux-amarula@amarulasolutions.com Received: by 2002:a17:902:12c:: with SMTP id 41ls101876plb.2.gmail; Tue, 19 May 2020 12:24:28 -0700 (PDT) X-Received: by 2002:a17:90b:344d:: with SMTP id lj13mr1192795pjb.160.1589916268232; Tue, 19 May 2020 12:24:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1589916268; cv=none; d=google.com; s=arc-20160816; b=gbHTlpnkL0cFu0fT/1u+gellcgY6Qy0hLK98Z7LK28HHUlNRXsauteQbJ8DJgzIc6w +W1a71aoCb5Q2yINoRC8KeHe3l3B+N7LmhQrBBC7yyq4ijSVltw3ka/Oi6Rxij664OR3 1cedvQhX8L0YYGo6TybDiivTh/EcAQxjRcSA3owIvBdw+rRQ4QgaSX/AG0/TDZbLtVTV 4LYGoYalHcJIgFmLPM2RFLHUOaU6ivizZapKpZXwMWQ11LS8r73f3XtU/lzZrw6705uE qVoo0l6eO2Vtuh516io9mfrtMreNGvgzVpR6qFZMAY44GhAeboIWOsZ8A3o7vgBDOL9B C+kA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:dkim-signature; bh=Ug5Mn9JNdBHG8t5caMWjKTyxBhHTQ22TxGk0ePPcLl4=; b=rNSvlcsMHx+gxB3+9V5tAh+sLznVLeezl41giUe6cUvzkk+SJVXfaJHu0C9/Cer9La K3SugLT1Q0Jexd9lf+JVQA2skK4EqwiQsWmQRNg2KmzD1qGJYHzojjAViRBK/cJa0oQo WB4evrDqYqpbj5KHhgPMynsYbLc9ZpUuU6xram2gozgBemEVlxzvymTjpeFOYQ6TIN5I paYMsh7rStWADoVauA1i4881X3TrmNuuBXuY+VIlRRc+zJa8j1iEZMSXKoHDz+rb7/Ts XeXQo4WgciWwz+ryytb6OR31F51nGhsQFFjZkuJECQQ1YsUPISi3a5VVXdnueQMdARDz Z5Gw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@amarulasolutions.com header.s=google header.b=cvm8jLjP; spf=pass (google.com: domain of jagan@amarulasolutions.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=jagan@amarulasolutions.com Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id z3sor410399plo.47.2020.05.19.12.24.28 for <linux-amarula@amarulasolutions.com> (Google Transport Security); Tue, 19 May 2020 12:24:28 -0700 (PDT) Received-SPF: pass (google.com: domain of jagan@amarulasolutions.com designates 209.85.220.65 as permitted sender) client-ip=209.85.220.65; X-Received: by 2002:a17:902:fe06:: with SMTP id g6mr973149plj.118.1589916267718; Tue, 19 May 2020 12:24:27 -0700 (PDT) Received: from localhost.localdomain ([2405:201:c809:c7d5:713c:4a0a:47eb:ffcd]) by smtp.gmail.com with ESMTPSA id w192sm222553pff.126.2020.05.19.12.24.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2020 12:24:26 -0700 (PDT) From: Jagan Teki <jagan@amarulasolutions.com> To: Rick Chen <rick@andestech.com>, Atish Patra <atish.patra@wdc.com>, Palmer Dabbelt <palmerdabbelt@google.com>, Bin Meng <bmeng.cn@gmail.com>, Paul Walmsley <paul.walmsley@sifive.com>, Anup Patel <anup.patel@wdc.com>, Sagar Kadam <sagar.kadam@sifive.com> Cc: u-boot@lists.denx.de, linux-amarula@amarulasolutions.com, Jagan Teki <jagan@amarulasolutions.com>, Mario Six <mario.six@gdsys.cc>, Tom Rini <trini@konsulko.com>, Simon Glass <sjg@chromium.org>, Jean-Jacques Hiblot <jjhiblot@ti.com> Subject: [PATCH v2 1/9] spl: Try to get SPL boot device via board_get_int Date: Wed, 20 May 2020 00:53:32 +0530 Message-Id: <20200519192340.16624-2-jagan@amarulasolutions.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200519192340.16624-1-jagan@amarulasolutions.com> References: <20200519192340.16624-1-jagan@amarulasolutions.com> MIME-Version: 1.0 X-Original-Sender: jagan@amarulasolutions.com X-Original-Authentication-Results: mx.google.com; dkim=pass header.i=@amarulasolutions.com header.s=google header.b=cvm8jLjP; spf=pass (google.com: domain of jagan@amarulasolutions.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=jagan@amarulasolutions.com Content-Type: text/plain; charset="UTF-8" Precedence: list Mailing-list: list linux-amarula@amarulasolutions.com; contact linux-amarula+owners@amarulasolutions.com List-ID: <linux-amarula.amarulasolutions.com> X-Spam-Checked-In-Group: linux-amarula@amarulasolutions.com X-Google-Group-Id: 476853432473 List-Post: <https://groups.google.com/a/amarulasolutions.com/group/linux-amarula/post>, <mailto:linux-amarula@amarulasolutions.com> List-Help: <https://support.google.com/a/amarulasolutions.com/bin/topic.py?topic=25838>, <mailto:linux-amarula+help@amarulasolutions.com> List-Archive: <https://groups.google.com/a/amarulasolutions.com/group/linux-amarula/> List-Unsubscribe: <mailto:googlegroups-manage+476853432473+unsubscribe@googlegroups.com>, <https://groups.google.com/a/amarulasolutions.com/group/linux-amarula/subscribe> |
| Series |
riscv: sifive/fu540: Booting from SPI
|
|
Commit Message
Jagan Teki
May 19, 2020, 7:23 p.m. UTC
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(-)
Comments
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!
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. 152781d4641e0e4c37b3a32f699cf99aeec877c8 "spl: fit: Allow the board to tell if more images must be loaded from FIT" Jagan.
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?
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? Jagan.
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.
On Sat, May 23, 2020 at 2:55 AM Tom Rini <trini@konsulko.com> wrote: > > 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. I don't understand why we have individual function calls though we have dm supported cases to get rid of those? then why would be the case for spl fit code to pump multiple images via dm board driver? What this patch is trying to do is fundamentally similar, like spl_fit is using a board driver to support multiple fit images where spl is using a board driver to support the desired boot device. Jagan.
On Mon, May 25, 2020 at 02:01:09PM +0530, Jagan Teki wrote: > On Sat, May 23, 2020 at 2:55 AM Tom Rini <trini@konsulko.com> wrote: > > > > 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. > > I don't understand why we have individual function calls though we > have dm supported cases to get rid of those? then why would be the > case for spl fit code to pump multiple images via dm board driver? > What this patch is trying to do is fundamentally similar, like spl_fit > is using a board driver to support multiple fit images where spl is > using a board driver to support the desired boot device. Because when using DM it needs to be to improve things overall, not just duplicate them. We have an abstraction for "figure out what device we've booted from" already. I don't see how your changes are improving the situation, just making the same abstraction with more calls. When we're dealing with SPL we need to be even more thoughtful than usual about size impacts. So the DM implementation of spl_boot_device() belongs elsewhere rather than making SPL do some checks and doing DM or not DM. Thanks!
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, +};