Change in coreboot[master]: sb/intel: Introduce SCMI Kconfig options

[Keith Hui (Code Review)]

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48921 )

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

sb/intel: Introduce SCMI Kconfig options

Introduce two options for enabling SMBus Control Method Interface [1] support.

1. HAVE_SCMI is to be set by southbridges that implements SCMI in the DSDT.
   That implementation should be conditional on USE_SCMI. See below.
2. USE_SCMI is guarded by HAVE_SCMI and actually provides the option to the
   user in Kconfig. Enabling it adds the SCMI implementation to the DSDT and
   make it usable. Boards that require this support for board function should
   set it in their BOARD_SPECIFIC_OPTIONS, which will lock this option on.

It will be used by CB:41735 but are designed for wider adoption e.g. by CB:44507

[1] http://smbus.org/specs/smbus_cmi10.pdf
[41735] https://review.coreboot.org/c/coreboot/+/41735
[44507] https://review.coreboot.org/c/coreboot/+/44507

Change-Id: Ib1e176b6495fa673fd46cbb8f8fd7c5a41ababde
Signed-off-by: Keith Hui 
---
M src/southbridge/intel/common/Kconfig
1 file changed, 19 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/48921/1

diff --git a/src/southbridge/intel/common/Kconfig b/src/southbridge/intel/common/Kconfig
index 3030d25..11ba094 100644
--- a/src/southbridge/intel/common/Kconfig
+++ b/src/southbridge/intel/common/Kconfig
@@ -105,3 +105,22 @@
    hex
    depends on SOUTHBRIDGE_INTEL_COMMON_SMBUS
    default 0x400
+
+config HAVE_SCMI
+	bool
+	help
+	  Selected by southbridges that implements SCMI in their DSDT.
+	  See http://smbus.org/specs/smbus_cmi10.pdf for details.
+
+config USE_SCMI
+	bool "Enable SMBus Control Method Interface (SCMI)"
+	depends on HAVE_SCMI && HAVE_ACPI_TABLES
+	help
+	  Adds extra access methods conforming to the SMBus Control Method Interface (SCMI)
+	  specification to the SMBus ACPI device. This provides alternate SMBus access if
+	  regular means are unavailable due to e.g. resource conflicts.
+
+	  ASUS P3B-F mainboard is known to need this.
+
+	  If you say Y here, you will have to blacklist the native driver and instead use
+	  i2c-scmi driver to access SMBus-connected devices.

[Keith Hui (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

sb/intel: Introduce SCMI Kconfig options

Introduce two options for enabling SMBus Control Method Interface [1] support.

1. HAVE_SCMI is to be set by southbridges that implements SCMI in the DSDT.
   That implementation should be conditional on USE_SCMI. See below.
2. USE_SCMI is guarded by HAVE_SCMI and actually provides the option to the
   user in Kconfig. Enabling it adds the SCMI implementation to the DSDT and
   make it usable. Boards that require this support for board function should
   set it in their BOARD_SPECIFIC_OPTIONS, which will lock this option on.

It will be used by CB:41735 but are designed for wider adoption e.g. by CB:44507

[1] http://smbus.org/specs/smbus_cmi10.pdf
[41735] https://review.coreboot.org/c/coreboot/+/41735
[44507] https://review.coreboot.org/c/coreboot/+/44507

Change-Id: Ib1e176b6495fa673fd46cbb8f8fd7c5a41ababde
Signed-off-by: Keith Hui 
---
M src/southbridge/intel/common/Kconfig
M src/southbridge/intel/i82371eb/Kconfig
M src/southbridge/intel/i82371eb/acpi/i82371eb.asl
A src/southbridge/intel/i82371eb/acpi/smbus.asl
4 files changed, 205 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/48921/2

[Keith Hui (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

sb/intel: Introduce SCMI Kconfig options

Introduce two options for enabling SMBus Control Method Interface [1] support.

1. HAVE_SCMI is to be set by southbridges that implements SCMI in the DSDT.
   That implementation should be conditional on USE_SCMI. See below.
2. USE_SCMI is guarded by HAVE_SCMI and actually provides the option to the
   user in Kconfig. Enabling it adds the SCMI implementation to the DSDT and
   make it usable. Boards that require this support for board function should
   set it in their BOARD_SPECIFIC_OPTIONS, which will lock this option on.

It will be used by CB:41735 but are designed for wider adoption e.g. by CB:44507

[1] http://smbus.org/specs/smbus_cmi10.pdf
[41735] https://review.coreboot.org/c/coreboot/+/41735
[44507] https://review.coreboot.org/c/coreboot/+/44507

Change-Id: Ib1e176b6495fa673fd46cbb8f8fd7c5a41ababde
Signed-off-by: Keith Hui 
---
M src/southbridge/intel/common/Kconfig.common
M src/southbridge/intel/i82371eb/Kconfig
M src/southbridge/intel/i82371eb/acpi/i82371eb.asl
A src/southbridge/intel/i82371eb/acpi/smbus.asl
4 files changed, 205 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/48921/3

[Keith Hui (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

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

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

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

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

sb/intel: Introduce SCMI Kconfig options

Introduce two options for enabling SMBus Control Method Interface [1] support.

1. HAVE_SCMI is to be set by southbridges that implements SCMI in the DSDT.
   That implementation should be conditional on USE_SCMI. See below.
2. USE_SCMI is guarded by HAVE_SCMI and actually provides the option to the
   user in Kconfig. Enabling it adds the SCMI implementation to the DSDT and
   make it usable. Boards that require this support for board function should
   set it in their BOARD_SPECIFIC_OPTIONS, which will lock this option on.

It will be used by CB:41735 but is designed for wider adoption e.g. by CB:44507

[1] http://smbus.org/specs/smbus_cmi10.pdf
[41735] https://review.coreboot.org/c/coreboot/+/41735
[44507] https://review.coreboot.org/c/coreboot/+/44507

Change-Id: Ib1e176b6495fa673fd46cbb8f8fd7c5a41ababde
Signed-off-by: Keith Hui 
---
M src/southbridge/intel/common/Kconfig.common
1 file changed, 19 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/48921/5

[Angel Pons (Code Review)]

Attention is currently required from: Keith Hui.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48921 )

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

Patch Set 5: Code-Review+1

(2 comments)

File src/southbridge/intel/common/Kconfig.common:

https://review.coreboot.org/c/coreboot/+/48921/comment/7484224c_058600b6 
PS5, Line 126: implements
nit: implement

https://review.coreboot.org/c/coreboot/+/48921/comment/4b68eb8e_eb1bbee9 
PS5, Line 131: HAVE_ACPI_TABLES
I'd move this dependency to `HAVE_SCMI`, and adjust the select in CB:41735 accordingly:

select HAVE_SCMI if HAVE_ACPI_TABLES

Let me know if this approach works.

[Angel Pons (Code Review)]

Attention is currently required from: Keith Hui.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48921 )

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

Patch Set 5:

(1 comment)

File src/southbridge/intel/common/Kconfig.common:

https://review.coreboot.org/c/coreboot/+/48921/comment/67a14fa3_8be8cb98 
PS5, Line 123: config HAVE_SCMI
Given that CB:41735 adds the relevant code in southbridge/intel/i82371eb, shouldn't these Kconfig options be placed there too?

[Keith Hui (Code Review)]

Hello build bot (Jenkins), Angel Pons, Patrick Rudolph,

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

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

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

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

sb/intel: Introduce SCMI Kconfig options

Introduce two options for enabling SMBus Control Method Interface [1] support.

1. HAVE_SCMI is to be set by southbridges that implements SCMI in the DSDT.
   That implementation should be conditional on USE_SCMI below.
2. USE_SCMI is guarded by HAVE_SCMI and actually provides the option to the
   user in Kconfig. Enabling it adds the SCMI implementation to the DSDT and
   make it usable. Boards that require this support for board function should
   set it in their BOARD_SPECIFIC_OPTIONS, which will lock this option on.

It will be used by CB:41735 but is designed for wider adoption e.g. by CB:44507

[1] http://smbus.org/specs/smbus_cmi10.pdf
[41735] https://review.coreboot.org/c/coreboot/+/41735
[44507] https://review.coreboot.org/c/coreboot/+/44507

Change-Id: Ib1e176b6495fa673fd46cbb8f8fd7c5a41ababde
Signed-off-by: Keith Hui 
---
M src/southbridge/intel/common/Kconfig.common
1 file changed, 19 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/48921/7

[Keith Hui (Code Review)]

Attention is currently required from: Angel Pons.
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48921 )

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

Patch Set 7:

(3 comments)

File src/southbridge/intel/common/Kconfig.common:

https://review.coreboot.org/c/coreboot/+/48921/comment/f808844c_b550e59a 
PS5, Line 123: config HAVE_SCMI
> Given that CB:41735 adds the relevant code in southbridge/intel/i82371eb, shouldn't these Kconfig op […]

These two are meant for more than just i82371eb. What impact would there be if these options begin in i82371eb and eventually have to move when more Intel southbridges implement this?

https://review.coreboot.org/c/coreboot/+/48921/comment/0b098800_b5cd4524 
PS5, Line 126: implements
> nit: implement

Done

https://review.coreboot.org/c/coreboot/+/48921/comment/80e2799e_79624912 
PS5, Line 131: HAVE_ACPI_TABLES
> I'd move this dependency to `HAVE_SCMI`, and adjust the select in CB:41735 accordingly: […]

Done - dependency moved to 41735 so this patch only adds the two Kconfig options.

[Angel Pons (Code Review)]

Attention is currently required from: Keith Hui.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48921 )

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

Patch Set 7:

(1 comment)

File src/southbridge/intel/common/Kconfig.common:

https://review.coreboot.org/c/coreboot/+/48921/comment/a19bf01b_1926a3ce 
PS5, Line 123: config HAVE_SCMI
> These two are meant for more than just i82371eb. […]

If the implementation isn't shared, I'd simply have separate Kconfig options. It's not a problem.

[Keith Hui (Code Review)]

Keith Hui has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48921 )

Change subject: sb/intel: Introduce SCMI Kconfig options
......................................................................

Abandoned

Revive when SCMI gets wider adoption.