[U-Boot] [PATCH] sifive: riscv: update Hifive Unleashed configuration infrastructure

[Sagar Kadam]

Hi Anup,

On Wed, Jul 31, 2019 at 1:50 PM Anup Patel <anup at brainfault.org> wrote:
>
> On Tue, Jul 30, 2019 at 11:01 PM Sagar Kadam <sagar.kadam at sifive.com> wrote:
> >
> > Hello Anup,
> >
> > On Tue, Jul 30, 2019 at 9:12 AM Anup Patel <anup at brainfault.org> wrote:
> > >
> > > On Mon, Jul 29, 2019 at 6:13 PM Sagar Shrikant Kadam
> > > <sagar.kadam at sifive.com> wrote:
> > > >
> > > > This patch aligns the current implementation of HiFive Unleashed
> > > > board configuration framework with the one described in doc/README.kconfig.
> > > >
> > > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
> > > > ---
> > > >  arch/riscv/Kconfig                           |   6 +-
> > > >  arch/riscv/cpu/generic/Kconfig               |  12 ---
> > > >  arch/riscv/cpu/generic/Makefile              |   6 --
> > > >  arch/riscv/cpu/generic/cpu.c                 |  35 -------
> > > >  arch/riscv/cpu/generic/dram.c                |  37 -------
> > > >  arch/riscv/cpu/u54-mc/Kconfig                |  12 +++
> > > >  arch/riscv/cpu/u54-mc/Makefile               |   6 ++
> > > >  arch/riscv/cpu/u54-mc/cpu.c                  |  35 +++++++
> > > >  arch/riscv/cpu/u54-mc/dram.c                 |  37 +++++++
> > > >  arch/riscv/include/asm/arch-fu540-c000/clk.h |  14 +++
> > > >  arch/riscv/include/asm/arch-generic/clk.h    |  14 ---
> > > >  board/sifive/fu540/Kconfig                   |  49 ----------
> > > >  board/sifive/fu540/MAINTAINERS               |   9 --
> > > >  board/sifive/fu540/Makefile                  |   5 -
> > > >  board/sifive/fu540/fu540.c                   | 139 ---------------------------
> > > >  board/sifive/hifive_unleashed/Kconfig        |  52 ++++++++++
> > > >  board/sifive/hifive_unleashed/MAINTAINERS    |   9 ++
> > > >  board/sifive/hifive_unleashed/Makefile       |   5 +
> > > >  board/sifive/hifive_unleashed/fu540.c        | 139 +++++++++++++++++++++++++++
> > > >  configs/hifive_unleashed_defconfig           |  11 +++
> > > >  configs/sifive_fu540_defconfig               |  11 ---
> > > >  include/configs/hifive_unleashed.h           |  47 +++++++++
> > > >  include/configs/sifive-fu540.h               |  47 ---------
> > > >  23 files changed, 370 insertions(+), 367 deletions(-)
> > > >  delete mode 100644 arch/riscv/cpu/generic/Kconfig
> > > >  delete mode 100644 arch/riscv/cpu/generic/Makefile
> > > >  delete mode 100644 arch/riscv/cpu/generic/cpu.c
> > > >  delete mode 100644 arch/riscv/cpu/generic/dram.c
> > > >  create mode 100644 arch/riscv/cpu/u54-mc/Kconfig
> > > >  create mode 100644 arch/riscv/cpu/u54-mc/Makefile
> > > >  create mode 100644 arch/riscv/cpu/u54-mc/cpu.c
> > > >  create mode 100644 arch/riscv/cpu/u54-mc/dram.c
> > > >  create mode 100644 arch/riscv/include/asm/arch-fu540-c000/clk.h
> > > >  delete mode 100644 arch/riscv/include/asm/arch-generic/clk.h
> > > >  delete mode 100644 board/sifive/fu540/Kconfig
> > > >  delete mode 100644 board/sifive/fu540/MAINTAINERS
> > > >  delete mode 100644 board/sifive/fu540/Makefile
> > > >  delete mode 100644 board/sifive/fu540/fu540.c
> > > >  create mode 100644 board/sifive/hifive_unleashed/Kconfig
> > > >  create mode 100644 board/sifive/hifive_unleashed/MAINTAINERS
> > > >  create mode 100644 board/sifive/hifive_unleashed/Makefile
> > > >  create mode 100644 board/sifive/hifive_unleashed/fu540.c
> > > >  create mode 100644 configs/hifive_unleashed_defconfig
> > > >  delete mode 100644 configs/sifive_fu540_defconfig
> > > >  create mode 100644 include/configs/hifive_unleashed.h
> > > >  delete mode 100644 include/configs/sifive-fu540.h
> > > >
> > >
> > > I agree with Bin's concerns.
> > >
> > > Please don't rename generic CPU support under arch/riscv
> > >
> > > We should think long-term here. If every SOC vendor starts adding
> > > their CPU support directory under arch/riscv then U-Boot RISC port
> > > will be eventually difficult to manage and we will also have duplicate
> > > code across various CPU support.
> > >
> > > IMHO, we should avoid adding new CPU support under arch/riscv
> > > as much as possible. We can call weak functions from generic CPU
> > > support and board support code can implement it. We should only
> > > add new CPU support under arch/riscv when we are not able to
> > > re-use generic CPU support.
> > >
> >
> > Yes, your points are valid. I am Ok with it.
> > My intent here was that as the support for riscv in U-boot is in its
> > early stages
> > and doing it now would be better as minimum changes will be required and
> > going ahead as other CPU vendors introduce their CPU under arch/riscv/
> > we could isolate a generic CPU code as it grows.
>
> Thanks.
>
> Let's try to make arch/riscv/cpu/generic extensible from start itself so
> that it is relatively easy to fit newer RISC-V SOCs and boards.
>
Thanks. So can I assume that we agree on adding new CPU support under
arch/riscv. Will provide a new patch for the same.
> >
> > >
> > > Other board support renaming is fine but there is lot of documentation
> > If board support renaming is fine. Shall I submit another patch
> > excluding the CPU
> > changes?
>
> Yes, please send another revision.
>
Yes, I will do send another revision.

> Also have consider updating U-Boot and OpenSBI documentation as
> separate patches.
>
Yes, I will also update the docs accordingly.

Thanks & BR,
Sagar Kadam
> Regards,
> Anup