Attention is currently required from: Lance Zhao, Caveh Jalali, Matt DeVillier, Tim Wawrzynczak, Boris Mittelberg.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74524 )
Change subject: [WIP] ACPI: Add usb_charge_mode_from_gnvs() ......................................................................
Patch Set 6:
(3 comments)
File src/acpi/acpi_pm.c:
https://review.coreboot.org/c/coreboot/+/74524/comment/b06c3fcb_8a094903 PS4, Line 69: *usb0_disable, bool *usb1_disable)
these variable names make the functionality much more clear, I'd recommend using them throughout the […]
Done
File src/ec/google/chromeec/smihandler.c:
https://review.coreboot.org/c/coreboot/+/74524/comment/9e673ea9_de879bc6 PS4, Line 48: chromeec_set_usb_charge_mode(bool u0disable, bool u1disable) : { : if (u0disable) : google_chromeec_set_usb_charge_mode(0, USB_CHARGE_MODE_DISABLED); : : if (u1disable) : google_chromeec_set_usb_charge_mode(1, USB_CHARGE_MODE_DISABLED);
any reason not to just pass in the sleep mode, and have chromeec_set_usb_charge_mode() get the disab […]
Done
File src/mainboard/google/auron/smihandler.c:
https://review.coreboot.org/c/coreboot/+/74524/comment/4ea67350_e286d079 PS6, Line 39: chromeec_set_usb_charge_mode(slp_typ); But now we have this special case to deal with.
I must be missing something here. The only platforms referencing S3U0, S3U1, S5U0 or S5U1 in ASL sources are the very first samsung/lumpy,stumpy and intel/emeraldlake2.
Seems to me like these are all using GNVS for no reason at all, seems to be a static board customisation value that does not change runtime?
-
Kyösti Mälkki (Code Review)