Messages in this thread | | | Date | Tue, 20 Aug 2019 09:17:17 -0500 | From | Bjorn Helgaas <> | Subject | Re: [PATCH] PCI: Add sysfs attribute for disabling PCIe link to downstream component |
| |
On Tue, Aug 20, 2019 at 12:58:20PM +0300, Mika Westerberg wrote: > On Mon, Aug 19, 2019 at 06:52:45PM -0500, Bjorn Helgaas wrote: > > > Right, it looks like we need some sort of flag there anyway. > > > > Does this mean you're looking at getting rid of "has_secondary_link", > > you think it's impossible, or you think it's not worth trying? > > I was of thinking that we need some flag anyway for the downstream port > (such as has_secondary_link) that tells us the which side of the port > the link is. > > > I'm pretty sure we could get rid of it by looking upstream, but I > > haven't actually tried it. > > So if we are downstream port, look at the parent and if it is also > downstream port (or root port) we change the type to upstream port > accordingly? That might work.
If we see a type of PCI_EXP_TYPE_ROOT_PORT or PCI_EXP_TYPE_PCIE_BRIDGE, I think we have to assume that's accurate (which we already do today -- for those types, we assume the device has a secondary link).
For a device that claims to be PCI_EXP_TYPE_DOWNSTREAM, if a parent device exists and is a Downstream Port (Root Port, Switch Downstream Port, and I suppose a PCI-to-PCIe bridge (this is basically pcie_downstream_port()), this device must actually be acting as a PCI_EXP_TYPE_UPSTREAM device.
If a device claiming to be PCI_EXP_TYPE_UPSTREAM has a parent that is PCI_EXP_TYPE_UPSTREAM, this device must actually be a PCI_EXP_TYPE_DOWNSTREAM port.
For PCI_EXP_TYPE_DOWNSTREAM and PCI_EXP_TYPE_UPSTREAM devices that don't have parents, we just have to assume they advertise the correct type (as we do today). There are sparc and virtualization configs like this.
> Another option may be to just add a quirk for these ports.
I don't really like the quirk approach because then we have to rely on user reports of something being broken.
> Only concern for both is that we have functions that rely on the type > such as pcie_capability_read_word() so if we change the type do we end > up breaking something? I did not check too closely, though.
I don't think we'll break anything that's not already broken because the type will reflect exactly what has_secondary_link now tells us. In fact, we might *fix* some things, e.g., pcie_capability_read_word() should work better if we fix the type that pcie_downstream_port() checks.
> I'm willing to cook a patch that fixes this once we have some consensus > what it should do ;-)
| |