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

[Anup Patel]

On Thu, Aug 1, 2019 at 8:26 PM Sagar Kadam <sagar.kadam at sifive.com> wrote:
>
> 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.

I think you misunderstood my previous comment.

We should use the generic CPU support from start itself and promote use of
generic CPU support as much as possible.

Adding specific CPU support under arch/riscv will require strong justification
about why generic CPU support is not suitable for particular SOC.

Regards,
Anup

> > >
> > > >
> > > > 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