diff mbox series

[U-Boot,09/30] riscv: move target selection into separate file

Message ID 20181019220743.15020-10-lukas.auer@aisec.fraunhofer.de
State Superseded
Delegated to: Andes
Headers show
Series General fixes / cleanup for RISC-V and improvements to qemu-riscv | expand

Commit Message

Lukas Auer Oct. 19, 2018, 10:07 p.m. UTC
Move the target selection into a separate file (Kconfig.board) to avoid
clutter once we support more boards.

Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
---

 arch/riscv/Kconfig       | 17 ++---------------
 arch/riscv/Kconfig.board | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 15 deletions(-)
 create mode 100644 arch/riscv/Kconfig.board

Comments

Bin Meng Oct. 22, 2018, 7:22 a.m. UTC | #1
Hi Lukas,

On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Move the target selection into a separate file (Kconfig.board) to avoid
> clutter once we support more boards.
>
> Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> ---
>
>  arch/riscv/Kconfig       | 17 ++---------------
>  arch/riscv/Kconfig.board | 14 ++++++++++++++
>  2 files changed, 16 insertions(+), 15 deletions(-)
>  create mode 100644 arch/riscv/Kconfig.board
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ce07fb4b55..10d17a0e18 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -4,21 +4,6 @@ menu "RISC-V architecture"
>  config SYS_ARCH
>         default "riscv"
>
> -choice
> -       prompt "Target select"
> -       optional
> -
> -config TARGET_AX25_AE350
> -       bool "Support ax25-ae350"
> -
> -config TARGET_QEMU_VIRT
> -       bool "Support QEMU Virt Board"
> -
> -endchoice
> -
> -source "board/AndesTech/ax25-ae350/Kconfig"
> -source "board/emulation/qemu-riscv/Kconfig"
> -
>  choice
>         prompt "Base ISA"
>         default ARCH_RV32I
> @@ -72,4 +57,6 @@ config 32BIT
>  config 64BIT
>         bool
>
> +source "arch/riscv/Kconfig.board"
> +

I am OK with moving board one to a separate file, though it looks no
other arch uses scuh convention in U-Boot :)

However, with this change, it lost the capability of overriding an
architecture defined Kconfig option at board level.

I have a patch @
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907a37f77b2a4257d81bb4daa2
to express such capability.

>  endmenu
> diff --git a/arch/riscv/Kconfig.board b/arch/riscv/Kconfig.board
> new file mode 100644
> index 0000000000..fcada760c8
> --- /dev/null
> +++ b/arch/riscv/Kconfig.board
> @@ -0,0 +1,14 @@
> +choice
> +       prompt "Target select"
> +       optional
> +
> +config TARGET_AX25_AE350
> +       bool "Support ax25-ae350"
> +
> +config TARGET_QEMU_VIRT
> +       bool "Support QEMU Virt Board"
> +
> +endchoice
> +
> +source "board/AndesTech/ax25-ae350/Kconfig"
> +source "board/emulation/qemu-riscv/Kconfig"
> --

Regards,
Bin
Rick Chen Oct. 23, 2018, 2:48 a.m. UTC | #2
> > Subject: Re: [PATCH 09/30] riscv: move target selection into separate file
> >
> > Hi Lukas,
> >
> > On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > wrote:
> > >
> > > Move the target selection into a separate file (Kconfig.board) to
> > > avoid clutter once we support more boards.
> > >
> > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > > ---
> > >
> > >  arch/riscv/Kconfig       | 17 ++---------------
> > >  arch/riscv/Kconfig.board | 14 ++++++++++++++
> > >  2 files changed, 16 insertions(+), 15 deletions(-)  create mode
> > > 100644 arch/riscv/Kconfig.board
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index
> > > ce07fb4b55..10d17a0e18 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -4,21 +4,6 @@ menu "RISC-V architecture"
> > >  config SYS_ARCH
> > >         default "riscv"
> > >
> > > -choice
> > > -       prompt "Target select"
> > > -       optional
> > > -
> > > -config TARGET_AX25_AE350
> > > -       bool "Support ax25-ae350"
> > > -
> > > -config TARGET_QEMU_VIRT
> > > -       bool "Support QEMU Virt Board"
> > > -
> > > -endchoice
> > > -
> > > -source "board/AndesTech/ax25-ae350/Kconfig"
> > > -source "board/emulation/qemu-riscv/Kconfig"
> > > -
> > >  choice
> > >         prompt "Base ISA"
> > >         default ARCH_RV32I
> > > @@ -72,4 +57,6 @@ config 32BIT
> > >  config 64BIT
> > >         bool
> > >
> > > +source "arch/riscv/Kconfig.board"
> > > +
> >
> > I am OK with moving board one to a separate file, though it looks no other arch
> > uses scuh convention in U-Boot :)
> >

Though no other arch use Kconfig.board here.
Because RISC-V is an open architechture.
Maybe separate board option from Kconfig to Kconfig.board is an good idea.

Rick

> > However, with this change, it lost the capability of overriding an architecture
> > defined Kconfig option at board level.
> >
> > I have a patch @
> > http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a65068941048290
> > 7a37f77b2a4257d81bb4daa2
> > to express such capability.
> >
> > >  endmenu
> > > diff --git a/arch/riscv/Kconfig.board b/arch/riscv/Kconfig.board new
> > > file mode 100644 index 0000000000..fcada760c8
> > > --- /dev/null
> > > +++ b/arch/riscv/Kconfig.board
> > > @@ -0,0 +1,14 @@
> > > +choice
> > > +       prompt "Target select"
> > > +       optional
> > > +
> > > +config TARGET_AX25_AE350
> > > +       bool "Support ax25-ae350"
> > > +
> > > +config TARGET_QEMU_VIRT
> > > +       bool "Support QEMU Virt Board"
> > > +
> > > +endchoice
> > > +
> > > +source "board/AndesTech/ax25-ae350/Kconfig"
> > > +source "board/emulation/qemu-riscv/Kconfig"
> > > --
> >
> > Regards,
> > Bin
Bin Meng Oct. 25, 2018, 2:50 a.m. UTC | #3
On Tue, Oct 23, 2018 at 10:48 AM Rick Chen <rickchen36@gmail.com> wrote:
>
> > > Subject: Re: [PATCH 09/30] riscv: move target selection into separate file
> > >
> > > Hi Lukas,
> > >
> > > On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > > wrote:
> > > >
> > > > Move the target selection into a separate file (Kconfig.board) to
> > > > avoid clutter once we support more boards.
> > > >
> > > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > > > ---
> > > >
> > > >  arch/riscv/Kconfig       | 17 ++---------------
> > > >  arch/riscv/Kconfig.board | 14 ++++++++++++++
> > > >  2 files changed, 16 insertions(+), 15 deletions(-)  create mode
> > > > 100644 arch/riscv/Kconfig.board
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index
> > > > ce07fb4b55..10d17a0e18 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -4,21 +4,6 @@ menu "RISC-V architecture"
> > > >  config SYS_ARCH
> > > >         default "riscv"
> > > >
> > > > -choice
> > > > -       prompt "Target select"
> > > > -       optional
> > > > -
> > > > -config TARGET_AX25_AE350
> > > > -       bool "Support ax25-ae350"
> > > > -
> > > > -config TARGET_QEMU_VIRT
> > > > -       bool "Support QEMU Virt Board"
> > > > -
> > > > -endchoice
> > > > -
> > > > -source "board/AndesTech/ax25-ae350/Kconfig"
> > > > -source "board/emulation/qemu-riscv/Kconfig"
> > > > -
> > > >  choice
> > > >         prompt "Base ISA"
> > > >         default ARCH_RV32I
> > > > @@ -72,4 +57,6 @@ config 32BIT
> > > >  config 64BIT
> > > >         bool
> > > >
> > > > +source "arch/riscv/Kconfig.board"
> > > > +
> > >
> > > I am OK with moving board one to a separate file, though it looks no other arch
> > > uses scuh convention in U-Boot :)
> > >
>
> Though no other arch use Kconfig.board here.
> Because RISC-V is an open architechture.
> Maybe separate board option from Kconfig to Kconfig.board is an good idea.
>

I suggest we keep current kconfig structure for now to be in
consistent with other archs.

Regards,
Bin
Lukas Auer Oct. 25, 2018, 11:37 a.m. UTC | #4
Hi Bin,

On Thu, 2018-10-25 at 10:50 +0800, Bin Meng wrote:
> On Tue, Oct 23, 2018 at 10:48 AM Rick Chen <rickchen36@gmail.com>
> wrote:
> > 
> > > > Subject: Re: [PATCH 09/30] riscv: move target selection into
> > > > separate file
> > > > 
> > > > Hi Lukas,
> > > > 
> > > > On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer <
> > > > lukas.auer@aisec.fraunhofer.de>
> > > > wrote:
> > > > > 
> > > > > Move the target selection into a separate file
> > > > > (Kconfig.board) to
> > > > > avoid clutter once we support more boards.
> > > > > 
> > > > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > > > > ---
> > > > > 
> > > > >  arch/riscv/Kconfig       | 17 ++---------------
> > > > >  arch/riscv/Kconfig.board | 14 ++++++++++++++
> > > > >  2 files changed, 16 insertions(+), 15 deletions(-)  create
> > > > > mode
> > > > > 100644 arch/riscv/Kconfig.board
> > > > > 
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index
> > > > > ce07fb4b55..10d17a0e18 100644
> > > > > --- a/arch/riscv/Kconfig
> > > > > +++ b/arch/riscv/Kconfig
> > > > > @@ -4,21 +4,6 @@ menu "RISC-V architecture"
> > > > >  config SYS_ARCH
> > > > >         default "riscv"
> > > > > 
> > > > > -choice
> > > > > -       prompt "Target select"
> > > > > -       optional
> > > > > -
> > > > > -config TARGET_AX25_AE350
> > > > > -       bool "Support ax25-ae350"
> > > > > -
> > > > > -config TARGET_QEMU_VIRT
> > > > > -       bool "Support QEMU Virt Board"
> > > > > -
> > > > > -endchoice
> > > > > -
> > > > > -source "board/AndesTech/ax25-ae350/Kconfig"
> > > > > -source "board/emulation/qemu-riscv/Kconfig"
> > > > > -
> > > > >  choice
> > > > >         prompt "Base ISA"
> > > > >         default ARCH_RV32I
> > > > > @@ -72,4 +57,6 @@ config 32BIT
> > > > >  config 64BIT
> > > > >         bool
> > > > > 
> > > > > +source "arch/riscv/Kconfig.board"
> > > > > +
> > > > 
> > > > I am OK with moving board one to a separate file, though it
> > > > looks no other arch
> > > > uses scuh convention in U-Boot :)
> > > > 
> > 
> > Though no other arch use Kconfig.board here.
> > Because RISC-V is an open architechture.
> > Maybe separate board option from Kconfig to Kconfig.board is an
> > good idea.
> > 
> 
> I suggest we keep current kconfig structure for now to be in
> consistent with other archs.
> 
> Regards,
> Bin

I previously worked with U-Boot on ARM processors. The ARM Kconfig is
already quite crowded with both board and ARM specific configuration
entries. That's the reason behind this patch, trying to avoid mixing
both processor and board specific entries.
At the moment, there are only two boards and therefore no real need for
this patch, so I am fine with dropping this patch.

Thanks,
Lukas
Lukas Auer Oct. 25, 2018, 11:39 a.m. UTC | #5
Hi Bin,

On Mon, 2018-10-22 at 15:22 +0800, Bin Meng wrote:
> Hi Lukas,
> 
> On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer
> <lukas.auer@aisec.fraunhofer.de> wrote:
> > 
> > Move the target selection into a separate file (Kconfig.board) to
> > avoid
> > clutter once we support more boards.
> > 
> > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > ---
> > 
> >  arch/riscv/Kconfig       | 17 ++---------------
> >  arch/riscv/Kconfig.board | 14 ++++++++++++++
> >  2 files changed, 16 insertions(+), 15 deletions(-)
> >  create mode 100644 arch/riscv/Kconfig.board
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index ce07fb4b55..10d17a0e18 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -4,21 +4,6 @@ menu "RISC-V architecture"
> >  config SYS_ARCH
> >         default "riscv"
> > 
> > -choice
> > -       prompt "Target select"
> > -       optional
> > -
> > -config TARGET_AX25_AE350
> > -       bool "Support ax25-ae350"
> > -
> > -config TARGET_QEMU_VIRT
> > -       bool "Support QEMU Virt Board"
> > -
> > -endchoice
> > -
> > -source "board/AndesTech/ax25-ae350/Kconfig"
> > -source "board/emulation/qemu-riscv/Kconfig"
> > -
> >  choice
> >         prompt "Base ISA"
> >         default ARCH_RV32I
> > @@ -72,4 +57,6 @@ config 32BIT
> >  config 64BIT
> >         bool
> > 
> > +source "arch/riscv/Kconfig.board"
> > +
> 
> I am OK with moving board one to a separate file, though it looks no
> other arch uses scuh convention in U-Boot :)
> 
> However, with this change, it lost the capability of overriding an
> architecture defined Kconfig option at board level.
> 
> I have a patch @
> 
http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907a37f77b2a4257d81bb4daa2
> to express such capability.
> 

This should be easy to fix by just moving the source statement, right?

Thanks,
Lukas

> >  endmenu
> > diff --git a/arch/riscv/Kconfig.board b/arch/riscv/Kconfig.board
> > new file mode 100644
> > index 0000000000..fcada760c8
> > --- /dev/null
> > +++ b/arch/riscv/Kconfig.board
> > @@ -0,0 +1,14 @@
> > +choice
> > +       prompt "Target select"
> > +       optional
> > +
> > +config TARGET_AX25_AE350
> > +       bool "Support ax25-ae350"
> > +
> > +config TARGET_QEMU_VIRT
> > +       bool "Support QEMU Virt Board"
> > +
> > +endchoice
> > +
> > +source "board/AndesTech/ax25-ae350/Kconfig"
> > +source "board/emulation/qemu-riscv/Kconfig"
> > --
> 
> Regards,
> Bin
Bin Meng Oct. 25, 2018, 1:32 p.m. UTC | #6
Hi Lukas,

On Thu, Oct 25, 2018 at 7:39 PM Auer, Lukas
<lukas.auer@aisec.fraunhofer.de> wrote:
>
> Hi Bin,
>
> On Mon, 2018-10-22 at 15:22 +0800, Bin Meng wrote:
> > Hi Lukas,
> >
> > On Sat, Oct 20, 2018 at 6:10 AM Lukas Auer
> > <lukas.auer@aisec.fraunhofer.de> wrote:
> > >
> > > Move the target selection into a separate file (Kconfig.board) to
> > > avoid
> > > clutter once we support more boards.
> > >
> > > Signed-off-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de>
> > > ---
> > >
> > >  arch/riscv/Kconfig       | 17 ++---------------
> > >  arch/riscv/Kconfig.board | 14 ++++++++++++++
> > >  2 files changed, 16 insertions(+), 15 deletions(-)
> > >  create mode 100644 arch/riscv/Kconfig.board
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index ce07fb4b55..10d17a0e18 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -4,21 +4,6 @@ menu "RISC-V architecture"
> > >  config SYS_ARCH
> > >         default "riscv"
> > >
> > > -choice
> > > -       prompt "Target select"
> > > -       optional
> > > -
> > > -config TARGET_AX25_AE350
> > > -       bool "Support ax25-ae350"
> > > -
> > > -config TARGET_QEMU_VIRT
> > > -       bool "Support QEMU Virt Board"
> > > -
> > > -endchoice
> > > -
> > > -source "board/AndesTech/ax25-ae350/Kconfig"
> > > -source "board/emulation/qemu-riscv/Kconfig"
> > > -
> > >  choice
> > >         prompt "Base ISA"
> > >         default ARCH_RV32I
> > > @@ -72,4 +57,6 @@ config 32BIT
> > >  config 64BIT
> > >         bool
> > >
> > > +source "arch/riscv/Kconfig.board"
> > > +
> >
> > I am OK with moving board one to a separate file, though it looks no
> > other arch uses scuh convention in U-Boot :)
> >
> > However, with this change, it lost the capability of overriding an
> > architecture defined Kconfig option at board level.
> >
> > I have a patch @
> >
> http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commitdiff;h=5a650689410482907a37f77b2a4257d81bb4daa2
> > to express such capability.
> >
>
> This should be easy to fix by just moving the source statement, right?
>

Yes. You can drop this patch.

Regards,
Bin
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ce07fb4b55..10d17a0e18 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -4,21 +4,6 @@  menu "RISC-V architecture"
 config SYS_ARCH
 	default "riscv"
 
-choice
-	prompt "Target select"
-	optional
-
-config TARGET_AX25_AE350
-	bool "Support ax25-ae350"
-
-config TARGET_QEMU_VIRT
-	bool "Support QEMU Virt Board"
-
-endchoice
-
-source "board/AndesTech/ax25-ae350/Kconfig"
-source "board/emulation/qemu-riscv/Kconfig"
-
 choice
 	prompt "Base ISA"
 	default ARCH_RV32I
@@ -72,4 +57,6 @@  config 32BIT
 config 64BIT
 	bool
 
+source "arch/riscv/Kconfig.board"
+
 endmenu
diff --git a/arch/riscv/Kconfig.board b/arch/riscv/Kconfig.board
new file mode 100644
index 0000000000..fcada760c8
--- /dev/null
+++ b/arch/riscv/Kconfig.board
@@ -0,0 +1,14 @@ 
+choice
+	prompt "Target select"
+	optional
+
+config TARGET_AX25_AE350
+	bool "Support ax25-ae350"
+
+config TARGET_QEMU_VIRT
+	bool "Support QEMU Virt Board"
+
+endchoice
+
+source "board/AndesTech/ax25-ae350/Kconfig"
+source "board/emulation/qemu-riscv/Kconfig"