Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46713 )
Change subject: driver/usb/acpi: Add power resources for devices on USB ports ......................................................................
Patch Set 2:
(1 comment)
Hmm, this makes me want to attack the power resource problem again... With the devtree alias patches in, expressing power resource dependencies is even cleaner, e.g.,:
``` chip drivers/generic/power_resource/ register "on" = "{{GPIO_L(GPP_A1), 50}, {GPIO_L(GPIO_B2), 1},}" register "off" = "{{GPIO_L(GPP_B2), 1}, {GPIO(GPP_A1), 10},}" # or even the current interface in generic/2c with the stop_gpio, reset_gpio, etc. # but I kind of like this explicit sequencing you get here. device generic 0 alias i2c1_power_res on end end chip drivers/i2c/generic use i2c1_power_res as power_resource; device i2c 1f on end end chip drivers/spi/generic use i2c1_power_res as power_resource; device spi 0 on end end ```
Each "resource" in the 'on' and 'off' list (i.e., GPIOs) can emit enable/disable methods that use reference counting to keep track of when it's safe to actually assert/deassert the pins.
WDYT? If you don't want to tackle that right now, factoring out the power resource fields into a common `struct power_resource_config` or similar would be better than copy-pasting these fields around into different drivers.
https://review.coreboot.org/c/coreboot/+/46713/2/src/drivers/usb/acpi/usb_ac... File src/drivers/usb/acpi/usb_acpi.c:
https://review.coreboot.org/c/coreboot/+/46713/2/src/drivers/usb/acpi/usb_ac... PS2, Line 65: * Should we apply Power Resource only for Internal ports. * Is this a question?