Re: [PATCH v4 04/11] PCI/ASPM: Use RMW accessors for changing LNKCTL

On Thu, Jul 13, 2023 at 03:44:58PM +0300, Ilpo Järvinen wrote:

Don't assume that the device is fully under the control of ASPM and use RMW capability accessors which do proper locking to avoid losing concurrent updates to the register values.

If configuration fails in pcie_aspm_configure_common_clock(), the function attempts to restore the old PCI_EXP_LNKCTL_CCC settings. Store only the old PCI_EXP_LNKCTL_CCC bit for the relevant devices rather than the content of the whole LNKCTL registers. It aligns better with how pcie_lnkctl_clear_and_set() expects its parameter and makes the code more obvious to understand.

...

int same_clock = 1;

• u16 reg16, parent_reg, child_reg[8];

• u16 reg16, parent_old_ccc, child_old_ccc[8];

u16 ccc;

struct pci_dev *child, *parent = link->pdev; struct pci_bus *linkbus = parent->subordinate;

...

ccc = same_clock ? PCI_EXP_LNKCTL_CCC : 0);

/* Configure downstream component, all functions */ list_for_each_entry(child, &linkbus->devices, bus_list) { pcie_capability_read_word(child, PCI_EXP_LNKCTL, &reg16);

• child_reg[PCI_FUNC(child->devfn)] = reg16;
  

• if (same_clock)
  

• 	reg16 |= PCI_EXP_LNKCTL_CCC;
  

• else
  

• 	reg16 &= ~PCI_EXP_LNKCTL_CCC;
  

• pcie_capability_write_word(child, PCI_EXP_LNKCTL, reg16);
  

• child_old_ccc[PCI_FUNC(child->devfn)] = reg16 & PCI_EXP_LNKCTL_CCC;
  

• pcie_capability_clear_and_set_word(child, PCI_EXP_LNKCTL,
  

• 				   PCI_EXP_LNKCTL_CCC,
  

• 				   same_clock ? PCI_EXP_LNKCTL_CCC : 0);
  

pcie_capability_clear_and_set_word(child, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_CCC, ccc);

}

...

• pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL,

• 			   PCI_EXP_LNKCTL_CCC,
  

• 			   same_clock ? PCI_EXP_LNKCTL_CCC : 0);
  

pcie_capability_clear_and_set_word(parent, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_CCC, ccc);

?