[Public]

> -----Original Message-----
> From: Mika Westerberg <mika.westerb...@linux.intel.com>
> Sent: Friday, February 11, 2022 04:24
> To: Limonciello, Mario <mario.limoncie...@amd.com>
> Cc: Bjorn Helgaas <bhelg...@google.com>; Andreas Noever
> <andreas.noe...@gmail.com>; open list:PCI SUBSYSTEM <linux-
> p...@vger.kernel.org>; open list:THUNDERBOLT DRIVER <linux-
> u...@vger.kernel.org>; open list:RADEON and AMDGPU DRM DRIVERS <amd-
> g...@lists.freedesktop.org>; open list:DRM DRIVERS <dri-
> de...@lists.freedesktop.org>; open list:DRM DRIVER FOR NVIDIA
> GEFORCE/QUADRO GPUS <nouv...@lists.freedesktop.org>; open list:X86
> PLATFORM DRIVERS <platform-driver-...@vger.kernel.org>; Michael Jamet
> <michael.ja...@intel.com>; Yehezkel Bernat <yehezkel...@gmail.com>;
> Lukas Wunner <lu...@wunner.de>; Deucher, Alexander
> <alexander.deuc...@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.limoncie...@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.

Reply via email to