On Thu, Oct 1, 2020 at 5:02 PM Krzysztof Wilczyński <kw@xxxxxxxxx> wrote: > > Unify ECAM-related constants into a single set of standard constants > defining memory address shift values for the byte-level address that can > be used when accessing the PCI Express Configuration Space, and then > move native PCI Express controller drivers to use newly introduced > definitions retiring any driver-specific ones. > > The ECAM ("Enhanced Configuration Access Mechanism") is defined by the > PCI Express specification (see PCI Express Base Specification, Revision > 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should > implement it the same way. Most of the native PCI Express controller > drivers define their ECAM-related constants, many of these could be > shared, or use open-coded values when setting the .bus_shift field of > the struct pci_ecam_ops. > > All of the newly added constants should remove ambiguity and reduce the > number of open-coded values, and also correlate more strongly with the > descriptions in the aforementioned specification (see Table 7-1 > "Enhanced Configuration Address Mapping", p. 677). > > There is no change to functionality. > > Suggested-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Signed-off-by: Krzysztof Wilczyński <kw@xxxxxxxxx> > --- > Changes in v3: > Updated commit message wording. > Updated regarding custom ECAM bus shift values and concerning PCI base > configuration space access for Type 1 access. > Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf() > and removed the "busdev" variable. > Removed surplus "relbus" variable from nwl_pcie_map_bus() and > xilinx_pcie_map_bus(). > Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET(). > > Changes in v2: > Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop > PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour > of using a single value for the device/function. > > drivers/pci/controller/dwc/pcie-al.c | 8 ++---- > drivers/pci/controller/dwc/pcie-hisi.c | 4 +-- > drivers/pci/controller/pci-host-generic.c | 4 +-- > drivers/pci/controller/pci-thunder-ecam.c | 2 +- > drivers/pci/controller/pci-thunder-pem.c | 13 +++++++-- > drivers/pci/controller/pci-xgene.c | 13 +++++++-- > drivers/pci/controller/pcie-rockchip-host.c | 27 +++++++++-------- > drivers/pci/controller/pcie-rockchip.h | 8 +----- > drivers/pci/controller/pcie-tango.c | 2 +- > drivers/pci/controller/pcie-xilinx-nwl.c | 9 ++---- > drivers/pci/controller/pcie-xilinx.c | 11 ++----- > drivers/pci/ecam.c | 4 +-- > include/linux/pci-ecam.h | 32 +++++++++++++++++++++ What about vmd which I mentioned? I also found iproc and brcmstb are ECAM (well, same shifts, but indirect addressing). [...] > +/* > + * Enhanced Configuration Access Mechanism (ECAM) > + * > + * N.B. This is a non-standard platform-specific ECAM bus shift value. For > + * standard values defined in the PCI Express Base Specification see > + * include/linux/pci-ecam.h. > + */ > +#define XGENE_PCIE_ECAM_BUS_SHIFT 16 Isn't this just CAM? Though perhaps CAM on PCIe is not standard... For CAM, there's also tegra, ftpci100, mvebu, and versatile. I think I'd drop CAM from this patch and do all of those in a separate patch. Rob