Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44397 )
Change subject: mb/google/dedede: Replace static Camera ACPI by driver for WDoo ......................................................................
Patch Set 9:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44397/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/waddledoo/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/44397/9/src/mainboard/google/dedede... PS9, Line 184: ACPI_DT_NAMESPACE_HID This is already the default for VCM so doesn't need to be specified.
https://review.coreboot.org/c/coreboot/+/44397/9/src/mainboard/google/dedede... PS9, Line 185: 0 I know this was copied from the asl, but I suspect there's a decent chance there already is (or will eventually be) a PRP0001 with UID 0. Offhand the closest I could find is src/mainboard/google/dedede/variants/baseboard/devicetree.cb using ACPI_DT_NAMESPACE_HID for the cr50, though it doesn't seem to specify a UID. It might be worth checking if there's a conflict on the system (if you have one handy) or just proactively use a higher number.
https://review.coreboot.org/c/coreboot/+/44397/9/src/mainboard/google/dedede... PS9, Line 196: ACPI_DT_NAMESPACE_HID I thought NVM devices used INT3499 (which is the mipi camera driver's default). I realize the asl you're replacing used ACPI_DT_NAMESPACE_HID. I had assumed the driver just auto-detects the chip, though I don't know this for certain nor know if it would work for this chip or not.
https://review.coreboot.org/c/coreboot/+/44397/9/src/mainboard/google/dedede... PS9, Line 229: "GPP_D13" #power_enable_2p8 : register "gpio_panel.gpio[1].gpio_num" = "GPP_D14" I didn't follow the ref-counting change but I'm assuming it's doing some ref-counting on D13 and D14 so that if both camera are on and only 1 is turned off then the other camera still stays on.