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

[Florian Fainelli]

+JimQ,

On 12/6/2020 12:16 PM, Krzysztof Wilczyński wrote:
> 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
> 

-- 
Florian