diff mbox series

[02/11] PCI: pciehp: Enable Command Completed Interrupt only if supported

Message ID 20220818135140.5996-3-kabel@kernel.org (mailing list archive)
State Superseded
Delegated to: Lorenzo Pieralisi
Headers show
Series PCI: aardvark controller changes BATCH 6 | expand

Commit Message

Marek Behún Aug. 18, 2022, 1:51 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.

We already check whether No Command Completed Support bit is set in
pcie_wait_cmd(), and do not wait in this case.

Let's not enable this Command Completed Interrupt at all if NCCS is set,
so that when users dump configuration space from userspace, the dump
does not confuse them by saying that Command Completed Interrupt is not
supported, but it is enabled.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
---
Changes since batch 5:
- changed commit message, previously we wrote that the change is needed
  to fix a bug where kernel was waiting for an event which did not
  come. This turns out to be false. See
  https://lore.kernel.org/linux-pci/20220818142243.4c046c59@dellmb/T/#u
---
 drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Lukas Wunner Aug. 21, 2022, 3:20 p.m. UTC | #1
On Thu, Aug 18, 2022 at 03:51:31PM +0200, Marek Behún wrote:
> 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.
> 
> We already check whether No Command Completed Support bit is set in
> pcie_wait_cmd(), and do not wait in this case.
> 
> Let's not enable this Command Completed Interrupt at all if NCCS is set,
> so that when users dump configuration space from userspace, the dump
> does not confuse them by saying that Command Completed Interrupt is not
> supported, but it is enabled.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>

Reviewed-by: Lukas Wunner <lukas@wunner.de>

I note however that this change isn't really necessary because CCIE
"must be hardwired to 0b" "If Command Completed notification is not
supported" per PCIe r6.0 sec 7.5.3.10.  It's purely a cosmetic issue.

Thanks,

Lukas
Lorenzo Pieralisi Sept. 28, 2022, 8:39 a.m. UTC | #2
On Thu, Aug 18, 2022 at 03:51:31PM +0200, Marek Behún wrote:
> 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.
> 
> We already check whether No Command Completed Support bit is set in
> pcie_wait_cmd(), and do not wait in this case.
> 
> Let's not enable this Command Completed Interrupt at all if NCCS is set,
> so that when users dump configuration space from userspace, the dump
> does not confuse them by saying that Command Completed Interrupt is not
> supported, but it is enabled.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
> Changes since batch 5:
> - changed commit message, previously we wrote that the change is needed
>   to fix a bug where kernel was waiting for an event which did not
>   come. This turns out to be false. See
>   https://lore.kernel.org/linux-pci/20220818142243.4c046c59@dellmb/T/#u
> ---
>  drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Hi Bjorn,

this patch is mixed with aardvark specific changes,
please let me know if it is fine for you to merge it.

Thanks,
Lorenzo

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