Ricardo Ribalda has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48789 )
Change subject: mb/google/poppy/natutilus: Add missing PR dependency. ......................................................................
mb/google/poppy/natutilus: Add missing PR dependency.
Depend on the Camera common Power Resource, introduced on the mainboard file.
Fixes: 64c03e3c ("mb/google/poppy: Fix race condition in acpi") BRANCH=poppy BUG=b:174941580 Signed-off-by: Ricardo Ribalda ribalda@chromium.org
Change-Id: Ifa6c70b7c02aec0112189eca573e76e53175d70d --- M src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/48789/1
diff --git a/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl b/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl index a390b61..cfc84b5 100644 --- a/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl +++ b/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl @@ -22,8 +22,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 () {
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48789 )
Change subject: mb/google/poppy/natutilus: Add missing PR dependency. ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG@7 PS1, Line 7: mb/google/poppy/natutilus: Add missing PR dependency. Please remove the dot/period at the end of the commit message.
https://chris.beams.io/posts/git-commit/
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG@8 PS1, Line 8: If you could describe the problem, that would be great.
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG@16 PS1, Line 16: Please remove the blank line.
Hello build bot (Jenkins), Furquan Shaikh, shkim,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48789
to look at the new patch set (#2).
Change subject: mb/google/poppy/natutilus: Add missing PR dependency ......................................................................
mb/google/poppy/natutilus: Add missing PR dependency
On commit 64c03e3c ("mb/google/poppy: Fix race condition in acpi"), we introduced a new Power Resource common to all the camera modules in order to resolve a race condition when both modules were in use (e.g. during startup).
The nautilus variant also used the Power Supply I2C2.PMIC.OVTH, which requires the new common PR, but the new dependency was not added.
Depend on the new Camera Common Power Resource.
Fixes: 64c03e3c ("mb/google/poppy: Fix race condition in acpi") BRANCH=poppy BUG=b:174941580 Signed-off-by: Ricardo Ribalda ribalda@chromium.org Change-Id: Ifa6c70b7c02aec0112189eca573e76e53175d70d --- M src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/48789/2
Hello build bot (Jenkins), Furquan Shaikh, shkim,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48789
to look at the new patch set (#3).
Change subject: mb/google/poppy/natutilus: Add missing PR dependency ......................................................................
mb/google/poppy/natutilus: Add missing PR dependency
On commit 64c03e3c ("mb/google/poppy: Fix race condition in acpi"), we introduced a new Power Resource common to all the camera modules, in order to resolve a race condition when both modules were in use (e.g. during startup).
The nautilus variant also used the Power Supply I2C2.PMIC.OVTH, which requires the new common PR, but the new dependency was not added.
Depend on the new Camera Common Power Resource.
Fixes: 64c03e3c ("mb/google/poppy: Fix race condition in acpi") BRANCH=poppy BUG=b:174941580 Signed-off-by: Ricardo Ribalda ribalda@chromium.org Change-Id: Ifa6c70b7c02aec0112189eca573e76e53175d70d --- M src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/48789/3
Ricardo Ribalda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48789 )
Change subject: mb/google/poppy/natutilus: Add missing PR dependency ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG@7 PS1, Line 7: mb/google/poppy/natutilus: Add missing PR dependency.
Please remove the dot/period at the end of the commit message. […]
Done
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG@8 PS1, Line 8:
If you could describe the problem, that would be great.
Done
Sorry about that. I guess I was too pissed for forgetting about nautilus and introducing this bug :(
https://review.coreboot.org/c/coreboot/+/48789/1//COMMIT_MSG@16 PS1, Line 16:
Please remove the blank line.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48789 )
Change subject: mb/google/poppy/natutilus: Add missing PR dependency ......................................................................
Patch Set 3: Code-Review+2
shkim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48789 )
Change subject: mb/google/poppy/natutilus: Add missing PR dependency ......................................................................
Patch Set 3: Code-Review+1
Tested on 4 nautilus DUTs here, no camera probing issue was observed on 1000 power cycle test. I'll verify again with CPFE image including this change later.
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48789 )
Change subject: mb/google/poppy/natutilus: Add missing PR dependency ......................................................................
mb/google/poppy/natutilus: Add missing PR dependency
On commit 64c03e3c ("mb/google/poppy: Fix race condition in acpi"), we introduced a new Power Resource common to all the camera modules, in order to resolve a race condition when both modules were in use (e.g. during startup).
The nautilus variant also used the Power Supply I2C2.PMIC.OVTH, which requires the new common PR, but the new dependency was not added.
Depend on the new Camera Common Power Resource.
Fixes: 64c03e3c ("mb/google/poppy: Fix race condition in acpi") BRANCH=poppy BUG=b:174941580 Signed-off-by: Ricardo Ribalda ribalda@chromium.org Change-Id: Ifa6c70b7c02aec0112189eca573e76e53175d70d Reviewed-on: https://review.coreboot.org/c/coreboot/+/48789 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: shkim sh_.kim@samsung.com --- M src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl 1 file changed, 2 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved shkim: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl b/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl index a390b61..cfc84b5 100644 --- a/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl +++ b/src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/cam0.asl @@ -22,8 +22,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 () {
-
Furquan Shaikh (Code Review) -
Paul Menzel (Code Review)
-
Ricardo Ribalda (Code Review)
-
shkim (Code Review)
-
Tim Wawrzynczak (Code Review)