Change in coreboot[master]: mb/google/zork: Disable unsupported SOC usb ports

[Rob Barnes (Code Review)]

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

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

mb/google/zork: Disable unsupported SOC usb ports

Disable USB ports that are not supported by the SOC.
Handles Dali, Picasso and Pollock.

BUG=b:158096224
TEST=Boot Picasso Ezkinil, observe ports disabled

Change-Id: Ie37e2ac6a3a26edf77823193e592310eb10b9f9c
Signed-off-by: Rob Barnes 
---
M src/soc/amd/picasso/chip.c
1 file changed, 30 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/42800/1

diff --git a/src/soc/amd/picasso/chip.c b/src/soc/amd/picasso/chip.c
index c510efe..fb5fb98 100644
--- a/src/soc/amd/picasso/chip.c
+++ b/src/soc/amd/picasso/chip.c
@@ -9,6 +9,7 @@
 #include <soc/data_fabric.h>
 #include <soc/iomap.h>
 #include <soc/pci_devs.h>
+#include <soc/soc_util.h>
 #include <soc/southbridge.h>
 #include "chip.h"
 #include <fsp/api.h>
@@ -127,12 +128,40 @@
    }
 }

+/**
+ * Disables USB ports not supported on the SOC variant.
+ */
+static void disable_unsupported_usb_ports(void) {
+	struct device *bus_a, *xhci0, *xhci1, *hub, *port;
+
+	bus_a = pcidev_path_on_root(PCI_DEVFN(8, 1));
+	xhci0 = pcidev_path_behind(bus_a->link_list, PCI_DEVFN(0,3));
+	hub = xhci0->link_list->children;
+	port = NULL;
+	while ((port = dev_bus_each_child(hub->link_list, port)) != NULL) {
+		unsigned int port_id = port->path.usb.port_id;
+		/* Picasso SOC only has 4 USB ports on XHCI0 */
+		if(!soc_is_raven2() && port->enabled && port_id > 3) {
+			printk(BIOS_INFO,"Disabling XHCI0 USB port %s because it is not supported on this SOC\n", dev_path(port));
+			dev_set_enabled(port, 0);
+		}
+	}
+
+	xhci1 = pcidev_path_behind(bus_a->link_list, PCI_DEVFN(0,4));
+	hub = xhci1->link_list->children;
+	/* Dali and Pollock do not have USB ports on XHCI1 */
+	if(soc_is_raven2() && hub->enabled) {
+		printk(BIOS_INFO,"Disabling XHCI1 USB hub because it is not supported on this SOC\n");
+		disable_children(hub->bus);
+	}
+}
+
 static void soc_init(void *chip_info)
 {
    default_dev_ops_root.write_acpi_tables = agesa_write_acpi_tables;

fsp_silicon_init(acpi_is_wakeup_s3());
-
+	disable_unsupported_usb_ports();
    data_fabric_set_mmio_np();
    southbridge_init(chip_info);
 }

[Rob Barnes (Code Review)]

Rob Barnes has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

mb/google/zork: Disable unsupported SOC usb ports

Disable USB ports that are not supported by the SOC.
Handles Dali, Picasso and Pollock.

BUG=b:158096224
TEST=Boot Picasso Ezkinil, observe ports disabled

Change-Id: Ie37e2ac6a3a26edf77823193e592310eb10b9f9c
Signed-off-by: Rob Barnes 
---
M src/soc/amd/picasso/chip.c
1 file changed, 30 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/42800/2

[Rob Barnes (Code Review)]

Rob Barnes has uploaded a new patch set (#3). ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

mb/google/zork: Disable unsupported SOC usb ports

Disable USB ports that are not supported by the SOC.
Handles Dali, Picasso and Pollock.

BUG=b:158096224
TEST=Boot Picasso Ezkinil, observe ports disabled

Change-Id: Ie37e2ac6a3a26edf77823193e592310eb10b9f9c
Signed-off-by: Rob Barnes 
---
M src/soc/amd/picasso/chip.c
1 file changed, 32 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/42800/3

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c 
File src/soc/amd/picasso/chip.c:

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 144: 4 USB
4 USB 3 ports, but IIRC additionally 2 USB 2 ports on the same XHCI 0 controller

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 154: 	/* Dali and Pollock do not have USB ports on XHCI1 */
they don't have XHCI 1; only XHCI 0

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c 
File src/soc/amd/picasso/chip.c:

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 145: soc_is_raven2
both RV2 and fused-down PCO, which can also be used in a Dali SKU, have a reduced number of ports. get_soc_type() == SOC_PICASSO is probably what should be used here. that doesn't take possible AM4 SKUs into consideration though.

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 155: soc_is_raven2
get_soc_type != SOC_PICASSO maybe? same to possible AM4 SKUs as above.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Patch Set 4:

should I add a soc_is_defeatured_sku() to soc_util? i think that's what we should be exporting there instead of soc_is_dali()

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Patch Set 4:
> Patch Set 4:
>
> should I add a soc_is_defeatured_sku() to soc_util? i think that's what we should be exporting there instead of soc_is_dali()

See CB:42833

[Rob Barnes (Code Review)]

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

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Patch Set 4:

(4 comments)

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c 
File src/soc/amd/picasso/chip.c:

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 144: 4 USB
> 4 USB 3 ports, but IIRC additionally 2 USB 2 ports on the same XHCI 0 controller

I'm referencing Chapter 11 of FP5 Processor Motherboard Design Guide. FP5 Type 1 (Picasso) has 4 USB2 + 4 USB3 ports on XHCI0 and 2 USB2 ports + 1 USB3 port on XHCI1. FP5 Type 2 (Dali) has 4 USB3 and 6 USB2 ports on XHCI0 and no XHCI1.

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 145: soc_is_raven2
> both RV2 and fused-down PCO, which can also be used in a Dali SKU, have a reduced number of ports. […]

Commit 30c62ab9130c95871ebb9f517894a9b6b9c4f7d8 has soc_is_picasso helper. We should get that upstream so it can be used here.

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 154: 	/* Dali and Pollock do not have USB ports on XHCI1 */
> they don't have XHCI 1; only XHCI 0

So disable disable_children(xhci1->bus).

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 155: soc_is_raven2
> get_soc_type != SOC_PICASSO maybe? same to possible AM4 SKUs as above.

Ack

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c 
File src/soc/amd/picasso/chip.c:

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 166: disable_unsupported_usb_ports
Maybe we add this code to the driver? https://review.coreboot.org/c/coreboot/+/41900/6

It knows how many ports exist for each controller.

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c 
File src/soc/amd/picasso/chip.c:

https://review.coreboot.org/c/coreboot/+/42800/4/src/soc/amd/picasso/chip.c@... 
PS4, Line 166: disable_unsupported_usb_ports
> Maybe we add this code to the driver? https://review.coreboot.org/c/coreboot/+/41900/6 […]

https://review.coreboot.org/c/coreboot/+/43354/2 adds the functionality to the driver. We can abandon this CL now.

[Rob Barnes (Code Review)]

Rob Barnes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/42800 )

Change subject: mb/google/zork: Disable unsupported SOC usb ports
......................................................................

Abandoned

https://review.coreboot.org/c/coreboot/+/43354/2