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