21 comments:
File src/mainboard/google/dedede/Kconfig:
Patch Set #9, Line 78: # Select this option to enable camera ACPI support on the variant.
Put this comment as a help string.
Patch Set #9, Line 81: default n
Keep it default n here. select VARIANT_HAS_ACPI_CAMERA under BOARD_GOOGLE_WADDLEDOO in Kconfig.name file.
File src/mainboard/google/dedede/variants/baseboard/gpio.c:
Nit: Remove the extra space. Here and lines 175, 177, 179.
Why PLTRST?
File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam0.asl:
Patch Set #9, Line 16: scope
scope -> Scope
Patch Set #9, Line 19: Method
Always use a space after //. Here and everywhere. Always use period at the end of the comment.
Nit: off
Patch Set #9, Line 22: PP1200
Nit: PP2800. Here and PON
Patch Set #9, Line 24: PP2800
Nit: PP1200. Here and PON
Patch Set #9, Line 43: GPP_D12
Why assert WCAM RESET line for Front Camera? Shouldn't it be UCAM RESET line(GPP_D15) - here and rest of the file?
//Enable PP1200 lane
STXS(GPP_D13)
//Enable PP2800 lane
STXS(GPP_D14)
Can PON method be invoked instead of duplication?
Patch Set #9, Line 55: Sleep(5)
Is that extra 5 ms sleep intentional?
Why can't you turn off clock 0 outside the if block? Disable the power lanes if the other sensor is already off.
//Disable PP1200 lane
CTXS(GPP_D13)
//Disable PP2800 lane
CTXS(GPP_D14)
POFF Method?
Shouldn't the UCAM reset line(GPP_D15) be deasserted after Powering off?
0x01
File src/mainboard/google/dedede/variants/baseboard/include/baseboard/acpi/cam1.asl:
//Enable PP1200 lane
STXS(GPP_D13)
//Enable PP2800 lane
STXS(GPP_D14)
PON Method?
Patch Set #9, Line 41: Sleep(5)
Is this sleep intentional and required?
Patch Set #9, Line 58: MCOF(1) // Clock 1
Same comment as in the previous file.
CTXS(GPP_D13)
//Disable PP2800 lane
CTXS(GPP_D14)
POFF Method?
No need to de-assert WCAM RESET Line (GPP_D12)?
To view, visit change 39360. To unsubscribe, or for help writing mail filters, visit settings.