linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled
@ 2019-03-18 14:31 Sergey Miroshnichenko
  2019-03-18 15:13 ` Lukas Wunner
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-18 14:31 UTC (permalink / raw)
  To: linux-pci; +Cc: linux, Sergey Miroshnichenko, Lukas Wunner

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(-)

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled
  2019-03-18 14:31 [PATCH] PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled Sergey Miroshnichenko
@ 2019-03-18 15:13 ` Lukas Wunner
  2019-03-18 15:42   ` Sergey Miroshnichenko
  0 siblings, 1 reply; 4+ messages in thread
From: Lukas Wunner @ 2019-03-18 15:13 UTC (permalink / raw)
  To: Sergey Miroshnichenko; +Cc: linux-pci, linux

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled
  2019-03-18 15:13 ` Lukas Wunner
@ 2019-03-18 15:42   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 4+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-18 15:42 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: linux-pci, linux


[-- Attachment #1.1: Type: text/plain, Size: 3399 bytes --]

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
>>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled
@ 2019-10-24 17:27 Sergey Miroshnichenko
  0 siblings, 0 replies; 4+ messages in thread
From: Sergey Miroshnichenko @ 2019-10-24 17:27 UTC (permalink / raw)
  To: linux-pci; +Cc: Bjorn Helgaas, linux, Sergey Miroshnichenko, Lukas Wunner

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).

Add a quirk for the PLX 8796 switch, which after disabling CCIE also stops
setting the PCI_EXP_SLTSTA_CC bit.

This quirk fixes the 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

Cc: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 drivers/pci/hotplug/pciehp_hpc.c | 18 ++++++++++++++++++
 include/linux/pci.h              |  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 1a522c1c4177..971258576be1 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -173,6 +173,10 @@ static void pcie_do_write_cmd(struct controller *ctrl, u16 cmd,
 	    (slot_ctrl_orig & CC_ERRATUM_MASK) == (slot_ctrl & CC_ERRATUM_MASK))
 		ctrl->cmd_busy = 0;
 
+	if (pdev->no_cmd_compl_wo_ccie &&
+	    !(ctrl->slot_ctrl & PCI_EXP_SLTCTL_CCIE))
+		ctrl->cmd_busy = 0;
+
 	/*
 	 * Optionally wait for the hardware to be ready for a new command,
 	 * indicating completion of the above issued command.
@@ -901,6 +905,18 @@ static void quirk_cmd_compl(struct pci_dev *pdev)
 			pdev->broken_cmd_compl = 1;
 	}
 }
+
+static void quirk_no_cmd_compl_wo_ccie(struct pci_dev *pdev)
+{
+	u32 slot_cap;
+
+	if (pci_is_pcie(pdev)) {
+		pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &slot_cap);
+		if (slot_cap & PCI_EXP_SLTCAP_HPC &&
+		    !(slot_cap & PCI_EXP_SLTCAP_NCCS))
+			pdev->no_cmd_compl_wo_ccie = 1;
+	}
+}
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
 			      PCI_CLASS_BRIDGE_PCI, 8, quirk_cmd_compl);
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_QCOM, 0x0400,
@@ -909,3 +925,5 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_QCOM, 0x0401,
 			      PCI_CLASS_BRIDGE_PCI, 8, quirk_cmd_compl);
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_HXT, 0x0401,
 			      PCI_CLASS_BRIDGE_PCI, 8, quirk_cmd_compl);
+DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_PLX, 0x8796,
+			      PCI_CLASS_BRIDGE_PCI, 8, quirk_no_cmd_compl_wo_ccie);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b5821134bdae..e61b92a15b19 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -437,6 +437,7 @@ struct pci_dev {
 
 #ifdef CONFIG_HOTPLUG_PCI_PCIE
 	unsigned int	broken_cmd_compl:1;	/* No compl for some cmds */
+	unsigned int	no_cmd_compl_wo_ccie:1;	/* No compl if CCIE is unset */
 #endif
 #ifdef CONFIG_PCIE_PTM
 	unsigned int	ptm_root:1;
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-10-24 17:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-18 14:31 [PATCH] PCI: pciehp: Don't wait for Command Completed if this interrupt is disabled Sergey Miroshnichenko
2019-03-18 15:13 ` Lukas Wunner
2019-03-18 15:42   ` Sergey Miroshnichenko
2019-10-24 17:27 Sergey Miroshnichenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).