Change in coreboot[master]: mb/google/zork: Cleanup bt reset_gpio removal

[Rob Barnes (Code Review)]

Rob Barnes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

mb/google/zork: Cleanup bt reset_gpio removal

Cleanup of bt reset_gpio removal function.

TEST=Boot and observe log showing bt reset_gpio was removed
BUG=b:157580724

Signed-off-by: Rob Barnes 
Change-Id: I1d40ad16dd3c624d4be89d9eea1835cc4e72c03d
---
M src/mainboard/google/zork/variants/baseboard/ramstage_common.c
1 file changed, 64 insertions(+), 67 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44273/1

diff --git a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c
index b2c5830..4f1be8a 100644
--- a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c
+++ b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c
@@ -51,87 +51,84 @@
    }
 }

-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.
+ * Removes reset_gpio from usb device in device tree.
  */
-static void baseboard_remove_bluetooth_reset_gpio(void)
+static void remove_usb_device_reset_gpio(const struct device_path usb_path[],
+					 size_t path_length)
 {
-	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__);
+	const struct device *usb_dev;
+	struct drivers_usb_acpi_config *usb_cfg;
+
+	usb_dev = find_dev_nested_path(
+		pci_root_bus(), usb_path, path_length);
+	if (!usb_dev) {
+		printk(BIOS_ERR, "%s: Failed to find USB device!", __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;
+	usb_cfg = config_of(usb_dev);
+	usb_cfg->reset_gpio.pin_count = 0;
+}

-	/* There's no bluetooth device on XHCI1 on Dalboz */
-	if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ))
-		return;
+/*
+ * The bluetooth device may be on XHCI0 or XHCI1 depending on SOC.
+ * There's no harm in removing reset_gpio from both here.
+ */
+static void baseboard_trembyle_remove_bluetooth_reset_gpio(void)
+{
+	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
+		}
+	};

-	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;
+	remove_usb_device_reset_gpio(xhci0_bt_path, ARRAY_SIZE(xhci0_bt_path));
+	remove_usb_device_reset_gpio(xhci1_bt_path, ARRAY_SIZE(xhci1_bt_path));
 }

void variant_bluetooth_update(void)
 {
-	if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ) || variant_uses_v3_schematics())
+	if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ) || variant_uses_v3_schematics()) {
    	return;
+	}

-	baseboard_remove_bluetooth_reset_gpio();
+	baseboard_trembyle_remove_bluetooth_reset_gpio();
 }

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44273/1/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:

https://review.coreboot.org/c/coreboot/+/44273/1/src/mainboard/google/zork/v... 
PS1, Line 129: 	if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ) || variant_uses_v3_schematics()) {
braces {} are not necessary for single statement blocks

[Rob Barnes (Code Review)]

Hello Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44273

to look at the new patch set (#2).

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

mb/google/zork: Cleanup bt reset_gpio removal

Cleanup of bt reset_gpio removal function.

TEST=Boot and observe log showing bt reset_gpio was removed
BUG=b:157580724

Signed-off-by: Rob Barnes 
Change-Id: I1d40ad16dd3c624d4be89d9eea1835cc4e72c03d
---
M src/mainboard/google/zork/variants/baseboard/ramstage_common.c
1 file changed, 62 insertions(+), 66 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44273/2

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/44273/2/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:

https://review.coreboot.org/c/coreboot/+/44273/2/src/mainboard/google/zork/v... 
PS2, Line 65: 	usb_dev = find_dev_nested_path(
flow on the same line?

https://review.coreboot.org/c/coreboot/+/44273/2/src/mainboard/google/zork/v... 
PS2, Line 68: 		printk(BIOS_ERR, "%s: Failed to find USB device!", __func__);
you can pass in a name so you can specify which xhci controller number you were expecting. That way you'll have better debug messages.

[Rob Barnes (Code Review)]

Hello build bot (Jenkins), Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44273

to look at the new patch set (#3).

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

mb/google/zork: Cleanup bt reset_gpio removal

Cleanup of bt reset_gpio removal function.

TEST=Boot and observe log showing bt reset_gpio was removed
BUG=b:157580724

Signed-off-by: Rob Barnes 
Change-Id: I1d40ad16dd3c624d4be89d9eea1835cc4e72c03d
---
M src/mainboard/google/zork/variants/baseboard/ramstage_common.c
1 file changed, 64 insertions(+), 65 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44273/3

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

Patch Set 3: Code-Review+2

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44273/3/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:

https://review.coreboot.org/c/coreboot/+/44273/3/src/mainboard/google/zork/v... 
PS3, Line 61: 					 size_t path_length, const char* debug_device_name)
"foo* bar" should be "foo *bar"

[Rob Barnes (Code Review)]

Hello build bot (Jenkins), Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44273

to look at the new patch set (#4).

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

mb/google/zork: Cleanup bt reset_gpio removal

Cleanup of bt reset_gpio removal function.

TEST=Boot and observe log showing bt reset_gpio was removed
BUG=b:157580724

Signed-off-by: Rob Barnes 
Change-Id: I1d40ad16dd3c624d4be89d9eea1835cc4e72c03d
---
M src/mainboard/google/zork/variants/baseboard/ramstage_common.c
1 file changed, 64 insertions(+), 65 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44273/4

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

Patch Set 4: Code-Review+2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Cleanup bt reset_gpio removal
......................................................................

Patch Set 4:

(3 comments)

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG@7 
PS4, Line 7: Cleanup
Clean up

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG@9 
PS4, Line 9: Cleanup
Clean up

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG@12 
PS4, Line 12: BUG=b:157580724
Please summarize the bug.

Change in coreboot[master]: mb/google/zork: Clean up bt reset_gpio removal

[Rob Barnes (Code Review)]

Hello build bot (Jenkins), Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/44273

to look at the new patch set (#5).

Change subject: mb/google/zork: Clean up bt reset_gpio removal
......................................................................

mb/google/zork: Clean up bt reset_gpio removal

Clean up of bt reset_gpio removal function.

TEST=Boot and observe log showing bt reset_gpio was removed
BUG=b:157580724 - Add reset_gpio for Bluetooth

Signed-off-by: Rob Barnes 
Change-Id: I1d40ad16dd3c624d4be89d9eea1835cc4e72c03d
---
M src/mainboard/google/zork/variants/baseboard/ramstage_common.c
1 file changed, 64 insertions(+), 65 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/44273/5

Change in coreboot[master]: mb/google/zork: Clean up bt reset_gpio removal

[Rob Barnes (Code Review)]

Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Clean up bt reset_gpio removal
......................................................................

Patch Set 5:

(4 comments)

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG@7 
PS4, Line 7: Cleanup
> Clean up

Done

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG@9 
PS4, Line 9: Cleanup
> Clean up

Done

https://review.coreboot.org/c/coreboot/+/44273/4//COMMIT_MSG@12 
PS4, Line 12: BUG=b:157580724
> Please summarize the bug.

The bug listed here is the original request for adding reset_gpio. This change is just some clean up to that code. I could change to BUG=none if that is more clear.

https://review.coreboot.org/c/coreboot/+/44273/2/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:

https://review.coreboot.org/c/coreboot/+/44273/2/src/mainboard/google/zork/v... 
PS2, Line 65: 	usb_dev = find_dev_nested_path(
> flow on the same line?

Yep, copy paste mistake.

Change in coreboot[master]: mb/google/zork: Clean up bt reset_gpio removal

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Clean up bt reset_gpio removal
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44273/2/src/mainboard/google/zork/v... 
File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:

https://review.coreboot.org/c/coreboot/+/44273/2/src/mainboard/google/zork/v... 
PS2, Line 68: 		printk(BIOS_ERR, "%s: Failed to find USB device!", __func__);
> you can pass in a name so you can specify which xhci controller number you were expecting. […]

Done

Change in coreboot[master]: mb/google/zork: Clean up bt reset_gpio removal

[Aaron Durbin (Code Review)]

Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44273 )

Change subject: mb/google/zork: Clean up bt reset_gpio removal
......................................................................

mb/google/zork: Clean up bt reset_gpio removal

Clean up of bt reset_gpio removal function.

TEST=Boot and observe log showing bt reset_gpio was removed
BUG=b:157580724 - Add reset_gpio for Bluetooth

Signed-off-by: Rob Barnes 
Change-Id: I1d40ad16dd3c624d4be89d9eea1835cc4e72c03d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44273
Reviewed-by: Aaron Durbin 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/zork/variants/baseboard/ramstage_common.c
1 file changed, 64 insertions(+), 65 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved

diff --git a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c
index 679f98c..664f659 100644
--- a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c
+++ b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c
@@ -127,81 +127,80 @@
    update_hp_int_odl();
 }

-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.
+ * Removes reset_gpio from usb 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.
+ * debug_device_name is used for debug messaging only.
  */
-static void baseboard_remove_bluetooth_reset_gpio(void)
+static void remove_usb_device_reset_gpio(const struct device_path usb_path[],
+					 size_t path_length, const char *debug_device_name)
 {
-	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__);
+	const struct device *usb_dev;
+	struct drivers_usb_acpi_config *usb_cfg;
+
+	usb_dev = find_dev_nested_path(pci_root_bus(), usb_path, path_length);
+	if (!usb_dev) {
+		printk(BIOS_ERR, "%s: Failed to find %s!", __func__, debug_device_name);
    	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;
+	usb_cfg = config_of(usb_dev);
+	usb_cfg->reset_gpio.pin_count = 0;
+}

-	/* There's no bluetooth device on XHCI1 on Dalboz */
-	if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ))
-		return;
+/*
+ * The bluetooth device may be on XHCI0 or XHCI1 depending on SOC.
+ * There's no harm in removing reset_gpio from both here.
+ */
+static void baseboard_trembyle_remove_bluetooth_reset_gpio(void)
+{
+	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
+		}
+	};

-	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;
+	remove_usb_device_reset_gpio(xhci0_bt_path, ARRAY_SIZE(xhci0_bt_path),
+				     "XHCI0 Bluetoth USB Device");
+	remove_usb_device_reset_gpio(xhci1_bt_path, ARRAY_SIZE(xhci1_bt_path),
+				     "XHCI1 Bluetoth USB Device");
 }

void variant_bluetooth_update(void)
@@ -209,7 +208,7 @@
    if (CONFIG(BOARD_GOOGLE_BASEBOARD_DALBOZ) || variant_uses_v3_schematics())
    	return;

-	baseboard_remove_bluetooth_reset_gpio();
+	baseboard_trembyle_remove_bluetooth_reset_gpio();
 }

void variant_touchscreen_update(void)