Re: [PATCH 1/3] PCI: Add standard PCI Config Address macros

[Bjorn Helgaas <helgaas@kernel.org>]

On Sat, Sep 24, 2022 at 11:24:02AM +0200, Pali Rohár wrote:
> Lot of PCI and PCIe controllers are using standard Config Address for PCI
> Configuration Mechanism #1 (as defined in PCI Local Bus Specification) or
> its extended version.
> 
> So introduce new macros PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() in
> include file drivers/pci/pci.h which can be suitable for PCI and PCIe
> controllers which uses this type of access to PCI config space.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> +#define PCI_CONF1_BUS_SHIFT	16 /* Bus number */
> +#define PCI_CONF1_DEV_SHIFT	11 /* Device number */
> +#define PCI_CONF1_FUNC_SHIFT	8  /* Function number */
> +
> +#define PCI_CONF1_BUS_MASK	0xff
> +#define PCI_CONF1_DEV_MASK	0x1f
> +#define PCI_CONF1_FUNC_MASK	0x7
> +#define PCI_CONF1_REG_MASK	0xfc /* Limit aligned offset to a maximum of 256B */

Since all the above are used only in the macros below, I personally
don't think they're really necessary and I would find it easier to
read if they were just open-coded, e.g.,

  +#define PCI_CONF1_BUS(x)     (((x) & 0xff) << 16)
  +#define PCI_CONF1_DEV(x)	(((x) & 0x1f) << 11)
  +#define PCI_CONF1_FUNC(x)	(((x) & 0x07) <<  8)
  +#define PCI_CONF1_REG(x)     ( (x) & 0xfc)

> +#define PCI_CONF1_ENABLE	BIT(31)
> +#define PCI_CONF1_BUS(x)	(((x) & PCI_CONF1_BUS_MASK) << PCI_CONF1_BUS_SHIFT)
> +#define PCI_CONF1_DEV(x)	(((x) & PCI_CONF1_DEV_MASK) << PCI_CONF1_DEV_SHIFT)
> +#define PCI_CONF1_FUNC(x)	(((x) & PCI_CONF1_FUNC_MASK) << PCI_CONF1_FUNC_SHIFT)
> +#define PCI_CONF1_REG(x)	((x) & PCI_CONF1_REG_MASK)