u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
From: Zong Li <zong.li@sifive.com>
To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: trini@konsulko.com, mark.kettenis@xs4all.nl,
	"Bharat Gooty" <bharat.gooty@broadcom.com>,
	"Rayagonda Kokatanur" <rayagonda.kokatanur@broadcom.com>,
	"Rick Chen" <rick@andestech.com>, Leo <ycliang@andestech.com>,
	"Thomas Fitzsimmons" <fitzsim@fitzsim.org>,
	"Simon Glass" <sjg@chromium.org>, "Bin Meng" <bmeng.cn@gmail.com>,
	"Marek Behún" <marek.behun@nic.cz>,
	"Green Wan" <green.wan@sifive.com>,
	"Sean Anderson" <seanga2@gmail.com>, "Lukas Auer" <lukas@auer.io>,
	"Brad Kim" <brad.kim@semifive.com>,
	"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"David Abdurachmanov" <david.abdurachmanov@sifive.com>,
	"Dimitri John Ledkov" <dimitri.ledkov@canonical.com>,
	"U-Boot Mailing List" <u-boot@lists.denx.de>
Subject: Re: [PATCH 1/3] treewide: Remove OF_PRIOR_STAGE from RISC-V boards
Date: Wed, 29 Sep 2021 16:33:07 +0800	[thread overview]
Message-ID: <CANXhq0oMp=GaaNSd3jhMx04oJe5f+Y7U8MYSfgTbFefNDTD1ew@mail.gmail.com> (raw)
In-Reply-To: <20210927064751.78591-1-ilias.apalodimas@linaro.org>

On Mon, Sep 27, 2021 at 2:48 PM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> At some point back in 2018 prior_stage_fdt_address and OF_PRIOR_STAGE got
> introduced,  in order to support a DTB handed over by an earlier stage boo
> loader.  However we have another option in the Kconfig (OF_BOARD) which has
> identical semantics.
>
> On RISC-V boards which during their startup,  some of the platforms, pick
> up the DTB from a1 and copy it in their private gd_t.  Apart from that they
> copy it to prior_stage_fdt_address,  if the Kconfig option is selected,
> which is unnecessary.
>
> So let's switch the config option for those boards to OF_BOARD and define
> the required board_fdt_blob_setup() for them.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  arch/riscv/cpu/cpu.c                    |  3 ---
>  arch/riscv/cpu/start.S                  |  5 -----
>  arch/riscv/dts/binman.dtsi              |  6 +++---
>  board/AndesTech/ax25-ae350/ax25-ae350.c |  1 -
>  board/emulation/qemu-riscv/qemu-riscv.c |  9 +++++++++
>  board/sifive/unleashed/unleashed.c      | 10 ++++------
>  board/sifive/unmatched/unmatched.c      | 10 ++++------
>  configs/ae350_rv32_defconfig            |  2 +-
>  configs/ae350_rv32_spl_defconfig        |  2 +-
>  configs/ae350_rv64_defconfig            |  2 +-
>  configs/ae350_rv64_spl_defconfig        |  2 +-
>  configs/qemu-riscv32_defconfig          |  2 +-
>  configs/qemu-riscv32_smode_defconfig    |  2 +-
>  configs/qemu-riscv32_spl_defconfig      |  2 +-
>  configs/qemu-riscv64_defconfig          |  2 +-
>  configs/qemu-riscv64_smode_defconfig    |  2 +-
>  configs/qemu-riscv64_spl_defconfig      |  2 +-
>  dts/Kconfig                             |  2 +-
>  18 files changed, 31 insertions(+), 35 deletions(-)
>
> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> index c894ac10b536..e16f1df30254 100644
> --- a/arch/riscv/cpu/cpu.c
> +++ b/arch/riscv/cpu/cpu.c
> @@ -16,9 +16,6 @@
>   * The variables here must be stored in the data section since they are used
>   * before the bss section is available.
>   */
> -#ifdef CONFIG_OF_PRIOR_STAGE
> -phys_addr_t prior_stage_fdt_address __section(".data");
> -#endif
>  #ifndef CONFIG_XIP
>  u32 hart_lottery __section(".data") = 0;
>
> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
> index 308b0a97a58f..76850ec9be2c 100644
> --- a/arch/riscv/cpu/start.S
> +++ b/arch/riscv/cpu/start.S
> @@ -142,11 +142,6 @@ call_harts_early_init:
>         bnez    tp, secondary_hart_loop
>  #endif
>
> -#ifdef CONFIG_OF_PRIOR_STAGE
> -       la      t0, prior_stage_fdt_address
> -       SREG    s1, 0(t0)
> -#endif
> -
>         jal     board_init_f_init_reserve
>
>         SREG    s1, GD_FIRMWARE_FDT_ADDR(gp)
> diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
> index d26cfdb78a9e..5757ef65ea4b 100644
> --- a/arch/riscv/dts/binman.dtsi
> +++ b/arch/riscv/dts/binman.dtsi
> @@ -48,7 +48,7 @@
>                                         };
>                                 };
>
> -#ifndef CONFIG_OF_PRIOR_STAGE
> +#ifndef CONFIG_OF_BOARD
>                                 @fdt-SEQ {
>                                         description = "NAME";
>                                         type = "flat_dt";
> @@ -60,7 +60,7 @@
>                         configurations {
>                                 default = "conf-1";
>
> -#ifndef CONFIG_OF_PRIOR_STAGE
> +#ifndef CONFIG_OF_BOARD
>                                 @conf-SEQ {
>  #else
>                                 conf-1 {
> @@ -68,7 +68,7 @@
>                                         description = "NAME";
>                                         firmware = "opensbi";
>                                         loadables = "uboot";
> -#ifndef CONFIG_OF_PRIOR_STAGE
> +#ifndef CONFIG_OF_BOARD
>                                         fdt = "fdt-SEQ";
>  #endif
>                                 };
> diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c
> index 81b0ee992372..4f03806272df 100644
> --- a/board/AndesTech/ax25-ae350/ax25-ae350.c
> +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
> @@ -21,7 +21,6 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -extern phys_addr_t prior_stage_fdt_address;
>  /*
>   * Miscellaneous platform dependent initializations
>   */
> diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c
> index dcfd3f20bee6..aa91ca91325c 100644
> --- a/board/emulation/qemu-riscv/qemu-riscv.c
> +++ b/board/emulation/qemu-riscv/qemu-riscv.c
> @@ -14,6 +14,8 @@
>  #include <virtio_types.h>
>  #include <virtio.h>
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  int board_init(void)
>  {
>         /*
> @@ -69,3 +71,10 @@ int board_fit_config_name_match(const char *name)
>         return 0;
>  }
>  #endif
> +
> +void *board_fdt_blob_setup(void)
> +{
> +       /* Stored the DTB address there during our init */
> +       return (void *)gd->arch.firmware_fdt_addr;
> +}
> +
> diff --git a/board/sifive/unleashed/unleashed.c b/board/sifive/unleashed/unleashed.c
> index 8cd514df3005..7e89c3f740a7 100644
> --- a/board/sifive/unleashed/unleashed.c
> +++ b/board/sifive/unleashed/unleashed.c
> @@ -116,12 +116,10 @@ int misc_init_r(void)
>
>  void *board_fdt_blob_setup(void)
>  {
> -       if (IS_ENABLED(CONFIG_OF_SEPARATE)) {
> -               if (gd->arch.firmware_fdt_addr)
> -                       return (ulong *)gd->arch.firmware_fdt_addr;
> -               else
> -                       return (ulong *)&_end;
> -       }
> +       if (gd->arch.firmware_fdt_addr)
> +               return (void *)gd->arch.firmware_fdt_addr;
> +       else
> +               return (void *)&_end;
>  }

I was wondering if we need to check CONFIG_OF_BOARD here? I'm not sure
whether we should distinguish the value of a1 register which is
meaningless. It means that if we don't expect the device tree to be
passed by prior stage, then the a1 register might be a trash value at
the beginning, so it would still return the arch.firmware_fdt_addr
here, rather than _end. And do you think that we should enable the
CONFIG_OF_BOARD for unmatched and unleashed? Because it seems to me
that we actually pass the device tree by prior stage (i.e. OpenSBI).

>
>  int board_init(void)
> diff --git a/board/sifive/unmatched/unmatched.c b/board/sifive/unmatched/unmatched.c
> index d90b252baef7..2f26f92fcb2b 100644
> --- a/board/sifive/unmatched/unmatched.c
> +++ b/board/sifive/unmatched/unmatched.c
> @@ -13,12 +13,10 @@
>
>  void *board_fdt_blob_setup(void)
>  {
> -       if (IS_ENABLED(CONFIG_OF_SEPARATE)) {
> -               if (gd->arch.firmware_fdt_addr)
> -                       return (ulong *)gd->arch.firmware_fdt_addr;
> -               else
> -                       return (ulong *)&_end;
> -       }
> +       if (gd->arch.firmware_fdt_addr)
> +               return (void *)gd->arch.firmware_fdt_addr;
> +       else
> +               return (void *)&_end;
>  }
>
>  int board_init(void)
> diff --git a/configs/ae350_rv32_defconfig b/configs/ae350_rv32_defconfig
> index 4e7a1686a64d..8b6c0b8a4a0a 100644
> --- a/configs/ae350_rv32_defconfig
> +++ b/configs/ae350_rv32_defconfig
> @@ -15,7 +15,7 @@ CONFIG_CMD_SF_TEST=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_BOOTP_PREFER_SERVERIP=y
>  CONFIG_CMD_CACHE=y
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> diff --git a/configs/ae350_rv32_spl_defconfig b/configs/ae350_rv32_spl_defconfig
> index 34c6af6e7e17..a0fe9b9a71df 100644
> --- a/configs/ae350_rv32_spl_defconfig
> +++ b/configs/ae350_rv32_spl_defconfig
> @@ -19,7 +19,7 @@ CONFIG_CMD_SF_TEST=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_BOOTP_PREFER_SERVERIP=y
>  CONFIG_CMD_CACHE=y
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
>  CONFIG_BOOTP_SEND_HOSTNAME=y
> diff --git a/configs/ae350_rv64_defconfig b/configs/ae350_rv64_defconfig
> index 05eee371ac2f..cb23cbd3d95e 100644
> --- a/configs/ae350_rv64_defconfig
> +++ b/configs/ae350_rv64_defconfig
> @@ -16,7 +16,7 @@ CONFIG_CMD_SF_TEST=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_BOOTP_PREFER_SERVERIP=y
>  CONFIG_CMD_CACHE=y
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> diff --git a/configs/ae350_rv64_spl_defconfig b/configs/ae350_rv64_spl_defconfig
> index 9cd7848c92eb..9ad312505db3 100644
> --- a/configs/ae350_rv64_spl_defconfig
> +++ b/configs/ae350_rv64_spl_defconfig
> @@ -20,7 +20,7 @@ CONFIG_CMD_SF_TEST=y
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_BOOTP_PREFER_SERVERIP=y
>  CONFIG_CMD_CACHE=y
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
>  CONFIG_BOOTP_SEND_HOSTNAME=y
> diff --git a/configs/qemu-riscv32_defconfig b/configs/qemu-riscv32_defconfig
> index 8ac16cf4186e..6fe133c268d7 100644
> --- a/configs/qemu-riscv32_defconfig
> +++ b/configs/qemu-riscv32_defconfig
> @@ -9,6 +9,6 @@ CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_CMD_BOOTEFI_SELFTEST=y
>  CONFIG_CMD_NVEDIT_EFI=y
>  # CONFIG_CMD_MII is not set
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> diff --git a/configs/qemu-riscv32_smode_defconfig b/configs/qemu-riscv32_smode_defconfig
> index 05eda439618f..c67e8206d1ab 100644
> --- a/configs/qemu-riscv32_smode_defconfig
> +++ b/configs/qemu-riscv32_smode_defconfig
> @@ -10,6 +10,6 @@ CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_CMD_BOOTEFI_SELFTEST=y
>  CONFIG_CMD_NVEDIT_EFI=y
>  # CONFIG_CMD_MII is not set
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> diff --git a/configs/qemu-riscv32_spl_defconfig b/configs/qemu-riscv32_spl_defconfig
> index ee81e552724d..77e81fac3af7 100644
> --- a/configs/qemu-riscv32_spl_defconfig
> +++ b/configs/qemu-riscv32_spl_defconfig
> @@ -12,6 +12,6 @@ CONFIG_DISPLAY_CPUINFO=y
>  CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_CMD_SBI=y
>  # CONFIG_CMD_MII is not set
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> diff --git a/configs/qemu-riscv64_defconfig b/configs/qemu-riscv64_defconfig
> index daf5d655d01f..90e87672aab0 100644
> --- a/configs/qemu-riscv64_defconfig
> +++ b/configs/qemu-riscv64_defconfig
> @@ -10,6 +10,6 @@ CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_CMD_BOOTEFI_SELFTEST=y
>  CONFIG_CMD_NVEDIT_EFI=y
>  # CONFIG_CMD_MII is not set
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> diff --git a/configs/qemu-riscv64_smode_defconfig b/configs/qemu-riscv64_smode_defconfig
> index 4a6416e2540b..0a8393903368 100644
> --- a/configs/qemu-riscv64_smode_defconfig
> +++ b/configs/qemu-riscv64_smode_defconfig
> @@ -13,6 +13,6 @@ CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_CMD_BOOTEFI_SELFTEST=y
>  CONFIG_CMD_NVEDIT_EFI=y
>  # CONFIG_CMD_MII is not set
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> diff --git a/configs/qemu-riscv64_spl_defconfig b/configs/qemu-riscv64_spl_defconfig
> index 429d4d814e65..a15e82dd3ee1 100644
> --- a/configs/qemu-riscv64_spl_defconfig
> +++ b/configs/qemu-riscv64_spl_defconfig
> @@ -13,6 +13,6 @@ CONFIG_DISPLAY_CPUINFO=y
>  CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_CMD_SBI=y
>  # CONFIG_CMD_MII is not set
> -CONFIG_OF_PRIOR_STAGE=y
> +CONFIG_OF_BOARD=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>  CONFIG_DM_MTD=y
> diff --git a/dts/Kconfig b/dts/Kconfig
> index dabe0080c1ef..39270b47f9f0 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -107,7 +107,7 @@ config OF_EMBED
>           Boards in the mainline U-Boot tree should not use it.
>
>  config OF_BOARD
> -       bool "Provided by the board at runtime"
> +       bool "Provided by the board (e.g a previous loader) at runtime"
>         depends on !SANDBOX
>         help
>           If this option is enabled, the device tree will be provided by
> --
> 2.33.0
>

  parent reply	other threads:[~2021-09-29  8:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-27  6:47 [PATCH 1/3] treewide: Remove OF_PRIOR_STAGE from RISC-V boards Ilias Apalodimas
2021-09-27  6:47 ` [PATCH 2/3] board: arm: Remove OF_PRIOR_STAGE Ilias Apalodimas
2021-09-27 20:15   ` Simon Glass
2021-09-27  6:47 ` [PATCH 3/3] treewide: " Ilias Apalodimas
2021-09-27 20:15   ` Simon Glass
2021-09-27 20:14 ` [PATCH 1/3] treewide: Remove OF_PRIOR_STAGE from RISC-V boards Simon Glass
2021-09-29  8:33 ` Zong Li [this message]
2021-09-29  9:02   ` Ilias Apalodimas
2021-09-29 10:17     ` Ilias Apalodimas
2021-09-29 11:51       ` Zong Li
2021-09-29 12:55         ` Ilias Apalodimas
2021-09-29 12:59           ` Mark Kettenis
2021-09-29 13:11             ` Ilias Apalodimas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANXhq0oMp=GaaNSd3jhMx04oJe5f+Y7U8MYSfgTbFefNDTD1ew@mail.gmail.com' \
    --to=zong.li@sifive.com \
    --cc=bharat.gooty@broadcom.com \
    --cc=bmeng.cn@gmail.com \
    --cc=brad.kim@semifive.com \
    --cc=david.abdurachmanov@sifive.com \
    --cc=dimitri.ledkov@canonical.com \
    --cc=fitzsim@fitzsim.org \
    --cc=green.wan@sifive.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=lukas@auer.io \
    --cc=marek.behun@nic.cz \
    --cc=mark.kettenis@xs4all.nl \
    --cc=rayagonda.kokatanur@broadcom.com \
    --cc=rick@andestech.com \
    --cc=seanga2@gmail.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    --cc=ycliang@andestech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).