Lijian Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31905
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
drivers/usb: Fix USB acpi gpio programming
GPIO programming for transition between D0 and D3 state will require power resources to be present. Otherwise the previously defined "reset-gpio" can't take effect.
BUG=b:123342945 TEST=Boot up on Arcada platform and extract SSDT table, find the GPIOIO entry and set/clear and "_ON/_OFF" present under USB devices after reset-gpio entry added in devicetree.cb file.
Signed-off-by: Lijian Zhao lijian.zhao@intel.com Change-Id: If312150b5065e2e3fb362e9433cdeff47064a35a --- M src/drivers/usb/acpi/chip.h M src/drivers/usb/acpi/usb_acpi.c 2 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/31905/1
diff --git a/src/drivers/usb/acpi/chip.h b/src/drivers/usb/acpi/chip.h index 512893d..51dec63 100644 --- a/src/drivers/usb/acpi/chip.h +++ b/src/drivers/usb/acpi/chip.h @@ -56,7 +56,29 @@ bool use_custom_pld; struct acpi_pld custom_pld;
+ /* Does the device have a power resource? */ + bool has_power_resource; + + /* GPIO used to take device out of reset or to put it into reset. */ struct acpi_gpio reset_gpio; + /* Delay to be inserted after device is taken out of reset. */ + unsigned int reset_delay_ms; + /* Delay to be inserted after device is put into reset. */ + unsigned int reset_off_delay_ms; + + /* GPIO used to enable device. */ + struct acpi_gpio enable_gpio; + /* Delay to be inserted after device is enabled. */ + unsigned int enable_delay_ms; + /* Delay to be inserted after device is disabled. */ + unsigned int enable_off_delay_ms; + + /* GPIO used to stop operation of device. */ + struct acpi_gpio stop_gpio; + /* Delay to be inserted after disabling stop. */ + unsigned int stop_delay_ms; + /* Delay to be inserted after enabling stop. */ + unsigned int stop_off_delay_ms; };
#endif /* __USB_ACPI_CHIP_H__ */ diff --git a/src/drivers/usb/acpi/usb_acpi.c b/src/drivers/usb/acpi/usb_acpi.c index 8c63d2a..e77472c 100644 --- a/src/drivers/usb/acpi/usb_acpi.c +++ b/src/drivers/usb/acpi/usb_acpi.c @@ -76,6 +76,22 @@ acpi_dp_write(dsd); }
+ /* Power Resource */ + if (config->has_power_resource) { + const struct acpi_power_res_params power_res_params = { + &config->reset_gpio, + config->reset_delay_ms, + config->reset_off_delay_ms, + &config->enable_gpio, + config->enable_delay_ms, + config->enable_off_delay_ms, + &config->stop_gpio, + config->stop_delay_ms, + config->stop_off_delay_ms + }; + acpi_device_add_power_res(&power_res_params); + } + acpigen_pop_len();
printk(BIOS_INFO, "%s: %s at %s\n", path,
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31905 )
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31905/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31905/2//COMMIT_MSG@8 PS2, Line 8: : GPIO programming for transition between D0 and D3 state will require : power resources to be present. Otherwise the previously defined : "reset-gpio" can't take effect. That is not the intention of the reset-gpio. It was designed to be used by the kernel driver only in cases where BT has entered a bad state and needs to be recovered. I don't think we want to assert the reset to BT device every time we suspend. It will prevent wake from BT (if that has to be a wake source).
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31905 )
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31905/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31905/2//COMMIT_MSG@8 PS2, Line 8: : GPIO programming for transition between D0 and D3 state will require : power resources to be present. Otherwise the previously defined : "reset-gpio" can't take effect.
That is not the intention of the reset-gpio. […]
Hi Furquan, That's the feedback from Amy as well, she did say the similar thing. Then how come the kernel driver can trigger the gpio pin, through "gpioio" resources?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31905 )
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31905/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31905/2//COMMIT_MSG@8 PS2, Line 8: : GPIO programming for transition between D0 and D3 state will require : power resources to be present. Otherwise the previously defined : "reset-gpio" can't take effect.
Hi Furquan, […]
This change outlines the details: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1...
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31905 )
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
I see, so just the _DSD is good enough? Then I don't need this fix any more, just my original patch with updated commit message. Now that's clear!
Lijian Zhao has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31905 )
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
Abandoned
Not necessary
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31905 )
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
Patch Set 2:
We need control USB camera power for better power consumption. We have camera_en pin as B11 on sarien platform.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31905 )
Change subject: drivers/usb: Fix USB acpi gpio programming ......................................................................
Patch Set 2:
This is issue number https://partnerissuetracker.corp.google.com/issues/129177593
-
EricR Lai (Code Review) -
Furquan Shaikh (Code Review)
-
Lijian Zhao (Code Review)