Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers

["Krzysztof Wilczyński" <kw@linux.com>]

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