On 9/15/2023 23:48, Lukas Wunner wrote:
On Fri, Sep 15, 2023 at 07:04:11AM -0500, Mario Limonciello wrote:
On 9/15/2023 02:08, Lukas Wunner wrote:
On Thu, Sep 14, 2023 at 09:33:54PM -0500, Mario Limonciello wrote:
+static bool child_has_amd_usb4(struct pci_dev *pdev)
+{
+ struct pci_dev *child = NULL;
+
+ while ((child = pci_get_class(PCI_CLASS_SERIAL_USB_USB4, child))) {
+ if (child->vendor != PCI_VENDOR_ID_AMD)
+ continue;
+ if (pcie_find_root_port(child) != pdev)
+ continue;
+ return true;
+ }
+
+ return false;
+}
What's the purpose of the pcie_find_root_port() check? PCI is a hierarchy,
not a graph, so a device cannot have any other Root Port but the one below
which you're searching.
If the purpose is to check that the port is a Root Port (if the PCI IDs
you're using in the DECLARE_PCI_FIXUP_* clauses match non-Root Ports),
check for pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT. (No need to
check for that in every loop iteration obviously, just check once in
the fixup.)
Thanks,
Lukas
The reason to look for it the way that I did was that there are multiple
root ports with the exact same PCI ID.
The problem only occurs on the root port that happens to have an AMD USB4
controller connected.
Yes but what's the purpose of the pcie_find_root_port(child) check
quoted above?
Thanks,
Lukas
You're right that if you look at this system alone that the check isn't
strictly necessary. It's to future proof the quirk. If a discrete USB4
controller was connected to the system it would be connected to a
different root port (the one that is used for PCI tunneling).
AMD doesn't have any of these devices, but if some day one was created
it could trip this codepath.
If you feel it's better to remove the check unless such a device is
created sure I can drop it.