Re: [PATCH] pci: Add standard PCIe ECAM macros

Stefan Roese Wed, 03 Nov 2021 23:51:54 -0700

On 03.11.21 01:01, Pali Rohár wrote:

Lot of PCIe controllers are using ECAM addressing. So add common ECAM
macros into U-Boot's pci.h header file which can be suitable for most
PCI controller drivers.


Replace custom ECAM address macros in every PCI controller driver by new
ECAM macros from U-Boot's pci.h header file.

Similar macros are defined also in Linux kernel. There is a small
difference between Linux and these new U-Boot macros.

U-Boot's PCIE_ECAM_OFFSET() takes device and function numbers in separate
arguments. Linux's PCIE_ECAM_OFFSET() takes device and function numbers
encoded in one argument. The reason is that U-Boot's PCI_DEVFN() macro is
different than Linux's PCI_SLOT() macro. So having device and function
numbers in separate arguments makes code more straightforward.

Signed-off-by: Pali Rohár <p...@kernel.org>
---
I have tested this patch only with pci-aardvark.c driver. So please
properly review this change.


Reviewed-by: Stefan Roese <s...@denx.de>

Thanks,
STefan

Look into following file to compare macros with Linux:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/pci-ecam.h
---
  drivers/pci/pci-aardvark.c        | 14 ++------------
  drivers/pci/pcie_ecam_generic.c   |  6 ++----
  drivers/pci/pcie_ecam_synquacer.c |  6 ++----
  drivers/pci/pcie_phytium.c        |  8 ++------
  drivers/pci/pcie_rockchip.c       | 13 ++-----------
  drivers/pci/pcie_xilinx.c         |  5 +----
  include/pci.h                     | 26 ++++++++++++++++++++++++++
  7 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/pci-aardvark.c b/drivers/pci/pci-aardvark.c
index 9e623b6e617b..407b9fa93da1 100644
--- a/drivers/pci/pci-aardvark.c
+++ b/drivers/pci/pci-aardvark.c
@@ -165,16 +165,6 @@
  #define PCIE_CONFIG_WR_TYPE0                  0xa
  #define PCIE_CONFIG_WR_TYPE1                  0xb

-/* PCI_BDF shifts 8bit, so we need extra 4bit shift */

-#define PCIE_BDF(b, d, f)                      (PCI_BDF(b, d, f) << 4)
-#define PCIE_CONF_BUS(bus)                     (((bus) & 0xff) << 20)
-#define PCIE_CONF_DEV(dev)                     (((dev) & 0x1f) << 15)
-#define PCIE_CONF_FUNC(fun)                    (((fun) & 0x7)      << 12)
-#define PCIE_CONF_REG(reg)                     ((reg) & 0xffc)
-#define PCIE_CONF_ADDR(bus, devfn, where)      \
-       (PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn))    | \
-        PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
-
  /* PCIe Retries & Timeout definitions */
  #define PIO_MAX_RETRIES                               1500
  #define PIO_WAIT_TIMEOUT                      1000
@@ -468,7 +458,7 @@ static int pcie_advk_read_config(const struct udevice *bus, 
pci_dev_t bdf,
        advk_writel(pcie, reg, PIO_CTRL);

/* Program the address registers */

-       reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | 
PCIE_CONF_REG(offset);
+       reg = PCIE_ECAM_OFFSET(busno, PCI_DEV(bdf), PCI_FUNC(bdf), (offset & 
~0x3));
        advk_writel(pcie, reg, PIO_ADDR_LS);
        advk_writel(pcie, 0, PIO_ADDR_MS);

@@ -625,7 +615,7 @@ static int pcie_advk_write_config(struct udevice *bus, pci_dev_t bdf,

        advk_writel(pcie, reg, PIO_CTRL);

/* Program the address registers */

-       reg = PCIE_BDF(busno, PCI_DEV(bdf), PCI_FUNC(bdf)) | 
PCIE_CONF_REG(offset);
+       reg = PCIE_ECAM_OFFSET(busno, PCI_DEV(bdf), PCI_FUNC(bdf), (offset & 
~0x3));
        advk_writel(pcie, reg, PIO_ADDR_LS);
        advk_writel(pcie, 0, PIO_ADDR_MS);
        dev_dbg(pcie->dev, "\tPIO req. - addr = 0x%08x\n", reg);
diff --git a/drivers/pci/pcie_ecam_generic.c b/drivers/pci/pcie_ecam_generic.c
index e83e5aff206b..09b6fc1fd5d5 100644
--- a/drivers/pci/pcie_ecam_generic.c
+++ b/drivers/pci/pcie_ecam_generic.c
@@ -46,10 +46,8 @@ static int pci_generic_ecam_conf_address(const struct 
udevice *bus,
        void *addr;

addr = pcie->cfg_base;

-       addr += (PCI_BUS(bdf) - pcie->first_busno) << 20;
-       addr += PCI_DEV(bdf) << 15;
-       addr += PCI_FUNC(bdf) << 12;
-       addr += offset;
+       addr += PCIE_ECAM_OFFSET(PCI_BUS(bdf) - pcie->first_busno,
+                                PCI_DEV(bdf), PCI_FUNC(bdf), offset);
        *paddress = addr;

return 0;

diff --git a/drivers/pci/pcie_ecam_synquacer.c 
b/drivers/pci/pcie_ecam_synquacer.c
index c6e7c59f8a64..e3e22891088e 100644
--- a/drivers/pci/pcie_ecam_synquacer.c
+++ b/drivers/pci/pcie_ecam_synquacer.c
@@ -235,10 +235,8 @@ static int pci_synquacer_ecam_conf_address(const struct 
udevice *bus,
        void *addr;

addr = pcie->cfg_base;

-       addr += (PCI_BUS(bdf) - pcie->first_busno) << 20;
-       addr += PCI_DEV(bdf) << 15;
-       addr += PCI_FUNC(bdf) << 12;
-       addr += offset;
+       addr += PCIE_ECAM_OFFSET(PCI_BUS(bdf) - pcie->first_busno,
+                                PCI_DEV(bdf), PCI_FUNC(bdf), offset);
        *paddress = addr;

return 0;

diff --git a/drivers/pci/pcie_phytium.c b/drivers/pci/pcie_phytium.c
index 752e1703215b..a80727625428 100644
--- a/drivers/pci/pcie_phytium.c
+++ b/drivers/pci/pcie_phytium.c
@@ -36,9 +36,7 @@ static int phytium_pci_skip_dev(pci_dev_t parent)
        unsigned short capreg;
        unsigned char port_type;

- addr += PCI_BUS(parent) << 20;

-       addr += PCI_DEV(parent) << 15;
-       addr += PCI_FUNC(parent) << 12;
+       addr += PCIE_ECAM_OFFSET(PCI_BUS(parent), PCI_DEV(parent), 
PCI_FUNC(parent), 0);

pos = 0x34;

        while (1) {
@@ -89,9 +87,7 @@ static int pci_phytium_conf_address(const struct udevice 
*bus, pci_dev_t bdf,
        bdf_parent = PCI_BDF((bus_no - 1), 0, 0);

addr = pcie->cfg_base;

-       addr += PCI_BUS(bdf) << 20;
-       addr += PCI_DEV(bdf) << 15;
-       addr += PCI_FUNC(bdf) << 12;
+       addr += PCIE_ECAM_OFFSET(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), 0);

if (bus_no > 0 && dev_no > 0) {

                if ((readb(addr + PCI_HEADER_TYPE) & 0x7f) !=
diff --git a/drivers/pci/pcie_rockchip.c b/drivers/pci/pcie_rockchip.c
index b0c91c0f430e..67039d2a29f8 100644
--- a/drivers/pci/pcie_rockchip.c
+++ b/drivers/pci/pcie_rockchip.c
@@ -101,15 +101,6 @@ struct rockchip_pcie {
        struct phy pcie_phy;
  };

-static int rockchip_pcie_off_conf(pci_dev_t bdf, uint offset)

-{
-       unsigned int bus = PCI_BUS(bdf);
-       unsigned int dev = PCI_DEV(bdf);
-       unsigned int func = PCI_FUNC(bdf);
-
-       return (bus << 20) | (dev << 15) | (func << 12) | (offset & ~0x3);
-}
-
  static int rockchip_pcie_rd_conf(const struct udevice *udev, pci_dev_t bdf,
                                 uint offset, ulong *valuep,
                                 enum pci_size_t size)
@@ -117,7 +108,7 @@ static int rockchip_pcie_rd_conf(const struct udevice 
*udev, pci_dev_t bdf,
        struct rockchip_pcie *priv = dev_get_priv(udev);
        unsigned int bus = PCI_BUS(bdf);
        unsigned int dev = PCI_DEV(bdf);
-       int where = rockchip_pcie_off_conf(bdf, offset);
+       int where = PCIE_ECAM_OFFSET(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), 
offset & ~0x3);
        ulong value;

if (bus == priv->first_busno && dev == 0) {

@@ -144,7 +135,7 @@ static int rockchip_pcie_wr_conf(struct udevice *udev, 
pci_dev_t bdf,
        struct rockchip_pcie *priv = dev_get_priv(udev);
        unsigned int bus = PCI_BUS(bdf);
        unsigned int dev = PCI_DEV(bdf);
-       int where = rockchip_pcie_off_conf(bdf, offset);
+       int where = PCIE_ECAM_OFFSET(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), 
offset & ~0x3);
        ulong old;

if (bus == priv->first_busno && dev == 0) {

diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c
index ae9a65b0a9e1..eb9ec97b74f3 100644
--- a/drivers/pci/pcie_xilinx.c
+++ b/drivers/pci/pcie_xilinx.c
@@ -76,10 +76,7 @@ static int pcie_xilinx_config_address(const struct udevice 
*udev, pci_dev_t bdf,
                return -ENODEV;

addr = pcie->cfg_base;

-       addr += bus << 20;
-       addr += dev << 15;
-       addr += func << 12;
-       addr += offset;
+       addr += PCIE_ECAM_OFFSET(bus, dev, func, offset);
        *paddress = addr;

return 0;

diff --git a/include/pci.h b/include/pci.h
index 797f224e2fc4..6c1094d72998 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -522,6 +522,32 @@

#include <pci_ids.h>+/*

+ * Enhanced Configuration Access Mechanism (ECAM)
+ *
+ * See PCI Express Base Specification, Revision 5.0, Version 1.0,
+ * Section 7.2.2, Table 7-1, p. 677.
+ */
+#define PCIE_ECAM_BUS_SHIFT    20 /* Bus number */
+#define PCIE_ECAM_DEV_SHIFT    15 /* Device number */
+#define PCIE_ECAM_FUNC_SHIFT   12 /* Function number */
+
+#define PCIE_ECAM_BUS_MASK     0xff
+#define PCIE_ECAM_DEV_MASK     0x1f
+#define PCIE_ECAM_FUNC_MASK    0x7
+#define PCIE_ECAM_REG_MASK     0xfff /* Limit offset to a maximum of 4K */
+
+#define PCIE_ECAM_BUS(x)       (((x) & PCIE_ECAM_BUS_MASK) << 
PCIE_ECAM_BUS_SHIFT)
+#define PCIE_ECAM_DEV(x)       (((x) & PCIE_ECAM_DEV_MASK) << 
PCIE_ECAM_DEV_SHIFT)
+#define PCIE_ECAM_FUNC(x)      (((x) & PCIE_ECAM_FUNC_MASK) << 
PCIE_ECAM_FUNC_SHIFT)
+#define PCIE_ECAM_REG(x)       ((x) & PCIE_ECAM_REG_MASK)
+
+#define PCIE_ECAM_OFFSET(bus, dev, func, where) \
+       (PCIE_ECAM_BUS(bus) | \
+        PCIE_ECAM_DEV(dev) | \
+        PCIE_ECAM_FUNC(func) | \
+        PCIE_ECAM_REG(where))
+
  #ifndef __ASSEMBLY__

#include <dm/pci.h>



Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

Re: [PATCH] pci: Add standard PCIe ECAM macros

Reply via email to