Change in ...coreboot[master]: arch/riscv: Don't select ARCH_RISCV_M on non-ARCH_RISCV platforms

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31311

Change subject: arch/riscv: Don't select ARCH_RISCV_M on non-ARCH_RISCV platforms
......................................................................

arch/riscv: Don't select ARCH_RISCV_M on non-ARCH_RISCV platforms

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
---
M src/arch/riscv/Kconfig
1 file changed, 1 insertion(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/31311/1

diff --git a/src/arch/riscv/Kconfig b/src/arch/riscv/Kconfig
index 9d325af..f5253bb 100644
--- a/src/arch/riscv/Kconfig
+++ b/src/arch/riscv/Kconfig
@@ -24,6 +24,7 @@
    # one implementation that will not have it due
    # to security concerns.
    bool
+	depends on ARCH_RISCV
    default n if ARCH_RISCV_M_DISABLED
    default y

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: Don't select ARCH_RISCV_M on non-ARCH_RISCV platforms
......................................................................

Patch Set 1:

Maybe add an `if` around everything below `ARCH_RISCV`?
The other options show up in all .configs as well, just
set to `n`.

Change in ...coreboot[master]: arch/riscv: make RISCV_ARCH options depend on select ARCH_RISCV

[Arthur Heymans (Code Review)]

Hello ron minnich, Jonathan Neuschäfer, build bot (Jenkins), Philipp Hug,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31311

to look at the new patch set (#2).

Change subject: arch/riscv: make RISCV_ARCH options depend on select ARCH_RISCV
......................................................................

arch/riscv: make RISCV_ARCH options depend on select ARCH_RISCV

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
---
M src/arch/riscv/Kconfig
1 file changed, 14 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/31311/2

Change in ...coreboot[master]: arch/riscv: make RISCV_ARCH options depend on select ARCH_RISCV

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: make RISCV_ARCH options depend on select ARCH_RISCV
......................................................................

Patch Set 2: Code-Review+1

Same seems to apply to all other architectures...

Change in ...coreboot[master]: arch/riscv: make RISCV_ARCH options depend on select ARCH_RISCV

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: make RISCV_ARCH options depend on select ARCH_RISCV
......................................................................

Patch Set 2:

Oh, btw. removing the `default n` from the unguarded
options would also remove their spurious entries.

Change in ...coreboot[master]: arch/*: make ARCH specific options depend on select ARCH_x

[Arthur Heymans (Code Review)]

Hello ron minnich, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Philipp Hug,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31311

to look at the new patch set (#3).

Change subject: arch/*: make ARCH specific options depend on select ARCH_x
......................................................................

arch/*: make ARCH specific options depend on select ARCH_x

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
---
M src/arch/arm/Kconfig
M src/arch/arm64/Kconfig
M src/arch/mips/Kconfig
M src/arch/ppc64/Kconfig
M src/arch/riscv/Kconfig
M src/arch/x86/Kconfig
6 files changed, 34 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/31311/3

Change in ...coreboot[master]: arch/*: make ARCH specific options depend on select ARCH_x

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/*: make ARCH specific options depend on select ARCH_x
......................................................................

Patch Set 3:

Let's see if jenkins thinks this is a good idea...

Change in ...coreboot[master]: arch/*: make ARCH specific options depend on select ARCH_x

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/*: make ARCH specific options depend on select ARCH_x
......................................................................

Patch Set 3:

I'll de per arch cleanups given that they are all a bit messy in this regard...

Change in ...coreboot[master]: arch/*: make ARCH specific options depend on select ARCH_x

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/*: make ARCH specific options depend on select ARCH_x
......................................................................

Patch Set 3: Code-Review+1

Change in ...coreboot[master]: arch/*: make ARCH specific options depend on select ARCH_x

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/*: make ARCH specific options depend on select ARCH_x
......................................................................

Patch Set 3:
> Let's see if jenkins thinks this is a good idea...

Even if, we should also check if the binaries (or
maybe just `config.h` files) stay the same. In
theory, defaults for one arch could have been
applied for another one by accident.

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV

[Arthur Heymans (Code Review)]

Hello ron minnich, Angel Pons, Julius Werner, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Philipp Hug,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31311

to look at the new patch set (#4).

Change subject: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV
......................................................................

arch/riscv: Make RISCV specific options depend on select ARCH_RISCV

Also don't define the default as this result in spurious lines in the
.config.

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
---
M src/arch/riscv/Kconfig
1 file changed, 12 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/31311/4

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV
......................................................................

Patch Set 4:
> Patch Set 3:
> > Let's see if jenkins thinks this is a good idea...
>
> Even if, we should also check if the binaries (or
> maybe just `config.h` files) stay the same. In
> theory, defaults for one arch could have been
> applied for another one by accident.

For the few boards I tested with BUILD_TIMELESS on this patchseries the binary was identical, but I should probably test all boards.

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV

[Philipp Hug (Code Review)]

Philipp Hug has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV
......................................................................

Patch Set 4: Code-Review+1

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV

[Arthur Heymans (Code Review)]

Hello ron minnich, Angel Pons, Julius Werner, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Philipp Hug,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31311

to look at the new patch set (#5).

Change subject: arch/riscv: Make RISCV specific options depend on select ARCH_RISCV
......................................................................

arch/riscv: Make RISCV specific options depend on select ARCH_RISCV

Also don't define the default as this result in spurious lines in the
.config.

The only difference in the generated config.h is that for most board
ARCH_RISCV_M goes from 1 to 0. This should not matter.

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
---
M src/arch/riscv/Kconfig
1 file changed, 12 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/31311/5

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on ARCH_RISCV

[Arthur Heymans (Code Review)]

Hello ron minnich, Angel Pons, Julius Werner, Jonathan Neuschäfer, build bot (Jenkins), Nico Huber, Philipp Hug,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/31311

to look at the new patch set (#6).

Change subject: arch/riscv: Make RISCV specific options depend on ARCH_RISCV
......................................................................

arch/riscv: Make RISCV specific options depend on ARCH_RISCV

Also don't define the default as this result in spurious lines in the
.config.

The only difference in the generated config.h is that for most board
ARCH_RISCV_M goes from 1 to 0. This should not matter.

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
---
M src/arch/riscv/Kconfig
1 file changed, 12 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/31311/6

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on ARCH_RISCV

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: Make RISCV specific options depend on ARCH_RISCV
......................................................................

Patch Set 6: Code-Review+2

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on ARCH_RISCV

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: Make RISCV specific options depend on ARCH_RISCV
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/#/c/31311/6//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31311/6//COMMIT_MSG@9 
PS6, Line 9: result
results

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on ARCH_RISCV

[Martin Roth (Code Review)]

Martin Roth has uploaded a new patch set (#7) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: Make RISCV specific options depend on ARCH_RISCV
......................................................................

arch/riscv: Make RISCV specific options depend on ARCH_RISCV

Also don't define the default as this results in spurious lines in the
.config.

The only difference in the generated config.h is that for most board
ARCH_RISCV_M goes from 1 to 0. This should not matter.

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
---
M src/arch/riscv/Kconfig
1 file changed, 12 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/31311/7

Change in ...coreboot[master]: arch/riscv: Make RISCV specific options depend on ARCH_RISCV

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31311 )

Change subject: arch/riscv: Make RISCV specific options depend on ARCH_RISCV
......................................................................

arch/riscv: Make RISCV specific options depend on ARCH_RISCV

Also don't define the default as this results in spurious lines in the
.config.

The only difference in the generated config.h is that for most board
ARCH_RISCV_M goes from 1 to 0. This should not matter.

Change-Id: I3e8c1cc5696d621e243696a3b5e34f62ab69a688
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31311
Reviewed-by: Patrick Rudolph 
Tested-by: build bot (Jenkins) 
---
M src/arch/riscv/Kconfig
1 file changed, 12 insertions(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved

diff --git a/src/arch/riscv/Kconfig b/src/arch/riscv/Kconfig
index 25a3980..a4f1788 100644
--- a/src/arch/riscv/Kconfig
+++ b/src/arch/riscv/Kconfig
@@ -1,6 +1,15 @@
+config ARCH_RISCV_RV64
+	bool
+	select ARCH_RISCV
+
+config ARCH_RISCV_RV32
+	bool
+	select ARCH_RISCV
+
 config ARCH_RISCV
    bool
-	default n
+
+if ARCH_RISCV

config RISCV_ARCH
    string
@@ -37,16 +46,6 @@
    bool
    default n

-config ARCH_RISCV_RV64
-	bool
-	default n
-	select ARCH_RISCV
-
-config ARCH_RISCV_RV32
-	bool
-	default n
-	select ARCH_RISCV
-
 config ARCH_RISCV_PMP
    bool
    default n
@@ -74,3 +73,5 @@

config RISCV_WORKING_HARTID
    int
+
+endif # if ARCH_RISCV