2024-04-29 10:47:01

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 00/10] PCI: Add generic Conf Type 0/1 helpers

This series replaces PCI_CONF1{,_EXT}_ADDRESS() with more generic
helpers and makes them more widely available by placing the new helpers
into include/linux/pci.h.

Most of what is under drivers/pci/controller is converted to use the
new helpers by this series. I left arch/ changes out from this because
they're quite varied so they would be harder to verify (and review)
except ARM/orion5x that I had to do now due to a naming conflict.
Nonetheless, there is plenty custom type 0/1 code under arch/ that
could now take advantage of the new helpers.

I've postponed touching pcie-mediatek.c because there's odd slot
calculation which I brought up in another thread.

Ilpo Järvinen (10):
ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way
PCI: Add helpers to calculate PCI Conf Type 0/1 addresses
ARM: orion5x: Pass devfn to orion5x_pci_hw_{rd,wr}_conf()
ARM: orion5x: Use generic PCI Conf Type 1 helper
PCI: ixp4xx: Use generic PCI Conf Type 0 helper
PCI: ixp4xx: Replace 1 with PCI_CONF1_TRANSACTION
PCI: Replace PCI_CONF1{,_EXT}_ADDRESS() with the new helpers
PCI: tegra: Use generic PCI Conf Type 1 helper
PCI: mvebu: Use generic PCI Conf Type 1 helper
PCI: v3: Use generic PCI Conf Type 0/1 helpers

arch/arm/mach-orion5x/pci.c | 54 +++++++----------
drivers/pci/controller/pci-ftpci100.c | 6 +-
drivers/pci/controller/pci-ixp4xx.c | 9 ++-
drivers/pci/controller/pci-mvebu.c | 13 +---
drivers/pci/controller/pci-tegra.c | 12 +---
drivers/pci/controller/pci-v3-semi.c | 6 +-
drivers/pci/controller/pcie-mt7621.c | 7 +--
drivers/pci/pci.h | 45 --------------
include/linux/pci.h | 86 +++++++++++++++++++++++++++
9 files changed, 123 insertions(+), 115 deletions(-)

--
2.39.2



2024-04-29 10:47:51

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 04/10] ARM: orion5x: Use generic PCI Conf Type 1 helper

Convert orion5x PCI code to use pci_conf1_ext_addr() from PCI core to
calculate PCI Configuration Type 1 address.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
arch/arm/mach-orion5x/pci.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
index 6376e1db6386..8b7d67549adf 100644
--- a/arch/arm/mach-orion5x/pci.c
+++ b/arch/arm/mach-orion5x/pci.c
@@ -216,15 +216,6 @@ static int __init pcie_setup(struct pci_sys_data *sys)
#define PCI_P2P_DEV_OFFS 24
#define PCI_P2P_DEV_MASK (0x1f << PCI_P2P_DEV_OFFS)

-/*
- * PCI_CONF_ADDR bits
- */
-#define ORION5X_PCI_CONF_REG(reg) ((reg) & 0xfc)
-#define ORION5X_PCI_CONF_FUNC(func) (((func) & 0x3) << 8)
-#define PCI_CONF_DEV(dev) (((dev) & 0x1f) << 11)
-#define PCI_CONF_BUS(bus) (((bus) & 0xff) << 16)
-#define PCI_CONF_ADDR_EN (1 << 31)
-
/*
* Internal configuration space
*/
@@ -276,9 +267,7 @@ static int orion5x_pci_hw_rd_conf(int bus, u8 devfn, u32 where,
unsigned long flags;
spin_lock_irqsave(&orion5x_pci_lock, flags);

- writel(PCI_CONF_BUS(bus) |
- PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
- ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
+ writel(pci_conf1_addr(bus, devfn, where, true), PCI_CONF_ADDR);

*val = readl(PCI_CONF_DATA);

@@ -300,9 +289,7 @@ static int orion5x_pci_hw_wr_conf(int bus, u8 devfn, u32 where,

spin_lock_irqsave(&orion5x_pci_lock, flags);

- writel(PCI_CONF_BUS(bus) |
- PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
- ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
+ writel(pci_conf1_addr(bus, devfn, where, true), PCI_CONF_ADDR);

if (size == 4) {
__raw_writel(val, PCI_CONF_DATA);
--
2.39.2


2024-04-29 12:01:48

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 05/10] PCI: ixp4xx: Use generic PCI Conf Type 0 helper

Convert Type 0 address calculation to use pci_conf0_offset() instead of
abusing PCI_CONF1_ADDRESS().

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/pci/controller/pci-ixp4xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-ixp4xx.c b/drivers/pci/controller/pci-ixp4xx.c
index acb85e0d5675..44639838df9c 100644
--- a/drivers/pci/controller/pci-ixp4xx.c
+++ b/drivers/pci/controller/pci-ixp4xx.c
@@ -188,8 +188,8 @@ static u32 ixp4xx_config_addr(u8 bus_num, u16 devfn, int where)
/* Root bus is always 0 in this hardware */
if (bus_num == 0) {
/* type 0 */
- return (PCI_CONF1_ADDRESS(0, 0, PCI_FUNC(devfn), where) &
- ~PCI_CONF1_ENABLE) | BIT(32-PCI_SLOT(devfn));
+ return pci_conf0_addr(devfn, where) |
+ BIT(32 - PCI_SLOT(devfn));
} else {
/* type 1 */
return (PCI_CONF1_ADDRESS(bus_num, PCI_SLOT(devfn),
--
2.39.2


2024-04-29 12:07:01

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 09/10] PCI: mvebu: Use generic PCI Conf Type 1 helper

Convert mvebu to use the pci_conf1_ext_addr() helper from PCI core
to calculate PCI Configuration Space address for Type 1 access.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/pci/controller/pci-mvebu.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
index 29fe09c99e7d..1908754ee6fd 100644
--- a/drivers/pci/controller/pci-mvebu.c
+++ b/drivers/pci/controller/pci-mvebu.c
@@ -45,15 +45,6 @@
#define PCIE_WIN5_BASE_OFF 0x1884
#define PCIE_WIN5_REMAP_OFF 0x188c
#define PCIE_CONF_ADDR_OFF 0x18f8
-#define PCIE_CONF_ADDR_EN 0x80000000
-#define PCIE_CONF_REG(r) ((((r) & 0xf00) << 16) | ((r) & 0xfc))
-#define PCIE_CONF_BUS(b) (((b) & 0xff) << 16)
-#define PCIE_CONF_DEV(d) (((d) & 0x1f) << 11)
-#define PCIE_CONF_FUNC(f) (((f) & 0x7) << 8)
-#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_CONF_ADDR_EN)
#define PCIE_CONF_DATA_OFF 0x18fc
#define PCIE_INT_CAUSE_OFF 0x1900
#define PCIE_INT_UNMASK_OFF 0x1910
@@ -361,7 +352,7 @@ static int mvebu_pcie_child_rd_conf(struct pci_bus *bus, u32 devfn, int where,

conf_data = port->base + PCIE_CONF_DATA_OFF;

- mvebu_writel(port, PCIE_CONF_ADDR(bus->number, devfn, where),
+ mvebu_writel(port, pci_conf1_ext_addr(bus->number, devfn, where, true),
PCIE_CONF_ADDR_OFF);

switch (size) {
@@ -397,7 +388,7 @@ static int mvebu_pcie_child_wr_conf(struct pci_bus *bus, u32 devfn,

conf_data = port->base + PCIE_CONF_DATA_OFF;

- mvebu_writel(port, PCIE_CONF_ADDR(bus->number, devfn, where),
+ mvebu_writel(port, pci_conf1_ext_addr(bus->number, devfn, where, true),
PCIE_CONF_ADDR_OFF);

switch (size) {
--
2.39.2


2024-04-29 13:02:42

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 10/10] PCI: v3: Use generic PCI Conf Type 0/1 helpers

Convert v3 to use pci_conf{0,1}_addr() from PCI core to calculate PCI
Configuration Space address for Type 0/1 access.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/pci/controller/pci-v3-semi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pci-v3-semi.c b/drivers/pci/controller/pci-v3-semi.c
index 460a825325dd..a07323148007 100644
--- a/drivers/pci/controller/pci-v3-semi.c
+++ b/drivers/pci/controller/pci-v3-semi.c
@@ -327,7 +327,7 @@ static void __iomem *v3_map_bus(struct pci_bus *bus,
* 3:1 = config cycle (101)
* 0 = PCI A1 & A0 are 0 (0)
*/
- address = PCI_FUNC(devfn) << 8;
+ address = pci_conf0_addr(devfn, offset);
mapaddress = V3_LB_MAP_TYPE_CONFIG;

if (slot > 12)
@@ -354,7 +354,7 @@ static void __iomem *v3_map_bus(struct pci_bus *bus,
* 0 = PCI A1 & A0 from host bus (1)
*/
mapaddress = V3_LB_MAP_TYPE_CONFIG | V3_LB_MAP_AD_LOW_EN;
- address = (busnr << 16) | (devfn << 8);
+ address = pci_conf1_addr(busnr, devfn, offset, false);
}

/*
@@ -375,7 +375,7 @@ static void __iomem *v3_map_bus(struct pci_bus *bus,
v3->base + V3_LB_BASE1);
writew(mapaddress, v3->base + V3_LB_MAP1);

- return v3->config_base + address + offset;
+ return v3->config_base + address;
}

static void v3_unmap_bus(struct v3_pci *v3)
--
2.39.2


2024-04-29 13:16:54

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 01/10] ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way

orion5x defines PCI_CONF_REG() and PCI_CONF_FUNC() that are problematic
because PCI core is going to introduce defines with the same names.

Add ORION5X prefix to those defines to avoid name conflicts.

Note: as this is part of series that replaces the code in question
anyway, only bare minimum renaming is done and other similarly named
macros are not touched.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
arch/arm/mach-orion5x/pci.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
index 3313bc5a63ea..77ddab90f448 100644
--- a/arch/arm/mach-orion5x/pci.c
+++ b/arch/arm/mach-orion5x/pci.c
@@ -219,8 +219,8 @@ static int __init pcie_setup(struct pci_sys_data *sys)
/*
* PCI_CONF_ADDR bits
*/
-#define PCI_CONF_REG(reg) ((reg) & 0xfc)
-#define PCI_CONF_FUNC(func) (((func) & 0x3) << 8)
+#define ORION5X_PCI_CONF_REG(reg) ((reg) & 0xfc)
+#define ORION5X_PCI_CONF_FUNC(func) (((func) & 0x3) << 8)
#define PCI_CONF_DEV(dev) (((dev) & 0x1f) << 11)
#define PCI_CONF_BUS(bus) (((bus) & 0xff) << 16)
#define PCI_CONF_ADDR_EN (1 << 31)
@@ -277,8 +277,8 @@ static int orion5x_pci_hw_rd_conf(int bus, int dev, u32 func,
spin_lock_irqsave(&orion5x_pci_lock, flags);

writel(PCI_CONF_BUS(bus) |
- PCI_CONF_DEV(dev) | PCI_CONF_REG(where) |
- PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
+ PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
+ ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);

*val = readl(PCI_CONF_DATA);

@@ -301,8 +301,8 @@ static int orion5x_pci_hw_wr_conf(int bus, int dev, u32 func,
spin_lock_irqsave(&orion5x_pci_lock, flags);

writel(PCI_CONF_BUS(bus) |
- PCI_CONF_DEV(dev) | PCI_CONF_REG(where) |
- PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
+ PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
+ ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);

if (size == 4) {
__raw_writel(val, PCI_CONF_DATA);
--
2.39.2


2024-04-29 13:16:55

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 02/10] PCI: Add helpers to calculate PCI Conf Type 0/1 addresses

Many places in arch and PCI controller code need to calculate PCI
Configuration Space Addresses for Type 0/1 accesses. There are small
variations between archs when it comes to bits outside of [10:2] (Type
0) and [24:2] (Type 1) but the basic calculation can still be
generalized.

drivers/pci/pci.h has PCI_CONF1{,_EXT}_ADDRESS() but due to their
location the use is limited to PCI subsys and the also always enable
PCI_CONF1_ENABLE which is not what all the callers want.

Add generic pci_conf{0,1}_addr() and pci_conf1_ext_addr() helpers into
include/linux/pci.h which can be reused by various parts of the kernel
that have to calculate PCI Conf Type 0/1 addresses.

The PCI_CONF* defines are needed by the new helpers so move also them
to include/linux/pci.h. The new helpers use true bitmasks and
FIELD_PREP() instead of open coded masking and shifting so adjust
PCI_CONF* definitions to match that.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/pci/pci.h | 43 ++---------------------
include/linux/pci.h | 85 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+), 40 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 17fed1846847..cf0530a60105 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -833,49 +833,12 @@ struct pci_devres {

struct pci_devres *find_pci_dr(struct pci_dev *pdev);

-/*
- * 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) \
(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
-#define PCI_CONF1_EXT_REG(x) (((x) & PCI_CONF1_EXT_REG_MASK) << PCI_CONF1_EXT_REG_SHIFT)
+ pci_conf1_addr(bus, PCI_DEVFN(dev, func), reg & ~0x3U))

#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \
- (PCI_CONF1_ADDRESS(bus, dev, func, reg) | \
- PCI_CONF1_EXT_REG(reg))
+ (PCI_CONF1_ENABLE | \
+ pci_conf1_ext_addr(bus, PCI_DEVFN(dev, func), reg & ~0x3U))

#endif /* DRIVERS_PCI_H */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 16493426a04f..4c4e3bb52a0a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -26,6 +26,8 @@
#include <linux/args.h>
#include <linux/mod_devicetable.h>

+#include <linux/bits.h>
+#include <linux/bitfield.h>
#include <linux/types.h>
#include <linux/init.h>
#include <linux/ioport.h>
@@ -1183,6 +1185,89 @@ void pci_sort_breadthfirst(void);
#define dev_is_pci(d) ((d)->bus == &pci_bus_type)
#define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))

+/*
+ * Config Address for PCI Configuration Mechanism #0/1
+ *
+ * See PCI Local Bus Specification, Revision 3.0,
+ * Section 3.2.2.3.2, Figure 3-1 and 3-2, p. 48-50.
+ */
+#define PCI_CONF_REG 0x000000ffU /* common for Type 0/1 */
+#define PCI_CONF_FUNC 0x00000700U /* common for Type 0/1 */
+#define PCI_CONF1_DEV 0x0000f800U
+#define PCI_CONF1_BUS 0x00ff0000U
+#define PCI_CONF1_ENABLE BIT(31)
+
+/**
+ * pci_conf0_addr - PCI Base Configuration Space address for Type 0 access
+ * @devfn: Device and function numbers (device number will be ignored)
+ * @reg: Base configuration space offset
+ *
+ * Calculates the PCI Configuration Space address for Type 0 accesses.
+ *
+ * Note: the caller is responsible for adding the bits outside of [10:0].
+ *
+ * Return: Base Configuration Space address.
+ */
+static inline u32 pci_conf0_addr(u8 devfn, u8 reg)
+{
+ return FIELD_PREP(PCI_CONF_FUNC, PCI_FUNC(devfn)) |
+ FIELD_PREP(PCI_CONF_REG, reg & ~3);
+}
+
+/**
+ * pci_conf1_addr - PCI Base Configuration Space address for Type 1 access
+ * @bus: Bus number of the device
+ * @devfn: Device and function numbers
+ * @reg: Base configuration space offset
+ * @enable: Assert enable bit (bit 31)
+ *
+ * Calculates the PCI Base Configuration Space (first 256 bytes) address for
+ * Type 1 accesses.
+ *
+ * Note: the caller is responsible for adding the bits outside of [24:2]
+ * and enable bit.
+ *
+ * Return: PCI Base Configuration Space address.
+ */
+static inline u32 pci_conf1_addr(u8 bus, u8 devfn, u8 reg, bool enable)
+{
+ return (enable ? PCI_CONF1_ENABLE : 0) |
+ FIELD_PREP(PCI_CONF1_BUS, bus) |
+ FIELD_PREP(PCI_CONF1_DEV | PCI_CONF_FUNC, devfn) |
+ FIELD_PREP(PCI_CONF_REG, reg & ~3);
+}
+
+/*
+ * 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 0x0f000000UL
+
+/**
+ * pci_conf1_ext_addr - PCI Configuration Space address for Type 1 access
+ * @bus: Bus number of the device
+ * @devfn: Device and function numbers
+ * @reg: Base or Extended Configuration space offset
+ * @enable: Assert enable bit (bit 31)
+ *
+ * Calculates the PCI Base and Extended (4096 bytes per PCI function)
+ * Configuration Space address for Type 1 accesses. This function assumes
+ * the Extended Conguration Space is using the reserved bits [27:24].
+ *
+ * Note: the caller is responsible for adding the bits outside of [27:2] and
+ * enable bit.
+ *
+ * Return: PCI Configuration Space address.
+ */
+static inline u32 pci_conf1_ext_addr(u8 bus, u8 devfn, u16 reg, bool enable)
+{
+ return FIELD_PREP(PCI_CONF1_EXT_REG, (reg & 0xf00) >> 8) |
+ pci_conf1_addr(bus, devfn, reg & 0xff, enable);
+}
+
/* Generic PCI functions exported to card drivers */

u8 pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
--
2.39.2


2024-04-29 13:53:26

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 07/10] PCI: Replace PCI_CONF1{,_EXT}_ADDRESS() with the new helpers

Replace the old PCI_CONF1{,_EXT}_ADDRESS() helpers used to calculate
PCI Configuration Space Type 1 addresses with the new
pci_conf1{,_ext}_offset() helpers that are more generic and more widely
available.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/pci/controller/pci-ftpci100.c | 6 ++----
drivers/pci/controller/pci-ixp4xx.c | 5 ++---
drivers/pci/controller/pcie-mt7621.c | 7 +++----
drivers/pci/pci.h | 8 --------
4 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/controller/pci-ftpci100.c b/drivers/pci/controller/pci-ftpci100.c
index ffdeed25e961..a8d0217a0b94 100644
--- a/drivers/pci/controller/pci-ftpci100.c
+++ b/drivers/pci/controller/pci-ftpci100.c
@@ -182,8 +182,7 @@ static int faraday_raw_pci_read_config(struct faraday_pci *p, int bus_number,
unsigned int fn, int config, int size,
u32 *value)
{
- writel(PCI_CONF1_ADDRESS(bus_number, PCI_SLOT(fn),
- PCI_FUNC(fn), config),
+ writel(pci_conf1_addr(bus_number, fn, config, true),
p->base + FTPCI_CONFIG);

*value = readl(p->base + FTPCI_DATA);
@@ -214,8 +213,7 @@ static int faraday_raw_pci_write_config(struct faraday_pci *p, int bus_number,
{
int ret = PCIBIOS_SUCCESSFUL;

- writel(PCI_CONF1_ADDRESS(bus_number, PCI_SLOT(fn),
- PCI_FUNC(fn), config),
+ writel(pci_conf1_addr(bus_number, fn, config, true),
p->base + FTPCI_CONFIG);

switch (size) {
diff --git a/drivers/pci/controller/pci-ixp4xx.c b/drivers/pci/controller/pci-ixp4xx.c
index ec0125344ca1..fd52f4a3ef31 100644
--- a/drivers/pci/controller/pci-ixp4xx.c
+++ b/drivers/pci/controller/pci-ixp4xx.c
@@ -192,9 +192,8 @@ static u32 ixp4xx_config_addr(u8 bus_num, u16 devfn, int where)
BIT(32 - PCI_SLOT(devfn));
} else {
/* type 1 */
- return (PCI_CONF1_ADDRESS(bus_num, PCI_SLOT(devfn),
- PCI_FUNC(devfn), where) &
- ~PCI_CONF1_ENABLE) | PCI_CONF1_TRANSACTION;
+ return pci_conf1_addr(bus_num, devfn, where, false) |
+ PCI_CONF1_TRANSACTION;
}
}

diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
index 79e225edb42a..2b2d9828a910 100644
--- a/drivers/pci/controller/pcie-mt7621.c
+++ b/drivers/pci/controller/pcie-mt7621.c
@@ -127,8 +127,7 @@ static void __iomem *mt7621_pcie_map_bus(struct pci_bus *bus,
unsigned int devfn, int where)
{
struct mt7621_pcie *pcie = bus->sysdata;
- u32 address = PCI_CONF1_EXT_ADDRESS(bus->number, PCI_SLOT(devfn),
- PCI_FUNC(devfn), where);
+ u32 address = pci_conf1_ext_addr(bus->number, devfn, where, true);

writel_relaxed(address, pcie->base + RALINK_PCI_CONFIG_ADDR);

@@ -143,7 +142,7 @@ static struct pci_ops mt7621_pcie_ops = {

static u32 read_config(struct mt7621_pcie *pcie, unsigned int dev, u32 reg)
{
- u32 address = PCI_CONF1_EXT_ADDRESS(0, dev, 0, reg);
+ u32 address = pci_conf1_ext_addr(0, PCI_DEVFN(dev, 0), reg, true);

pcie_write(pcie, address, RALINK_PCI_CONFIG_ADDR);
return pcie_read(pcie, RALINK_PCI_CONFIG_DATA);
@@ -152,7 +151,7 @@ static u32 read_config(struct mt7621_pcie *pcie, unsigned int dev, u32 reg)
static void write_config(struct mt7621_pcie *pcie, unsigned int dev,
u32 reg, u32 val)
{
- u32 address = PCI_CONF1_EXT_ADDRESS(0, dev, 0, reg);
+ u32 address = pci_conf1_ext_addr(0, PCI_DEVFN(dev, 0), reg, true);

pcie_write(pcie, address, RALINK_PCI_CONFIG_ADDR);
pcie_write(pcie, val, RALINK_PCI_CONFIG_DATA);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index cf0530a60105..fdf9624b0b12 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -833,12 +833,4 @@ struct pci_devres {

struct pci_devres *find_pci_dr(struct pci_dev *pdev);

-#define PCI_CONF1_ADDRESS(bus, dev, func, reg) \
- (PCI_CONF1_ENABLE | \
- pci_conf1_addr(bus, PCI_DEVFN(dev, func), reg & ~0x3U))
-
-#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \
- (PCI_CONF1_ENABLE | \
- pci_conf1_ext_addr(bus, PCI_DEVFN(dev, func), reg & ~0x3U))
-
#endif /* DRIVERS_PCI_H */
--
2.39.2


2024-04-29 14:09:16

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 01/10] ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way

On Mon, Apr 29, 2024 at 01:46:24PM +0300, Ilpo J?rvinen wrote:
> orion5x defines PCI_CONF_REG() and PCI_CONF_FUNC() that are problematic
> because PCI core is going to introduce defines with the same names.
>
> Add ORION5X prefix to those defines to avoid name conflicts.
>
> Note: as this is part of series that replaces the code in question
> anyway, only bare minimum renaming is done and other similarly named
> macros are not touched.
>
> Signed-off-by: Ilpo J?rvinen <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2024-04-29 14:38:47

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 01/10] ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way

On Mon, Apr 29, 2024 at 01:46:24PM +0300, Ilpo J?rvinen wrote:
> orion5x defines PCI_CONF_REG() and PCI_CONF_FUNC() that are problematic
> because PCI core is going to introduce defines with the same names.
>
> Add ORION5X prefix to those defines to avoid name conflicts.
>
> Note: as this is part of series that replaces the code in question
> anyway, only bare minimum renaming is done and other similarly named
> macros are not touched.
>
> Signed-off-by: Ilpo J?rvinen <[email protected]>

Hi Ilpo

What branch do these apply to? I wanted to test them, but i get hunks
rejected:

git am < [email protected]
Applying: ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way
Applying: ARM: orion5x: Use generic PCI Conf Type 1 helper
error: patch failed: arch/arm/mach-orion5x/pci.c:276
error: arch/arm/mach-orion5x/pci.c: patch does not apply
Patch failed at 0002 ARM: orion5x: Use generic PCI Conf Type 1 helper

I tried linux-next, v6.9-rc6, pci:next

Thanks
Andrew

2024-04-29 14:53:25

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH 01/10] ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way

On Mon, 29 Apr 2024, Andrew Lunn wrote:

> On Mon, Apr 29, 2024 at 01:46:24PM +0300, Ilpo J?rvinen wrote:
> > orion5x defines PCI_CONF_REG() and PCI_CONF_FUNC() that are problematic
> > because PCI core is going to introduce defines with the same names.
> >
> > Add ORION5X prefix to those defines to avoid name conflicts.
> >
> > Note: as this is part of series that replaces the code in question
> > anyway, only bare minimum renaming is done and other similarly named
> > macros are not touched.
> >
> > Signed-off-by: Ilpo J?rvinen <[email protected]>
>
> Hi Ilpo
>
> What branch do these apply to? I wanted to test them, but i get hunks
> rejected:
>
> git am < [email protected]
> Applying: ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way
> Applying: ARM: orion5x: Use generic PCI Conf Type 1 helper
> error: patch failed: arch/arm/mach-orion5x/pci.c:276
> error: arch/arm/mach-orion5x/pci.c: patch does not apply
> Patch failed at 0002 ARM: orion5x: Use generic PCI Conf Type 1 helper
>
> I tried linux-next, v6.9-rc6, pci:next

Hi,

On top of pci:main (so v6.9-rc1).

"ARM: orion5x: Use generic PCI Conf Type 1 helper" should be only 4th
patch but your command seems to apply it as 2nd patch (is the mbx file
having them out-of-order?).

--
i.

2024-04-29 15:00:26

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 08/10] PCI: tegra: Use generic PCI Conf Type 1 helper

Convert tegra to use the pci_conf1_ext_addr() helper from PCI core to
calculate PCI Configuration Space address for Type 1 access.

Signed-off-by: Ilpo Järvinen <[email protected]>
---

Note: where mask is changed from 0xff -> 0xfc by this change.
---
drivers/pci/controller/pci-tegra.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 038d974a318e..a86e88c6b87a 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -415,13 +415,6 @@ static inline u32 pads_readl(struct tegra_pcie *pcie, unsigned long offset)
* address (access to which generates correct config transaction) falls in
* this 4 KiB region.
*/
-static unsigned int tegra_pcie_conf_offset(u8 bus, unsigned int devfn,
- unsigned int where)
-{
- return ((where & 0xf00) << 16) | (bus << 16) | (PCI_SLOT(devfn) << 11) |
- (PCI_FUNC(devfn) << 8) | (where & 0xff);
-}
-
static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
unsigned int devfn,
int where)
@@ -440,10 +433,9 @@ static void __iomem *tegra_pcie_map_bus(struct pci_bus *bus,
}
}
} else {
- unsigned int offset;
- u32 base;
+ u32 base, offset;

- offset = tegra_pcie_conf_offset(bus->number, devfn, where);
+ offset = pci_conf1_ext_addr(bus->number, devfn, where, false);

/* move 4 KiB window to offset within the FPCI region */
base = 0xfe100000 + ((offset & ~(SZ_4K - 1)) >> 8);
--
2.39.2


2024-04-29 16:15:11

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 03/10] ARM: orion5x: Pass devfn to orion5x_pci_hw_{rd,wr}_conf()

Pass the usual devfn instead of individual components into
orion5x_pci_hw_{rd,wr}_conf() to make the change into
pci_conf1_offset() in an upcoming commit easier.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
arch/arm/mach-orion5x/pci.c | 45 +++++++++++++++++++------------------
1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
index 77ddab90f448..6376e1db6386 100644
--- a/arch/arm/mach-orion5x/pci.c
+++ b/arch/arm/mach-orion5x/pci.c
@@ -270,15 +270,15 @@ static int orion5x_pci_local_bus_nr(void)
return((conf & PCI_P2P_BUS_MASK) >> PCI_P2P_BUS_OFFS);
}

-static int orion5x_pci_hw_rd_conf(int bus, int dev, u32 func,
- u32 where, u32 size, u32 *val)
+static int orion5x_pci_hw_rd_conf(int bus, u8 devfn, u32 where,
+ u32 size, u32 *val)
{
unsigned long flags;
spin_lock_irqsave(&orion5x_pci_lock, flags);

writel(PCI_CONF_BUS(bus) |
- PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
- ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
+ PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
+ ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);

*val = readl(PCI_CONF_DATA);

@@ -292,8 +292,8 @@ static int orion5x_pci_hw_rd_conf(int bus, int dev, u32 func,
return PCIBIOS_SUCCESSFUL;
}

-static int orion5x_pci_hw_wr_conf(int bus, int dev, u32 func,
- u32 where, u32 size, u32 val)
+static int orion5x_pci_hw_wr_conf(int bus, u8 devfn, u32 where,
+ u32 size, u32 val)
{
unsigned long flags;
int ret = PCIBIOS_SUCCESSFUL;
@@ -301,8 +301,8 @@ static int orion5x_pci_hw_wr_conf(int bus, int dev, u32 func,
spin_lock_irqsave(&orion5x_pci_lock, flags);

writel(PCI_CONF_BUS(bus) |
- PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
- ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
+ PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
+ ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);

if (size == 4) {
__raw_writel(val, PCI_CONF_DATA);
@@ -347,8 +347,7 @@ static int orion5x_pci_rd_conf(struct pci_bus *bus, u32 devfn,
return PCIBIOS_DEVICE_NOT_FOUND;
}

- return orion5x_pci_hw_rd_conf(bus->number, PCI_SLOT(devfn),
- PCI_FUNC(devfn), where, size, val);
+ return orion5x_pci_hw_rd_conf(bus->number, devfn, where, size, val);
}

static int orion5x_pci_wr_conf(struct pci_bus *bus, u32 devfn,
@@ -357,8 +356,7 @@ static int orion5x_pci_wr_conf(struct pci_bus *bus, u32 devfn,
if (!orion5x_pci_valid_config(bus->number, devfn))
return PCIBIOS_DEVICE_NOT_FOUND;

- return orion5x_pci_hw_wr_conf(bus->number, PCI_SLOT(devfn),
- PCI_FUNC(devfn), where, size, val);
+ return orion5x_pci_hw_wr_conf(bus->number, devfn, where, size, val);
}

static struct pci_ops pci_ops = {
@@ -375,12 +373,14 @@ static void __init orion5x_pci_set_bus_nr(int nr)
* PCI-X mode
*/
u32 pcix_status, bus, dev;
+ u8 devfn;
bus = (p2p & PCI_P2P_BUS_MASK) >> PCI_P2P_BUS_OFFS;
dev = (p2p & PCI_P2P_DEV_MASK) >> PCI_P2P_DEV_OFFS;
- orion5x_pci_hw_rd_conf(bus, dev, 0, PCIX_STAT, 4, &pcix_status);
+ devfn = PCI_DEVFN(dev, 0);
+ orion5x_pci_hw_rd_conf(bus, devfn, PCIX_STAT, 4, &pcix_status);
pcix_status &= ~PCIX_STAT_BUS_MASK;
pcix_status |= (nr << PCIX_STAT_BUS_OFFS);
- orion5x_pci_hw_wr_conf(bus, dev, 0, PCIX_STAT, 4, pcix_status);
+ orion5x_pci_hw_wr_conf(bus, devfn, PCIX_STAT, 4, pcix_status);
} else {
/*
* PCI Conventional mode
@@ -393,15 +393,16 @@ static void __init orion5x_pci_set_bus_nr(int nr)

static void __init orion5x_pci_master_slave_enable(void)
{
- int bus_nr, func, reg;
+ int bus_nr, reg;
+ u8 devfn;
u32 val;

bus_nr = orion5x_pci_local_bus_nr();
- func = PCI_CONF_FUNC_STAT_CMD;
+ devfn = PCI_DEVFN(0, PCI_CONF_FUNC_STAT_CMD);
reg = PCI_CONF_REG_STAT_CMD;
- orion5x_pci_hw_rd_conf(bus_nr, 0, func, reg, 4, &val);
+ orion5x_pci_hw_rd_conf(bus_nr, devfn, reg, 4, &val);
val |= (PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
- orion5x_pci_hw_wr_conf(bus_nr, 0, func, reg, 4, val | 0x7);
+ orion5x_pci_hw_wr_conf(bus_nr, devfn, reg, 4, val | 0x7);
}

static void __init orion5x_setup_pci_wins(void)
@@ -424,7 +425,7 @@ static void __init orion5x_setup_pci_wins(void)

for (i = 0; i < dram->num_cs; i++) {
const struct mbus_dram_window *cs = dram->cs + i;
- u32 func = PCI_CONF_FUNC_BAR_CS(cs->cs_index);
+ u8 devfn = PCI_DEVFN(0, PCI_CONF_FUNC_BAR_CS(cs->cs_index));
u32 reg;
u32 val;

@@ -432,15 +433,15 @@ static void __init orion5x_setup_pci_wins(void)
* Write DRAM bank base address register.
*/
reg = PCI_CONF_REG_BAR_LO_CS(cs->cs_index);
- orion5x_pci_hw_rd_conf(bus, 0, func, reg, 4, &val);
+ orion5x_pci_hw_rd_conf(bus, devfn, reg, 4, &val);
val = (cs->base & 0xfffff000) | (val & 0xfff);
- orion5x_pci_hw_wr_conf(bus, 0, func, reg, 4, val);
+ orion5x_pci_hw_wr_conf(bus, devfn, reg, 4, val);

/*
* Write DRAM bank size register.
*/
reg = PCI_CONF_REG_BAR_HI_CS(cs->cs_index);
- orion5x_pci_hw_wr_conf(bus, 0, func, reg, 4, 0);
+ orion5x_pci_hw_wr_conf(bus, devfn, reg, 4, 0);
writel((cs->size - 1) & 0xfffff000,
PCI_BAR_SIZE_DDR_CS(cs->cs_index));
writel(cs->base & 0xfffff000,
--
2.39.2


2024-04-29 16:17:21

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 06/10] PCI: ixp4xx: Replace 1 with PCI_CONF1_TRANSACTION

Add PCI_CONF1_TRANSACTION and replace literal 1 within PCI
Configuration Space Type 1 address with it.

Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/pci/controller/pci-ixp4xx.c | 2 +-
include/linux/pci.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-ixp4xx.c b/drivers/pci/controller/pci-ixp4xx.c
index 44639838df9c..ec0125344ca1 100644
--- a/drivers/pci/controller/pci-ixp4xx.c
+++ b/drivers/pci/controller/pci-ixp4xx.c
@@ -194,7 +194,7 @@ static u32 ixp4xx_config_addr(u8 bus_num, u16 devfn, int where)
/* type 1 */
return (PCI_CONF1_ADDRESS(bus_num, PCI_SLOT(devfn),
PCI_FUNC(devfn), where) &
- ~PCI_CONF1_ENABLE) | 1;
+ ~PCI_CONF1_ENABLE) | PCI_CONF1_TRANSACTION;
}
}

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4c4e3bb52a0a..df613428bc4d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1191,6 +1191,7 @@ void pci_sort_breadthfirst(void);
* See PCI Local Bus Specification, Revision 3.0,
* Section 3.2.2.3.2, Figure 3-1 and 3-2, p. 48-50.
*/
+#define PCI_CONF1_TRANSACTION BIT(0)
#define PCI_CONF_REG 0x000000ffU /* common for Type 0/1 */
#define PCI_CONF_FUNC 0x00000700U /* common for Type 0/1 */
#define PCI_CONF1_DEV 0x0000f800U
--
2.39.2


2024-04-29 19:46:09

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 09/10] PCI: mvebu: Use generic PCI Conf Type 1 helper

On Mon, Apr 29, 2024 at 01:46:32PM +0300, Ilpo J?rvinen wrote:
> Convert mvebu to use the pci_conf1_ext_addr() helper from PCI core
> to calculate PCI Configuration Space address for Type 1 access.
>
> Signed-off-by: Ilpo J?rvinen <[email protected]>

Tested on a kirkwood QNAP and an Armanda XP. The kirkwood correctly
reports there is nothing on the PCIe bus. The XP finds the two 88W8864
WiFi devices, but there is no mainline driver for it, and it also
finds an Etron Technology USB controller, which enumerates O.K.

Tested-by: Andrew Lunn <[email protected]>

Andrew

2024-05-03 08:43:48

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 05/10] PCI: ixp4xx: Use generic PCI Conf Type 0 helper

On Mon, Apr 29, 2024 at 12:47 PM Ilpo Järvinen
<[email protected]> wrote:

> Convert Type 0 address calculation to use pci_conf0_offset() instead of
> abusing PCI_CONF1_ADDRESS().
>
> Signed-off-by: Ilpo Järvinen <[email protected]>

Looks better indeed.
Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2024-05-03 08:44:10

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 06/10] PCI: ixp4xx: Replace 1 with PCI_CONF1_TRANSACTION

On Mon, Apr 29, 2024 at 12:47 PM Ilpo Järvinen
<[email protected]> wrote:

> Add PCI_CONF1_TRANSACTION and replace literal 1 within PCI
> Configuration Space Type 1 address with it.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2024-05-03 08:44:28

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 07/10] PCI: Replace PCI_CONF1{,_EXT}_ADDRESS() with the new helpers

On Mon, Apr 29, 2024 at 12:47 PM Ilpo Järvinen
<[email protected]> wrote:

> Replace the old PCI_CONF1{,_EXT}_ADDRESS() helpers used to calculate
> PCI Configuration Space Type 1 addresses with the new
> pci_conf1{,_ext}_offset() helpers that are more generic and more widely
> available.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2024-05-03 08:44:41

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 10/10] PCI: v3: Use generic PCI Conf Type 0/1 helpers

On Mon, Apr 29, 2024 at 12:48 PM Ilpo Järvinen
<[email protected]> wrote:

> Convert v3 to use pci_conf{0,1}_addr() from PCI core to calculate PCI
> Configuration Space address for Type 0/1 access.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2024-05-03 09:45:18

by Sergio Paracuellos

[permalink] [raw]
Subject: Re: [PATCH 07/10] PCI: Replace PCI_CONF1{,_EXT}_ADDRESS() with the new helpers

Hi,

On Mon, Apr 29, 2024 at 12:47 PM Ilpo Järvinen
<[email protected]> wrote:
>
> Replace the old PCI_CONF1{,_EXT}_ADDRESS() helpers used to calculate
> PCI Configuration Space Type 1 addresses with the new
> pci_conf1{,_ext}_offset() helpers that are more generic and more widely
> available.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
> drivers/pci/controller/pci-ftpci100.c | 6 ++----
> drivers/pci/controller/pci-ixp4xx.c | 5 ++---
> drivers/pci/controller/pcie-mt7621.c | 7 +++----
> drivers/pci/pci.h | 8 --------
> 4 files changed, 7 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-ftpci100.c b/drivers/pci/controller/pci-ftpci100.c
> index ffdeed25e961..a8d0217a0b94 100644
> --- a/drivers/pci/controller/pci-ftpci100.c
> +++ b/drivers/pci/controller/pci-ftpci100.c
> @@ -182,8 +182,7 @@ static int faraday_raw_pci_read_config(struct faraday_pci *p, int bus_number,
> unsigned int fn, int config, int size,
> u32 *value)
> {
> - writel(PCI_CONF1_ADDRESS(bus_number, PCI_SLOT(fn),
> - PCI_FUNC(fn), config),
> + writel(pci_conf1_addr(bus_number, fn, config, true),
> p->base + FTPCI_CONFIG);
>
> *value = readl(p->base + FTPCI_DATA);
> @@ -214,8 +213,7 @@ static int faraday_raw_pci_write_config(struct faraday_pci *p, int bus_number,
> {
> int ret = PCIBIOS_SUCCESSFUL;
>
> - writel(PCI_CONF1_ADDRESS(bus_number, PCI_SLOT(fn),
> - PCI_FUNC(fn), config),
> + writel(pci_conf1_addr(bus_number, fn, config, true),
> p->base + FTPCI_CONFIG);
>
> switch (size) {
> diff --git a/drivers/pci/controller/pci-ixp4xx.c b/drivers/pci/controller/pci-ixp4xx.c
> index ec0125344ca1..fd52f4a3ef31 100644
> --- a/drivers/pci/controller/pci-ixp4xx.c
> +++ b/drivers/pci/controller/pci-ixp4xx.c
> @@ -192,9 +192,8 @@ static u32 ixp4xx_config_addr(u8 bus_num, u16 devfn, int where)
> BIT(32 - PCI_SLOT(devfn));
> } else {
> /* type 1 */
> - return (PCI_CONF1_ADDRESS(bus_num, PCI_SLOT(devfn),
> - PCI_FUNC(devfn), where) &
> - ~PCI_CONF1_ENABLE) | PCI_CONF1_TRANSACTION;
> + return pci_conf1_addr(bus_num, devfn, where, false) |
> + PCI_CONF1_TRANSACTION;
> }
> }
>
> diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
> index 79e225edb42a..2b2d9828a910 100644
> --- a/drivers/pci/controller/pcie-mt7621.c
> +++ b/drivers/pci/controller/pcie-mt7621.c
> @@ -127,8 +127,7 @@ static void __iomem *mt7621_pcie_map_bus(struct pci_bus *bus,
> unsigned int devfn, int where)
> {
> struct mt7621_pcie *pcie = bus->sysdata;
> - u32 address = PCI_CONF1_EXT_ADDRESS(bus->number, PCI_SLOT(devfn),
> - PCI_FUNC(devfn), where);
> + u32 address = pci_conf1_ext_addr(bus->number, devfn, where, true);
>
> writel_relaxed(address, pcie->base + RALINK_PCI_CONFIG_ADDR);
>
> @@ -143,7 +142,7 @@ static struct pci_ops mt7621_pcie_ops = {
>
> static u32 read_config(struct mt7621_pcie *pcie, unsigned int dev, u32 reg)
> {
> - u32 address = PCI_CONF1_EXT_ADDRESS(0, dev, 0, reg);
> + u32 address = pci_conf1_ext_addr(0, PCI_DEVFN(dev, 0), reg, true);
>
> pcie_write(pcie, address, RALINK_PCI_CONFIG_ADDR);
> return pcie_read(pcie, RALINK_PCI_CONFIG_DATA);
> @@ -152,7 +151,7 @@ static u32 read_config(struct mt7621_pcie *pcie, unsigned int dev, u32 reg)
> static void write_config(struct mt7621_pcie *pcie, unsigned int dev,
> u32 reg, u32 val)
> {
> - u32 address = PCI_CONF1_EXT_ADDRESS(0, dev, 0, reg);
> + u32 address = pci_conf1_ext_addr(0, PCI_DEVFN(dev, 0), reg, true);
>
> pcie_write(pcie, address, RALINK_PCI_CONFIG_ADDR);
> pcie_write(pcie, val, RALINK_PCI_CONFIG_DATA);
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index cf0530a60105..fdf9624b0b12 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -833,12 +833,4 @@ struct pci_devres {
>
> struct pci_devres *find_pci_dr(struct pci_dev *pdev);
>
> -#define PCI_CONF1_ADDRESS(bus, dev, func, reg) \
> - (PCI_CONF1_ENABLE | \
> - pci_conf1_addr(bus, PCI_DEVFN(dev, func), reg & ~0x3U))
> -
> -#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \
> - (PCI_CONF1_ENABLE | \
> - pci_conf1_ext_addr(bus, PCI_DEVFN(dev, func), reg & ~0x3U))
> -
> #endif /* DRIVERS_PCI_H */
> --
> 2.39.2
>

I have tested in a GnuBee v1 board based on mt7621 and all PCI
enumeration and so on is working properly. Hence, for MT7621:

Acked-by: Sergio Paracuellos <[email protected]>
Tested-by: Sergio Paracuellos <[email protected]>

Thanks,
Sergio Paracuellos

2024-05-05 16:38:41

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH 01/10] ARM: orion5x: Rename PCI_CONF_{REG,FUNC}() out of the way

Ilpo Järvinen <[email protected]> writes:

> orion5x defines PCI_CONF_REG() and PCI_CONF_FUNC() that are problematic
> because PCI core is going to introduce defines with the same names.
>
> Add ORION5X prefix to those defines to avoid name conflicts.
>
> Note: as this is part of series that replaces the code in question
> anyway, only bare minimum renaming is done and other similarly named
> macros are not touched.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>

Acked-by: Gregory CLEMENT <[email protected]>

As some other patches of the series depend on patches in the PCIe
subsystem, the best approach would be to let you apply the series
through the PCIe subsystem.

Thanks,

Gregory
> ---
> arch/arm/mach-orion5x/pci.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
> index 3313bc5a63ea..77ddab90f448 100644
> --- a/arch/arm/mach-orion5x/pci.c
> +++ b/arch/arm/mach-orion5x/pci.c
> @@ -219,8 +219,8 @@ static int __init pcie_setup(struct pci_sys_data *sys)
> /*
> * PCI_CONF_ADDR bits
> */
> -#define PCI_CONF_REG(reg) ((reg) & 0xfc)
> -#define PCI_CONF_FUNC(func) (((func) & 0x3) << 8)
> +#define ORION5X_PCI_CONF_REG(reg) ((reg) & 0xfc)
> +#define ORION5X_PCI_CONF_FUNC(func) (((func) & 0x3) << 8)
> #define PCI_CONF_DEV(dev) (((dev) & 0x1f) << 11)
> #define PCI_CONF_BUS(bus) (((bus) & 0xff) << 16)
> #define PCI_CONF_ADDR_EN (1 << 31)
> @@ -277,8 +277,8 @@ static int orion5x_pci_hw_rd_conf(int bus, int dev, u32 func,
> spin_lock_irqsave(&orion5x_pci_lock, flags);
>
> writel(PCI_CONF_BUS(bus) |
> - PCI_CONF_DEV(dev) | PCI_CONF_REG(where) |
> - PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
> + PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
> + ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
>
> *val = readl(PCI_CONF_DATA);
>
> @@ -301,8 +301,8 @@ static int orion5x_pci_hw_wr_conf(int bus, int dev, u32 func,
> spin_lock_irqsave(&orion5x_pci_lock, flags);
>
> writel(PCI_CONF_BUS(bus) |
> - PCI_CONF_DEV(dev) | PCI_CONF_REG(where) |
> - PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
> + PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
> + ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
>
> if (size == 4) {
> __raw_writel(val, PCI_CONF_DATA);
> --
> 2.39.2

2024-05-05 16:39:17

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH 04/10] ARM: orion5x: Use generic PCI Conf Type 1 helper

Ilpo Järvinen <[email protected]> writes:

> Convert orion5x PCI code to use pci_conf1_ext_addr() from PCI core to
> calculate PCI Configuration Type 1 address.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>

Acked-by: Gregory CLEMENT <[email protected]>

As some other patches of the series depend on patches in the PCIe
subsystem, the best approach would be to let you apply the series
through the PCIe subsystem.

Thanks,

Gregory

> ---
> arch/arm/mach-orion5x/pci.c | 17 ++---------------
> 1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
> index 6376e1db6386..8b7d67549adf 100644
> --- a/arch/arm/mach-orion5x/pci.c
> +++ b/arch/arm/mach-orion5x/pci.c
> @@ -216,15 +216,6 @@ static int __init pcie_setup(struct pci_sys_data *sys)
> #define PCI_P2P_DEV_OFFS 24
> #define PCI_P2P_DEV_MASK (0x1f << PCI_P2P_DEV_OFFS)
>
> -/*
> - * PCI_CONF_ADDR bits
> - */
> -#define ORION5X_PCI_CONF_REG(reg) ((reg) & 0xfc)
> -#define ORION5X_PCI_CONF_FUNC(func) (((func) & 0x3) << 8)
> -#define PCI_CONF_DEV(dev) (((dev) & 0x1f) << 11)
> -#define PCI_CONF_BUS(bus) (((bus) & 0xff) << 16)
> -#define PCI_CONF_ADDR_EN (1 << 31)
> -
> /*
> * Internal configuration space
> */
> @@ -276,9 +267,7 @@ static int orion5x_pci_hw_rd_conf(int bus, u8 devfn, u32 where,
> unsigned long flags;
> spin_lock_irqsave(&orion5x_pci_lock, flags);
>
> - writel(PCI_CONF_BUS(bus) |
> - PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
> - ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
> + writel(pci_conf1_addr(bus, devfn, where, true), PCI_CONF_ADDR);
>
> *val = readl(PCI_CONF_DATA);
>
> @@ -300,9 +289,7 @@ static int orion5x_pci_hw_wr_conf(int bus, u8 devfn, u32 where,
>
> spin_lock_irqsave(&orion5x_pci_lock, flags);
>
> - writel(PCI_CONF_BUS(bus) |
> - PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
> - ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
> + writel(pci_conf1_addr(bus, devfn, where, true), PCI_CONF_ADDR);
>
> if (size == 4) {
> __raw_writel(val, PCI_CONF_DATA);
> --
> 2.39.2

2024-05-05 16:39:37

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH 03/10] ARM: orion5x: Pass devfn to orion5x_pci_hw_{rd,wr}_conf()

Ilpo Järvinen <[email protected]> writes:

> Pass the usual devfn instead of individual components into
> orion5x_pci_hw_{rd,wr}_conf() to make the change into
> pci_conf1_offset() in an upcoming commit easier.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>


Acked-by: Gregory CLEMENT <[email protected]>

As some other patches of the series depend on patches in the PCIe
subsystem, the best approach would be to let you apply the series
through the PCIe subsystem.

Thanks,

Gregory


> ---
> arch/arm/mach-orion5x/pci.c | 45 +++++++++++++++++++------------------
> 1 file changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
> index 77ddab90f448..6376e1db6386 100644
> --- a/arch/arm/mach-orion5x/pci.c
> +++ b/arch/arm/mach-orion5x/pci.c
> @@ -270,15 +270,15 @@ static int orion5x_pci_local_bus_nr(void)
> return((conf & PCI_P2P_BUS_MASK) >> PCI_P2P_BUS_OFFS);
> }
>
> -static int orion5x_pci_hw_rd_conf(int bus, int dev, u32 func,
> - u32 where, u32 size, u32 *val)
> +static int orion5x_pci_hw_rd_conf(int bus, u8 devfn, u32 where,
> + u32 size, u32 *val)
> {
> unsigned long flags;
> spin_lock_irqsave(&orion5x_pci_lock, flags);
>
> writel(PCI_CONF_BUS(bus) |
> - PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
> - ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
> + PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
> + ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
>
> *val = readl(PCI_CONF_DATA);
>
> @@ -292,8 +292,8 @@ static int orion5x_pci_hw_rd_conf(int bus, int dev, u32 func,
> return PCIBIOS_SUCCESSFUL;
> }
>
> -static int orion5x_pci_hw_wr_conf(int bus, int dev, u32 func,
> - u32 where, u32 size, u32 val)
> +static int orion5x_pci_hw_wr_conf(int bus, u8 devfn, u32 where,
> + u32 size, u32 val)
> {
> unsigned long flags;
> int ret = PCIBIOS_SUCCESSFUL;
> @@ -301,8 +301,8 @@ static int orion5x_pci_hw_wr_conf(int bus, int dev, u32 func,
> spin_lock_irqsave(&orion5x_pci_lock, flags);
>
> writel(PCI_CONF_BUS(bus) |
> - PCI_CONF_DEV(dev) | ORION5X_PCI_CONF_REG(where) |
> - ORION5X_PCI_CONF_FUNC(func) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
> + PCI_CONF_DEV(PCI_SLOT(devfn)) | ORION5X_PCI_CONF_REG(where) |
> + ORION5X_PCI_CONF_FUNC(PCI_FUNC(devfn)) | PCI_CONF_ADDR_EN, PCI_CONF_ADDR);
>
> if (size == 4) {
> __raw_writel(val, PCI_CONF_DATA);
> @@ -347,8 +347,7 @@ static int orion5x_pci_rd_conf(struct pci_bus *bus, u32 devfn,
> return PCIBIOS_DEVICE_NOT_FOUND;
> }
>
> - return orion5x_pci_hw_rd_conf(bus->number, PCI_SLOT(devfn),
> - PCI_FUNC(devfn), where, size, val);
> + return orion5x_pci_hw_rd_conf(bus->number, devfn, where, size, val);
> }
>
> static int orion5x_pci_wr_conf(struct pci_bus *bus, u32 devfn,
> @@ -357,8 +356,7 @@ static int orion5x_pci_wr_conf(struct pci_bus *bus, u32 devfn,
> if (!orion5x_pci_valid_config(bus->number, devfn))
> return PCIBIOS_DEVICE_NOT_FOUND;
>
> - return orion5x_pci_hw_wr_conf(bus->number, PCI_SLOT(devfn),
> - PCI_FUNC(devfn), where, size, val);
> + return orion5x_pci_hw_wr_conf(bus->number, devfn, where, size, val);
> }
>
> static struct pci_ops pci_ops = {
> @@ -375,12 +373,14 @@ static void __init orion5x_pci_set_bus_nr(int nr)
> * PCI-X mode
> */
> u32 pcix_status, bus, dev;
> + u8 devfn;
> bus = (p2p & PCI_P2P_BUS_MASK) >> PCI_P2P_BUS_OFFS;
> dev = (p2p & PCI_P2P_DEV_MASK) >> PCI_P2P_DEV_OFFS;
> - orion5x_pci_hw_rd_conf(bus, dev, 0, PCIX_STAT, 4, &pcix_status);
> + devfn = PCI_DEVFN(dev, 0);
> + orion5x_pci_hw_rd_conf(bus, devfn, PCIX_STAT, 4, &pcix_status);
> pcix_status &= ~PCIX_STAT_BUS_MASK;
> pcix_status |= (nr << PCIX_STAT_BUS_OFFS);
> - orion5x_pci_hw_wr_conf(bus, dev, 0, PCIX_STAT, 4, pcix_status);
> + orion5x_pci_hw_wr_conf(bus, devfn, PCIX_STAT, 4, pcix_status);
> } else {
> /*
> * PCI Conventional mode
> @@ -393,15 +393,16 @@ static void __init orion5x_pci_set_bus_nr(int nr)
>
> static void __init orion5x_pci_master_slave_enable(void)
> {
> - int bus_nr, func, reg;
> + int bus_nr, reg;
> + u8 devfn;
> u32 val;
>
> bus_nr = orion5x_pci_local_bus_nr();
> - func = PCI_CONF_FUNC_STAT_CMD;
> + devfn = PCI_DEVFN(0, PCI_CONF_FUNC_STAT_CMD);
> reg = PCI_CONF_REG_STAT_CMD;
> - orion5x_pci_hw_rd_conf(bus_nr, 0, func, reg, 4, &val);
> + orion5x_pci_hw_rd_conf(bus_nr, devfn, reg, 4, &val);
> val |= (PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> - orion5x_pci_hw_wr_conf(bus_nr, 0, func, reg, 4, val | 0x7);
> + orion5x_pci_hw_wr_conf(bus_nr, devfn, reg, 4, val | 0x7);
> }
>
> static void __init orion5x_setup_pci_wins(void)
> @@ -424,7 +425,7 @@ static void __init orion5x_setup_pci_wins(void)
>
> for (i = 0; i < dram->num_cs; i++) {
> const struct mbus_dram_window *cs = dram->cs + i;
> - u32 func = PCI_CONF_FUNC_BAR_CS(cs->cs_index);
> + u8 devfn = PCI_DEVFN(0, PCI_CONF_FUNC_BAR_CS(cs->cs_index));
> u32 reg;
> u32 val;
>
> @@ -432,15 +433,15 @@ static void __init orion5x_setup_pci_wins(void)
> * Write DRAM bank base address register.
> */
> reg = PCI_CONF_REG_BAR_LO_CS(cs->cs_index);
> - orion5x_pci_hw_rd_conf(bus, 0, func, reg, 4, &val);
> + orion5x_pci_hw_rd_conf(bus, devfn, reg, 4, &val);
> val = (cs->base & 0xfffff000) | (val & 0xfff);
> - orion5x_pci_hw_wr_conf(bus, 0, func, reg, 4, val);
> + orion5x_pci_hw_wr_conf(bus, devfn, reg, 4, val);
>
> /*
> * Write DRAM bank size register.
> */
> reg = PCI_CONF_REG_BAR_HI_CS(cs->cs_index);
> - orion5x_pci_hw_wr_conf(bus, 0, func, reg, 4, 0);
> + orion5x_pci_hw_wr_conf(bus, devfn, reg, 4, 0);
> writel((cs->size - 1) & 0xfffff000,
> PCI_BAR_SIZE_DDR_CS(cs->cs_index));
> writel(cs->base & 0xfffff000,
> --
> 2.39.2