On Tue, 21 Nov 2023, Shuai Xue wrote: > The clear and set pattern is commonly used for accessing PCI config, > move the helper pci_clear_and_set_dword() from aspm.c into PCI header. > In addition, rename to pci_clear_and_set_config_dword() to retain the > "config" information and match the other accessors. > > No functional change intended. > > Signed-off-by: Shuai Xue <xueshuai@xxxxxxxxxxxxxxxxx> > Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Tested-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx> > --- > drivers/pci/access.c | 12 ++++++++ > drivers/pci/pcie/aspm.c | 65 +++++++++++++++++++---------------------- > include/linux/pci.h | 2 ++ > 3 files changed, 44 insertions(+), 35 deletions(-) > > diff --git a/drivers/pci/access.c b/drivers/pci/access.c > index 6554a2e89d36..6449056b57dd 100644 > --- a/drivers/pci/access.c > +++ b/drivers/pci/access.c > @@ -598,3 +598,15 @@ int pci_write_config_dword(const struct pci_dev *dev, int where, > return pci_bus_write_config_dword(dev->bus, dev->devfn, where, val); > } > EXPORT_SYMBOL(pci_write_config_dword); > + > +void pci_clear_and_set_config_dword(const struct pci_dev *dev, int pos, > + u32 clear, u32 set) Just noting that annoyingly the ordering within the name is inconsistent between: pci_clear_and_set_config_dword() and pcie_capability_clear_and_set_dword() And if changed, it would be again annoyingly inconsistent with pci_read/write_config_*(), oh well... And renaming pci_read/write_config_* into the hierarchical pci_config_read/write_*() form for would touch only ~6k lines... ;-D > + pci_clear_and_set_config_dword(child, > + child->l1ss + PCI_L1SS_CTL1, 0, > + cl1_2_enables); Adding clear and set only variants into the header like there are for pcie_capability_*() would remove the need to add those 0 parameters. IMO, it improves code readability considerably. -- i.
