[Public]
-----Original Message----- From: Mika Westerberg mika.westerberg@linux.intel.com Sent: Friday, February 11, 2022 04:24 To: Limonciello, Mario Mario.Limonciello@amd.com Cc: Bjorn Helgaas bhelgaas@google.com; Andreas Noever andreas.noever@gmail.com; open list:PCI SUBSYSTEM <linux- pci@vger.kernel.org>; open list:THUNDERBOLT DRIVER <linux- usb@vger.kernel.org>; open list:RADEON and AMDGPU DRM DRIVERS <amd- gfx@lists.freedesktop.org>; open list:DRM DRIVERS <dri- devel@lists.freedesktop.org>; open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS nouveau@lists.freedesktop.org; open list:X86 PLATFORM DRIVERS platform-driver-x86@vger.kernel.org; Michael Jamet michael.jamet@intel.com; Yehezkel Bernat YehezkelShB@gmail.com; Lukas Wunner lukas@wunner.de; Deucher, Alexander Alexander.Deucher@amd.com Subject: Re: [PATCH v2 3/9] PCI: drop `is_thunderbolt` attribute
Hi Mario,
On Thu, Feb 10, 2022 at 04:43:23PM -0600, Mario Limonciello wrote:
The `is_thunderbolt` attribute is currently a dumping ground for a variety of things.
Instead use the driver core removable attribute to indicate the detail a device is attached to a thunderbolt or USB4 chain.
Signed-off-by: Mario Limonciello mario.limonciello@amd.com
drivers/pci/pci.c | 2 +- drivers/pci/probe.c | 20 +++++++------------- drivers/platform/x86/apple-gmux.c | 2 +- include/linux/pci.h | 5 ++--- 4 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 9ecce435fb3f..1264984d5e6d 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2955,7 +2955,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) return true;
/* Even the oldest 2010 Thunderbolt controller supports D3. */
if (bridge->is_thunderbolt)
if (dev_is_removable(&bridge->dev))For this, I'm not entirely sure this is what we want. The purpose of this check is to enable port power management of Apple systems with Intel Thunderbolt controller and therefore checking for "removable" here is kind of misleading IMHO.
I wonder if we could instead remove the check completely here and rely on the below:
if (platform_pci_bridge_d3(bridge)) return true;
and that would then look like:
static inline bool platform_pci_bridge_d3(struct pci_dev *dev) { if (pci_use_mid_pm()) return false;
if (acpi_pci_bridge_d3(dev)) return true;
if (device_property_read_bool(&dev->dev, "HotPlugSupportInD3")) return true;
return false; }
and then make a quirk in quirks.c that adds the software node property for the Apple systems? Or something along those lines.
@Lukas, what do you think?
I took a stab at doing this for V3, but I'm unsure whether ALL of the TBT controllers in pci_ids.h have been used in Apple laptops, so it might be a bit wasteful of a quirk list. If there is a known list somewhere that is shorter than that, it may be possible to pare down. Lukas, if you can please look closely at patch 3 of v3.