Rob Barnes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 14 to trembyle baseboard devicetree Add bluetooth reset gpio 143 to dalboz baseboard devicetree
BUG=b:157580724 TEST=Boot Ezkinil, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb 2 files changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/42832/1
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb index d541529..d20115f 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb @@ -161,6 +161,7 @@ chip drivers/usb/acpi register "desc" = ""Bluetooth"" register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_143)" device usb 2.5 on end end chip drivers/usb/acpi diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb index 47ff8ba..c1d8288 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb @@ -189,6 +189,7 @@ chip drivers/usb/acpi register "desc" = ""Bluetooth"" register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_14)" device usb 2.5 on end end end @@ -208,6 +209,7 @@ chip drivers/usb/acpi register "desc" = ""Bluetooth"" register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_14)" device usb 2.1 on end end chip drivers/usb/acpi
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Set Ready For Review
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42832
to look at the new patch set (#4).
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 14 to trembyle baseboard devicetree Add bluetooth reset gpio 143 to dalboz baseboard devicetree
BUG=b:157580724 TEST=Boot Ezkinil with Realtek 8822CE, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb 2 files changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/42832/4
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb:
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... PS3, Line 272: GPIO_14 Trembyle EVT 2.2 schematic shows AGPIO14/USB_OC4_L attached to USB_A1_FAULT_ODL
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb:
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... PS3, Line 272: GPIO_14
Trembyle EVT 2. […]
APGIO14 was changed from USB_A1_FAULT_ODL input to BT_DISABLE output in version 3 of the schematic. We handle this override in zork/variants/*/gpio.c. I guess it may be necessary to add an override for this ACPI entry too?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb:
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... PS3, Line 272: GPIO_14
APGIO14 was changed from USB_A1_FAULT_ODL input to BT_DISABLE output in version 3 of the schematic. […]
We would need this override if we are supporting designs using EVT 2.2 schematics.
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb:
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... PS3, Line 272: GPIO_14
We would need this override if we are supporting designs using EVT 2.2 schematics.
Do we have the infrastructure for doing board id specific ACPI overrides or do we need to build this first?
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb:
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... PS3, Line 272: GPIO_14
Do we have the infrastructure for doing board id specific ACPI overrides or do we need to build this […]
variant_devtree_update seems like the right place.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb:
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... PS3, Line 272: GPIO_14
variant_devtree_update seems like the right place.
You might want to consider adding `variant_bluetooth_update()` called from `mainboard_devtree_update()` since it applies to all variants. Currently, it looks like `variant_devtree_update()` is being implemented by two variants already. `variant_bluetooth_update()` can then be implemented by trembyle and dalboz references only.
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42832
to look at the new patch set (#6).
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 14 to trembyle baseboard devicetree Add bluetooth reset gpio 143 to dalboz baseboard devicetree
BUG=b:157580724 TEST=Boot Ezkinil with Realtek 8822CE, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb 2 files changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/42832/6
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 7:
This can be squashed into the previous CL.
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42832
to look at the new patch set (#9).
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 143 to dalboz baseboard devicetree Add bluetooth reset gpio 14 to trembyle baseboard devicetree Remove bluetooth reset_gpio when not supported on a specfic board variant.
BUG=b:157580724 TEST=Boot Ezkinil with Realtek 8822CE, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 6 files changed, 98 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/42832/9
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb:
https://review.coreboot.org/c/coreboot/+/42832/3/src/mainboard/google/zork/v... PS3, Line 272: GPIO_14
You might want to consider adding `variant_bluetooth_update()` called from `mainboard_devtree_update […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 9: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 9: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/42832/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42832/9//COMMIT_MSG@11 PS9, Line 11: specfic nit: specific
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... PS9, Line 320: #if !DEVTREE_EARLY I think this whole block can be moved to ramstage_common.c. Then you won't need the guard for DEVTREE_EARLY and the function `baseboard_remove_bluetooth_reset_gpio()` doesn't need to exposed outside the file scope.
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... PS9, Line 38: audio nit: bluetooth
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Angel Pons, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42832
to look at the new patch set (#10).
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 143 to dalboz baseboard devicetree Add bluetooth reset gpio 14 to trembyle baseboard devicetree Remove bluetooth reset_gpio when not supported on a specific board variant.
BUG=b:157580724 TEST=Boot Ezkinil with Realtek 8822CE, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 5 files changed, 92 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/42832/10
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42832/10/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/42832/10/src/mainboard/google/zork/... PS10, Line 2: #include <console/console.h>
https://review.coreboot.org/c/coreboot/+/42832/10/src/mainboard/google/zork/... PS10, Line 9: Probably need: #include <device/device.h> #include <drivers/usb/acpi/chip.h>
too?
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Angel Pons, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42832
to look at the new patch set (#11).
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 143 to dalboz baseboard devicetree Add bluetooth reset gpio 14 to trembyle baseboard devicetree Remove bluetooth reset_gpio when not supported on a specific board variant.
BUG=b:157580724 TEST=Boot Ezkinil with Realtek 8822CE, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 5 files changed, 95 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/42832/11
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 11:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... PS9, Line 38: audio
nit: bluetooth
Done
https://review.coreboot.org/c/coreboot/+/42832/10/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/42832/10/src/mainboard/google/zork/... PS10, Line 2:
#include <console/console. […]
Done
https://review.coreboot.org/c/coreboot/+/42832/10/src/mainboard/google/zork/... PS10, Line 9:
Probably need: […]
Done
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42832/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42832/9//COMMIT_MSG@11 PS9, Line 11: specfic
nit: specific
Done
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/42832/9/src/mainboard/google/zork/v... PS9, Line 320: #if !DEVTREE_EARLY
I think this whole block can be moved to ramstage_common.c. […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 11: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/42832/11/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/42832/11/src/mainboard/google/zork/... PS11, Line 117: nit: extra blank line not required.
https://review.coreboot.org/c/coreboot/+/42832/11/src/mainboard/google/zork/... PS11, Line 118: /* There's no bluetooth device on XHCI1 on Dalboz */ : if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ)) : return; nit: Is this check required now? This function should not get called for dalboz at all based on the check in line 134.
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Angel Pons, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42832
to look at the new patch set (#12).
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 143 to dalboz baseboard devicetree Add bluetooth reset gpio 14 to trembyle baseboard devicetree Remove bluetooth reset_gpio when not supported on a specific board variant.
BUG=b:157580724 TEST=Boot Ezkinil with Realtek 8822CE, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com --- M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 5 files changed, 94 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/42832/12
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42832/11/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/42832/11/src/mainboard/google/zork/... PS11, Line 117:
nit: extra blank line not required.
Done
https://review.coreboot.org/c/coreboot/+/42832/11/src/mainboard/google/zork/... PS11, Line 118: /* There's no bluetooth device on XHCI1 on Dalboz */ : if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ)) : return;
nit: Is this check required now? This function should not get called for dalboz at all based on the […]
It's just defensive. It's not clear from the function name that it does not apply to dalboz.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 12: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
mb/google/zork: Add Bluetooth reset gpios to devicetree
Add bluetooth reset gpio 143 to dalboz baseboard devicetree Add bluetooth reset gpio 14 to trembyle baseboard devicetree Remove bluetooth reset_gpio when not supported on a specific board variant.
BUG=b:157580724 TEST=Boot Ezkinil with Realtek 8822CE, observe log [ 12.240720] Bluetooth: af_bluetooth.c:bt_init() HCI device and connection manager initialized [ 12.249272] Bluetooth: hci_sock.c:hci_sock_init() HCI socket layer initialized [ 12.256520] Bluetooth: l2cap_sock.c:l2cap_init_sockets() L2CAP socket layer initialized [ 12.264575] Bluetooth: sco.c:sco_init() SCO socket layer initialized [ 12.273700] usb 3-2: GPIO lookup for consumer reset [ 12.273702] usb 3-2: using ACPI for GPIO lookup [ 12.273705] acpi device:18: GPIO: looking up reset-gpios [ 12.273707] acpi device:18: GPIO: looking up reset-gpio [ 12.273711] acpi device:18: GPIO: _DSD returned device:18 0 0 0 [ 12.273737] gpio gpiochip0: Persistence not supported for GPIO 14 [ 12.273960] usbcore: registered new interface driver btusb
Change-Id: I14e3ef099d5b8f48c915b41284039b3508dec975 Signed-off-by: Rob Barnes robbarnes@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42832 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 5 files changed, 94 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/zork/mainboard.c b/src/mainboard/google/zork/mainboard.c index 1ce87a5..1e4ea06 100644 --- a/src/mainboard/google/zork/mainboard.c +++ b/src/mainboard/google/zork/mainboard.c @@ -135,6 +135,7 @@ static void mainboard_devtree_update(void) { variant_audio_update(); + variant_bluetooth_update(); variant_devtree_update(); }
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb index 0b3c1f1..e2d9183 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb @@ -235,6 +235,7 @@ chip drivers/usb/acpi register "desc" = ""Bluetooth"" register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_143)" device usb 2.5 on end end chip drivers/usb/acpi diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb index 787f00b..bbef702 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb @@ -263,6 +263,7 @@ chip drivers/usb/acpi register "desc" = ""Bluetooth"" register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_14)" device usb 2.5 on end end end @@ -282,6 +283,7 @@ chip drivers/usb/acpi register "desc" = ""Bluetooth"" register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_14)" device usb 2.1 on end end chip drivers/usb/acpi diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index aa1a3c8..6b9bbfd 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -35,6 +35,8 @@ void variant_devtree_update(void); /* Update audio configuration in devicetree during ramstage. */ void variant_audio_update(void); +/* Update bluetooth configuration in devicetree during ramstage. */ +void variant_bluetooth_update(void); /* Configure PCIe GPIOs as per variant sequencing requirements. */ void variant_pcie_gpio_configure(void);
diff --git a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c index 511d60d..b2c5830 100644 --- a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c +++ b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c @@ -2,7 +2,10 @@
#include <acpi/acpi_device.h> #include <baseboard/variants.h> +#include <console/console.h> +#include <device/device.h> #include <drivers/amd/i2s_machine_dev/chip.h> +#include <drivers/usb/acpi/chip.h> #include <ec/google/chromeec/ec.h> #include <soc/gpio.h> #include <soc/pci_devs.h> @@ -47,3 +50,88 @@ break; } } + +static const struct device_path xhci0_bt_path[] = { + { + .type = DEVICE_PATH_PCI, + .pci.devfn = PCIE_GPP_A_DEVFN + }, + { + .type = DEVICE_PATH_PCI, + .pci.devfn = XHCI0_DEVFN + }, + { + .type = DEVICE_PATH_USB, + .usb.port_type = 0, + .usb.port_id = 0 + }, + { + .type = DEVICE_PATH_USB, + .usb.port_type = 2, + .usb.port_id = 5 + } +}; + +static const struct device_path xhci1_bt_path[] = { + { + .type = DEVICE_PATH_PCI, + .pci.devfn = PCIE_GPP_A_DEVFN + }, + { + .type = DEVICE_PATH_PCI, + .pci.devfn = XHCI1_DEVFN + }, + { + .type = DEVICE_PATH_USB, + .usb.port_type = 0, + .usb.port_id = 0 + }, + { + .type = DEVICE_PATH_USB, + .usb.port_type = 2, + .usb.port_id = 1 + } +}; + +/* + * Removes reset_gpio from bluetooth device in device tree. + * + * The bluetooth device may be on XHCI0 or XHCI1 depending on SOC. + * There's no harm in removing from both here. + */ +static void baseboard_remove_bluetooth_reset_gpio(void) +{ + const struct device *xhci0_bt_dev, *xhci1_bt_dev; + struct drivers_usb_acpi_config *xhci0_bt_cfg, *xhci1_bt_cfg; + + xhci0_bt_dev = find_dev_nested_path( + pci_root_bus(), xhci0_bt_path, ARRAY_SIZE(xhci0_bt_path)); + if (!xhci0_bt_dev) { + printk(BIOS_ERR, "%s: Failed to find bluetooth device on XHCI0!", __func__); + return; + } + /* config_of dies on failure, so a NULL check is not required */ + xhci0_bt_cfg = config_of(xhci0_bt_dev); + xhci0_bt_cfg->reset_gpio.pin_count = 0; + + /* There's no bluetooth device on XHCI1 on Dalboz */ + if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ)) + return; + + xhci1_bt_dev = find_dev_nested_path( + pci_root_bus(), xhci1_bt_path, ARRAY_SIZE(xhci1_bt_path)); + if (!xhci1_bt_dev) { + printk(BIOS_ERR, "%s: Failed to find bluetooth device on XHCI1!", __func__); + return; + } + xhci1_bt_cfg = config_of(xhci1_bt_dev); + xhci1_bt_cfg->reset_gpio.pin_count = 0; +} + +void variant_bluetooth_update(void) +{ + if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ) || variant_uses_v3_schematics()) + return; + + baseboard_remove_bluetooth_reset_gpio(); +}
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... PS13, Line 115: xhci0_bt_cfg->reset_gpio.pin_count = 0; This whole sequence could have used a helper function that took a path and string name as to not duplicate the logic too much. We're also bailing early above which wouldn't fix up the latter one below.
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... PS13, Line 118: if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ)) There is already a dalboz check below. This is redundant.
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... PS13, Line 115: xhci0_bt_cfg->reset_gpio.pin_count = 0;
This whole sequence could have used a helper function that took a path and string name as to not dup […]
I'll put up a CL.
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... PS13, Line 118: if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ))
There is already a dalboz check below. This is redundant.
This is just defensive in case it's called by dalboz for whatever reason. We could remove it and change the name of the function to baseboard_trembyle_remove_bluetooth_reset_gpio.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42832 )
Change subject: mb/google/zork: Add Bluetooth reset gpios to devicetree ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/42832/13/src/mainboard/google/zork/... PS13, Line 118: if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ))
This is just defensive in case it's called by dalboz for whatever reason. […]
This function is local to this compilation unit and there's one caller which already had a guard.
-
Aaron Durbin (Code Review) -
Angel Pons (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Raul Rangel (Code Review)
-
Rob Barnes (Code Review)