Change in coreboot[master]: soc/intel/apl: Add ACPI for SBMBUS control
Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SBMBUS control ...................................................................... soc/intel/apl: Add ACPI for SBMBUS control Based on acpidump from AMI firmware on the Kontron mAL10 COMe module and SMBus Control Methods Interface Specification, Ver 1.0, 1999. Signed-off-by: Maxim Polyakov <max.senia.poliak@gmail.com> Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 --- A src/soc/intel/apollolake/acpi/smbus.asl M src/soc/intel/apollolake/acpi/southbridge.asl 2 files changed, 795 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/44507/1 diff --git a/src/soc/intel/apollolake/acpi/smbus.asl b/src/soc/intel/apollolake/acpi/smbus.asl new file mode 100644 index 0000000..b41c0f2 --- /dev/null +++ b/src/soc/intel/apollolake/acpi/smbus.asl @@ -0,0 +1,792 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device (SBUS) +{ + /* Intel SMBus Controller 0:1f.1 */ + Name (_ADR, 0x001F0001) + + OperationRegion (SMBP, PCI_Config, 0x40, 0xC0) + Field (SMBP, DWordAcc, NoLock, Preserve) + { + , 2, + I2CE, 1 + } + + OperationRegion (SMPB, PCI_Config, 0x20, 0x04) + Field (SMPB, DWordAcc, NoLock, Preserve) + { + , 5, + SBAR, 11 + } + + OperationRegion (SMBI, SystemIO, (SBAR << 0x05), 0x10) + Field (SMBI, ByteAcc, NoLock, Preserve) + { + HSTS, 8, // Host Status + Offset (0x02), + HCON, 8, // Host Control + HCOM, 8, // Host Command + TXSA, 8, // Transmit Slave Address + DAT0, 8, // Host Data 0 + DAT1, 8, // Host Data 1 + HBDR, 8, // Host Block Data Byte + PECR, 8, // Packet Error Check + RXSA, 8, // Receive Slave Data + SDAT, 16 + } + + // SMBus Send Byte + // Arg0: Address + // Arg1: Data + // Return: 1 = Success, 0=Failure + Method (SSXB, 2, Serialized) + { + If (STRT ()) + { // Is the SMBus Controller Ready? + Return (Zero) + } + + // Send Byte + // SMBus Enable + I2CE = Zero + HSTS = 0xBF + TXSA = Arg0 // Write Address + HCOM = Arg1 // Write Data + HCON = 0x48 // Start + Byte Data Protocol + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (One) // Success + } + + Return (Zero) + } + + // SMBus Receive Byte + // Arg0: Address + // Return: 0xffff = Failure, Data (8bit) = Success + Method (SRXB, 1, Serialized) + { + // Is the SMBus Controller Ready? + If (STRT ()) + { + Return (0xFFFF) + } + + // Receive Byte + I2CE = Zero // SMBus Enable + HSTS = 0xBF + TXSA = (Arg0 | One) // Write Address + HCON = 0x44 + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (DAT0) // Success + } + + Return (0xFFFF) + } + + // SMBus Write Byte + // Arg0: Address + // Arg1: Command + // Arg2: Data + // Return: 1 = Success, 0=Failure + Method (SWRB, 3, Serialized) + { + If (STRT ()) + { // Is the SMBus Controller Ready? + Return (Zero) + } + + I2CE = Zero // SMBus Enable + HSTS = 0xBF + TXSA = Arg0 // Write Address + HCOM = Arg1 // Write Command + DAT0 = Arg2 // Write Data + HCON = 0x48 // Start + Byte Protocol + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (One) // Success + } + + Return (Zero) + } + + // SMBus Read Byte + // Arg0: Address + // Arg1: Command + // Return: 0xffff = Failure, Data (8bit) = Success + Method (SRDB, 2, Serialized) + { + If (STRT ()) + { // Is the SMBus Controller Ready? + Return (0xFFFF) + } + + I2CE = Zero // SMBus Enable + HSTS = 0xBF + // Write Address + TXSA = (Arg0 | One) + HCOM = Arg1 // Command + HCON = 0x48 // Start + If (COMP ()) + { + HSTS |= 0xFF // Clean up + Return (DAT0) // Success + } + + Return (0xFFFF) + } + + // SWRW - write word + // Arg0: Address + // Arg1: Command + // Arg2: Data + // Return: 0 on failure, 1, on success + Method (SWRW, 3, Serialized) + { + If (STRT ()) + { + Return (Zero) + } + + I2CE = Zero + HSTS = 0xBF + TXSA = Arg0 + HCOM = Arg1 + DAT1 = (Arg2 & 0xFF) + DAT0 = ((Arg2 >> 0x08) & 0xFF) + HCON = 0x4C + If (COMP ()) + { + HSTS |= 0xFF + Return (One) + } + + Return (Zero) + } + + // SRDW - read word + // Arg0: Address + // Arg1: Command + // Return: 0xffffffff on failure, the read word otherwise + Method (SRDW, 2, Serialized) + { + If (STRT ()) + { + Return (0xFFFF) + } + + I2CE = Zero + HSTS = 0xBF + TXSA = (Arg0 | One) + HCOM = Arg1 + HCON = 0x4C + If (COMP ()) + { + HSTS |= 0xFF + Return (((DAT0 << 0x08) | DAT1)) + } + + Return (0xFFFFFFFF) + } + + // SBLW - write block data + // Arg0: Address + // Arg1: Command + // Arg2: Data - data[0] is the number of bytes that follow + // Return: 0 on failure, 1, on success + Method (SBLW, 4, Serialized) + { + If (STRT ()) + { + Return (Zero) + } + + I2CE = Arg3 + HSTS = 0xBF + TXSA = Arg0 + HCOM = Arg1 + DAT0 = SizeOf (Arg2) + Local1 = Zero + HBDR = DerefOf (Arg2 [Zero]) + HCON = 0x54 + While ((SizeOf (Arg2) > Local1)) + { + Local0 = 0x0FA0 + While ((!(HSTS & 0x80) && Local0)) + { + Local0-- + Stall (0x32) + } + + If (!Local0) + { + KILL () + Return (Zero) + } + + HSTS = 0x80 + Local1++ + If ((SizeOf (Arg2) > Local1)) + { + HBDR = DerefOf (Arg2 [Local1]) + } + } + + If (COMP ()) + { + HSTS |= 0xFF + Return (One) + } + + Return (Zero) + } + + // SBLR - read block data + // Arg0: Address + // Arg1: Command + // Return: 0 on failure, the read data otherwise + Method (SBLR, 3, Serialized) + { + Name (TBUF, Buffer (0x0100){}) + If (STRT ()) + { + Return (Zero) + } + + I2CE = Arg2 + HSTS = 0xBF + TXSA = (Arg0 | One) + HCOM = Arg1 + HCON = 0x54 + Local0 = 0x0FA0 + While ((!(HSTS & 0x80) && Local0)) + { + Local0-- + Stall (0x32) + } + + If (!Local0) + { + KILL () + Return (Zero) + } + + TBUF [Zero] = DAT0 /* \_SB_.PCI0.SBUS.DAT0 */ + HSTS = 0x80 + Local1 = One + While ((Local1 < DerefOf (TBUF [Zero]))) + { + Local0 = 0x0FA0 + While ((!(HSTS & 0x80) && Local0)) + { + Local0-- + Stall (0x32) + } + + If (!Local0) + { + KILL () + Return (Zero) + } + + TBUF [Local1] = HBDR /* \_SB_.PCI0.SBUS.HBDR */ + HSTS = 0x80 + Local1++ + } + + If (COMP ()) + { + HSTS |= 0xFF + Return (TBUF) /* \_SB_.PCI0.SBUS.SBLR.TBUF */ + } + + Return (Zero) + } + + // Wait for SMBus to become ready + Method (STRT, 0, Serialized) + { + // Timeout 200ms + Local0 = 0xC8 + + While (Local0) + { + If ((HSTS & 0x40)) + { // Is used? + // timeout-- + Local0-- + + // Wait 1ms + Sleep (One) + If ((Local0 == Zero)) + { + Return (One) + } + } + Else + { + // We're ready + Local0 = Zero + } + } + + // Timeout 200ms (50us * 4000) + Local0 = 0x0FA0 + While (Local0) + { + If ((HSTS & One)) + { // Is the host busy? + // timeout-- + Local0-- + + // Wait 50us + Stall (0x32) + + If ((Local0 == Zero)) + { + KILL () + } + } + Else + { // Success + Return (Zero) + } + } + // Failure + Return (One) + } + + // Check if last operation completed + // return Failure = 0, Success = 1 + Method (COMP, 0, Serialized) + { + Local0 = 0x0FA0 // Timeout 200ms in 50us steps + While (Local0) + { + If ((HSTS & 0x02)) + { // Completion Status? + // Operation Completed + Return (One) + } + Else + { + Local0-- + Stall (0x32) + If ((Local0 == Zero)) + { + KILL () + } + } + } + Return (Zero) // Failure + } + + // Kill all SMBus communication + Method (KILL, 0, Serialized) + { + HCON |= 0x02 // Send Kill + HSTS |= 0xFF // Clean Status + } + + // SMBus transactional synchronization + Mutex (SBX0, 0x00) + + OperationRegion (SBU0, SystemIO, (SBAR << 0x05), 0x10) + Field (SBU0, ByteAcc, NoLock, Preserve) + { + HSTA, 8, + Offset (0x02), + HCNT, 8, + HCMD, 8, + HADD, 8, + HDT0, 8, + HDT1, 8, + BLKD, 8, + Offset (0x0D), + AUCT, 8 + } + + // SWTC - Wait for Transaction Complete + // Wait until the previous SMBus transaction has completed + // Arg0 : Timeout Value + // return + // 0x00 : OK + // 0x07 : Unknown Failure + // 0x10 : Address Not Acknowledged + // 0x11 : Device Error + // 0x12 : Command Access Denied + // 0x13 : Unknown Error + // 0x17 : Device Access Denied + // 0x18 : Timeout + // 0x19 : Unsupported Protocol + // 0x1A : Bus Busy + // 0x1F : PEC (CRC-8) Error + Method (SWTC, 1, NotSerialized) + { + Local0 = Arg0 + Local2 = 0x07 + // The previous command has completed when the protocol + // register is equal to 0 (zero). Wait <timeout> ms + // for this to occur. + Local1 = One + While ((Local1 == One)) + { + Local3 = (HSTA & 0x1E) + If ((Local3 != Zero)) + { + If ((Local3 == 0x02)) + { + Local2 = Zero + } + Else + { + Local2 = 0x07 + } + + Local1 = Zero + } + // Transaction isn't complete. Check for timeout, and if not, + // sleep 10ms and loop again. + ElseIf ((Local0 < 0x0A)) + { + Local2 = 0x18 // ERROR: Timeout occurred. + Local1 = Zero // Terminate loop. + } + Else + { + Sleep (0x0A) + Local0 -= 0x0A + } + } + Return (Local2) + } + + // SBR - SMBus Read + // Arg0 : Protocol (Integer) + // Arg1 : Slave Address (Integer) + // Arg2 : Command (Integer) + // return + // (0) : Status (Integer) + // (1) : Data Length (Integer) + // (2) : Data (Integer | Buffer) + Method (SBR, 3, Serialized) + { + I2CE = Zero + Local0 = Package (0x12) + { + 0x07, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero, + Zero + } + + // Make sure the protocol is valid, if not return the + // 'invalid protocol' status code. Note that this segment + // does not support packet error checking. + If ((Arg0 != 0x03)) + { // Read Quick + If ((Arg0 != 0x05)) + { // Receive Byte + If ((Arg0 != 0x07)) + { // Read Byte + If ((Arg0 != 0x09)) + { // Read Word + If ((Arg0 != 0x0B)) + { // Read Block + Local0 [Zero] = 0x19 + Return (Local0) + } + } + } + } + } + + // Acquire the SMBus mutex to ensure transactional synchronization. + If ((Acquire (SBX0, 0xFFFF) == Zero)) + { + // Initiate the transaction by writing the slave address, + // command, and protocol registers. Note that the command + // code and data length are always written, even if the + // protocol doesn't require it (it + // will be ignored). + HSTA = (HSTA | 0xFF) + HADD = (Arg1 | One) + HCMD = Arg2 + If ((Arg0 == 0x03)) + { // Read Quick + HCNT = ((HCNT & 0xA0) | 0x40) + } + + If ((Arg0 == 0x05)) + { // Receive Byte + HCNT = ((HCNT & 0xA0) | 0x44) + } + + If ((Arg0 == 0x07)) + { // Read Byte + HCNT = ((HCNT & 0xA0) | 0x48) + } + + If ((Arg0 == 0x09)) + { // Read Word + HCNT = ((HCNT & 0xA0) | 0x4C) + } + + If ((Arg0 == 0x0B)) + { // Read Block + AUCT &= 0xFD + HCNT = ((HCNT & 0xA0) | 0x54) + Local1 = Zero + } + Else + { + // Wait for completion + Local1 = SWTC (0x03E8) + } + + // Save the status code, data size, and data into the return + // package (if required by the protocol). + Local0 [Zero] = Local1 + If ((Local1 == Zero)) + { + If ((Arg0 == 0x05)) + { // Receive Byte + Local0 [One] = One + Local0 [0x02] = HDT0 + } + + If ((Arg0 == 0x07)) + { // Read Byte + Local0 [One] = One + Local0 [0x02] = HDT0 + } + + If ((Arg0 == 0x09)) + { // Read Word + Local0 [One] = 0x02 + Local2 = HDT1 + Local2 <<= 0x08 + Local2 += HDT0 + Local0 [0x02] = Local2 + } + + If ((Arg0 == 0x0B)) + { // Read Block + Local0 [Zero] = 0x07 + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x17 + Release (SBX0) + Return (Local0) + } + + Local0 [Zero] = Zero + Local2 = HDT0 + If ((Local2 > 0x10)) + { + Local2 = 0x10 + } + + Local0 [One] = Local2 + Local0 [0x02] = BLKD + Local3 = 0x03 + HSTA = 0x80 + While ((Local3 < (Local2 + 0x02))) + { + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x18 + Release (SBX0) + Return (Local0) + } + + Local0 [Local3] = BLKD + HSTA = 0x80 + Local3++ + } + } + } + + Release (SBX0) + } + + Return (Local0) + } + + // _SBW - SMBus Write + // + // Arg0 : Protocol Value (Integer) + // Arg1 : Slave Address (Integer) + // Arg2 : Command Code (Integer) + // Arg3 : Data Length (Integer) + // Arg4 : Data (Integer | Buffer) + // + // return Value (Package): + // (0) : Status (Integer) + Method (SBW, 5, Serialized) + { + I2CE = Zero + // Local0 is the return package. The status code is defaulted + // to 'unknown failure' (0x07). + Local0 = Package (0x01) + { + 0x07 + } + + // Make sure the protocol is valid, if not return the + // 'invalid protocol' status code. Note that this segment + // does not support packet error checking or the 'process call' + // protocol. + If ((Arg0 != 0x02)) + { // Write Quick + If ((Arg0 != 0x04)) + { // Send Byte + If ((Arg0 != 0x06)) + { // Write Byte + If ((Arg0 != 0x08)) + { // Write Word + If ((Arg0 != 0x0A)) + { // Write Block + Local0 [Zero] = 0x19 + Return (Local0) + } + } + } + } + } + + // Acquire the SMBus mutex to ensure transactional synchronization. + If ((Acquire (SBX0, 0xFFFF) == Zero)) + { + HSTA = (HSTA | 0xFF) + HADD = (Arg1 & 0xFE) + HCMD = Arg2 + If ((Arg0 == 0x02)) + { // Write Quick + HCNT = ((HCNT & 0xA0) | 0x40) + } + + If ((Arg0 == 0x04)) + { // Send Byte + HCNT = ((HCNT & 0xA0) | 0x44) + } + + If ((Arg0 == 0x06)) + { // Write Byte + HDT0 = Arg4 + HCNT = ((HCNT & 0xA0) | 0x48) + } + + If ((Arg0 == 0x08)) + { // Write Word + HDT0 = (Arg4 & 0xFF) + HDT1 = (Arg4 >> 0x08) + HCNT = ((HCNT & 0xA0) | 0x4C) + } + + If ((Arg0 == 0x0A)) + { // Write Block + If ((Arg3 > 0x10)) + { + Local0 [Zero] = 0x12 + Release (SBX0) + Return (Local0) + } + + If ((Arg3 == Zero)) + { + Local0 [Zero] = 0x12 + Release (SBX0) + Return (Local0) + } + + AUCT &= 0xFD + HDT0 = Arg3 + Local3 = One + BLKD = DerefOf (Arg4 [Zero]) + HCNT = ((HCNT & 0x80) | 0x54) + While ((Arg3 > Local3)) + { + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x18 + Release (SBX0) + Return (Local0) + } + + BLKD = DerefOf (Arg4 [Local3]) + Local3++ + HSTA = 0x80 + } + + Local4 = 0x9C40 + While ((!(HSTA & 0x80) && Local4)) + { + Local4-- + Stall (0x32) + } + + If (!Local4) + { + HCNT |= 0x02 + HSTA |= 0xFF + Local0 [Zero] = 0x18 + Release (SBX0) + Return (Local0) + } + + HSTA = 0x80 + } + + Local0 [Zero] = SWTC (0x03E8) + Release (SBX0) + } + + Return (Local0) + } +} diff --git a/src/soc/intel/apollolake/acpi/southbridge.asl b/src/soc/intel/apollolake/acpi/southbridge.asl index f4d1497..9a5a5ec 100644 --- a/src/soc/intel/apollolake/acpi/southbridge.asl +++ b/src/soc/intel/apollolake/acpi/southbridge.asl @@ -20,6 +20,9 @@ /* LPC */ #include <soc/intel/common/block/acpi/acpi/lpc.asl> +/* SMBus 0:1f.1 */ +#include "smbus.asl" + /* eMMC */ #include "scs.asl" -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 1 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: newchange
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SBMBUS control ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG@7 PS2, Line 7: SBMBUS Is SBM in here correct? -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 2 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 17 Aug 2020 07:11:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SBMBUS control ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG@11 PS2, Line 11: Tested how? -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 2 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 17 Aug 2020 07:11:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello Lance Zhao, Felix Singer, build bot (Jenkins), Angel Pons, Andrey Petrov, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/44507 to look at the new patch set (#3). Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... soc/intel/apl: Add ACPI for SMBus control Add APCI code that allows the OS to control devices on the SMBus. This is based on acpidump from AMI firmware on the Kontron mAL10 COMe [1] module and SMBus Control Methods Interface Specification, Ver 1.0, 1999. TEST = After loading the nct7802 module on the Kontron mAL-10 with Linux OS, we can read the hwm registers, see temperature and fan speed: user@user-apl:~$ sensors coretemp-isa-0000 Adapter: ISA adapter Package id 0: +41.0°C (high = +110.0°C, crit = +110.0°C) Core 0: +40.0°C (high = +110.0°C, crit = +110.0°C) Core 1: +40.0°C (high = +110.0°C, crit = +110.0°C) Core 2: +41.0°C (high = +110.0°C, crit = +110.0°C) Core 3: +41.0°C (high = +110.0°C, crit = +110.0°C) nct7802-i2c-0-2e Adapter: SMBus CMI adapter cmi in0: +3.35 V (min = +0.00 V, max = +4.09 V) in1: +1.92 V in3: +1.21 V (min = +0.00 V, max = +2.05 V) in4: +1.68 V (min = +0.00 V, max = +2.05 V) fan1: 0 RPM (min = 0 RPM) fan2: 868 RPM (min = 0 RPM) fan3: 0 RPM (min = 0 RPM) temp1: +42.5°C (low = +0.0°C, high = +85.0°C) (crit = +100.0°C) sensor = thermistor temp4: +44.0°C (low = +0.0°C, high = +85.0°C) (crit = +100.0°C) temp6: +0.0°C [1] https://review.coreboot.org/c/coreboot/+/39133 Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Signed-off-by: Maxim Polyakov <max.senia.poliak@gmail.com> --- A src/soc/intel/apollolake/acpi/smbus.asl M src/soc/intel/apollolake/acpi/southbridge.asl 2 files changed, 490 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/44507/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 3 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 4: (2 comments) Thanks for the review https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG@7 PS2, Line 7: SBMBUS
Is SBM in here correct? Fixed
https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG@11 PS2, Line 11:
Tested how? TEST = After loading the nct7802 module on the Kontron mAL10 with Linux OS, we can read the hwm registers, see temperature and fan speed
Also a new i2c device appears in /sys/bus/i2c/devices and the nct7802 module uses it. nct7802-i2c-0-2e -> i2c-0 Adapter: SMBus CMI adapter cmi Before that, the module could not see the hwm device. -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 4 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 28 Aug 2020 10:11:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/44507/2//COMMIT_MSG@11 PS2, Line 11:
TEST = After loading the nct7802 module on the Kontron mAL10 with Linux OS, we can read the hwm regi […] Ack
-- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 6 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 01 Sep 2020 20:04:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maxim Polyakov <max.senia.poliak@gmail.com> Comment-In-Reply-To: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: comment
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 9: (3 comments) https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... File src/soc/intel/apollolake/acpi/smbus.asl: https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 66: While ((Local1 == One)) maybe : While (Local1 == One) https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 69: If ((Local3 != Zero)) maybe If (Local3 != Zero) https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 170: HSTS = (HSTS | 0xFF) HSTS |= 0xFF -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 9 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 27 Sep 2020 14:11:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 9: (10 comments) Oh no https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... File src/soc/intel/apollolake/acpi/smbus.asl: https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 8: #if CONFIG(SOC_INTEL_COMMON_BLOCK_SMBUS_ACPI_DRIVER) Why not put this inside a smbus_methods.asl in common code and include it if desired? You can place the first part inside a `Scope (SBUS)` block for simplicity https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 48: // 0x00 : OK : // 0x07 : Unknown Failure : // 0x10 : Address Not Acknowledged : // 0x11 : Device Error : // 0x12 : Command Access Denied : // 0x13 : Unknown Error : // 0x17 : Device Access Denied : // 0x18 : Timeout : // 0x19 : Unsupported Protocol : // 0x1A : Bus Busy : // 0x1F : PEC (CRC-8) Error Use #defines for these? https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 65: One I would: s/Zero/0/g s/One/1/g https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 82: // Transaction isn't complete. Check for timeout, and if not, sleep 10ms and line over 96 characters. Please try to split this into two parts of about the same length. https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 92: 0x0A Does using hex make this easier to read? https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 145: If ((Arg0 != 0x03)) // Read Quick Command codes should really be #defined somewhere. Also, this would be better done within the first switch-case like block inside the mutex acquired section. https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 168: it they (command code and data length) https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 208: 0x03E8 huh? https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 219: { Read quick? https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 334: If ((Arg0 != 0x02)) // Write Quick : { : If ((Arg0 != 0x04)) // Send Byte : { : If ((Arg0 != 0x06)) // Write Byte : { : If ((Arg0 != 0x08)) // Write Word : { : If ((Arg0 != 0x0A)) // Write Block : { : Local0 [Zero] = 0x19 : Return (Local0) : } : } : } : } : } Why not check this within the mutex section? -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 9 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 27 Sep 2020 14:29:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 9:
Patch Set 9:
(10 comments)
Oh no
(this is just because this ASL code looks like it came out of IASL-disassembled vendor firmware) -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 9 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 27 Sep 2020 14:39:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 9: (1 comment) https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... File src/soc/intel/apollolake/acpi/smbus.asl: https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 65: One
I would: […] not sure here ... looks like the ASL+ compiler will use Zero and One (optimization ...?) and it convert decimals to hex :)
maybe we need a rule for ("Zero" and "One") -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 9 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 27 Sep 2020 14:40:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 9: (1 comment) https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... File src/soc/intel/apollolake/acpi/smbus.asl: https://review.coreboot.org/c/coreboot/+/44507/9/src/soc/intel/apollolake/ac... PS9, Line 65: One
not sure here ... […] The use of '0' and '1' are more readable. Thx to Nico.
-- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 9 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 27 Sep 2020 14:50:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 9: (1 comment) Why can't the OS use its own SMBus driver? On Linux, i2c-i801 is responsible for 0x5ad4 (APL's SMBus PCI dev). Are we using/ reserving it in ASL? or do we maybe not expose the PCI device? https://review.coreboot.org/c/coreboot/+/44507/9//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/44507/9//COMMIT_MSG@10 PS9, Line 10: dump from AMI firmware What license is this under? are you allowed to share it under the GPL? -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 9 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 27 Sep 2020 15:05:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 12: (3 comments) https://review.coreboot.org/c/coreboot/+/44507/12/src/soc/intel/apollolake/a... File src/soc/intel/apollolake/acpi/smbus.asl: https://review.coreboot.org/c/coreboot/+/44507/12/src/soc/intel/apollolake/a... PS12, Line 65: Local1 = One : While ((Local1 == One)) Local1 = 1 While (Local1 == 1) https://review.coreboot.org/c/coreboot/+/44507/12/src/soc/intel/apollolake/a... PS12, Line 68: Local3 = (HSTS & 0x1E) Local3 = HSTS & 0x1E https://review.coreboot.org/c/coreboot/+/44507/12/src/soc/intel/apollolake/a... PS12, Line 69: If ((Local3 != Zero)) If (Local3 != 0) -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 02 Oct 2020 10:04:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 12: Code-Review+1 Sweet! I tried to do SCMI for p3b-f in cb:41735. With this I don't have to bend a rule with iasl to get those methods in. -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 02 Oct 2020 11:24:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 12: I would really like to understand this. I've tried your mAL10 port without this SMBus patch on Linux 4.19: SMBus works fine. For instance `decode-dimms` shows me the SPD info (didn't test nct7802 as I seem to lack the module). -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 11 Oct 2020 20:41:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 12:
Patch Set 12:
I would really like to understand this. I've tried your mAL10 port without this SMBus patch on Linux 4.19: SMBus works fine. For instance `decode-dimms` shows me the SPD info (didn't test nct7802 as I seem to lack the module).
I apologize for the delay. Yes, you're right, this also works with i2c-i801 driver on Linux. $ sensors coretemp-isa-0000 Adapter: ISA adapter Package id 0: +39.0°C (high = +110.0°C, crit = +110.0°C) Core 0: +39.0°C (high = +110.0°C, crit = +110.0°C) Core 1: +39.0°C (high = +110.0°C, crit = +110.0°C) Core 2: +40.0°C (high = +110.0°C, crit = +110.0°C) Core 3: +40.0°C (high = +110.0°C, crit = +110.0°C) nct7802-i2c-5-2e Adapter: SMBus I801 adapter at efa0 in0: +3.35 V (min = +0.00 V, max = +4.09 V) in1: +1.91 V in3: +1.22 V (min = +0.00 V, max = +2.05 V) in4: +1.68 V (min = +0.00 V, max = +2.05 V) fan1: 0 RPM (min = 0 RPM) fan2: 0 RPM (min = 0 RPM) fan3: 0 RPM (min = 0 RPM) temp1: +41.1°C (low = +0.0°C, high = +85.0°C) (crit = +100.0°C) sensor = thermistor temp4: +43.0°C (low = +0.0°C, high = +85.0°C) (crit = +100.0°C) temp6: +0.0°C But It seems to me that in the case of Windows, these methods are needed. Unfortunately I can not test right now. -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 11 Nov 2020 15:57:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Maxim Polyakov. Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 12: (1 comment) Patchset: PS12: Is this still active ? -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Comment-Date: Wed, 16 Nov 2022 09:05:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Attention is currently required from: Maxim Polyakov. Elyes Haouas has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 12: (1 comment) Patchset: PS12:
Is this still active ? any update?
-- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Comment-Date: Mon, 02 Jan 2023 07:41:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Elyes Haouas <ehaouas@noos.fr> Gerrit-MessageType: comment
Attention is currently required from: Elyes Haouas. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Patch Set 12: (1 comment) Patchset: PS12:
any update? No, it is no longer needed. Linux uses i2c-i801 driver to read sensors, that's enough. Thanks for the review.
-- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Elyes Haouas <ehaouas@noos.fr> Gerrit-Comment-Date: Mon, 09 Jan 2023 21:10:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Elyes Haouas <ehaouas@noos.fr> Gerrit-MessageType: comment
Maxim Polyakov has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44507 ) Change subject: soc/intel/apl: Add ACPI for SMBus control ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/44507 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3846266d3155bbe831123fef4751748ff0e00511 Gerrit-Change-Number: 44507 Gerrit-PatchSet: 12 Gerrit-Owner: Maxim Polyakov <max.senia.poliak@gmail.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Keith Hui <buurin@gmail.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas@noos.fr> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: abandon
participants (7)
-
Angel Pons (Code Review) -
Elyes Haouas (Code Review)
-
HAOUAS Elyes (Code Review)
-
Keith Hui (Code Review)
-
Maxim Polyakov (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)