[PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers

Krzysztof Wilczyński kw at linux.com
Sun Nov 15 00:50:57 EST 2020


On 20-10-04 19:53:06, Florian Fainelli wrote:

Hi Florian,

Sorry for taking a long time to get back to you.

[...]
> This appears to be correct, so:
> 
> Acked-by: Florian Fainelli <f.fainelli at gmail.com>

Thank you!
 
> however, I would have defined a couple of additional helper macros and do:
> 
> 	idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEV(devfn) |
> PCIE_ECAM_FUN(devfn);
> 
> for clarity.
> 
[...]
> For instance, adding these two:
> 
> #define PCIE_ECAM_DEV(x)		(((x) & 0x1f) << PCIE_ECAM_DEV_SHIFT)
> #define PCIE_ECAM_FUN(x)		(((x) & 0x7) << PCIE_ECAM_FUN_SHIFT)
> 
> may be clearer for use in drivers like pcie-brcmstb.c that used to treat the
> device function in terms of device and function (though it was called slot
> there).

Regarding the suggestion above - it has been like that initially, albeit
Bjorn suggested that there is no need to reply on the macros that use
PCI_SLOT() and PCI_FUNC() macros, see:

https://lore.kernel.org/linux-pci/20200922232715.GA2238688@bjorn-Precision-5520/

I would be happy to put the macros back if there is a value in having
the extra macros added - perhaps for clarify, as you suggest.

Krzysztof



More information about the Linux-rockchip mailing list