On Wed, Jan 17, 2018 at 04:48:39PM +0100, Lukas Wunner wrote:
> Certain Thunderbolt 1 controllers claim to support Command Completed events
> (value of 0b in the No Command Completed Support field of the Slot
> Capabilities register) but in reality neither set the Command Completed bit
> in the Slot Status register nor signal a Command Completed interrupt:
>
> 8086:1513 CV82524 [Light Ridge 4C 2010]
> 8086:151a DSL2310 [Eagle Ridge 2C 2011]
> 8086:151b CVL2510 [Light Peak 2C 2010]
> 8086:1547 DSL3510 [Cactus Ridge 4C 2012]
> 8086:1548 DSL3310 [Cactus Ridge 2C 2012]
> 8086:1549 DSL2210 [Port Ridge 1C 2011]
>
> All newer chips (Redwood Ridge and onwards) set the bit correctly to 1b.
>
> The user-visible impact is that after unplugging such a device, 2 seconds
> elapse until pciehp is unbound. That's because on ->remove,
> pcie_write_cmd() is called via pcie_disable_notification() and every call
> to pcie_write_cmd() takes 2 seconds (1 second for each invocation of
> pcie_wait_cmd()):
>
> [ 337.942727] pciehp 0000:0a:00.0:pcie204: Timeout on hotplug command 0x1038 (issued 21176 msec ago)
> [ 340.014735] pciehp 0000:0a:00.0:pcie204: Timeout on hotplug command 0x0000 (issued 2072 msec ago)
>
> That by itself has always been unpleasant, but the situation has become
> worse with commit cc27b735ad3a ("PCI/portdrv: Turn off PCIe services
> during shutdown"): Now pciehp is unbound on ->shutdown. Because
> Thunderbolt controllers typically have 4 hotplug ports, every reboot and
> shutdown is now delayed by 8 seconds, plus another 2 seconds for every
> attached Thunderbolt 1 device.
>
> Thunderbolt hotplug slots are not physical slots that one inserts cards
> into, but rather logical hotplug slots implemented in silicon. Devices
> appear beyond those logical slots once a PCI tunnel is established on top
> of the Thunderbolt Converged I/O switch. One would expect commands written
> to the Slot Control register to be executed immediately by the silicon,
> so for simplicity we always assume NoCompl+ for Thunderbolt ports.
>
> Fixes: cc27b735ad3a ("PCI/portdrv: Turn off PCIe services during shutdown")
> Tested-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v4.12+
> Cc: Sinan Kaya <okaya@xxxxxxxxxxxxxx>
> Cc: Yehezkel Bernat <yehezkel.bernat@xxxxxxxxx>
> Cc: Michael Jamet <michael.jamet@xxxxxxxxx>
> Cc: Andreas Noever <andreas.noever@xxxxxxxxx>
Applied to pci/hotplug for v4.16, thanks!
> ---
> Changes since v1:
> - Rephrase code comment for clarity. (Bjorn)
> - Amend commit message with info provided by Mika.
> - Adhere to "Make Bjorn's life easier" guide.
>
> drivers/pci/hotplug/pciehp_hpc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 7bab0606f1a9..a89d8b990228 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -848,6 +848,13 @@ struct controller *pcie_init(struct pcie_device *dev)
> if (pdev->hotplug_user_indicators)
> slot_cap &= ~(PCI_EXP_SLTCAP_AIP | PCI_EXP_SLTCAP_PIP);
>
> + /*
> + * We assume no Thunderbolt controllers support Command Complete events,
> + * but some controllers falsely claim they do.
> + */
> + if (pdev->is_thunderbolt)
> + slot_cap |= PCI_EXP_SLTCAP_NCCS;
> +
> ctrl->slot_cap = slot_cap;
> mutex_init(&ctrl->ctrl_lock);
> init_waitqueue_head(&ctrl->queue);
> --
> 2.15.1
>
