Re: [PATCH v2] PCI/ASPM: Disable L1 before disabling L1ss

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Oct 03, 2024 at 06:55:03PM +0530, Ajay Agarwal wrote:
> The current sequence in the driver for L1ss update is as follows.
> 
> Disable L1ss
> Disable L1
> Enable L1ss as required
> Enable L1 if required
> 
> With this sequence, a bus hang is observed during the L1ss
> disable sequence when the RC CPU attempts to clear the RC L1ss
> register after clearing the EP L1ss register. It looks like the
> RC attempts to enter L1ss again and at the same time, access to
> RC L1ss register fails because aux clk is still not active.
>
> PCIe spec r6.2, section 5.5.4, recommends that setting either
> or both of the enable bits for ASPM L1 PM Substates must be done
> while ASPM L1 is disabled. My interpretation here is that
> clearing L1ss should also be done when L1 is disabled. Thereby,
> change the sequence as follows.
> 
> Disable L1
> Disable L1ss
> Enable L1ss as required
> Enable L1 if required

I think we also write the L1.2 enable bits in PCI_L1SS_CTL1 in
aspm_calc_l12_info() when ASPM L1 may be enabled:

  pcie_aspm_init_link_state
    pcie_aspm_cap_init
      pcie_capability_read_word(PCI_EXP_LNKCTL)
      aspm_l1ss_init
        aspm_calc_l12_info
          pci_clear_and_set_config_dword(PCI_L1SS_CTL1, PCI_L1SS_CTL1_L1_2_MASK)

That looks like another path where we should make a similar change.
What do you think?

Bjorn