Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/23056 )
Change subject: mainboard/google/poppy/variants/nautilus - MIPI camera asl files for Nautilus ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/#/c/23056/2/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/ipu_mainboard.asl:
https://review.coreboot.org/#/c/23056/2/src/mainboard/google/poppy/variants/... PS2, Line 18: /* Define two ports for CIO2 device where endpoint of port0 : is connected to CAM0 and endpoint of port1 is connected to CAM1 */ I imagine there is only one port and endpoint where only CAM0 is connected.
https://review.coreboot.org/#/c/23056/2/src/mainboard/google/poppy/variants/... PS2, Line 25: Package () { "port1", "PRT1" }, not needed, there is only one sensor.
https://review.coreboot.org/#/c/23056/2/src/mainboard/google/poppy/variants/... PS2, Line 52: Name (PRT1, Package () { : ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package () { : Package () { "port", 1 }, /* csi 1 */ : }, : ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), : Package () { : Package () { "endpoint0", "EP10" }, : } : }) Do we need this? Only one sensor on MIPI.
https://review.coreboot.org/#/c/23056/2/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/nautilus/include/variant/acpi/mipi_camera.asl:
https://review.coreboot.org/#/c/23056/2/src/mainboard/google/poppy/variants/... PS2, Line 16: Scope (_SB.PCI0.I2C2) : { : Device (PMIC) : { : Name (_HID, "INT3472") /* _HID: Hardware ID */ : Name (_UID, Zero) /* _UID: Unique ID */ : Name (_DDN, "TPS68470 PMIC") /* _DDN: DOS Device Name */ : Name (CAMD, 0x64) : : Method (_STA, 0, NotSerialized) /* _STA: Status */ : { : Return (0x0F) : } : : Method (PMON, 0, Serialized) { : /* : * Turn on 3V3_VDD. It takes around There is a lot of code which is a copy of src/mainboard/google/poppy/variants/baseboard/include/baseboard/acpi/mipi_camera.asl, specially the PMIC. Can we utilize the common code from that file. Things may will turn murky. Feel free to split the original file.
https://review.coreboot.org/#/c/23056/2/src/mainboard/google/poppy/variants/... PS2, Line 338: /* Increment only if VSIC < 4 */ : If (LLess (VSIC, 4)) { For nautilus, this reference count will come down to 3 ideally (CAM0, VCM, NVM). hmmmm...if we decide to split the common code then this value should also be configurable or derived from somewhere. Can't think of a right now.