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

[Sagar Kadam]

Hi Anup,

On Thu, Aug 1, 2019 at 9:26 PM Anup Patel <anup at brainfault.org> wrote:
>
> 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.
>

Ohh, Ok, thanks for clarifying my assumption.

> Adding specific CPU support under arch/riscv will require strong justification
> about why generic CPU support is not suitable for particular SOC.
>
> Regards,
> Anup
>
Regards,
Sagar
> > > >
> > > > >
> > > > > 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