Attention is currently required from: David Wu, Tim Wawrzynczak, Paul Fagerburg, Nick Vaccaro. Jim Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49302 )
Change subject: mb/google/volteer/var/voema: Add camera ACPI configuration ......................................................................
Patch Set 1:
(5 comments)
File src/mainboard/google/volteer/variants/voema/include/variant/acpi/mipi_camera.asl:
https://review.coreboot.org/c/coreboot/+/49302/comment/33253742_712eac4b PS1, Line 105: }
do you need a "link-frequencies" entry here, or "address-width" ?
no need to add link-frequency in IPU endpoint entry. we add it in sensor device entry. sensor driver gets and reports it to isys driver
https://review.coreboot.org/c/coreboot/+/49302/comment/f0f55939_7d5500f3 PS1, Line 113: PowerResource (RCPR, 0x00, 0x0000) : { : Name (STA, 0) : Method (_ON, 0, Serialized) /* Rear camera_ON_: Power On */ : { : /* Enable IMG_CLK */ : MCON(3,1) /* Clock 3, 19.2MHz */ : : /* Pull RST low */ : CTXS(GPP_F15) : : /* Pull SNRPWR_EN high */ : STXS(GPP_H14) : : If (REFC == 0) : { : /* Pull PWREN high */ : STXS(GPP_H20) : } : Sleep(2) /* reset pulse width */ : : REFC++ : : /* Pull RST high */ : STXS(GPP_F15) : : Sleep(1) /* t2 */ : : STA = 1 : } : Method (_OFF, 0, Serialized) /* Rear camera _OFF: Power Off */ : { : /* Disable IMG_CLK */ : Sleep(1) /* t0+t1 */ : MCOF(3) /* Clock 3 */ : : /* Pull RST low */ : CTXS(GPP_F15) : : If (REFC == 1) : { : /* Pull PWREN low */ : CTXS(GPP_H20) : } : REFC-- : : /* Pull SNRPWR_EN low */ : CTXS(GPP_H14) : : STA = 0 : } : Method (_STA, 0, NotSerialized) /* _STA: Status */ : { : Return (STA) : } : } :
This PowerResource is also not referenced anywhere.
Done
https://review.coreboot.org/c/coreboot/+/49302/comment/f05f9c64_9bd09ba7 PS1, Line 170: PowerResource (VCPR, 0x00, 0x0000) : { : Name (STA, 0) : Method (_ON, 0, Serialized) /* VCPR_ON_: VCM Power On */ : { : If (REFC == 0) : { : /* Pull PWREN high */ : STXS(GPP_H20) : } : REFC++ : STA = 1 : } : Method (_OFF, 0, Serialized) /* VCPR_OFF: VCM Power Off */ : { : If (REFC == 1) : { : /* Pull PWREN low */ : CTXS(GPP_H20) : } : REFC-- : STA = 0 : } : Method (_STA, 0, NotSerialized) /* _STA: Status */ : { : Return (STA) : } : }
I don't see this PowerResource referenced anywhere...
Done
https://review.coreboot.org/c/coreboot/+/49302/comment/15997931_03b542ac PS1, Line 198:
nit: extra line
Done
https://review.coreboot.org/c/coreboot/+/49302/comment/4e1f1a34_9025335e PS1, Line 235: Sleep(1) /* t0+t1 */
A delay is required before anything else is done? Is this supposed to be after the call to MCOF?
Datasheet has statement saying, "the requirement is that XVLCK must be active for time t1 after the last SCCB transaction or after the MIPI frame end short packet, whichever is the later event." and therefore we will need to have this delay before turning off MCOF