On Mon, Aug 4, 2025, at 05:06, Hans Zhang wrote:
> On 2025/8/1 19:30, Gerd Bayer wrote:
>> On Fri, 2025-08-01 at 16:24 +0530, Manivannan Sadhasivam wrote:
> }
>
> +#define CDNS_PCI_OP_READ(size, type, len) \
> +static inline int cdns_pcie_read_cfg_##size \
> + (struct cdns_pcie *pcie, int where, type *val) \
> +{ \
> + if (len == 4) \
> + *val = cdns_pcie_readl(pcie, where); \
> + else if (len == 2) \
> + *val = cdns_pcie_readw(pcie, where); \
> + else if (len == 1) \
> + *val = cdns_pcie_readb(pcie, where); \
> + else \
> + return PCIBIOS_BAD_REGISTER_NUMBER; \
> + \
> + return PCIBIOS_SUCCESSFUL; \
> +}
> +
> +CDNS_PCI_OP_READ(byte, u8, 1)
> +CDNS_PCI_OP_READ(word, u16, 2)
> +CDNS_PCI_OP_READ(dword, u32, 4)
> +
This is fine for the calling conventions, but the use of a macro
doesn't really help readability, so I'd suggest just having
separate inline functions if they are even needed.
> @@ -112,17 +110,17 @@ int pci_bus_read_config(void *priv, unsigned int
> devfn, int where, u32 size,
> ({ \
> int __ttl = PCI_FIND_CAP_TTL; \
> u8 __id, __found_pos = 0; \
> - u32 __pos = (start); \
> - u32 __ent; \
> + u8 __pos = (start); \
> + u16 __ent; \
> \
> - read_cfg(args, __pos, 1, &__pos); \
> + read_cfg##_byte(args, __pos, &__pos); \
> \
> while (__ttl--) { \
> if (__pos < PCI_STD_HEADER_SIZEOF) \
> break; \
> \
> __pos = ALIGN_DOWN(__pos, 4); \
> - read_cfg(args, __pos, 2, &__ent); \
> + read_cfg##_word(args, __pos, &__ent); \
> \
> __id = FIELD_GET(PCI_CAP_ID_MASK, __ent); \
> if (__id == 0xff) \
I still don't feel great about this macro either, and suspect
we should have a better abstraction with fixed types and a
global function to do it, but I don't see anything obviously
wrong here either.
Arnd
