On Fri, Oct 13, 2023 at 02:20:04PM +0300, Ilpo Järvinen wrote: > Use defines instead of literals and replace custom masking and shifts > with FIELD_GET() where it makes sense. > ... > 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_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; > + ctl &= ~PCI_EXP_DPC_CTL_EN_NONFATAL; This has been a little obtuse from the beginning. The original clears bits 0, 1, 3, then sets bits 0 and 3. The new code sets bits 0, 3, then clears bit 1. These are equivalent, but it's definitely some work to verify it. I think the point is to enable DPC on ERR_FATAL (but not ERR_NONFATAL) and to enable DPC interrupts. What about something like this? #define PCI_EXP_DPC_CTL_EN_MASK 0x0003 ctl &= ~PCI_EXP_DPC_CTL_EN_MASK; 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); > - pci_info(pdev, "enabled with IRQ %d\n", dev->irq); > > + 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", > cap & PCI_EXP_DPC_IRQ, FLAG(cap, PCI_EXP_DPC_CAP_RP_EXT), > FLAG(cap, PCI_EXP_DPC_CAP_POISONED_TLP), > -- > 2.30.2 >
