Re: [PATCH 1/4] PCI / ACPI: Identify external PCI devices

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Mon, Nov 12, 2018 at 07:02:03PM +0100, Lukas Wunner wrote:
> On Mon, Nov 12, 2018 at 07:06:25PM +0300, Mika Westerberg wrote:
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1378,6 +1378,27 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
> >  	}
> >  }
> >  
> > +static void set_pcie_external(struct pci_dev *dev)
> > +{
> > +	struct pci_dev *parent;
> > +
> > +	/*
> > +	 * Walk up the device hierarchy and check for any upstream
> > +	 * bridge that has is_external_facing set to true. This means
> > +	 * the hierarchy is below PCIe port that is exposed externally
> > +	 * (such as Thunderbolt).
> > +	 */
> > +	parent = pci_upstream_bridge(dev);
> > +	while (parent) {
> > +		if (parent->is_external) {
> > +			dev->is_external = true;
> > +			break;
> > +		}
> > +
> > +		parent = pci_upstream_bridge(parent);
> > +	}
> > +}
> > +
> 
> This looks pretty much like a duplication of the is_thunderbolt bit
> in struct pci_dev and the pci_is_thunderbolt_attached() helper.
> 
> Why constrain the functionality to ports with the _DSD property
> instead of making it available for *any* Thunderbolt port?

I assume it is because this is just not needed for Thuderbolt ports but
rather for all PCIe devices that are "external" (whatever that is
supposed to mean), ie it is valid also for PCIe slots.

To be frank the concept (and Microsoft _DSD bindings) seems a bit vague
and not thoroughly defined and I would question its detection at
PCI/ACPI core level, I would hope this can be clarified at ACPI
specification level, at least.

Thanks,
Lorenzo