[PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
Krzysztof Wilczyński
kw at linux.com
Sun Dec 6 15:16:36 EST 2020
Hello Nicolas, Florian and Florian,
[...]
> -/* Configuration space read/write support */
> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
> -{
> - return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
> - | ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
> - | (busnr << PCIE_EXT_BUSNUM_SHIFT)
> - | (reg & ~3);
> -}
> -
> static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
> int where)
> {
> @@ -716,7 +704,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
> return PCI_SLOT(devfn) ? NULL : base + where;
>
> /* For devices, write to the config space index register */
> - idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
> + idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
> writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
> return base + PCIE_EXT_CFG_DATA + where;
> }
[...]
Passing the hard-coded 0 as the "reg" argument here never actually did
anything, thus the 32 bit alignment was never correctly enforced.
My question would be: should this be 32 bit aligned? It seems like the
intention was to perhaps make the alignment? I am sadly not intimately
familiar with his hardware, so I am not sure if there is something to
fix here or not.
Also, I wonder whether it would be safe to pass the offset (the "where"
variable) rather than hard-coded 0?
Thank you for help in advance!
Bjorn also asked the same question:
https://lore.kernel.org/linux-pci/20201120203428.GA272511@bjorn-Precision-5520/
Krzysztof
More information about the Linux-rockchip
mailing list