3 comments:
File src/drivers/usb/pci_xhci/chip.h:
Patch Set #5, Line 11: __XHCI_ACPI_CHIP_H__
__USB_XHCI_PCI_CHIP_H__
File src/drivers/usb/pci_xhci/pci_xhci.c:
I was thinking some more about this. PCI drivers in coreboot add an array of PCI device IDs ending in 0 which is used to match the driver to a particular device. Why not use that here as well?
File src/drivers/usb/pci_xhci/pci_xhci.c:
Sorry about the delay in review here. I was giving this some thought. All PCI drivers in coreboot provide a PCI device ID table with a list of IDs that the driver supports. XHCI controller is a property/attribute of the SoC itself and not mainboard dependent. I am thinking that:
1. We should add a list of PCI device IDs here that determines which XHCI controllers are supported by this driver.
2. DRIVERS_USB_PCI_XHCI should really be selected by the SoC.
3. This also keeps the organization within device tree consistent i.e. pci devices don't generally live under chip drivers of their own.
4. I think the reason you need the chip driver here is because of the wake property being passed into this driver. What do you think about having a SoC callback here soc_get_controller_wake_gpe(const struct device *controller, int *gpe) which returns -1 using a weak definition in this file and can be overridden by SoC if required. For platforms that have a fixed GPE for XHCI wake can return that GPE# directly in SoC code. For platforms that do not have a fixed GPE (e.g. picasso) can allow mainboards to set xhci_wake_gpe in devicetree under chip/soc/ and use that to return the right value for the callback.
Thoughts?
To view, visit change 41900. To unsubscribe, or for help writing mail filters, visit settings.