Change in coreboot[master]: sb/intel/i82371eb: Implement SMBus Control Method Interface support
Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This is an attempt to expand the ASL SMBus routines into a complete SMBus Control Method Interface (SCMI), that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
Because SCMI methods begin with an underscore, which iasl considers "unknown reserved name", on which our build process errors out, it must also be told to ignore this particular warning, at least for asus/p3b-f.
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M Makefile.inc M src/southbridge/intel/i82371eb/acpi/i82371eb.asl 2 files changed, 183 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/1
diff --git a/Makefile.inc b/Makefile.inc index 210e9cf..d1aebd3 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -253,11 +253,17 @@ # "Multiple types (Device object requires either a _HID or _ADR, but not both)" MULTIPLE_TYPES_WARNING = 3073
+UNKNOWN_RESERVED_NAME = 3133 + ifeq ($(CONFIG_SOUTHBRIDGE_INTEL_LYNXPOINT)$(CONFIG_SOC_INTEL_BROADWELL),y) IGNORED_IASL_WARNINGS = -vw $(EMPTY_RESOURCE_TEMPLATE_WARNING) -vw $(REDUNDANT_OFFSET_REMARK) -vw $(MULTIPLE_TYPES_WARNING) else +ifeq ($(CONFIG_BOARD_ASUS_P3B_F),y) +IGNORED_IASL_WARNINGS = -vw $(EMPTY_RESOURCE_TEMPLATE_WARNING) -vw $(REDUNDANT_OFFSET_REMARK) -vw $(UNKNOWN_RESERVED_NAME) +else IGNORED_IASL_WARNINGS = -vw $(EMPTY_RESOURCE_TEMPLATE_WARNING) -vw $(REDUNDANT_OFFSET_REMARK) endif +endif
define asl_template $(CONFIG_CBFS_PREFIX)/$(1).aml-file = $(obj)/$(1).aml diff --git a/src/southbridge/intel/i82371eb/acpi/i82371eb.asl b/src/southbridge/intel/i82371eb/acpi/i82371eb.asl index b64036e..27c2a3c 100644 --- a/src/southbridge/intel/i82371eb/acpi/i82371eb.asl +++ b/src/southbridge/intel/i82371eb/acpi/i82371eb.asl @@ -2,6 +2,8 @@
#include "southbridge/intel/i82371eb/i82371eb.h"
+#define SCMI_MUTEX SBX0 + /* Declares assorted devices that falls under this southbridge. */ Device (PX40) { @@ -135,4 +137,179 @@ }) Return (BUF1) } +/* Begin SCMI implementation */ + Mutex(SCMI_MUTEX, 1) + + Device (SMB0) + { + OperationRegion (SM00, SystemIO, SMBUS_IO_BASE, 7) + /* Should have been SMBUS01, but Intel iasl won't accept */ + Name (_HID, "SMB0001") + + Field (SM00, ByteAcc, NoLock, Preserve) + { + HSTS, 8, + Offset (0x02), + CTLR, 8, + CMDR, 8, + ADDR, 8, + DAT0, 8, + DAT1, 8 + } + Method (_CRS, 0, NotSerialized) + { + Name (BUF1, ResourceTemplate () + { + /* SMBus register ports */ + IO (Decode16, SMBUS_IO_BASE, SMBUS_IO_BASE, 0x01, 0x10, ) + }) + Return (BUF1) + } + /** + * SCMI: SMBus Information + * + * @return SMBus information buffer + */ + Method (_SBI, 0) + { + Local0 = Package(2) {0x10, Buffer() + { + 0x10, 0x10, /* SCMI and SMBus versions, both 1.0 */ + 0, 0, + 3, /* Device count */ + 0x2d, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0x48, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0x49, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0, + 0, 0, 0, 0 + } + }; + return (Local0) + } + /** + * Read from a SMBus device. + * + * @param Arg0 Protocol + * @param Arg1 Slave address + * @param Arg2 Command (register index in our case) + * @return Package (status code, data length, data) + */ + Method (_SBR, 3) { + Local2 = Package(3) {0, 0, 0} + if (Arg0 == 0x07) + { + Local0 = 0x48 /* Start a byte data R/W access */ + Local3 = 1; /* 1 byte */ + } + elseif (Arg0 == 0x09) + { + Local0 = 0x4c /* Start a word data R/W access */ + } + else + { + Index(Local2, 0) = 0x19 // Protocol not supported + return (Local2) + } + if (HSTS & 0x01) + { + Index(Local2, 0) = 0x1a // Bus busy + return (Local2) + } + if (Acquire(SCMI_MUTEX, 100)) + { + Index(Local2, 0) = 0x18 // Timeout + } else { + ADDR = (Arg1 << 1) | 0x01 /* Set bit 0, this is a read */ + CMDR = Arg2 + HSTS = 0xFF + CTLR = Local0 + + while (HSTS & 0x01) + { + Stall(1) + } + if (HSTS & 0x06) + { + Index(Local2, 0) = 0x11 /* Device error */ + } + else + { + Local1 = DAT0 + /* Get high byte of word read */ + if (Arg0 == 0x09) + { + Local1 |= (DAT1 << 8) + Local3 = 2 /* 2 bytes */ + } + Index(Local2, 1) = Local3 + Index(Local2, 2) = Local1 + } + Release(SCMI_MUTEX); + } + return (Local2); + } + /** + * Write to a SMBus device. + * + * @param Arg0 Protocol + * @param Arg1 Slave address + * @param Arg2 Command (register index in our case) + * @param Arg3 Data length + * @param Arg4 Data + * @return Status code + */ + Method (_SBW, 5) { + if (Arg0 == 0x06) + { + Local0 = 0x48 /* Start a byte data R/W access */ + } + elseif (Arg0 == 0x08) + { + Local0 = 0x4c /* Start a word data R/W access */ + } + else + { + Return (0x19) //* Protocol not supported */ + } + if (HSTS & 0x01) + { + Return (0x1a) /* Bus busy */ + } + if (Acquire(SCMI_MUTEX, 100)) + { + Local1 = 0x18 /* Timeout */ + } + else + { + Local1 = 0 + ADDR = Arg1 << 1 + CMDR = Arg2 + DAT0 = Arg4 + if (Arg0 == 0x08) { + DAT1 = Arg4 >> 8 + } + HSTS = 0xFF + CTLR = Local0 + while (HSTS & 0x01) + { + Stall(1) + } + if (HSTS & 0x06) + { + Local1 = 0x11 /* Device error */ + } + Release(SCMI_MUTEX) + } + return (Local1); + } + /* _SBT, _SBA not implemented */ + } +/* End SCMI implementation */ }
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 3:
This change is ready for review.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/Kconfig:
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... PS4, Line 9: config HAVE_SCMI : bool : default y if SOUTHBRIDGE_INTEL_I82371EB : help : Southbridges select Y if their DSDT implements SCMI. : 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 may be your only access to SMBus : devices if other means fail. : : ASUS P3B-F mainboard requires 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. This needs to be guarded:
if SOUTHBRIDGE_INTEL_I82371EB
config HAVE_SCMI ...
endif
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41735/4/Makefile.inc File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41735/4/Makefile.inc@266 PS4, Line 266: # Ignore names with leading underscore so that SCMI methods can be implemented. : # See cb:41735 : UNKNOWN_RESERVED_NAME = 3133 please wait IASL version 20200925 https://review.coreboot.org/c/coreboot/+/45750
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/acpi/smbus.asl:
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... PS4, Line 134: Index(Local0, 0) = Local1 please use ASL 2.0 syntax.
Local0 [0] = Local1
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41735
to look at the new patch set (#5).
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This change grows the ASL SMBus routines into a complete enough SMBus Control Method Interface (SCMI) implementation, that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
The new routines are in a separate file, with OpRegion aligned with other Intel southbridges, so that it can be more widely adopted with minimal edits.
This change is guarded by a user-selectable Kconfig option, but forced enabled for asus/p3b-f. Reason is other i82371eb boards may benefit from, but do not require this change.
Developed and tested on asus/p3b-f, where: - Linux 5.4.42 autoloads i2c-scmi (4.4.14 cannot) - dmesg shows no more conflict with i2c-piix4 blacklisted - i2cdetect can find the 3 i2c addresses of AS991227F - with w83781d module loaded, lm-sensors can get readings
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/Kconfig M src/southbridge/intel/i82371eb/acpi/i82371eb.asl A src/southbridge/intel/i82371eb/acpi/smbus.asl 3 files changed, 205 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/5
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41735
to look at the new patch set (#6).
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This change grows the ASL SMBus routines into a complete enough SMBus Control Method Interface (SCMI) implementation, that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
The new routines are in a separate file, with OpRegion aligned with other Intel southbridges, so that it can be more widely adopted with minimal edits.
This change is guarded by a user-selectable Kconfig option, but forced enabled for asus/p3b-f. Reason is other i82371eb boards may benefit from, but do not require this change.
Developed and tested on asus/p3b-f, where: - Linux 5.4.42 autoloads i2c-scmi (4.4.14 cannot) - dmesg shows no more conflict with i2c-piix4 blacklisted - i2cdetect can find the 3 i2c addresses of AS991227F - with w83781d module loaded, lm-sensors can get readings
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/Kconfig M src/southbridge/intel/i82371eb/acpi/i82371eb.asl A src/southbridge/intel/i82371eb/acpi/smbus.asl 3 files changed, 205 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/6
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/Kconfig:
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... PS4, Line 9: config HAVE_SCMI : bool : default y if SOUTHBRIDGE_INTEL_I82371EB : help : Southbridges select Y if their DSDT implements SCMI. : 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 may be your only access to SMBus : devices if other means fail. : : ASUS P3B-F mainboard requires 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.
This needs to be guarded: […]
Still not guarded. The options would show on other platforms. If you intend to allow other southbridges to use SCMI, the Kconfig options should be declared elsewhere. In any case, I'd appreciate a reply.
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/acpi/smbus.asl:
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... PS4, Line 134: Index(Local0, 0) = Local1
please use ASL 2.0 syntax. […]
Done
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... File src/southbridge/intel/i82371eb/Kconfig:
https://review.coreboot.org/c/coreboot/+/41735/4/src/southbridge/intel/i8237... PS4, Line 9: config HAVE_SCMI : bool : default y if SOUTHBRIDGE_INTEL_I82371EB : help : Southbridges select Y if their DSDT implements SCMI. : 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 may be your only access to SMBus : devices if other means fail. : : ASUS P3B-F mainboard requires 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.
Still not guarded. The options would show on other platforms. […]
This option is dependent on HAVE_SCMI being set, and should not show unless it is set in the southbridge (and have SCMI interfaces implemented). Seeing wider SCMI adoption elsewhere starting with CB:44507, I am preparing a separate patch to move these options to src/acpi/Kconfig. I'm going to run a few builds to make sure before submitting.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41735
to look at the new patch set (#7).
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This change grows the ASL SMBus routines into a complete enough SMBus Control Method Interface (SCMI) implementation, that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
The new routines are in a separate file, with OpRegion aligned with other Intel southbridges, so that it can be more widely adopted with minimal edits.
This change is guarded by a user-selectable Kconfig option, but forced enabled for asus/p3b-f. Reason is other i82371eb boards may benefit from, but do not require this change.
Developed and tested on asus/p3b-f, where: - Linux 5.4.42 autoloads i2c-scmi (4.4.14 cannot) - dmesg shows no more conflict with i2c-piix4 blacklisted - i2cdetect can find the 3 i2c addresses of AS991227F - with w83781d module loaded, lm-sensors can get readings
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/Kconfig M src/southbridge/intel/i82371eb/acpi/i82371eb.asl A src/southbridge/intel/i82371eb/acpi/smbus.asl 3 files changed, 186 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/7
Hello build bot (Jenkins), Martin Roth, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41735
to look at the new patch set (#8).
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This change grows the ASL SMBus routines into a complete enough SMBus Control Method Interface (SCMI) implementation, that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
The new routines are in a separate file, with OpRegion aligned with other Intel southbridges, so that it can be more widely adopted with minimal edits.
This change is guarded by a user-selectable Kconfig option, but forced enabled for asus/p3b-f. Reason is other i82371eb boards may benefit from, but do not require this change.
Because SCMI methods begin with an underscore, which iasl considers "unknown reserved name", on which our build process errors out, it must also be told to ignore this particular warning.
Developed and tested on asus/p3b-f, where: - Linux 5.4.42 autoloads i2c-scmi (4.4.14 cannot) - dmesg shows no more conflict with i2c-piix4 blacklisted - i2cdetect can find the 3 i2c addresses of AS991227F - with w83781d module loaded, lm-sensors can get readings
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/Kconfig M src/southbridge/intel/i82371eb/acpi/i82371eb.asl A src/southbridge/intel/i82371eb/acpi/smbus.asl 3 files changed, 186 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/8
Attention is currently required from: Keith Hui. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 8: Code-Review+1
(2 comments)
File src/southbridge/intel/i82371eb/acpi/smbus.asl:
https://review.coreboot.org/c/coreboot/+/41735/comment/c47b1aa6_c2a1bb08 PS8, Line 7: /* Should have been SMBUS01, but Intel iasl won't accept */ That's because `SMBUS01` is not a valid ACPI HID. `SMB0001` at least conforms to the required format, but I'm not sure if it's an official HID.
There's also the `EisaId` method in ASL, not sure if it's needed here.
https://review.coreboot.org/c/coreboot/+/41735/comment/41073a9a_749fd5d5 PS8, Line 25: Your board may need adjustment How would one do this adjustment?
Attention is currently required from: Angel Pons. Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 8:
(3 comments)
File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41735/comment/c58af6c3_b2445025 PS4, Line 266: # Ignore names with leading underscore so that SCMI methods can be implemented. : # See cb:41735 : UNKNOWN_RESERVED_NAME = 3133
please wait IASL version 20200925 […]
Ack
File src/southbridge/intel/i82371eb/Kconfig:
https://review.coreboot.org/c/coreboot/+/41735/comment/883f7c73_b0fc74b4 PS4, Line 9: config HAVE_SCMI : bool : default y if SOUTHBRIDGE_INTEL_I82371EB : help : Southbridges select Y if their DSDT implements SCMI. : 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 may be your only access to SMBus : devices if other means fail. : : ASUS P3B-F mainboard requires 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.
This option is dependent on HAVE_SCMI being set, and should not show unless it is set in the southbr […]
This has been split off to CB:48921.
File src/southbridge/intel/i82371eb/acpi/smbus.asl:
https://review.coreboot.org/c/coreboot/+/41735/comment/daec03de_4d391873 PS8, Line 25: Your board may need adjustment
How would one do this adjustment?
By editing this device list. Updated comment to follow in next patch set.
Attention is currently required from: Keith Hui. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 8:
(1 comment)
File src/southbridge/intel/i82371eb/acpi/smbus.asl:
https://review.coreboot.org/c/coreboot/+/41735/comment/3738cd01_8609b7e7 PS8, Line 25: Your board may need adjustment
By editing this device list. Updated comment to follow in next patch set.
Hmmm, if this is mainboard-specific, I'd rather have it defined in mainboard-specific code. One approach that doesn't increase coreboot.rom space usage would be to include a mainboard-provided header from here that defines _SBI
Attention is currently required from: Keith Hui. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 8:
(1 comment)
File src/southbridge/intel/i82371eb/acpi/smbus.asl:
https://review.coreboot.org/c/coreboot/+/41735/comment/931faf39_8327260d PS8, Line 25: Your board may need adjustment
Hmmm, if this is mainboard-specific, I'd rather have it defined in mainboard-specific code. […]
Another way would be to define _SBI in a `Scope (SMB0)` block in mainboard code.
Hello build bot (Jenkins), Martin Roth - Personal, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41735
to look at the new patch set (#9).
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This change grows the ASL SMBus routines into a complete enough SMBus Control Method Interface (SCMI) implementation, that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
The new routines are in a separate file, with OpRegion aligned with other Intel southbridges, so that it can be more widely adopted with minimal edits.
This change is guarded by a user-selectable Kconfig option, forced selected for asus/p3b-f. Reason is other i82371eb boards may benefit from, but do not require this change.
Developed and tested on asus/p3b-f, where: - Linux 5.4.42 autoloads i2c-scmi (4.4.14 cannot) - dmesg shows no more conflict with i2c-piix4 blacklisted - i2cdetect can find the 3 i2c addresses of AS991227F - with w83781d module loaded, lm-sensors can get readings
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/Kconfig M src/southbridge/intel/i82371eb/acpi/i82371eb.asl A src/southbridge/intel/i82371eb/acpi/smbus.asl 3 files changed, 163 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/9
Hello build bot (Jenkins), Martin Roth, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41735
to look at the new patch set (#10).
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This change grows the ASL SMBus routines into a complete enough SMBus Control Method Interface (SCMI) implementation, that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
The new routines are in a separate file, with OpRegion aligned with other Intel southbridges, so that it can be more widely adopted with minimal edits.
This change is guarded by a user-selectable Kconfig option, forced selected for asus/p3b-f. Reason is other i82371eb boards may benefit from, but do not require this change.
Developed and tested on asus/p3b-f, where: - Linux 5.4.42 autoloads i2c-scmi (4.4.14 cannot) - dmesg shows no more conflict with i2c-piix4 blacklisted - i2cdetect can find the 3 i2c addresses of AS991227F - with w83781d module loaded, lm-sensors can get readings
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/Kconfig M src/southbridge/intel/i82371eb/acpi/i82371eb.asl A src/southbridge/intel/i82371eb/acpi/smbus.asl 3 files changed, 183 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/10
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 10:
(2 comments)
File src/southbridge/intel/i82371eb/Kconfig:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-139931): https://review.coreboot.org/c/coreboot/+/41735/comment/c5c0ccf4_b56f2f5b PS10, Line 3: trailing whitespace
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-139931): https://review.coreboot.org/c/coreboot/+/41735/comment/a2f6deb8_f3ff95c4 PS10, Line 25: trailing whitespace
Attention is currently required from: Keith Hui. Hello build bot (Jenkins), Martin Roth, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41735
to look at the new patch set (#11).
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
sb/intel/i82371eb: Implement SMBus Control Method Interface support
Upcoming ACPI support for asus/p3b-f mainboard contains ASL code to manipulate its AS99127F chip over SMBus before and after suspend, to blink front panel lights, turn off fans, among other things. This code caused an ACPI/PCI resource conflict that disabled access to the entire SMBus, rendering its hardware monitor inoperative, and all resolution attempts so far have not worked.
This change grows the ASL SMBus routines into a complete enough SMBus Control Method Interface (SCMI) implementation, that Linux kernel can use via the i2c-scmi driver, sidestepping this conflict.
The new routines are in a separate file, with OpRegion aligned with other Intel southbridges, so that it can be more widely adopted with minimal edits.
This change is guarded by a user-selectable Kconfig option, forced selected for asus/p3b-f. Reason is other i82371eb boards may benefit from, but do not require this change.
Developed and tested on asus/p3b-f, where: - Linux 5.4.42 autoloads i2c-scmi (4.4.14 cannot) - dmesg shows no more conflict with i2c-piix4 blacklisted - i2cdetect can find the 3 i2c addresses of AS991227F - with w83781d module loaded, lm-sensors can get readings
Change-Id: I97deb2d292af25e42068474f48e11c94a7480be6 Signed-off-by: Keith Hui buurin@gmail.com --- M src/southbridge/intel/i82371eb/Kconfig M src/southbridge/intel/i82371eb/acpi/i82371eb.asl A src/southbridge/intel/i82371eb/acpi/smbus.asl 3 files changed, 183 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/41735/11
Attention is currently required from: Keith Hui.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41735 )
Change subject: sb/intel/i82371eb: Implement SMBus Control Method Interface support ......................................................................
Patch Set 11: Code-Review+1
(1 comment)
File src/southbridge/intel/i82371eb/Kconfig:
https://review.coreboot.org/c/coreboot/+/41735/comment/0c04d1df_cf3f52d5 PS11, Line 4: if SOUTHBRIDGE_INTEL_I82371EB : : config SOUTHBRIDGE_OPTIONS : def_bool y Is it necessary to change this?
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
HAOUAS Elyes (Code Review)
-
Keith Hui (Code Review)