
Hi Paul,
On Fri, 26 Nov 2021 at 03:43, Pali Rohár pali@kernel.org wrote:
Lot of PCI and PCIe controllers are using standard Config Address for PCI Configuration Mechanism #1 or its extended version.
So add PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() macros into U-Boot's pci.h header file which can be suitable for most PCI and PCIe controller drivers. Drivers do not have to invent their own macros and can use these new U-Boot macros.
Signed-off-by: Pali Rohár pali@kernel.org
include/pci.h | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Thoughts below
diff --git a/include/pci.h b/include/pci.h index 6c1094d72998..0ea41a7e1ba2 100644 --- a/include/pci.h +++ b/include/pci.h @@ -522,6 +522,51 @@
#include <pci_ids.h>
+/*
- Config Address for PCI Configuration Mechanism #1
- See PCI Local Bus Specification, Revision 3.0,
- Section 3.2.2.3.2, Figure 3-2, p. 50.
- */
+#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 */
+#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)
+#define PCI_CONF1_ADDRESS(bus, dev, func, reg) \
Just for brevity, how about _ADDR() instead of _ADDRESS() ?
(PCI_CONF1_ENABLE | \
PCI_CONF1_BUS(bus) | \
PCI_CONF1_DEV(dev) | \
PCI_CONF1_FUNC(func) | \
PCI_CONF1_REG(reg))
+/*
- Extension of PCI Config Address for accessing extended PCIe registers
- No standardized specification, but used on lot of non-ECAM-compliant ARM SoCs
- or on AMD Barcelona and new CPUs. Reserved bits [27:24] of PCI Config Address
- are used for specifying additional 4 high bits of PCI Express register.
- */
+#define PCI_CONF1_EXT_REG_SHIFT 16 +#define PCI_CONF1_EXT_REG_MASK 0xf00
How about s/EXT_REG/EXT/ ?
For shifts and masks we normally like to define the mask in terms of the shift, so:
+#define PCI_CONF1_EXT_REG_MASK (0xf00 << PCI_CONF1_EXT_REG_SHIFT)
+#define PCI_CONF1_EXT_REG(x) (((x) & PCI_CONF1_EXT_REG_MASK) << PCI_CONF1_EXT_REG_SHIFT)
+#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \
(PCI_CONF1_ADDRESS(bus, dev, func, reg) | \
PCI_CONF1_EXT_REG(reg))
/*
- Enhanced Configuration Access Mechanism (ECAM)
-- 2.20.1
Regards, Simon