[PATCH 1/1 RFC] treewide: Deprecate OF_PRIOR_STAGE

[Simon Glass]

Hi Ilias,

On Fri, 24 Sept 2021 at 07:10, Ilias Apalodimas
<ilias.apalodimas at 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 boot
> loader.  However we have another option in the Kconfig (OF_BOARD) which has
> identical semantics.
>
> A good example of this is RISC-V boards which during their startup,
> pick up the DTB from a1 and copy it in their private gd_t.  Apart from that
> they also copy it to prior_stage_fdt_address,  if the Kconfig option is
> selected,  which seems unnecessary(??).
>
> This is mostly an RFC,  trying to figure out if I am missing some subtle
> functionality,  which would justify having 2 Kconfig options doing similar
> things present.
>
> - Should we do this?

I think one option is better than two. I have a slight preference for
OF_PRIOR_STAGE because it is board-agnostic, but I'm not sure it
matters, since some of these boards are doing strange things anyway
and cannot use OF_PRIOR_STAGE. So let's go with this.

> - Doesn't OF_BOARD and OF_PRIOR_STAGE practically mean "Someone else is
>   going to pass me my DTB".  Why should we care if that someone is a prior
>   bootloader or runtime memory generated on the fly by U-Boot?  It all
>   boils down to having a *board* specific callback for that.

More generally, I think OF_BOARD is basically 'opt out of the normal
flow and do something special'.

So at some point I would like to define what 'normal' is. At present,
normal is OF_SEPARATE which means that the devicetree is packed with
U-Boot.

Really we want to add a second 'normal', to permit a devicetree (and
perhaps other stuff) to be passed in. I think this should be that a
bloblist is passed in, which can contain a devicetree. If it does,
then the one in U-Boot is ignored.

There should be a standard way to do this with U-Boot. Apart from the
arch-specific selection of machine registers, the standard way should
work for all boards, at some indeterminate point in the future.

> - RISC-V binman should get rid of the option as well if we decide to go
>   though with this (but I have no idea what RISC-V expects there).
> - Can we apply similar logic to OF_HOSTFILE?  It seems like we could just
>   have a board_fdt_blob_setup() for the sandbox that reads the file we
>   want and get rid of another Kconfig option.

May as well. I cannot see a down side but see how it goes.

>
> Note that the original board which introduced CONFIG_OF_PRIOR_STAGE is still
> there.  If someone cares enough I guess he could fix that as well, but I don't
> have the board around, so I prefer keeping it as is and mark the option as
> deprecated. For that board we could  also keep the prior_stage_fdt_address
> without the Kconfig option and simply copy the location there, but the
> board must define it's own board_fdt_blob_setup().  That would get rid of
> the Kconfig option entirely instead of limiting it to that board only.

Just remove it. That's why we have maintainers and we cannot keep this
around for one board. It really should not have got in anyway IMO.

The next step (after removing OF_PRIOR_STAGE) is to make OF_BOARD a
bool, i.e. taking it out of the choice. Then these boards use
OF_SEPARATE, have an in-try devicetree and use OF_BOARD to override
that at runtime.

Step 3 is to define the second normal, as above.

>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>  arch/riscv/cpu/cpu.c                    |  3 ---
>  arch/riscv/cpu/start.S                  |  5 -----
>  board/emulation/qemu-riscv/qemu-riscv.c |  8 ++++++++
>  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/bcm7260_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                             |  3 ++-
>  lib/fdtdec.c                            |  4 ++++
>  18 files changed, 33 insertions(+), 32 deletions(-)
>

Regards,
Simon