Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38683 )
Change subject: usb: add a new function soc_get_usb_ctlr ......................................................................
usb: add a new function soc_get_usb_ctlr
The new function should be provided by SoCs or mainboards, and it simply returns the 'struct device *' representing the USB root controller.
Change-Id: I7dd2569dd615722a69f2dfd75622362e56a424ea Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/device/Makefile.inc A src/device/usb_device.c A src/include/device/usb.h M src/soc/intel/common/block/xhci/xhci.c 4 files changed, 43 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/38683/1
diff --git a/src/device/Makefile.inc b/src/device/Makefile.inc index 966ca0d..33d81b9 100644 --- a/src/device/Makefile.inc +++ b/src/device/Makefile.inc @@ -58,3 +58,5 @@ verstage-y += mmio.c romstage-y += mmio.c ramstage-y += mmio.c + +ramstage-y += usb_device.c diff --git a/src/device/usb_device.c b/src/device/usb_device.c new file mode 100644 index 0000000..723ad1b --- /dev/null +++ b/src/device/usb_device.c @@ -0,0 +1,16 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <device/device.h> +#include <device/usb.h> + +/* Return the devicetree entry for the USB controller */ +__weak struct device *soc_get_usb_ctlr(void) +{ + return NULL; +} diff --git a/src/include/device/usb.h b/src/include/device/usb.h new file mode 100644 index 0000000..bd0f190 --- /dev/null +++ b/src/include/device/usb.h @@ -0,0 +1,19 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef _DEVICE_USB_H_ +#define _DEVICE_USB_H_ + +/** + * Return the devicetree entry for the USB controller. + * + * SoC-specific by nature. + */ +struct device *soc_get_usb_ctlr(void); + +#endif /* _DEVICE_USB_H_ */ diff --git a/src/soc/intel/common/block/xhci/xhci.c b/src/soc/intel/common/block/xhci/xhci.c index 4b8a5cc..222b215 100644 --- a/src/soc/intel/common/block/xhci/xhci.c +++ b/src/soc/intel/common/block/xhci/xhci.c @@ -19,6 +19,7 @@ #include <device/device.h> #include <device/pci.h> #include <device/pci_ids.h> +#include <device/usb.h> #include <drivers/usb/acpi/chip.h> #include <intelblocks/acpi.h> #include <intelblocks/xhci.h> @@ -107,6 +108,11 @@
__weak void soc_xhci_init(struct device *dev) { /* no-op */ }
+struct device *soc_get_usb_ctlr(void) +{ + return pcidev_path_on_root(PCH_DEVFN_XHCI); +} + static struct device_operations usb_xhci_ops = { .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources,
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38683 )
Change subject: usb: add a new function soc_get_usb_ctlr ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38683/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38683/1//COMMIT_MSG@9 PS1, Line 9: should Please give the motivation why. Why isn't iterating over the devicetree items sufficient parsing the class member of struct device?
https://review.coreboot.org/c/coreboot/+/38683/1//COMMIT_MSG@10 PS1, Line 10: root That to imprecise. On older Intel platforms there are multiple controllers (EHCI, 2x UHCI).
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38683 )
Change subject: usb: add a new function soc_get_usb_ctlr ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38683/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38683/1//COMMIT_MSG@10 PS1, Line 10: root
That to imprecise. On older Intel platforms there are multiple controllers (EHCI, 2x UHCI).
That's fair. This is to support b/146506369, adding USB-PD information (like the port's power and data roles, etc.) in the SSDT under the EC object. But if we ever decide to use an SoC with more than one controller, this becomes brittle. Let me think a little more on this one.
Tim Wawrzynczak has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/38683 )
Change subject: usb: add a new function soc_get_usb_ctlr ......................................................................
Abandoned
Thanks for pushing back Patrick, I found a better way that doesn't require any board or soc-specific mechanism
-
Patrick Rudolph (Code Review) -
Tim Wawrzynczak (Code Review)