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>
diff --git a/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam0.asl b/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam0.asl
index 8f3ae53..2d5dd0a 100644
--- a/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam0.asl
+++ b/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam0.asl
@@ -23,8 +23,8 @@
)
})
- Name (_PR0, Package () { ^^I2C2.PMIC.OVTH })
- Name (_PR3, Package () { ^^I2C2.PMIC.OVTH })
+ Name (_PR0, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVTH})
+ Name (_PR3, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVTH})
/* Port0 of CAM0 is connected to port0 of CIO2 device */
Name (_DSD, Package () {
diff --git a/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam1.asl b/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam1.asl
index 7e20575..c41ebca 100644
--- a/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam1.asl
+++ b/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/cam1.asl
@@ -23,8 +23,8 @@
)
})
- Name (_PR0, Package () { ^^I2C2.PMIC.OVFI })
- Name (_PR3, Package () { ^^I2C2.PMIC.OVFI })
+ Name (_PR0, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVFI})
+ Name (_PR3, Package () {^^I2C2.PMIC.OVCM, ^^I2C2.PMIC.OVFI})
/* Port0 of CAM1 is connected to port1 of CIO2 device */
Name (_DSD, Package () {
diff --git a/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/camera_pmic.asl b/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/camera_pmic.asl
index 5ad10ef..842005a 100644
--- a/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/camera_pmic.asl
+++ b/src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/camera_pmic.asl
@@ -313,76 +313,50 @@
PODV, 32,
}
- /* CLE0 and CLE1 are used to determine if both the clocks
- are enabled or disabled. */
- Mutex (MUTC, 0)
- Name (CLE0, 0)
- Name (CLE1, 0)
- Method (CLKE, 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)) {
- /* Enable clocks only when a sensor is turned on and
- both the clocks are disabled */
- If (LNot (LOr (CLE0, CLE1))) {
- /* Set boost clock divider */
- BODI = 3
- /* Set buck clock divider */
- BUDI = 2
- /* Set the PLL_REF_CLK cyles */
- PSWR = 19
- /* Set the reference crystal divider */
- XTDV = 170
- /* Set PLL feedback divider */
- PLDV = 32
- /* Set PLL output divider for HCLK_A */
- PODV = 1
- /* Set PLL output divider for HCLK_B */
- PDV2 = 1
- /* Enable clocks for both the sensors
- * CFG1: output selection for HCLK_A and
- * HCLK_B.
- * CFG2: set drive strength for HCLK_A
- * and HCLK_B.
- */
- CFG2 = 5
- CFG1 = 10
- /* Enable PLL output, crystal oscillator
- * input capacitance control and set
- * Xtal oscillator as clock source.
- */
- PCTL = 209
- 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)
+ Method (CLK, 1, Serialized) {
+ If (LEqual (Arg0, Zero)) {
+ 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
+ Sleep(1)
+ } ElseIf (LEqual (Arg0, 1)) {
+ /* Set boost clock divider */
+ BODI = 3
+ /* Set buck clock divider */
+ BUDI = 2
+ /* Set the PLL_REF_CLK cyles */
+ PSWR = 19
+ /* Set the reference crystal divider */
+ XTDV = 170
+ /* Set PLL feedback divider */
+ PLDV = 32
+ /* Set PLL output divider for HCLK_A */
+ PODV = 1
+ /* Set PLL output divider for HCLK_B */
+ PDV2 = 1
+ /* Enable clocks for both the sensors
+ * CFG1: output selection for HCLK_A and
+ * HCLK_B.
+ * CFG2: set drive strength for HCLK_A
+ * and HCLK_B.
+ */
+ CFG2 = 5
+ CFG1 = 10
+ /* Enable PLL output, crystal oscillator
+ * input capacitance control and set
+ * Xtal oscillator as clock source.
+ */
+ PCTL = 209
+ Sleep(1)
}
}
@@ -425,17 +399,43 @@
}
}
- /* Power resource methods for CAM0 */
- PowerResource (OVTH, 0, 0) {
+ /* Power resource methods for both CAMs */
+ PowerResource (OVCM, 0, 0) {
Name (STA, 0)
Method (_ON, 0, Serialized) {
- /* TODO: Read Voltage and Sleep values from Sensor Obj */
If (LEqual (AVBL, 1)) {
If (LEqual (STA, 0)) {
/* Enable VSIO regulator +
daisy chain */
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.CGP2()
@@ -447,9 +447,6 @@
DCVA = 12
VDCT = 1
- \_SB.PCI0.I2C2.PMIC.CLKE()
- CLE0 = 1
-
/*
* Wait for all regulator
* outputs to settle.
@@ -473,16 +470,12 @@
If (LEqual (AVBL, 1)) {
If (LEqual (STA, 1)) {
Sleep(2)
- CLE0 = 0
- \_SB.PCI0.I2C2.PMIC.CLKD()
- Sleep(2)
\_SB.PCI0.I2C2.PMIC.CRST(0)
Sleep(3)
VDCT = 0
Sleep(3)
VACT = 0
Sleep(1)
- DOVD(0)
}
}
STA = 0
@@ -493,16 +486,12 @@
}
/* Power resource methods for CAM1 */
- PowerResource (OVFI, 0, 0) {
+ PowerResource (OVFI, 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)) {
- /* Enable VSIO regulator +
- daisy chain */
- DOVD(1)
-
/* Set VAUX2 as 1.8006 V */
AX2V = 52
VAX2 = 1 /* Enable VAUX2 */
@@ -522,9 +511,6 @@
/* Wait for VDD to settle. */
Sleep(1)
- \_SB.PCI0.I2C2.PMIC.CLKE()
- CLE1 = 1
-
\_SB.PCI0.I2C2.PMIC.CGP5(1)
/*
* Ensure 10 ms between
@@ -540,9 +526,6 @@
If (LEqual (AVBL, 1)) {
If (LEqual (STA, 1)) {
Sleep(2)
- CLE1 = 0
- \_SB.PCI0.I2C2.PMIC.CLKD()
- Sleep(2)
\_SB.PCI0.I2C2.PMIC.CGP5(0)
Sleep(3)
VAX1 = 0
@@ -551,7 +534,6 @@
Sleep(1)
VAX2 = 0
Sleep(1)
- DOVD(0)
}
STA = 0
}