PCI: ARM/ARM64: remove pci_fixup_irqs() usage

Current iproc driver host bridge controller driver requires struct pci_bus to be created in order to carry out PCI link checks with standard PCI config space accessors.

This struct pci_bus dependency is fictitious and burdens the driver with unneeded constraints (eg to use separate APIs to create and scan the root bus).

Add PCI raw config space accessors to PCIe iproc driver and remove the fictitious struct pci_bus dependency.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Scott Branden <[email protected]>

Cc Ray Jui <[email protected]>

Cc Jon Mason <[email protected]>

---
  drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
  1 file changed, 74 insertions(+), 20 deletions(-)

drivers/pci/host/pcie-iproc.c +74 -20

index 0f39bd2..e48b8e2 100644
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
   * Note access to the configuration registers are protected at the higher layer
   * by 'pci_lock' in drivers/pci/access.c
   */
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+					    int busno,
  					    unsigned int devfn,
  					    int where)
  {
-	struct iproc_pcie *pcie = iproc_data(bus);
  	unsigned slot = PCI_SLOT(devfn);
  	unsigned fn = PCI_FUNC(devfn);
-	unsigned busno = bus->number;
  	u32 val;
  	u16 offset;
 

@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
  		return (pcie->base + offset);
  }
 
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+						unsigned int devfn,
+						int where)
+{
+	return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+				      where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+				       unsigned int devfn, int where,
+				       int size, u32 *val)
+{
+	void __iomem *addr;
+
+	addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	*val = readl(addr);
+
+	if (size <= 2)
+		*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+					unsigned int devfn, int where,
+					int size, u32 val)
+{
+	void __iomem *addr;
+	u32 mask, tmp;
+
+	addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+	if (!addr)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (size == 4) {
+		writel(val, addr);
+		return PCIBIOS_SUCCESSFUL;
+	}
+
+	mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+	tmp = readl(addr) & mask;
+	tmp |= val << ((where & 0x3) * 8);
+	writel(tmp, addr);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
  static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
  				    int where, int size, u32 *val)
  {

@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
  }
 
  static struct pci_ops iproc_pcie_ops = {
-	.map_bus = iproc_pcie_map_cfg_bus,
+	.map_bus = iproc_pcie_bus_map_cfg_bus,
  	.read = iproc_pcie_config_read32,
  	.write = iproc_pcie_config_write32,
  };

@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
  	msleep(100);
  }
 
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
  {
  	struct device *dev = pcie->dev;
-	u8 hdr_type;
-	u32 link_ctrl, class, val;
-	u16 pos = PCI_EXP_CAP, link_status;
+	u32 hdr_type, link_ctrl, link_status, class, val;
+	u16 pos = PCI_EXP_CAP;
  	bool link_is_active = false;
 
  	/*

@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
  	}
 
  	/* make sure we are not in EP mode */
-	pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+	iproc_pci_raw_config_read32(pcie,  0, PCI_HEADER_TYPE, 1, &hdr_type);
  	if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
  		dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
  		return -EFAULT;

@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
  #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
  #define PCI_CLASS_BRIDGE_MASK      0xffff00
  #define PCI_CLASS_BRIDGE_SHIFT     8
-	pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+	iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+				    4, &class);
  	class &= ~PCI_CLASS_BRIDGE_MASK;
  	class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
-	pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+	iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+				     4, class);
 
  	/* check link status to see if link is active */
-	pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+	iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+				    2, &link_status);
  	if (link_status & PCI_EXP_LNKSTA_NLW)
  		link_is_active = true;
 

@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
  #define PCI_TARGET_LINK_SPEED_MASK    0xf
  #define PCI_TARGET_LINK_SPEED_GEN2    0x2
  #define PCI_TARGET_LINK_SPEED_GEN1    0x1
-		pci_bus_read_config_dword(bus, 0,
-					  pos + PCI_EXP_LNKCTL2,
+		iproc_pci_raw_config_read32(pcie, 0,
+					  pos + PCI_EXP_LNKCTL2, 4,
  					  &link_ctrl);
  		if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
  		    PCI_TARGET_LINK_SPEED_GEN2) {
  			link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
  			link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
-			pci_bus_write_config_dword(bus, 0,
-					   pos + PCI_EXP_LNKCTL2,
-					   link_ctrl);
+			iproc_pci_raw_config_write32(pcie, 0,
+						pos + PCI_EXP_LNKCTL2,
+						4, link_ctrl);
  			msleep(100);
 
-			pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
-						 &link_status);
+			iproc_pci_raw_config_read32(pcie, 0,
+						pos + PCI_EXP_LNKSTA,
+						2, &link_status);
  			if (link_status & PCI_EXP_LNKSTA_NLW)
  				link_is_active = true;
  		}

@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
  	}
  	pcie->root_bus = bus;
 
-	ret = iproc_pcie_check_link(pcie, bus);
+	ret = iproc_pcie_check_link(pcie);
  	if (ret) {
  		dev_err(dev, "no PCIe EP device detected\n");
  		goto err_rm_root_bus;

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

Thanks,

Ray

Quoted 181 lines · Lorenzo Pieralisi

Current iproc driver host bridge controller driver requires struct pci_bus to be created in order to carry out PCI link checks with standard PCI config space accessors.

This struct pci_bus dependency is fictitious and burdens the driver with unneeded constraints (eg to use separate APIs to create and scan the root bus).

Add PCI raw config space accessors to PCIe iproc driver and remove the fictitious struct pci_bus dependency.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Scott Branden <[email protected]>

Cc Ray Jui <[email protected]>

Cc Jon Mason <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 74 insertions(+), 20 deletions(-)

drivers/pci/host/pcie-iproc.c +74 -20

index 0f39bd2..e48b8e2 100644
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
  * Note access to the configuration registers are protected at the higher layer
  * by 'pci_lock' in drivers/pci/access.c
  */
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+					    int busno,
 					    unsigned int devfn,
 					    int where)
 {
-	struct iproc_pcie *pcie = iproc_data(bus);
 	unsigned slot = PCI_SLOT(devfn);
 	unsigned fn = PCI_FUNC(devfn);
-	unsigned busno = bus->number;
 	u32 val;
 	u16 offset;

@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
 		return (pcie->base + offset);
 }
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+						unsigned int devfn,
+						int where)
+{
+	return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+				      where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+				       unsigned int devfn, int where,
+				       int size, u32 *val)
+{
+	void __iomem *addr;
+
+	addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+	if (!addr) {
+		*val = ~0;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	*val = readl(addr);
+
+	if (size <= 2)
+		*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+					unsigned int devfn, int where,
+					int size, u32 val)
+{
+	void __iomem *addr;
+	u32 mask, tmp;
+
+	addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+	if (!addr)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (size == 4) {
+		writel(val, addr);
+		return PCIBIOS_SUCCESSFUL;
+	}
+
+	mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+	tmp = readl(addr) & mask;
+	tmp |= val << ((where & 0x3) * 8);
+	writel(tmp, addr);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
 static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
 				    int where, int size, u32 *val)
 {

@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
 }
 static struct pci_ops iproc_pcie_ops = {
-	.map_bus = iproc_pcie_map_cfg_bus,
+	.map_bus = iproc_pcie_bus_map_cfg_bus,
 	.read = iproc_pcie_config_read32,
 	.write = iproc_pcie_config_write32,
 };

@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
 	msleep(100);
 }
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
-	u8 hdr_type;
-	u32 link_ctrl, class, val;
-	u16 pos = PCI_EXP_CAP, link_status;
+	u32 hdr_type, link_ctrl, link_status, class, val;
+	u16 pos = PCI_EXP_CAP;
 	bool link_is_active = false;
 	/*

@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 	}
 	/* make sure we are not in EP mode */
-	pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+	iproc_pci_raw_config_read32(pcie,  0, PCI_HEADER_TYPE, 1, &hdr_type);
 	if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
 		dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
 		return -EFAULT;

@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
 #define PCI_CLASS_BRIDGE_MASK      0xffff00
 #define PCI_CLASS_BRIDGE_SHIFT     8
-	pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+	iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+				    4, &class);
 	class &= ~PCI_CLASS_BRIDGE_MASK;
 	class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
-	pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+	iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+				     4, class);
 	/* check link status to see if link is active */
-	pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+	iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+				    2, &link_status);
 	if (link_status & PCI_EXP_LNKSTA_NLW)
 		link_is_active = true;

@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_TARGET_LINK_SPEED_MASK    0xf
 #define PCI_TARGET_LINK_SPEED_GEN2    0x2
 #define PCI_TARGET_LINK_SPEED_GEN1    0x1
-		pci_bus_read_config_dword(bus, 0,
-					  pos + PCI_EXP_LNKCTL2,
+		iproc_pci_raw_config_read32(pcie, 0,
+					  pos + PCI_EXP_LNKCTL2, 4,
 					  &link_ctrl);
 		if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
 		    PCI_TARGET_LINK_SPEED_GEN2) {
 			link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
 			link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
-			pci_bus_write_config_dword(bus, 0,
-					   pos + PCI_EXP_LNKCTL2,
-					   link_ctrl);
+			iproc_pci_raw_config_write32(pcie, 0,
+						pos + PCI_EXP_LNKCTL2,
+						4, link_ctrl);
 			msleep(100);
-			pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
-						 &link_status);
+			iproc_pci_raw_config_read32(pcie, 0,
+						pos + PCI_EXP_LNKSTA,
+						2, &link_status);
 			if (link_status & PCI_EXP_LNKSTA_NLW)
 				link_is_active = true;
 		}

@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
 	}
 	pcie->root_bus = bus;
-	ret = iproc_pcie_check_link(pcie, bus);
+	ret = iproc_pcie_check_link(pcie);
 	if (ret) {
 		dev_err(dev, "no PCIe EP device detected\n");
 		goto err_rm_root_bus;

[dropped rock-chips maintainers, email bounces]

Quoted 9 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

Quoted 1 line

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Thanks for testing it, please let me know.

Thank you !
Lorenzo

Quoted 180 lines

Thanks,

Ray

Quoted 173 lines · Lorenzo Pieralisi

Current iproc driver host bridge controller driver requires struct pci_bus to be created in order to carry out PCI link checks with standard PCI config space accessors.

This struct pci_bus dependency is fictitious and burdens the driver with unneeded constraints (eg to use separate APIs to create and scan the root bus).

Add PCI raw config space accessors to PCIe iproc driver and remove the fictitious struct pci_bus dependency.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Scott Branden <[email protected]>

Cc Ray Jui <[email protected]>

Cc Jon Mason <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 74 insertions(+), 20 deletions(-)

drivers/pci/host/pcie-iproc.c +73 -19

index 0f39bd2..e48b8e2 100644
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
  * Note access to the configuration registers are protected at the higher layer
  * by 'pci_lock' in drivers/pci/access.c
  */
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+                                        int busno,
                                        unsigned int devfn,
                                        int where)
 {
-    struct iproc_pcie *pcie = iproc_data(bus);
    unsigned slot = PCI_SLOT(devfn);
    unsigned fn = PCI_FUNC(devfn);
-    unsigned busno = bus->number;
    u32 val;
    u16 offset;

@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
            return (pcie->base + offset);
 }
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+                                            unsigned int devfn,
+                                            int where)
+{
+    return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+                                  where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+                                   unsigned int devfn, int where,
+                                   int size, u32 *val)
+{
+    void __iomem *addr;
+
+    addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+    if (!addr) {
+            *val = ~0;
+            return PCIBIOS_DEVICE_NOT_FOUND;
+    }
+
+    *val = readl(addr);
+
+    if (size <= 2)
+            *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+    return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+                                    unsigned int devfn, int where,
+                                    int size, u32 val)
+{
+    void __iomem *addr;
+    u32 mask, tmp;
+
+    addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+    if (!addr)
+            return PCIBIOS_DEVICE_NOT_FOUND;
+
+    if (size == 4) {
+            writel(val, addr);
+            return PCIBIOS_SUCCESSFUL;
+    }
+
+    mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+    tmp = readl(addr) & mask;
+    tmp |= val << ((where & 0x3) * 8);
+    writel(tmp, addr);
+
+    return PCIBIOS_SUCCESSFUL;
+}
+
 static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
                                int where, int size, u32 *val)
 {

@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
 }
 static struct pci_ops iproc_pcie_ops = {
-    .map_bus = iproc_pcie_map_cfg_bus,
+    .map_bus = iproc_pcie_bus_map_cfg_bus,
    .read = iproc_pcie_config_read32,
    .write = iproc_pcie_config_write32,
 };

@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
    msleep(100);
 }
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
 {
    struct device *dev = pcie->dev;
-    u8 hdr_type;
-    u32 link_ctrl, class, val;
-    u16 pos = PCI_EXP_CAP, link_status;
+    u32 hdr_type, link_ctrl, link_status, class, val;
+    u16 pos = PCI_EXP_CAP;
    bool link_is_active = false;
    /*

@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
    }
    /* make sure we are not in EP mode */
-    pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+    iproc_pci_raw_config_read32(pcie,  0, PCI_HEADER_TYPE, 1, &hdr_type);
    if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
            dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
            return -EFAULT;

@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
 #define PCI_CLASS_BRIDGE_MASK      0xffff00
 #define PCI_CLASS_BRIDGE_SHIFT     8
-    pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+    iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+                                4, &class);
    class &= ~PCI_CLASS_BRIDGE_MASK;
    class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
-    pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+    iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+                                 4, class);
    /* check link status to see if link is active */
-    pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+    iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+                                2, &link_status);
    if (link_status & PCI_EXP_LNKSTA_NLW)
            link_is_active = true;

@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_TARGET_LINK_SPEED_MASK    0xf
 #define PCI_TARGET_LINK_SPEED_GEN2    0x2
 #define PCI_TARGET_LINK_SPEED_GEN1    0x1
-            pci_bus_read_config_dword(bus, 0,
-                                      pos + PCI_EXP_LNKCTL2,
+            iproc_pci_raw_config_read32(pcie, 0,
+                                      pos + PCI_EXP_LNKCTL2, 4,
                                      &link_ctrl);
            if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
                PCI_TARGET_LINK_SPEED_GEN2) {
                    link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
                    link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
-                    pci_bus_write_config_dword(bus, 0,
-                                       pos + PCI_EXP_LNKCTL2,
-                                       link_ctrl);
+                    iproc_pci_raw_config_write32(pcie, 0,
+                                            pos + PCI_EXP_LNKCTL2,
+                                            4, link_ctrl);
                    msleep(100);
-                    pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
-                                             &link_status);
+                    iproc_pci_raw_config_read32(pcie, 0,
+                                            pos + PCI_EXP_LNKSTA,
+                                            2, &link_status);
                    if (link_status & PCI_EXP_LNKSTA_NLW)
                            link_is_active = true;
            }

Quoted 9 lines

drivers/pci/host/pcie-iproc.c int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) · line 1319

Quoted 8 lines

drivers/pci/host/pcie-iproc.c +1 -1

@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
    }
    pcie->root_bus = bus;
-    ret = iproc_pcie_check_link(pcie, bus);
+    ret = iproc_pcie_check_link(pcie);
    if (ret) {
            dev_err(dev, "no PCIe EP device detected\n");
            goto err_rm_root_bus;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 14 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

Quoted 3 lines

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Hi Bjorn,

please help to review following changes, to avoid merge conflicts since Lorenzo's changes in iproc driver is touching basic config APIs.

[PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP [PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC

Regards,
Oza.

Quoted 200 lines

Thanks for testing it, please let me know.

Thank you !
Lorenzo

Quoted 180 lines

Thanks,

Ray

Quoted 173 lines · Lorenzo Pieralisi

Current iproc driver host bridge controller driver requires struct pci_bus to be created in order to carry out PCI link checks with standard PCI config space accessors.

This struct pci_bus dependency is fictitious and burdens the driver with unneeded constraints (eg to use separate APIs to create and scan the root bus).

Add PCI raw config space accessors to PCIe iproc driver and remove the fictitious struct pci_bus dependency.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Scott Branden <[email protected]>

Cc Ray Jui <[email protected]>

Cc Jon Mason <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 74 insertions(+), 20 deletions(-)

drivers/pci/host/pcie-iproc.c +73 -19

index 0f39bd2..e48b8e2 100644
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
  * Note access to the configuration registers are protected at the higher layer
  * by 'pci_lock' in drivers/pci/access.c
  */
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+                                        int busno,
                                        unsigned int devfn,
                                        int where)
 {
-    struct iproc_pcie *pcie = iproc_data(bus);
    unsigned slot = PCI_SLOT(devfn);
    unsigned fn = PCI_FUNC(devfn);
-    unsigned busno = bus->number;
    u32 val;
    u16 offset;

@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
            return (pcie->base + offset);
 }
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+                                            unsigned int devfn,
+                                            int where)
+{
+    return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+                                  where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+                                   unsigned int devfn, int where,
+                                   int size, u32 *val)
+{
+    void __iomem *addr;
+
+    addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+    if (!addr) {
+            *val = ~0;
+            return PCIBIOS_DEVICE_NOT_FOUND;
+    }
+
+    *val = readl(addr);
+
+    if (size <= 2)
+            *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+    return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+                                    unsigned int devfn, int where,
+                                    int size, u32 val)
+{
+    void __iomem *addr;
+    u32 mask, tmp;
+
+    addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+    if (!addr)
+            return PCIBIOS_DEVICE_NOT_FOUND;
+
+    if (size == 4) {
+            writel(val, addr);
+            return PCIBIOS_SUCCESSFUL;
+    }
+
+    mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+    tmp = readl(addr) & mask;
+    tmp |= val << ((where & 0x3) * 8);
+    writel(tmp, addr);
+
+    return PCIBIOS_SUCCESSFUL;
+}
+
 static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
                                int where, int size, u32 *val)
 {

@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
 }
 static struct pci_ops iproc_pcie_ops = {
-    .map_bus = iproc_pcie_map_cfg_bus,
+    .map_bus = iproc_pcie_bus_map_cfg_bus,
    .read = iproc_pcie_config_read32,
    .write = iproc_pcie_config_write32,
 };

@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
    msleep(100);
 }
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
 {
    struct device *dev = pcie->dev;
-    u8 hdr_type;
-    u32 link_ctrl, class, val;
-    u16 pos = PCI_EXP_CAP, link_status;
+    u32 hdr_type, link_ctrl, link_status, class, val;
+    u16 pos = PCI_EXP_CAP;
    bool link_is_active = false;
    /*

@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
    }
    /* make sure we are not in EP mode */
-    pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+    iproc_pci_raw_config_read32(pcie,  0, PCI_HEADER_TYPE, 1, &hdr_type);
    if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
            dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
            return -EFAULT;

@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
 #define PCI_CLASS_BRIDGE_MASK      0xffff00
 #define PCI_CLASS_BRIDGE_SHIFT     8
-    pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+    iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+                                4, &class);
    class &= ~PCI_CLASS_BRIDGE_MASK;
    class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
-    pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+    iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+                                 4, class);
    /* check link status to see if link is active */
-    pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+    iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+                                2, &link_status);
    if (link_status & PCI_EXP_LNKSTA_NLW)
            link_is_active = true;

@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_TARGET_LINK_SPEED_MASK    0xf
 #define PCI_TARGET_LINK_SPEED_GEN2    0x2
 #define PCI_TARGET_LINK_SPEED_GEN1    0x1
-            pci_bus_read_config_dword(bus, 0,
-                                      pos + PCI_EXP_LNKCTL2,
+            iproc_pci_raw_config_read32(pcie, 0,
+                                      pos + PCI_EXP_LNKCTL2, 4,
                                      &link_ctrl);
            if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
                PCI_TARGET_LINK_SPEED_GEN2) {
                    link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
                    link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
-                    pci_bus_write_config_dword(bus, 0,
-                                       pos + PCI_EXP_LNKCTL2,
-                                       link_ctrl);
+                    iproc_pci_raw_config_write32(pcie, 0,
+                                            pos + PCI_EXP_LNKCTL2,
+                                            4, link_ctrl);
                    msleep(100);
-                    pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
-                                             &link_status);
+                    iproc_pci_raw_config_read32(pcie, 0,
+                                            pos + PCI_EXP_LNKSTA,
+                                            2, &link_status);
                    if (link_status & PCI_EXP_LNKSTA_NLW)
                            link_is_active = true;
            }

Quoted 9 lines

drivers/pci/host/pcie-iproc.c int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) · line 1319

Quoted 8 lines

drivers/pci/host/pcie-iproc.c +1 -1

@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
    }
    pcie->root_bus = bus;
-    ret = iproc_pcie_check_link(pcie, bus);
+    ret = iproc_pcie_check_link(pcie);
    if (ret) {
            dev_err(dev, "no PCIe EP device detected\n");
            goto err_rm_root_bus;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

Thanks a lot,
Lorenzo

Quoted 213 lines

Hi Bjorn,

please help to review following changes, to avoid merge conflicts since Lorenzo's changes in iproc driver is touching basic config APIs.

[PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP [PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC

Regards,
Oza.

Quoted 200 lines

Thanks for testing it, please let me know.

Thank you !
Lorenzo

Quoted 180 lines

Thanks,

Ray

Quoted 173 lines · Lorenzo Pieralisi

Current iproc driver host bridge controller driver requires struct pci_bus to be created in order to carry out PCI link checks with standard PCI config space accessors.

This struct pci_bus dependency is fictitious and burdens the driver with unneeded constraints (eg to use separate APIs to create and scan the root bus).

Add PCI raw config space accessors to PCIe iproc driver and remove the fictitious struct pci_bus dependency.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Scott Branden <[email protected]>

Cc Ray Jui <[email protected]>

Cc Jon Mason <[email protected]>

---
 drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 74 insertions(+), 20 deletions(-)

drivers/pci/host/pcie-iproc.c +73 -19

index 0f39bd2..e48b8e2 100644
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
  * Note access to the configuration registers are protected at the higher layer
  * by 'pci_lock' in drivers/pci/access.c
  */
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+                                        int busno,
                                        unsigned int devfn,
                                        int where)
 {
-    struct iproc_pcie *pcie = iproc_data(bus);
    unsigned slot = PCI_SLOT(devfn);
    unsigned fn = PCI_FUNC(devfn);
-    unsigned busno = bus->number;
    u32 val;
    u16 offset;

@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
            return (pcie->base + offset);
 }
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+                                            unsigned int devfn,
+                                            int where)
+{
+    return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+                                  where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+                                   unsigned int devfn, int where,
+                                   int size, u32 *val)
+{
+    void __iomem *addr;
+
+    addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+    if (!addr) {
+            *val = ~0;
+            return PCIBIOS_DEVICE_NOT_FOUND;
+    }
+
+    *val = readl(addr);
+
+    if (size <= 2)
+            *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+    return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+                                    unsigned int devfn, int where,
+                                    int size, u32 val)
+{
+    void __iomem *addr;
+    u32 mask, tmp;
+
+    addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+    if (!addr)
+            return PCIBIOS_DEVICE_NOT_FOUND;
+
+    if (size == 4) {
+            writel(val, addr);
+            return PCIBIOS_SUCCESSFUL;
+    }
+
+    mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+    tmp = readl(addr) & mask;
+    tmp |= val << ((where & 0x3) * 8);
+    writel(tmp, addr);
+
+    return PCIBIOS_SUCCESSFUL;
+}
+
 static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
                                int where, int size, u32 *val)
 {

@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
 }
 static struct pci_ops iproc_pcie_ops = {
-    .map_bus = iproc_pcie_map_cfg_bus,
+    .map_bus = iproc_pcie_bus_map_cfg_bus,
    .read = iproc_pcie_config_read32,
    .write = iproc_pcie_config_write32,
 };

@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
    msleep(100);
 }
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
 {
    struct device *dev = pcie->dev;
-    u8 hdr_type;
-    u32 link_ctrl, class, val;
-    u16 pos = PCI_EXP_CAP, link_status;
+    u32 hdr_type, link_ctrl, link_status, class, val;
+    u16 pos = PCI_EXP_CAP;
    bool link_is_active = false;
    /*

@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
    }
    /* make sure we are not in EP mode */
-    pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+    iproc_pci_raw_config_read32(pcie,  0, PCI_HEADER_TYPE, 1, &hdr_type);
    if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
            dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
            return -EFAULT;

@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
 #define PCI_CLASS_BRIDGE_MASK      0xffff00
 #define PCI_CLASS_BRIDGE_SHIFT     8
-    pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+    iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+                                4, &class);
    class &= ~PCI_CLASS_BRIDGE_MASK;
    class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
-    pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+    iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+                                 4, class);
    /* check link status to see if link is active */
-    pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+    iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+                                2, &link_status);
    if (link_status & PCI_EXP_LNKSTA_NLW)
            link_is_active = true;

@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
 #define PCI_TARGET_LINK_SPEED_MASK    0xf
 #define PCI_TARGET_LINK_SPEED_GEN2    0x2
 #define PCI_TARGET_LINK_SPEED_GEN1    0x1
-            pci_bus_read_config_dword(bus, 0,
-                                      pos + PCI_EXP_LNKCTL2,
+            iproc_pci_raw_config_read32(pcie, 0,
+                                      pos + PCI_EXP_LNKCTL2, 4,
                                      &link_ctrl);
            if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
                PCI_TARGET_LINK_SPEED_GEN2) {
                    link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
                    link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
-                    pci_bus_write_config_dword(bus, 0,
-                                       pos + PCI_EXP_LNKCTL2,
-                                       link_ctrl);
+                    iproc_pci_raw_config_write32(pcie, 0,
+                                            pos + PCI_EXP_LNKCTL2,
+                                            4, link_ctrl);
                    msleep(100);
-                    pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
-                                             &link_status);
+                    iproc_pci_raw_config_read32(pcie, 0,
+                                            pos + PCI_EXP_LNKSTA,
+                                            2, &link_status);
                    if (link_status & PCI_EXP_LNKSTA_NLW)
                            link_is_active = true;
            }

Quoted 9 lines

drivers/pci/host/pcie-iproc.c int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res) · line 1319

Quoted 8 lines

drivers/pci/host/pcie-iproc.c +1 -1

@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
    }
    pcie->root_bus = bus;
-    ret = iproc_pcie_check_link(pcie, bus);
+    ret = iproc_pcie_check_link(pcie);
    if (ret) {
            dev_err(dev, "no PCIe EP device detected\n");
            goto err_rm_root_bus;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 21 lines · Lorenzo Pieralisi

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 8 lines

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
root at bcm958742k:~# dmesg | grep nvme

[    3.855466] nvme 0000:01:00.0: assign irq: got 0
[    3.855469] nvme 0000:01:00.0: assigning IRQ 00
[    3.855515] nvme nvme0: pci function 0000:01:00.0
[    4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    4.276787] nvme 0000:01:00.0: enabling bus mastering
[    4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h

        modified:   arch/arm/kernel/bios32.c
        modified:   arch/arm/mach-dove/pcie.c
        modified:   arch/arm/mach-iop13xx/pci.c
        modified:   arch/arm/mach-iop13xx/pci.h
        modified:   arch/arm/mach-mv78xx0/pcie.c
        modified:   arch/arm/mach-orion5x/common.h
        modified:   arch/arm/mach-orion5x/pci.c
        modified:   arch/arm64/kernel/pci.c
        modified:   drivers/of/of_pci_irq.c
        modified:   drivers/pci/Makefile
        modified:   drivers/pci/host/pci-aardvark.c
        modified:   drivers/pci/host/pci-ftpci100.c
        modified:   drivers/pci/host/pci-host-common.c
        modified:   drivers/pci/host/pci-tegra.c
        modified:   drivers/pci/host/pci-versatile.c
        modified:   drivers/pci/host/pci-xgene.c
        modified:   drivers/pci/host/pcie-altera.c
        modified:   drivers/pci/host/pcie-iproc-bcma.c
        modified:   drivers/pci/host/pcie-iproc-platform.c
        modified:   drivers/pci/host/pcie-iproc.c
        modified:   drivers/pci/host/pcie-iproc.h

        modified:   drivers/pci/host/pcie-rcar.c
        modified:   drivers/pci/host/pcie-rockchip.c
        modified:   drivers/pci/host/pcie-xilinx-nwl.c
        modified:   drivers/pci/host/pcie-xilinx.c
        modified:   drivers/pci/pci-driver.c
        modified:   drivers/pci/probe.c
        modified:   drivers/pci/setup-irq.c
        modified:   include/linux/pci.h

Quoted 73 lines · Oza Oza

Quoted 21 lines · Lorenzo Pieralisi

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 8 lines

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
root at bcm958742k:~# dmesg | grep nvme

[    3.855466] nvme 0000:01:00.0: assign irq: got 0
[    3.855469] nvme 0000:01:00.0: assigning IRQ 00
[    3.855515] nvme nvme0: pci function 0000:01:00.0
[    4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    4.276787] nvme 0000:01:00.0: enabling bus mastering
[    4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h

        modified:   arch/arm/kernel/bios32.c
        modified:   arch/arm/mach-dove/pcie.c
        modified:   arch/arm/mach-iop13xx/pci.c
        modified:   arch/arm/mach-iop13xx/pci.h
        modified:   arch/arm/mach-mv78xx0/pcie.c
        modified:   arch/arm/mach-orion5x/common.h
        modified:   arch/arm/mach-orion5x/pci.c
        modified:   arch/arm64/kernel/pci.c
        modified:   drivers/of/of_pci_irq.c
        modified:   drivers/pci/Makefile
        modified:   drivers/pci/host/pci-aardvark.c
        modified:   drivers/pci/host/pci-ftpci100.c
        modified:   drivers/pci/host/pci-host-common.c
        modified:   drivers/pci/host/pci-tegra.c
        modified:   drivers/pci/host/pci-versatile.c
        modified:   drivers/pci/host/pci-xgene.c
        modified:   drivers/pci/host/pcie-altera.c
        modified:   drivers/pci/host/pcie-iproc-bcma.c
        modified:   drivers/pci/host/pcie-iproc-platform.c
        modified:   drivers/pci/host/pcie-iproc.c
        modified:   drivers/pci/host/pcie-iproc.h

Quoted 9 lines

        modified:   drivers/pci/host/pcie-rcar.c
        modified:   drivers/pci/host/pcie-rockchip.c
        modified:   drivers/pci/host/pcie-xilinx-nwl.c
        modified:   drivers/pci/host/pcie-xilinx.c
        modified:   drivers/pci/pci-driver.c
        modified:   drivers/pci/probe.c
        modified:   drivers/pci/setup-irq.c
        modified:   include/linux/pci.h

Did you test with or without my change in bcm/master to emulate legacy interrupt through dummy IRQ domain?

Thanks,

Ray

Quoted 86 lines · Ray Jui

Quoted 73 lines · Oza Oza

Quoted 21 lines · Lorenzo Pieralisi

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 8 lines

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
root at bcm958742k:~# dmesg | grep nvme

[    3.855466] nvme 0000:01:00.0: assign irq: got 0
[    3.855469] nvme 0000:01:00.0: assigning IRQ 00
[    3.855515] nvme nvme0: pci function 0000:01:00.0
[    4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    4.276787] nvme 0000:01:00.0: enabling bus mastering
[    4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h

        modified:   arch/arm/kernel/bios32.c
        modified:   arch/arm/mach-dove/pcie.c
        modified:   arch/arm/mach-iop13xx/pci.c
        modified:   arch/arm/mach-iop13xx/pci.h
        modified:   arch/arm/mach-mv78xx0/pcie.c
        modified:   arch/arm/mach-orion5x/common.h
        modified:   arch/arm/mach-orion5x/pci.c
        modified:   arch/arm64/kernel/pci.c
        modified:   drivers/of/of_pci_irq.c
        modified:   drivers/pci/Makefile
        modified:   drivers/pci/host/pci-aardvark.c
        modified:   drivers/pci/host/pci-ftpci100.c
        modified:   drivers/pci/host/pci-host-common.c
        modified:   drivers/pci/host/pci-tegra.c
        modified:   drivers/pci/host/pci-versatile.c
        modified:   drivers/pci/host/pci-xgene.c
        modified:   drivers/pci/host/pcie-altera.c
        modified:   drivers/pci/host/pcie-iproc-bcma.c
        modified:   drivers/pci/host/pcie-iproc-platform.c
        modified:   drivers/pci/host/pcie-iproc.c
        modified:   drivers/pci/host/pcie-iproc.h

Quoted 9 lines

        modified:   drivers/pci/host/pcie-rcar.c
        modified:   drivers/pci/host/pcie-rockchip.c
        modified:   drivers/pci/host/pcie-xilinx-nwl.c
        modified:   drivers/pci/host/pcie-xilinx.c
        modified:   drivers/pci/pci-driver.c
        modified:   drivers/pci/probe.c
        modified:   drivers/pci/setup-irq.c
        modified:   include/linux/pci.h

Quoted 8 lines

Did you test with or without my change in bcm/master to emulate legacy interrupt through dummy IRQ domain?

Thanks,

Ray

Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now. please mainline those changes.

Hi Lorenzo,

Legacy IRQ is working fine as well wiht your patches.

cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

NVMe data transfer is also fine.

Regards,
Oza.

Hi Oza/Lorenzo,

Quoted 110 lines · Oza Oza

Quoted 86 lines · Ray Jui

Quoted 73 lines · Oza Oza

Quoted 21 lines · Lorenzo Pieralisi

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 8 lines

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
root at bcm958742k:~# dmesg | grep nvme

[    3.855466] nvme 0000:01:00.0: assign irq: got 0
[    3.855469] nvme 0000:01:00.0: assigning IRQ 00
[    3.855515] nvme nvme0: pci function 0000:01:00.0
[    4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    4.276787] nvme 0000:01:00.0: enabling bus mastering
[    4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h

        modified:   arch/arm/kernel/bios32.c
        modified:   arch/arm/mach-dove/pcie.c
        modified:   arch/arm/mach-iop13xx/pci.c
        modified:   arch/arm/mach-iop13xx/pci.h
        modified:   arch/arm/mach-mv78xx0/pcie.c
        modified:   arch/arm/mach-orion5x/common.h
        modified:   arch/arm/mach-orion5x/pci.c
        modified:   arch/arm64/kernel/pci.c
        modified:   drivers/of/of_pci_irq.c
        modified:   drivers/pci/Makefile
        modified:   drivers/pci/host/pci-aardvark.c
        modified:   drivers/pci/host/pci-ftpci100.c
        modified:   drivers/pci/host/pci-host-common.c
        modified:   drivers/pci/host/pci-tegra.c
        modified:   drivers/pci/host/pci-versatile.c
        modified:   drivers/pci/host/pci-xgene.c
        modified:   drivers/pci/host/pcie-altera.c
        modified:   drivers/pci/host/pcie-iproc-bcma.c
        modified:   drivers/pci/host/pcie-iproc-platform.c
        modified:   drivers/pci/host/pcie-iproc.c
        modified:   drivers/pci/host/pcie-iproc.h

Quoted 9 lines

        modified:   drivers/pci/host/pcie-rcar.c
        modified:   drivers/pci/host/pcie-rockchip.c
        modified:   drivers/pci/host/pcie-xilinx-nwl.c
        modified:   drivers/pci/host/pcie-xilinx.c
        modified:   drivers/pci/pci-driver.c
        modified:   drivers/pci/probe.c
        modified:   drivers/pci/setup-irq.c
        modified:   include/linux/pci.h

Quoted 8 lines

Did you test with or without my change in bcm/master to emulate legacy interrupt through dummy IRQ domain?

Thanks,

Ray

Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now. please mainline those changes.

Hi Lorenzo,

Legacy IRQ is working fine as well wiht your patches.

cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

NVMe data transfer is also fine.

Quoted 3 lines

Regards,
Oza.

Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been sanity tested for link detection and legacy interrupt support. No regression is seen. Thanks, Oza.

I'll send out the INTx irqdomain support patch when I have time.

Ray

Quoted 113 lines · Ray Jui

Hi Oza/Lorenzo,

Quoted 110 lines · Oza Oza

Quoted 86 lines · Ray Jui

Quoted 73 lines · Oza Oza

Quoted 21 lines · Lorenzo Pieralisi

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 8 lines

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
root at bcm958742k:~# dmesg | grep nvme

[    3.855466] nvme 0000:01:00.0: assign irq: got 0
[    3.855469] nvme 0000:01:00.0: assigning IRQ 00
[    3.855515] nvme nvme0: pci function 0000:01:00.0
[    4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    4.276787] nvme 0000:01:00.0: enabling bus mastering
[    4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h

        modified:   arch/arm/kernel/bios32.c
        modified:   arch/arm/mach-dove/pcie.c
        modified:   arch/arm/mach-iop13xx/pci.c
        modified:   arch/arm/mach-iop13xx/pci.h
        modified:   arch/arm/mach-mv78xx0/pcie.c
        modified:   arch/arm/mach-orion5x/common.h
        modified:   arch/arm/mach-orion5x/pci.c
        modified:   arch/arm64/kernel/pci.c
        modified:   drivers/of/of_pci_irq.c
        modified:   drivers/pci/Makefile
        modified:   drivers/pci/host/pci-aardvark.c
        modified:   drivers/pci/host/pci-ftpci100.c
        modified:   drivers/pci/host/pci-host-common.c
        modified:   drivers/pci/host/pci-tegra.c
        modified:   drivers/pci/host/pci-versatile.c
        modified:   drivers/pci/host/pci-xgene.c
        modified:   drivers/pci/host/pcie-altera.c
        modified:   drivers/pci/host/pcie-iproc-bcma.c
        modified:   drivers/pci/host/pcie-iproc-platform.c
        modified:   drivers/pci/host/pcie-iproc.c
        modified:   drivers/pci/host/pcie-iproc.h

Quoted 9 lines

        modified:   drivers/pci/host/pcie-rcar.c
        modified:   drivers/pci/host/pcie-rockchip.c
        modified:   drivers/pci/host/pcie-xilinx-nwl.c
        modified:   drivers/pci/host/pcie-xilinx.c
        modified:   drivers/pci/pci-driver.c
        modified:   drivers/pci/probe.c
        modified:   drivers/pci/setup-irq.c
        modified:   include/linux/pci.h

Quoted 8 lines

Did you test with or without my change in bcm/master to emulate legacy interrupt through dummy IRQ domain?

Thanks,

Ray

Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now. please mainline those changes.

Hi Lorenzo,

Legacy IRQ is working fine as well wiht your patches.

cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

NVMe data transfer is also fine.

Quoted 7 lines

Quoted 3 lines

Regards,
Oza.

Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been sanity tested for link detection and legacy interrupt support. No regression is seen. Thanks, Oza.

Yes, that's how I take it too, I did not understand what was triggering the first regression reported - my series should work on top of mainline if the current code works and should not depend on any other patch being merged - please make sure that's the case to prevent unwanted regressions in case other patches do not make it into the mainline.

Thank you very much for your help in testing it.

Lorenzo

Quoted 8 lines

I'll send out the INTx irqdomain support patch when I have time.

Ray

Quoted 122 lines · Lorenzo Pieralisi

Quoted 113 lines · Ray Jui

Hi Oza/Lorenzo,

Quoted 110 lines · Oza Oza

Quoted 86 lines · Ray Jui

Quoted 73 lines · Oza Oza

Quoted 21 lines · Lorenzo Pieralisi

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 8 lines

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
root at bcm958742k:~# dmesg | grep nvme

[    3.855466] nvme 0000:01:00.0: assign irq: got 0
[    3.855469] nvme 0000:01:00.0: assigning IRQ 00
[    3.855515] nvme nvme0: pci function 0000:01:00.0
[    4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    4.276787] nvme 0000:01:00.0: enabling bus mastering
[    4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h

        modified:   arch/arm/kernel/bios32.c
        modified:   arch/arm/mach-dove/pcie.c
        modified:   arch/arm/mach-iop13xx/pci.c
        modified:   arch/arm/mach-iop13xx/pci.h
        modified:   arch/arm/mach-mv78xx0/pcie.c
        modified:   arch/arm/mach-orion5x/common.h
        modified:   arch/arm/mach-orion5x/pci.c
        modified:   arch/arm64/kernel/pci.c
        modified:   drivers/of/of_pci_irq.c
        modified:   drivers/pci/Makefile
        modified:   drivers/pci/host/pci-aardvark.c
        modified:   drivers/pci/host/pci-ftpci100.c
        modified:   drivers/pci/host/pci-host-common.c
        modified:   drivers/pci/host/pci-tegra.c
        modified:   drivers/pci/host/pci-versatile.c
        modified:   drivers/pci/host/pci-xgene.c
        modified:   drivers/pci/host/pcie-altera.c
        modified:   drivers/pci/host/pcie-iproc-bcma.c
        modified:   drivers/pci/host/pcie-iproc-platform.c
        modified:   drivers/pci/host/pcie-iproc.c
        modified:   drivers/pci/host/pcie-iproc.h

Quoted 9 lines

        modified:   drivers/pci/host/pcie-rcar.c
        modified:   drivers/pci/host/pcie-rockchip.c
        modified:   drivers/pci/host/pcie-xilinx-nwl.c
        modified:   drivers/pci/host/pcie-xilinx.c
        modified:   drivers/pci/pci-driver.c
        modified:   drivers/pci/probe.c
        modified:   drivers/pci/setup-irq.c
        modified:   include/linux/pci.h

Quoted 8 lines

Did you test with or without my change in bcm/master to emulate legacy interrupt through dummy IRQ domain?

Thanks,

Ray

Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now. please mainline those changes.

Hi Lorenzo,

Legacy IRQ is working fine as well wiht your patches.

cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

NVMe data transfer is also fine.

Quoted 7 lines

Quoted 3 lines

Regards,
Oza.

Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been sanity tested for link detection and legacy interrupt support. No regression is seen. Thanks, Oza.

Quoted 7 lines

Yes, that's how I take it too, I did not understand what was triggering the first regression reported - my series should work on top of mainline if the current code works and should not depend on any other patch being merged - please make sure that's the case to prevent unwanted regressions in case other patches do not make it into the mainline.

Thank you very much for your help in testing it.

Hi Lorenzo,

I have ported and tested my inbound memory patches on top of your patches and that work fine as well.
This is mainly to do with IOVA reservations.

Regards,
Oza.

Quoted 11 lines

Lorenzo

Quoted 8 lines

I'll send out the INTx irqdomain support patch when I have time.

Ray

Quoted 133 lines · Oza Oza

Quoted 122 lines · Lorenzo Pieralisi

Quoted 113 lines · Ray Jui

Hi Oza/Lorenzo,

Quoted 110 lines · Oza Oza

Quoted 86 lines · Ray Jui

Quoted 73 lines · Oza Oza

Quoted 21 lines · Lorenzo Pieralisi

Quoted 20 lines · Oza Oza

Quoted 17 lines · Lorenzo Pieralisi

[dropped rock-chips maintainers, email bounces]

Quoted 10 lines · Ray Jui

Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42 patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Quoted 8 lines

Quoted 4 lines

Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC, and ran basic fio data transfer.. and it looks okay.

Thank you. If you could also check it is all OK from a legacy IRQ allocation (ie same as before applying series) I'd be grateful.

I disabled msi and it should default to legacy IRQ, looks like there is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
root at bcm958742k:~# dmesg | grep nvme

[    3.855466] nvme 0000:01:00.0: assign irq: got 0
[    3.855469] nvme 0000:01:00.0: assigning IRQ 00
[    3.855515] nvme nvme0: pci function 0000:01:00.0
[    4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[    4.276787] nvme 0000:01:00.0: enabling bus mastering
[    4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h

        modified:   arch/arm/kernel/bios32.c
        modified:   arch/arm/mach-dove/pcie.c
        modified:   arch/arm/mach-iop13xx/pci.c
        modified:   arch/arm/mach-iop13xx/pci.h
        modified:   arch/arm/mach-mv78xx0/pcie.c
        modified:   arch/arm/mach-orion5x/common.h
        modified:   arch/arm/mach-orion5x/pci.c
        modified:   arch/arm64/kernel/pci.c
        modified:   drivers/of/of_pci_irq.c
        modified:   drivers/pci/Makefile
        modified:   drivers/pci/host/pci-aardvark.c
        modified:   drivers/pci/host/pci-ftpci100.c
        modified:   drivers/pci/host/pci-host-common.c
        modified:   drivers/pci/host/pci-tegra.c
        modified:   drivers/pci/host/pci-versatile.c
        modified:   drivers/pci/host/pci-xgene.c
        modified:   drivers/pci/host/pcie-altera.c
        modified:   drivers/pci/host/pcie-iproc-bcma.c
        modified:   drivers/pci/host/pcie-iproc-platform.c
        modified:   drivers/pci/host/pcie-iproc.c
        modified:   drivers/pci/host/pcie-iproc.h

Quoted 9 lines

        modified:   drivers/pci/host/pcie-rcar.c
        modified:   drivers/pci/host/pcie-rockchip.c
        modified:   drivers/pci/host/pcie-xilinx-nwl.c
        modified:   drivers/pci/host/pcie-xilinx.c
        modified:   drivers/pci/pci-driver.c
        modified:   drivers/pci/probe.c
        modified:   drivers/pci/setup-irq.c
        modified:   include/linux/pci.h

Quoted 8 lines

Did you test with or without my change in bcm/master to emulate legacy interrupt through dummy IRQ domain?

Thanks,

Ray

Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now. please mainline those changes.

Hi Lorenzo,

Legacy IRQ is working fine as well wiht your patches.

cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

NVMe data transfer is also fine.

Quoted 7 lines

Quoted 3 lines

Regards,
Oza.

Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been sanity tested for link detection and legacy interrupt support. No regression is seen. Thanks, Oza.

Quoted 7 lines

Yes, that's how I take it too, I did not understand what was triggering the first regression reported - my series should work on top of mainline if the current code works and should not depend on any other patch being merged - please make sure that's the case to prevent unwanted regressions in case other patches do not make it into the mainline.

Thank you very much for your help in testing it.

Quoted 20 lines

Hi Lorenzo,

I have ported and tested my inbound memory patches on top of your patches and that work fine as well.
This is mainly to do with IOVA reservations.

Regards,
Oza.

Quoted 11 lines

Lorenzo

Quoted 8 lines

I'll send out the INTx irqdomain support patch when I have time.

Ray

Hi Bjorn,

I have made v8 for inbound memory patches as you suggested, on top of Lorenzo's patches.
but I can not post them since Lorenzo's patches have not made in.

Current ftpci100 driver host bridge controller driver requires struct pci_bus to be created in order to mask and clear IRQs using standard PCI bus config accessors.

This struct pci_bus dependency is fictitious and burdens the driver with unneeded constraints (eg to use separate APIs to create and scan the root bus).

Add PCI raw config space accessors to PCIe ftpci100 driver and remove the fictitious struct pci_bus dependency.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Linus Walleij <[email protected]>

When probing the PCI host controller driver, if an error condition is trigger the probe function code does not free memory allocated for the struct pci_host_bridge resulting in memory leakage.

Move the struct pci_host_bridge allocation over to the respective devm interface to fix the issue.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Bjorn Helgaas <[email protected]>