diff mbox series

[07/18] PCI: pciehp: Enable Command Completed Interrupt only if supported

Message ID 20220220193346.23789-8-kabel@kernel.org (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show
Series PCI: aardvark controller changes BATCH 5 | expand

Commit Message

Marek Behún Feb. 20, 2022, 7:33 p.m. UTC 
From: Pali Rohár <pali@kernel.org>

The No Command Completed Support bit in the Slot Capabilities register
indicates whether Command Completed Interrupt Enable is unsupported.

Enable this interrupt only in the case it is supported.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Lukas Wunner May 9, 2022, 4:01 a.m. UTC | #1 
On Sun, Feb 20, 2022 at 08:33:35PM +0100, Marek Behún wrote:
> The No Command Completed Support bit in the Slot Capabilities register
> indicates whether Command Completed Interrupt Enable is unsupported.
> 
> Enable this interrupt only in the case it is supported.
[...]
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -817,7 +817,9 @@ static void pcie_enable_notification(struct controller *ctrl)
>  	else
>  		cmd |= PCI_EXP_SLTCTL_PDCE;
>  	if (!pciehp_poll_mode)
> -		cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
> +		cmd |= PCI_EXP_SLTCTL_HPIE;
> +	if (!pciehp_poll_mode && !NO_CMD_CMPL(ctrl))
> +		cmd |= PCI_EXP_SLTCTL_CCIE;

Looks okay to me in principle, I'm just wondering why this change is
necessary, i.e. what issue are you seeing without it?

Thanks,

Lukas
Pali Rohár May 13, 2022, 4:59 p.m. UTC | #2 
On Monday 09 May 2022 06:01:39 Lukas Wunner wrote:
> On Sun, Feb 20, 2022 at 08:33:35PM +0100, Marek Behún wrote:
> > The No Command Completed Support bit in the Slot Capabilities register
> > indicates whether Command Completed Interrupt Enable is unsupported.
> > 
> > Enable this interrupt only in the case it is supported.
> [...]
> > --- a/drivers/pci/hotplug/pciehp_hpc.c
> > +++ b/drivers/pci/hotplug/pciehp_hpc.c
> > @@ -817,7 +817,9 @@ static void pcie_enable_notification(struct controller *ctrl)
> >  	else
> >  		cmd |= PCI_EXP_SLTCTL_PDCE;
> >  	if (!pciehp_poll_mode)
> > -		cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
> > +		cmd |= PCI_EXP_SLTCTL_HPIE;
> > +	if (!pciehp_poll_mode && !NO_CMD_CMPL(ctrl))
> > +		cmd |= PCI_EXP_SLTCTL_CCIE;
> 
> Looks okay to me in principle, I'm just wondering why this change is
> necessary, i.e. what issue are you seeing without it?
> 
> Thanks,
> 
> Lukas

This is that case which I described in previous email. Kernel was
waiting for completion, but if (emulated) Root Port does not support
completion event then there were timeouts.
diff mbox series

Patch

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 373bb396fe22..838eb6cc3ec7 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -817,7 +817,9 @@  static void pcie_enable_notification(struct controller *ctrl)
 	else
 		cmd |= PCI_EXP_SLTCTL_PDCE;
 	if (!pciehp_poll_mode)
-		cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
+		cmd |= PCI_EXP_SLTCTL_HPIE;
+	if (!pciehp_poll_mode && !NO_CMD_CMPL(ctrl))
+		cmd |= PCI_EXP_SLTCTL_CCIE;
 
 	mask = (PCI_EXP_SLTCTL_PDCE | PCI_EXP_SLTCTL_ABPE |
 		PCI_EXP_SLTCTL_PFDE |