Rajmohan Mani has posted comments on this change. ( https://review.coreboot.org/27350 )
Change subject: mb/google/atlas: MIPI camera ASL files for Atlas ......................................................................
Patch Set 5:
(8 comments)
mipi_camera.asl - Please align this name with other variants (e.g cam0.asl)
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/atlas/devicetree.cb:
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 43: # FIXME: enable once MIPI is ready Please remove these comments
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 44: # FIXME: enable once MIPI is ready Ditto
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/atlas/include/variant/acpi/ipu_mainboard.asl:
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 18: /* Define two ports for CIO2 device where endpoint of port0 : is connected to CAM0 and endpoint of port1 is connected to CAM1 */ Please fix these comments as Atlas uses a single camera.
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/atlas/include/variant/acpi/mipi_camera.asl:
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 20: INT3478 Where does this ACPI id come from?
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 32: AddressingMode7Bit, "\_SB.PCI0.I2C3", : 0x00, ResourceConsumer, , Tab needed
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 45: Sleep(11); I need to look into the data sheet to understand the timing requirements. Would be nice if you can add some details about this delay.
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 62: FCPR FCPR naming convention is used to refer that it'a front camera. In Atlas, is this a front or rear camera? Please update all comments about front / rear as needed.
https://review.coreboot.org/#/c/27350/5/src/mainboard/google/poppy/variants/... PS5, Line 123: SSDB I don't believe SSDB is currently used. Please remove.
-
Rajmohan Mani (Code Review)