mb/google/poppy: Fix race condition in acpi camera_pmic

Newer kernels can re-schedule new acpi command calls during a Sleep().

This causes that the following trace fails to detect the cameras:
[   15.764725] drivers/acpi/power.c:358 Power resource [OVFI] turned on start
[   15.772180] drivers/acpi/power.c:358 Power resource [OVTH] turned on start
[   15.834970] drivers/acpi/power.c:362 Power resource [OVFI] turned on start
[   15.852456] drivers/acpi/power.c:415 Power resource [OVFI] turned off start
[   15.955987] drivers/acpi/power.c:420 Power resource [OVFI] turned off end
ERROR!!
[   16.030896] drivers/acpi/power.c:362 Power resource [OVTH] turned on end

Which can be triggered more frequently if the Sleep() commands in OVTH
 _ON Method are increased.

To avoid the race condition, we create a new Power Resource that
handles the common resources of both cameras and make both cameras
depend on that resource. This also simplifies the acpi table by removing
a Mutex.

BRANCH=poppy
BUG=b:171955583
TEST=while true; do if ssh $DUT "dmesg | grep \"failed to find sensor\" "; then  break;  fi; ssh $DUT reboot; sleep 30 ; done
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Change-Id: I25df0225699759c1828b8791c5bdee66529858a7
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48631
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This commit is contained in:
Ricardo Ribalda 2020-12-14 23:33:42 +01:00 committed by Furquan Shaikh
parent 1a8c20324a
commit 752fc6026f
3 changed files with 78 additions and 96 deletions

View File

@ -23,8 +23,8 @@ Scope (\_SB.PCI0.I2C2)
) )
}) })
Name (_PR0, Package () { ^^I2C2.PMIC.OVTH }) Name (_PR0, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVTH})
Name (_PR3, Package () { ^^I2C2.PMIC.OVTH }) Name (_PR3, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVTH})
/* Port0 of CAM0 is connected to port0 of CIO2 device */ /* Port0 of CAM0 is connected to port0 of CIO2 device */
Name (_DSD, Package () { Name (_DSD, Package () {

View File

@ -23,8 +23,8 @@ Scope (\_SB.PCI0.I2C4)
) )
}) })
Name (_PR0, Package () { ^^I2C2.PMIC.OVFI }) Name (_PR0, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVFI})
Name (_PR3, Package () { ^^I2C2.PMIC.OVFI }) Name (_PR3, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVFI})
/* Port0 of CAM1 is connected to port1 of CIO2 device */ /* Port0 of CAM1 is connected to port1 of CIO2 device */
Name (_DSD, Package () { Name (_DSD, Package () {

View File

@ -313,20 +313,22 @@ Scope (\_SB.PCI0.I2C2)
PODV, 32, PODV, 32,
} }
/* CLE0 and CLE1 are used to determine if both the clocks Method (CLK, 1, Serialized) {
are enabled or disabled. */ If (LEqual (Arg0, Zero)) {
Mutex (MUTC, 0) BODI = 0
Name (CLE0, 0) BUDI = 0
Name (CLE1, 0) PSWR = 0
Method (CLKE, 0, Serialized) { XTDV = 0
/* save Acquire result so we can check for PLDV = 0
Mutex acquired */ PODV = 0
Store (Acquire (MUTC, 1000), Local0) PDV2 = 0
/* check for Mutex acquired */ /* Disable clocks for both the
If (LEqual (Local0, Zero)) { sensors */
/* Enable clocks only when a sensor is turned on and CFG2 = 0
both the clocks are disabled */ CFG1 = 0
If (LNot (LOr (CLE0, CLE1))) { PCTL = 0
Sleep(1)
} ElseIf (LEqual (Arg0, 1)) {
/* Set boost clock divider */ /* Set boost clock divider */
BODI = 3 BODI = 3
/* Set buck clock divider */ /* Set buck clock divider */
@ -356,34 +358,6 @@ Scope (\_SB.PCI0.I2C2)
PCTL = 209 PCTL = 209
Sleep(1) Sleep(1)
} }
Release (MUTC)
}
}
/* Clocks need to be disabled only if both the sensors are
turned off */
Method (CLKD, 0, Serialized) {
/* save Acquire result so we can check for
Mutex acquired */
Store (Acquire (MUTC, 1000), Local0)
/* check for Mutex acquired */
If (LEqual (Local0, Zero)) {
If (LNot (LOr (CLE0, CLE1))) {
BODI = 0
BUDI = 0
PSWR = 0
XTDV = 0
PLDV = 0
PODV = 0
PDV2 = 0
/* Disable clocks for both the
sensors */
CFG2 = 0
CFG1 = 0
PCTL = 0
}
Release (MUTC)
}
} }
/* Reference count for VSIO */ /* Reference count for VSIO */
@ -425,17 +399,43 @@ Scope (\_SB.PCI0.I2C2)
} }
} }
/* Power resource methods for CAM0 */ /* Power resource methods for both CAMs */
PowerResource (OVTH, 0, 0) { PowerResource (OVCM, 0, 0) {
Name (STA, 0) Name (STA, 0)
Method (_ON, 0, Serialized) { Method (_ON, 0, Serialized) {
/* TODO: Read Voltage and Sleep values from Sensor Obj */
If (LEqual (AVBL, 1)) { If (LEqual (AVBL, 1)) {
If (LEqual (STA, 0)) { If (LEqual (STA, 0)) {
/* Enable VSIO regulator + /* Enable VSIO regulator +
daisy chain */ daisy chain */
DOVD(1) DOVD(1)
CLK(1)
STA = 1
}
}
}
Method (_OFF, 0, Serialized) {
If (LEqual (AVBL, 1)) {
If (LEqual (STA, 1)) {
CLK(0)
Sleep(2)
DOVD(0)
}
}
STA = 0
}
Method (_STA, 0, NotSerialized) {
Return (STA)
}
}
/* Power resource methods for CAM0 */
PowerResource (OVTH, 0, 1) {
Name (STA, 0)
Method (_ON, 0, Serialized) {
/* TODO: Read Voltage and Sleep values from Sensor Obj */
If (LEqual (AVBL, 1)) {
If (LEqual (STA, 0)) {
\_SB.PCI0.I2C2.PMIC.CGP1() \_SB.PCI0.I2C2.PMIC.CGP1()
\_SB.PCI0.I2C2.PMIC.CGP2() \_SB.PCI0.I2C2.PMIC.CGP2()
@ -447,9 +447,6 @@ Scope (\_SB.PCI0.I2C2)
DCVA = 12 DCVA = 12
VDCT = 1 VDCT = 1
\_SB.PCI0.I2C2.PMIC.CLKE()
CLE0 = 1
/* /*
* Wait for all regulator * Wait for all regulator
* outputs to settle. * outputs to settle.
@ -472,9 +469,6 @@ Scope (\_SB.PCI0.I2C2)
Method (_OFF, 0, Serialized) { Method (_OFF, 0, Serialized) {
If (LEqual (AVBL, 1)) { If (LEqual (AVBL, 1)) {
If (LEqual (STA, 1)) { If (LEqual (STA, 1)) {
Sleep(2)
CLE0 = 0
\_SB.PCI0.I2C2.PMIC.CLKD()
Sleep(2) Sleep(2)
\_SB.PCI0.I2C2.PMIC.CRST(0) \_SB.PCI0.I2C2.PMIC.CRST(0)
Sleep(3) Sleep(3)
@ -482,7 +476,6 @@ Scope (\_SB.PCI0.I2C2)
Sleep(3) Sleep(3)
VACT = 0 VACT = 0
Sleep(1) Sleep(1)
DOVD(0)
} }
} }
STA = 0 STA = 0
@ -493,16 +486,12 @@ Scope (\_SB.PCI0.I2C2)
} }
/* Power resource methods for CAM1 */ /* Power resource methods for CAM1 */
PowerResource (OVFI, 0, 0) { PowerResource (OVFI, 0, 1) {
Name (STA, 0) Name (STA, 0)
Method (_ON, 0, Serialized) { Method (_ON, 0, Serialized) {
/* TODO: Read Voltage and Sleep values from Sensor Obj */ /* TODO: Read Voltage and Sleep values from Sensor Obj */
If (LEqual (AVBL, 1)) { If (LEqual (AVBL, 1)) {
If (LEqual (STA, 0)) { If (LEqual (STA, 0)) {
/* Enable VSIO regulator +
daisy chain */
DOVD(1)
/* Set VAUX2 as 1.8006 V */ /* Set VAUX2 as 1.8006 V */
AX2V = 52 AX2V = 52
VAX2 = 1 /* Enable VAUX2 */ VAX2 = 1 /* Enable VAUX2 */
@ -522,9 +511,6 @@ Scope (\_SB.PCI0.I2C2)
/* Wait for VDD to settle. */ /* Wait for VDD to settle. */
Sleep(1) Sleep(1)
\_SB.PCI0.I2C2.PMIC.CLKE()
CLE1 = 1
\_SB.PCI0.I2C2.PMIC.CGP5(1) \_SB.PCI0.I2C2.PMIC.CGP5(1)
/* /*
* Ensure 10 ms between * Ensure 10 ms between
@ -539,9 +525,6 @@ Scope (\_SB.PCI0.I2C2)
Method (_OFF, 0, Serialized) { Method (_OFF, 0, Serialized) {
If (LEqual (AVBL, 1)) { If (LEqual (AVBL, 1)) {
If (LEqual (STA, 1)) { If (LEqual (STA, 1)) {
Sleep(2)
CLE1 = 0
\_SB.PCI0.I2C2.PMIC.CLKD()
Sleep(2) Sleep(2)
\_SB.PCI0.I2C2.PMIC.CGP5(0) \_SB.PCI0.I2C2.PMIC.CGP5(0)
Sleep(3) Sleep(3)
@ -551,7 +534,6 @@ Scope (\_SB.PCI0.I2C2)
Sleep(1) Sleep(1)
VAX2 = 0 VAX2 = 0
Sleep(1) Sleep(1)
DOVD(0)
} }
STA = 0 STA = 0
} }