Gaggery Tsai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30754
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
mb/google/poppy/variant/atlas: enable USB acpi
Main objective for this change is to export the bluetooth reset gpio to the kernel for use in an rf-kill operation. To do so, we enable USB acpi and define all of the USB2 devices, which includes bluetooth's reset gpio information.
BUG=b:122540489 BRANCH=None TEST=emerge-atlas coreboot chromeos-bootimage $cat sys/firmware/acpi/tables/SSDT > /tmp/ssdt.dat & retrieve ssdt.dat from DUT & $iasl -d ./ssdt.dat & check the HS03 node is with "reset-gpio" under _DSD object
Change-Id: I411ef707782655361bd1b8ac2b914b8ae64defeb Signed-off-by: Gaggery Tsai gaggery.tsai@intel.com --- M src/mainboard/google/poppy/Kconfig M src/mainboard/google/poppy/variants/atlas/devicetree.cb 2 files changed, 25 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/30754/1
diff --git a/src/mainboard/google/poppy/Kconfig b/src/mainboard/google/poppy/Kconfig index 59abe72..197986d 100644 --- a/src/mainboard/google/poppy/Kconfig +++ b/src/mainboard/google/poppy/Kconfig @@ -150,6 +150,7 @@ select DRIVERS_I2C_MAX98373 select DRIVERS_I2C_DA7219 select DRIVERS_SPI_ACPI + select DRIVERS_USB_ACPI select EXCLUDE_NATIVE_SD_INTERFACE select MAINBOARD_HAS_SPI_TPM_CR50 select VARIANT_HAS_CAMERA_ACPI diff --git a/src/mainboard/google/poppy/variants/atlas/devicetree.cb b/src/mainboard/google/poppy/variants/atlas/devicetree.cb index f8a6e6d..97c10b0 100644 --- a/src/mainboard/google/poppy/variants/atlas/devicetree.cb +++ b/src/mainboard/google/poppy/variants/atlas/devicetree.cb @@ -268,7 +268,30 @@ device pci 00.0 on end # Host Bridge device pci 02.0 on end # Integrated Graphics Device device pci 13.0 off end # Integrated Sensor Hub - device pci 14.0 on end # USB xHCI + device pci 14.0 on + chip drivers/usb/acpi + register "desc" = ""Root Hub"" + register "type" = "UPC_TYPE_HUB" + device usb 0.0 on + chip drivers/usb/acpi + register "desc" = ""USB Type C Port 1"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + device usb 2.0 on end + end + chip drivers/usb/acpi + register "desc" = ""Bluetooth"" + register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_E2)" + device usb 2.2 on end + end + chip drivers/usb/acpi + register "desc" = ""USB Type C Port 2"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + device usb 2.4 on end + end + end + end + end # USB xHCI device pci 14.1 on end # USB xDCI (OTG) device pci 14.2 on end # Thermal Subsystem device pci 15.0 on
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1: Code-Review+1
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1:
Any reason not to add the USB3 devices?
Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1:
Patch Set 1:
Any reason not to add the USB3 devices?
The purpose of this patch to add a GPIO for rfkill operation. Not sure about the ask for USB3.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Any reason not to add the USB3 devices?
The purpose of this patch to add a GPIO for rfkill operation. Not sure about the ask for USB3.
It is more about completeness, if you are going to fill out the USB2 you might as well add the USB3 entries. Otherwise people might see this and think it is complete.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1: Code-Review+1
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/atlas/devicetree.cb:
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... PS1, Line 284: reset_gpio usb-acpi doesn't seem to have the concept of an enable_gpio. is there a way to define this pin as an enable pin similar to what we do for the touchscreen a few lines down?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/atlas/devicetree.cb:
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... PS1, Line 284: reset_gpio
usb-acpi doesn't seem to have the concept of an enable_gpio. […]
Reset gpio is not used for asserting/de-asserting reset to a device using _ON/_OFF routines for USB. It is exposed to allow drivers to use it if required i.e. Bluetooth driver can use it to toggle reset if bluetooth gets into a bad state.
What is the use case for an enable gpio?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/atlas/devicetree.cb:
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... PS1, Line 284: reset_gpio
usb-acpi doesn't seem to have the concept of an enable_gpio. […]
The reset here is a specific hack for intel bluetooth: https://patchwork.kernel.org/patch/10777829/
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... File src/mainboard/google/poppy/variants/atlas/devicetree.cb:
https://review.coreboot.org/#/c/30754/1/src/mainboard/google/poppy/variants/... PS1, Line 284: reset_gpio
The reset here is a specific hack for intel bluetooth: https://patchwork.kernel. […]
GPP_E2 goes to the device's W_DISABLE2# pin which completely shuts down bluetooth - it's not really a classic reset pin. i was contemplating leaving this off (asserted) in coreboot until something higher up decides to enable bluetooth.
i don't see the timing requirements for this signal in the EPS. the driver duncan mentioned does a 100ms blip.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
Patch Set 1: Code-Review+2
ok, fair enough. looks like this scheme is how we're supporting reset.
Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30754 )
Change subject: mb/google/poppy/variant/atlas: enable USB acpi ......................................................................
mb/google/poppy/variant/atlas: enable USB acpi
Main objective for this change is to export the bluetooth reset gpio to the kernel for use in an rf-kill operation. To do so, we enable USB acpi and define all of the USB2 devices, which includes bluetooth's reset gpio information.
BUG=b:122540489 BRANCH=None TEST=emerge-atlas coreboot chromeos-bootimage $cat sys/firmware/acpi/tables/SSDT > /tmp/ssdt.dat & retrieve ssdt.dat from DUT & $iasl -d ./ssdt.dat & check the HS03 node is with "reset-gpio" under _DSD object
Change-Id: I411ef707782655361bd1b8ac2b914b8ae64defeb Signed-off-by: Gaggery Tsai gaggery.tsai@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/30754 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nick Vaccaro nvaccaro@google.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Caveh Jalali caveh@google.com --- M src/mainboard/google/poppy/Kconfig M src/mainboard/google/poppy/variants/atlas/devicetree.cb 2 files changed, 25 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Caveh Jalali: Looks good to me, approved Nick Vaccaro: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/poppy/Kconfig b/src/mainboard/google/poppy/Kconfig index 419b10e..b7f479f 100644 --- a/src/mainboard/google/poppy/Kconfig +++ b/src/mainboard/google/poppy/Kconfig @@ -151,6 +151,7 @@ select DRIVERS_I2C_MAX98373 select DRIVERS_I2C_DA7219 select DRIVERS_SPI_ACPI + select DRIVERS_USB_ACPI select EXCLUDE_NATIVE_SD_INTERFACE select MAINBOARD_HAS_SPI_TPM_CR50 select VARIANT_HAS_CAMERA_ACPI diff --git a/src/mainboard/google/poppy/variants/atlas/devicetree.cb b/src/mainboard/google/poppy/variants/atlas/devicetree.cb index ad79bca..1ea28a0 100644 --- a/src/mainboard/google/poppy/variants/atlas/devicetree.cb +++ b/src/mainboard/google/poppy/variants/atlas/devicetree.cb @@ -265,7 +265,30 @@ device pci 00.0 on end # Host Bridge device pci 02.0 on end # Integrated Graphics Device device pci 13.0 off end # Integrated Sensor Hub - device pci 14.0 on end # USB xHCI + device pci 14.0 on + chip drivers/usb/acpi + register "desc" = ""Root Hub"" + register "type" = "UPC_TYPE_HUB" + device usb 0.0 on + chip drivers/usb/acpi + register "desc" = ""USB Type C Port 1"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + device usb 2.0 on end + end + chip drivers/usb/acpi + register "desc" = ""Bluetooth"" + register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_E2)" + device usb 2.2 on end + end + chip drivers/usb/acpi + register "desc" = ""USB Type C Port 2"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + device usb 2.4 on end + end + end + end + end # USB xHCI device pci 14.1 on end # USB xDCI (OTG) device pci 14.2 on end # Thermal Subsystem device pci 15.0 on
-
Caveh Jalali (Code Review) -
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Gaggery Tsai (Code Review)
-
Nick Vaccaro (Code Review)
-
Paul Menzel (Code Review)