| Message ID | 20220126071853.1940111-2-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/2] PCI/AER: Disable AER when link is in L2/L3 ready, L2 and L3 state | expand |
Hi, On Wed, Jan 26, 2022 at 03:18:52PM +0800, Kai-Heng Feng wrote: > Since TLP and DLLP transmission is disabled for a Link in L2/L3 Ready, > L2 and L3, and DPC depends on AER, so also disable DPC here. Here too I think it is good to mention that the DPC "service" never implemented the PM hooks in the first place > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> One minor comment below, but other than that looks good, Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/pci/pcie/dpc.c | 61 +++++++++++++++++++++++++++++++----------- > 1 file changed, 45 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c > index 3e9afee02e8d1..9585c10b7c577 100644 > --- a/drivers/pci/pcie/dpc.c > +++ b/drivers/pci/pcie/dpc.c > @@ -343,13 +343,34 @@ void pci_dpc_init(struct pci_dev *pdev) > } > } > > +static void dpc_enable(struct pcie_device *dev) > +{ > + struct pci_dev *pdev = dev->port; > + u16 ctl; > + > + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > + Drop the empty line here. > + ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; > + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > +} > + > +static void dpc_disable(struct pcie_device *dev) > +{ > + struct pci_dev *pdev = dev->port; > + u16 ctl; > + > + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > + ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); > + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > +}
On Wed, Jan 26, 2022 at 7:10 PM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Hi, > > On Wed, Jan 26, 2022 at 03:18:52PM +0800, Kai-Heng Feng wrote: > > Since TLP and DLLP transmission is disabled for a Link in L2/L3 Ready, > > L2 and L3, and DPC depends on AER, so also disable DPC here. > > Here too I think it is good to mention that the DPC "service" never > implemented the PM hooks in the first place Will amend the commit message a bit. > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > One minor comment below, but other than that looks good, > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > --- > > drivers/pci/pcie/dpc.c | 61 +++++++++++++++++++++++++++++++----------- > > 1 file changed, 45 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c > > index 3e9afee02e8d1..9585c10b7c577 100644 > > --- a/drivers/pci/pcie/dpc.c > > +++ b/drivers/pci/pcie/dpc.c > > @@ -343,13 +343,34 @@ void pci_dpc_init(struct pci_dev *pdev) > > } > > } > > > > +static void dpc_enable(struct pcie_device *dev) > > +{ > > + struct pci_dev *pdev = dev->port; > > + u16 ctl; > > + > > + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > > + > > Drop the empty line here. OK, will do. Kai-Heng > > > + ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; > > + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > > +} > > + > > +static void dpc_disable(struct pcie_device *dev) > > +{ > > + struct pci_dev *pdev = dev->port; > > + u16 ctl; > > + > > + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > > + ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); > > + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > > +}
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 3e9afee02e8d1..9585c10b7c577 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -343,13 +343,34 @@ void pci_dpc_init(struct pci_dev *pdev) } } +static void dpc_enable(struct pcie_device *dev) +{ + struct pci_dev *pdev = dev->port; + u16 ctl; + + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); + + ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); +} + +static void dpc_disable(struct pcie_device *dev) +{ + struct pci_dev *pdev = dev->port; + u16 ctl; + + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); + ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); +} + #define FLAG(x, y) (((x) & (y)) ? '+' : '-') static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; struct device *device = &dev->device; int status; - u16 ctl, cap; + u16 cap; if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) return -ENOTSUPP; @@ -364,10 +385,7 @@ static int dpc_probe(struct pcie_device *dev) } pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CAP, &cap); - pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); - - ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; - pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); + dpc_enable(dev); pci_info(pdev, "enabled with IRQ %d\n", dev->irq); pci_info(pdev, "error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n", @@ -380,22 +398,33 @@ static int dpc_probe(struct pcie_device *dev) return status; } -static void dpc_remove(struct pcie_device *dev) +static int dpc_suspend(struct pcie_device *dev) { - struct pci_dev *pdev = dev->port; - u16 ctl; + dpc_disable(dev); + return 0; +} - pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); - ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); - pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); +static int dpc_resume(struct pcie_device *dev) +{ + dpc_enable(dev); + return 0; +} + +static void dpc_remove(struct pcie_device *dev) +{ + dpc_disable(dev); } static struct pcie_port_service_driver dpcdriver = { - .name = "dpc", - .port_type = PCIE_ANY_PORT, - .service = PCIE_PORT_SERVICE_DPC, - .probe = dpc_probe, - .remove = dpc_remove, + .name = "dpc", + .port_type = PCIE_ANY_PORT, + .service = PCIE_PORT_SERVICE_DPC, + .probe = dpc_probe, + .suspend = dpc_suspend, + .resume = dpc_resume, + .runtime_suspend = dpc_suspend, + .runtime_resume = dpc_resume, + .remove = dpc_remove, }; int __init pcie_dpc_init(void)
Since TLP and DLLP transmission is disabled for a Link in L2/L3 Ready, L2 and L3, and DPC depends on AER, so also disable DPC here. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/pci/pcie/dpc.c | 61 +++++++++++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 16 deletions(-)