diff mbox series

PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled

Message ID 20190318143129.6640-1-s.miroshnichenko@yadro.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled | expand

Commit Message

Sergei Miroshnichenko March 18, 2019, 2:31 p.m. UTC 
pcie_init() invokes pcie_disable_notification() for empty slots, which in
turn disables the Command Complete interrupt (unset PCI_EXP_SLTCTL_CCIE)
and right after that waits for Command Complete event (PCI_EXP_SLTSTA_CC).

This results in longer boot time - 4 seconds of delay for each empty slot:

[    4.200325] pciehp 0042:04:08.0:pcie204: Slot #40 AttnBtn+ PwrCtrl+ MRL- AttnInd+ PwrInd+ HotPlug+ Surprise- Interlock- NoCompl- LLActRep+
[    4.200607] pciehp 0042:04:08.0:pcie204: pciehp_get_power_status: SLOTCTRL 80 value read 1c0
[    6.217938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2020 msec ago)
[    6.217966] pciehp 0042:04:08.0:pcie204: pcie_disable_notification: SLOTCTRL 80 write cmd 0
[    6.237938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2040 msec ago)
[    8.257939] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2020 msec ago)
[    8.257974] pciehp 0042:04:08.0:pcie204: pciehp_power_off_slot: SLOTCTRL 80 write cmd 400
[    8.258034] pci_bus 0042:07: dev 00, created physical slot 40
[    8.277941] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2040 msec ago)
[    8.277967] pciehp 0042:04:08.0:pcie204: pcie_enable_notification: SLOTCTRL 80 write cmd 1031

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
Cc: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Lukas Wunner March 18, 2019, 3:13 p.m. UTC | #1 
On Mon, Mar 18, 2019 at 05:31:29PM +0300, Sergey Miroshnichenko wrote:
> pcie_init() invokes pcie_disable_notification() for empty slots, which in
> turn disables the Command Complete interrupt (unset PCI_EXP_SLTCTL_CCIE)
> and right after that waits for Command Complete event (PCI_EXP_SLTSTA_CC).
> 
> This results in longer boot time - 4 seconds of delay for each empty slot:
> 
> [    4.200325] pciehp 0042:04:08.0:pcie204: Slot #40 AttnBtn+ PwrCtrl+ MRL- AttnInd+ PwrInd+ HotPlug+ Surprise- Interlock- NoCompl- LLActRep+
> [    4.200607] pciehp 0042:04:08.0:pcie204: pciehp_get_power_status: SLOTCTRL 80 value read 1c0
> [    6.217938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2020 msec ago)
> [    6.217966] pciehp 0042:04:08.0:pcie204: pcie_disable_notification: SLOTCTRL 80 write cmd 0
> [    6.237938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2040 msec ago)
> [    8.257939] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2020 msec ago)
> [    8.257974] pciehp 0042:04:08.0:pcie204: pciehp_power_off_slot: SLOTCTRL 80 write cmd 400
> [    8.258034] pci_bus 0042:07: dev 00, created physical slot 40
> [    8.277941] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2040 msec ago)
> [    8.277967] pciehp 0042:04:08.0:pcie204: pcie_enable_notification: SLOTCTRL 80 write cmd 1031

Those "Timeout on hotplug command" messages look suspicious.  The port
claims to support Command Completed Events, but doesn't seem to set the
Command Completed bit in the Slot Status register.  Ultimately that's
the reason for the delays you're seeing.

We already have quirks for various Intel and Qualcomm ports with broken
Command Completed support (see at the bottom of pciehp_hpc.c) and we
know that older Thunderbolt chips erroneously claim to support Command
Completed Events but in reality don't support it (see quirk in pcie_init()).
Maybe the hardware you're dealing with needs such a quirk as well?

Thanks,

Lukas

> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 6a2365cd794e..eec5d6107360 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -118,8 +118,10 @@ static void pcie_wait_cmd(struct controller *ctrl)
>  	if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE &&
>  	    ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
>  		rc = wait_event_timeout(ctrl->queue, !ctrl->cmd_busy, timeout);
> -	else
> +	else if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
>  		rc = pcie_poll_cmd(ctrl, jiffies_to_msecs(timeout));
> +	else
> +		return;
>  
>  	if (!rc)
>  		ctrl_info(ctrl, "Timeout on hotplug command %#06x (issued %u msec ago)\n",
> -- 
> 2.20.1
>
Sergei Miroshnichenko March 18, 2019, 3:42 p.m. UTC | #2 
Hello Lukas,

On 3/18/19 6:13 PM, Lukas Wunner wrote:
> On Mon, Mar 18, 2019 at 05:31:29PM +0300, Sergey Miroshnichenko wrote:
>> pcie_init() invokes pcie_disable_notification() for empty slots, which in
>> turn disables the Command Complete interrupt (unset PCI_EXP_SLTCTL_CCIE)
>> and right after that waits for Command Complete event (PCI_EXP_SLTSTA_CC).
>>
>> This results in longer boot time - 4 seconds of delay for each empty slot:
>>
>> [    4.200325] pciehp 0042:04:08.0:pcie204: Slot #40 AttnBtn+ PwrCtrl+ MRL- AttnInd+ PwrInd+ HotPlug+ Surprise- Interlock- NoCompl- LLActRep+
>> [    4.200607] pciehp 0042:04:08.0:pcie204: pciehp_get_power_status: SLOTCTRL 80 value read 1c0
>> [    6.217938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2020 msec ago)
>> [    6.217966] pciehp 0042:04:08.0:pcie204: pcie_disable_notification: SLOTCTRL 80 write cmd 0
>> [    6.237938] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x01c0 (issued 2040 msec ago)
>> [    8.257939] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2020 msec ago)
>> [    8.257974] pciehp 0042:04:08.0:pcie204: pciehp_power_off_slot: SLOTCTRL 80 write cmd 400
>> [    8.258034] pci_bus 0042:07: dev 00, created physical slot 40
>> [    8.277941] pciehp 0042:04:08.0:pcie204: Timeout on hotplug command 0x05c0 (issued 2040 msec ago)
>> [    8.277967] pciehp 0042:04:08.0:pcie204: pcie_enable_notification: SLOTCTRL 80 write cmd 1031
> 
> Those "Timeout on hotplug command" messages look suspicious.  The port
> claims to support Command Completed Events, but doesn't seem to set the
> Command Completed bit in the Slot Status register.  Ultimately that's
> the reason for the delays you're seeing.
> 
> We already have quirks for various Intel and Qualcomm ports with broken
> Command Completed support (see at the bottom of pciehp_hpc.c) and we
> know that older Thunderbolt chips erroneously claim to support Command
> Completed Events but in reality don't support it (see quirk in pcie_init()).
> Maybe the hardware you're dealing with needs such a quirk as well?
Thanks, I'll rework this as a quirk for PEX 8796. The "Command
Completed" seems to be half-broken there: the SLTSTA_CC bit is always
zero without SLTCTL_CCIE, but works just fine with SLTCTL_CCIE, so the
pciehp_isr() wakes up the pcie_wait_cmd().

Best regards,
Serge

> 
> Thanks,
> 
> Lukas
> 
>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> Cc: Lukas Wunner <lukas@wunner.de>
>> ---
>>  drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
>> index 6a2365cd794e..eec5d6107360 100644
>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -118,8 +118,10 @@ static void pcie_wait_cmd(struct controller *ctrl)
>>  	if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE &&
>>  	    ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
>>  		rc = wait_event_timeout(ctrl->queue, !ctrl->cmd_busy, timeout);
>> -	else
>> +	else if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
>>  		rc = pcie_poll_cmd(ctrl, jiffies_to_msecs(timeout));
>> +	else
>> +		return;
>>  
>>  	if (!rc)
>>  		ctrl_info(ctrl, "Timeout on hotplug command %#06x (issued %u msec ago)\n",
>> -- 
>> 2.20.1
>>
diff mbox series

Patch

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 6a2365cd794e..eec5d6107360 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -118,8 +118,10 @@  static void pcie_wait_cmd(struct controller *ctrl)
 	if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE &&
 	    ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
 		rc = wait_event_timeout(ctrl->queue, !ctrl->cmd_busy, timeout);
-	else
+	else if (ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE)
 		rc = pcie_poll_cmd(ctrl, jiffies_to_msecs(timeout));
+	else
+		return;
 
 	if (!rc)
 		ctrl_info(ctrl, "Timeout on hotplug command %#06x (issued %u msec ago)\n",