diff options
author | Ricardo Ribalda <ribalda@chromium.org> | 2020-12-14 23:33:42 +0100 |
---|---|---|
committer | Furquan Shaikh <furquan@google.com> | 2020-12-16 06:29:20 +0000 |
commit | 752fc6026f786818681a02c4144268a3a1a03391 (patch) | |
tree | fe61b1d2407e1dd112169d64319e5c70157f263b | |
parent | 1a8c20324aea1795c9bb4dbb2d162d1ba19f5287 (diff) | |
download | coreboot-752fc6026f786818681a02c4144268a3a1a03391.tar.gz |
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>
3 files changed, 78 insertions, 96 deletions
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 8f3ae536b4..2d5dd0a289 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 @@ Scope (\_SB.PCI0.I2C2) ) }) - 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 7e205752ea..c41ebca7fe 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 @@ Scope (\_SB.PCI0.I2C4) ) }) - 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 5ad10effa8..842005abd7 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 @@ Scope (\_SB.PCI0.I2C2) 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 @@ Scope (\_SB.PCI0.I2C2) } } - /* 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 @@ Scope (\_SB.PCI0.I2C2) DCVA = 12 VDCT = 1 - \_SB.PCI0.I2C2.PMIC.CLKE() - CLE0 = 1 - /* * Wait for all regulator * outputs to settle. @@ -473,16 +470,12 @@ Scope (\_SB.PCI0.I2C2) 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 @@ Scope (\_SB.PCI0.I2C2) } /* 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 @@ Scope (\_SB.PCI0.I2C2) /* 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 @@ Scope (\_SB.PCI0.I2C2) 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 @@ Scope (\_SB.PCI0.I2C2) Sleep(1) VAX2 = 0 Sleep(1) - DOVD(0) } STA = 0 } |