Re: [PATCH v5 3/9] PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 01, 2018 at 04:52:11PM -0500, Bjorn Helgaas wrote:
> On Mon, Apr 16, 2018 at 01:34:47PM +0300, Mika Westerberg wrote:
> > After suspend/resume cycle the Presence Detect or Data Link Layer Status
> > Changed bits might be set and if we don't clear them those events will
> > not fire anymore and nothing happens for instance when a device is now
> > hot-unplugged.
> > 
> > Fix this by clearing those bits in a newly introduced function
> > pcie_reenable_notification(). This should be fine because immediately
> > after we check if the adapter is still present by reading directly from
> > the status register.
> 
> I want to understand why we need to handle this differently between
> the boot-time probe path and the resume path.
> 
> This patch clears PCI_EXP_SLTSTA_PDC and PCI_EXP_SLTSTA_DLLSC in the
> resume path:
> 
>   pciehp_resume
>     pcie_reenable_notification
>       # clear PDC DLLSC
>       pcie_enable_notification
>         # set DLLSCE ABPE/PDCE HPIE CCIE
>     pciehp_get_adapter_status
>       # read PCI_EXP_SLTSTA

The Slot Control register is already written during the ->resume_noirq
phase via:

  pci_pm_resume_noirq
    pci_pm_default_resume_early
    pci_restore_state
      pci_restore_pcie_state

>From that point on, slot events are enabled, but they're not received
because interrupts are disabled until the ->resume_early phase commences.

My guess is that if the hotplug port uses edge-triggered MSI/MSI-X,
an interrupt may have already been sent during ->resume_noirq, but
was lost because interrupts were disabled.  Clearing the Slot Status
register thus acknowledges any lost interrupts.

If this theory is correct, it should probably be included in the
commit message and/or a code comment so that it's clear what's
going on.

Thanks,

Lukas



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux