Skip to contentSkip to conversation

Make PCIe hotplug work with Freescale PCIe controllers

PATCH In review Rojhalat Ibrahim 3 participants 8 messages Mar 11, 2013 – Mar 13, 2013 PowerPC

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Make PCIe hotplug work with Freescale PCIe controllers

Hi,

this issue was brought up before.
See this thread: https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-July/099529.html

The following patch works for me.
Hot-added devices appear after "echo 1 > /sys/bus/pci/rescan". I tested it with a P5020DS development system.

---
 arch/powerpc/sysdev/indirect_pci.c |   53 ++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 9 deletions(-)
arch/powerpc/sysdev/indirect_pci.c +44 -9
index 82fdad8..aa36009 100644
@@ -20,22 +20,19 @@
 #include <asm/pci-bridge.h>
 #include <asm/machdep.h>
 
+#ifdef CONFIG_FSL_PCI
+#include <sysdev/fsl_pci.h>
+#endif
+
 static int
-indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
-		     int len, u32 *val)
+read_config_no_link_check(struct pci_bus *bus, unsigned int devfn, int offset,
+			  int len, u32 *val)
 {
 	struct pci_controller *hose = pci_bus_to_host(bus);
 	volatile void __iomem *cfg_data;
 	u8 cfg_type = 0;
 	u32 bus_no, reg;
 
-	if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) {
-		if (bus->number != hose->first_busno)
-			return PCIBIOS_DEVICE_NOT_FOUND;
-		if (devfn != 0)
-			return PCIBIOS_DEVICE_NOT_FOUND;
-	}
-
 	if (ppc_md.pci_exclude_device)
 		if (ppc_md.pci_exclude_device(hose, bus->number, devfn))
 			return PCIBIOS_DEVICE_NOT_FOUND;
@@ -78,6 +75,44 @@ indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
 	return PCIBIOS_SUCCESSFUL;
 }
 
+#ifdef CONFIG_FSL_PCI
+static int fsl_pcie_check_link(struct pci_bus *bus)
+{
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	u32 val = 0;
+
+	read_config_no_link_check(bus, 0, PCIE_LTSSM, 4, &val);
+	if (val < PCIE_LTSSM_L0) {
+		hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+		return 1;
+	} else {
+		hose->indirect_type &= ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+		return 0;
+	}
+}
+#endif
+
+static int
+indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
+		     int len, u32 *val)
+{
+	struct pci_controller *hose = pci_bus_to_host(bus);
+
+#ifdef CONFIG_FSL_PCI
+	if ((bus->number == hose->first_busno) && (devfn == 0)) {
+		fsl_pcie_check_link(bus);
+	}
+#endif
+	if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) {
+		if (bus->number != hose->first_busno)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		if (devfn != 0)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	return read_config_no_link_check(bus, devfn, offset, len, val);
+}
+
 static int
 indirect_write_config(struct pci_bus *bus, unsigned int devfn, int offset,
 		      int len, u32 val)
#1 lore raw

Rojhalat Ibrahim <[email protected]> wrote on 2013/03/11 15:47:30:

Quoted 5 lines

Hi,

this issue was brought up before.
See this thread:

https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-July/099529.html

Quoted 6 lines

The following patch works for me.
Hot-added devices appear after "echo 1 > /sys/bus/pci/rescan". I tested it with a P5020DS development system.

Nice! This LOOKS ok to me(cannot test ATM as I am lagging behind)

Jocke

Quoted 2 lines
---
 arch/powerpc/sysdev/indirect_pci.c |   53 

++++++++++++++++++++++++++++++-------

Review

Quoted 3 lines
 1 file changed, 44 insertions(+), 9 deletions(-)
(quoted diff) +0 -0

b/arch/powerpc/sysdev/indirect_pci.c

arch/powerpc/sysdev/indirect_pci.c line 24

Quoted 13 lines

index 82fdad8..aa36009 100644

arch/powerpc/sysdev/indirect_pci.c +4 -1
@@ -20,22 +20,19 @@
 #include <asm/pci-bridge.h>
 #include <asm/machdep.h>
 
+#ifdef CONFIG_FSL_PCI
+#include <sysdev/fsl_pci.h>
+#endif
+
 static int
-indirect_read_config(struct pci_bus *bus, unsigned int devfn, int 

offset,

arch/powerpc/sysdev/indirect_pci.c line 28

Quoted 2 lines
arch/powerpc/sysdev/indirect_pci.c +1 -1
-           int len, u32 *val)
+read_config_no_link_check(struct pci_bus *bus, unsigned int devfn, int 

offset,

arch/powerpc/sysdev/indirect_pci.c indirect_read_config(struct pci_bus *bus, unsigned

Quoted 18 lines
arch/powerpc/sysdev/indirect_pci.c +1 -7
+           int len, u32 *val)
 {
    struct pci_controller *hose = pci_bus_to_host(bus);
    volatile void __iomem *cfg_data;
    u8 cfg_type = 0;
    u32 bus_no, reg;
 
-   if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) {
-      if (bus->number != hose->first_busno)
-         return PCIBIOS_DEVICE_NOT_FOUND;
-      if (devfn != 0)
-         return PCIBIOS_DEVICE_NOT_FOUND;
-   }
-
    if (ppc_md.pci_exclude_device)
       if (ppc_md.pci_exclude_device(hose, bus->number, devfn))
          return PCIBIOS_DEVICE_NOT_FOUND;
@@ -78,6 +75,44 @@ indirect_read_config(struct pci_bus *bus, unsigned 

int

Quoted 13 lines
arch/powerpc/sysdev/indirect_pci.c +9 -0
 devfn, int offset,
    return PCIBIOS_SUCCESSFUL;
 }
 
+#ifdef CONFIG_FSL_PCI
+static int fsl_pcie_check_link(struct pci_bus *bus)
+{
+   struct pci_controller *hose = pci_bus_to_host(bus);
+   u32 val = 0;
+
+   read_config_no_link_check(bus, 0, PCIE_LTSSM, 4, &val);
+   if (val < PCIE_LTSSM_L0) {
+      hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;

arch/powerpc/sysdev/indirect_pci.c indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,

Quoted 10 lines
arch/powerpc/sysdev/indirect_pci.c +10 -0
+      return 1;
+   } else {
+      hose->indirect_type &= ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+      return 0;
+   }
+}
+#endif
+
+static int
+indirect_read_config(struct pci_bus *bus, unsigned int devfn, int 

offset,

Quoted 11 lines
arch/powerpc/sysdev/indirect_pci.c +11 -0
+           int len, u32 *val)
+{
+   struct pci_controller *hose = pci_bus_to_host(bus);
+
+#ifdef CONFIG_FSL_PCI
+   if ((bus->number == hose->first_busno) && (devfn == 0)) {
+      fsl_pcie_check_link(bus);
+   }
+#endif
+   if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) {
+      if (bus->number != hose->first_busno)

arch/powerpc/sysdev/indirect_pci.c indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,

Quoted 10 lines
arch/powerpc/sysdev/indirect_pci.c +8 -0
+         return PCIBIOS_DEVICE_NOT_FOUND;
+      if (devfn != 0)
+         return PCIBIOS_DEVICE_NOT_FOUND;
+   }
+
+   return read_config_no_link_check(bus, devfn, offset, len, val);
+}
+
 static int
 indirect_write_config(struct pci_bus *bus, unsigned int devfn, int 

offset,

Quoted 2 lines

int len, u32 val)

#2 lore raw
In reply to Rojhalat Ibrahim
Quoted 4 lines · Rojhalat Ibrahim

Hi,
=20
this issue was brought up before.=20
See this thread: =

https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-July/099529.html

Quoted 8 lines

=20
The following patch works for me.
Hot-added devices appear after "echo 1 > /sys/bus/pci/rescan". I tested it with a P5020DS development system.
=20

---
arch/powerpc/sysdev/indirect_pci.c | 53 =

++++++++++++++++++++++++++++++-------

Quoted 1 line
1 file changed, 44 insertions(+), 9 deletions(-)

Rather than do it this way, we should do something like:

fsl_indirect_read_config() {
link check
if (link)
indirect_read_config()
}

and just add fsl_indirect_{r,w}_config into fsl_pci.c

- k

=

Quoted 2 lines

=20

= renamed from arch/powerpc/sysdev/indirect_pci.c +0 -0

b/arch/powerpc/sysdev/indirect_pci.c

Quoted 11 lines

index 82fdad8..aa36009 100644

arch/powerpc/sysdev/indirect_pci.c +0 -0
@@ -20,22 +20,19 @@
 #include <asm/pci-bridge.h>
 #include <asm/machdep.h>

=20

arch/powerpc/sysdev/indirect_pci.c +4 -0
+#ifdef CONFIG_FSL_PCI
+#include <sysdev/fsl_pci.h>
+#endif
+
Quoted 2 lines

static int
-indirect_read_config(struct pci_bus *bus, unsigned int devfn, int =

offset,

arch/powerpc/sysdev/indirect_pci.c

Quoted 2 lines
arch/powerpc/sysdev/indirect_pci.c +1 -1
-		     int len, u32 *val)
+read_config_no_link_check(struct pci_bus *bus, unsigned int devfn, =

int offset,

arch/powerpc/sysdev/indirect_pci.c indirect_read_config(struct pci_bus *bus, unsigned =

Quoted 18 lines
arch/powerpc/sysdev/indirect_pci.c +1 -0
+			  int len, u32 *val)
{
	struct pci_controller *hose =3D pci_bus_to_host(bus);
	volatile void __iomem *cfg_data;
	u8 cfg_type =3D 0;
	u32 bus_no, reg;

=20

arch/powerpc/sysdev/indirect_pci.c +0 -7
-	if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) {
-		if (bus->number !=3D hose->first_busno)
-			return PCIBIOS_DEVICE_NOT_FOUND;
-		if (devfn !=3D 0)
-			return PCIBIOS_DEVICE_NOT_FOUND;
-	}
-
	if (ppc_md.pci_exclude_device)
		if (ppc_md.pci_exclude_device(hose, bus->number, devfn))
			return PCIBIOS_DEVICE_NOT_FOUND;
arch/powerpc/sysdev/indirect_pci.c +0 -0
@@ -78,6 +75,44 @@ indirect_read_config(struct pci_bus *bus, unsigned =

int devfn, int offset,

Quoted 5 lines
arch/powerpc/sysdev/indirect_pci.c +0 -0
 	return PCIBIOS_SUCCESSFUL;
 }

=20

arch/powerpc/sysdev/indirect_pci.c +2 -0
+#ifdef CONFIG_FSL_PCI
+static int fsl_pcie_check_link(struct pci_bus *bus)

arch/powerpc/sysdev/indirect_pci.c indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,

Quoted 10 lines
arch/powerpc/sysdev/indirect_pci.c +10 -0
+{
+	struct pci_controller *hose =3D pci_bus_to_host(bus);
+	u32 val =3D 0;
+
+	read_config_no_link_check(bus, 0, PCIE_LTSSM, 4, &val);
+	if (val < PCIE_LTSSM_L0) {
+		hose->indirect_type |=3D PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+		return 1;
+	} else {
+		hose->indirect_type &=3D =

~PPC_INDIRECT_TYPE_NO_PCIE_LINK;

arch/powerpc/sysdev/indirect_pci.c indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,

Quoted 7 lines
arch/powerpc/sysdev/indirect_pci.c +7 -0
+		return 0;
+	}
+}
+#endif
+
+static int
+indirect_read_config(struct pci_bus *bus, unsigned int devfn, int =

offset,

arch/powerpc/sysdev/indirect_pci.c indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,

Quoted 6 lines
arch/powerpc/sysdev/indirect_pci.c +6 -0
+		     int len, u32 *val)
+{
+	struct pci_controller *hose =3D pci_bus_to_host(bus);
+
+#ifdef CONFIG_FSL_PCI
+	if ((bus->number =3D=3D hose->first_busno) && (devfn =3D=3D 0)) =

{

Quoted 13 lines
arch/powerpc/sysdev/indirect_pci.c +13 -0
+		fsl_pcie_check_link(bus);
+	}
+#endif
+	if (hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK) {
+		if (bus->number !=3D hose->first_busno)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		if (devfn !=3D 0)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	return read_config_no_link_check(bus, devfn, offset, len, val);
+}
+
Quoted 2 lines

static int
indirect_write_config(struct pci_bus *bus, unsigned int devfn, int =

offset,

Quoted 1 line

int len, u32 val)

#3 lore raw
Quoted 9 lines · On Monday 11 March 2013 12:17:42 Kumar Gala wrote:

Rather than do it this way, we should do something like:

fsl_indirect_read_config() {
link check
if (link)
indirect_read_config()
}

Quoted 4 lines

and just add fsl_indirect_{r,w}_config into fsl_pci.c

- k

Ok, how about this:

---
 arch/powerpc/sysdev/fsl_pci.c |   49 
++++++++++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 4 deletions(-)
arch/powerpc/sysdev/fsl_pci.c +2 -0
index 682084d..693db9f 100644
@@ -36,6 +36,8 @@
 
 static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
 
+static struct pci_ops *indirect_pci_ops;
+
 static void quirk_fsl_pcie_header(struct pci_dev *dev)
 {
 	u8 hdr_type;
@@ -64,6 +66,45 @@ static int __init fsl_pcie_check_link(struct pci_controller 

*hose)
return 0;
}

+static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int devfn,
+ int offset, int len, u32 *val)
+{
+ struct pci_controller *hose = pci_bus_to_host(bus);
+
+ // check the link status
+ if ((bus->number == hose->first_busno) && (devfn == 0)) {
+ u32 ltssm = 0;
+ indirect_pci_ops->read(bus, 0, PCIE_LTSSM, 4, &ltssm);
+ if (ltssm < PCIE_LTSSM_L0) {
+ hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+ } else {
+ hose->indirect_type &= ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+ }
+ }
+ return indirect_pci_ops->read(bus, devfn, offset, len, val);
+}
+
+static int fsl_indirect_write_config(struct pci_bus *bus, unsigned int devfn,
+ int offset, int len, u32 val)
+{
+ return indirect_pci_ops->write(bus, devfn, offset, len, val);
+}
+
+static struct pci_ops fsl_indirect_pci_ops =
+{
+ .read = fsl_indirect_read_config,
+ .write = fsl_indirect_write_config,
+};
+
+static void __init fsl_setup_indirect_pci(struct pci_controller* hose,
+ resource_size_t cfg_addr,
+ resource_size_t cfg_data, u32 flags)
+{
+ setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
+ indirect_pci_ops = hose->ops;
+ hose->ops = &fsl_indirect_pci_ops;
+}
+
#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)

#define MAX_PHYS_ADDR_BITS 40
@@ -461,8 +502,8 @@ int __init fsl_add_bridge(struct platform_device *pdev, int is_primary)
hose->first_busno = bus_range ? bus_range[0] : 0x0; hose->last_busno = bus_range ? bus_range[1] : 0xff;

- setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
- PPC_INDIRECT_TYPE_BIG_ENDIAN);
+ fsl_setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
+ PPC_INDIRECT_TYPE_BIG_ENDIAN);

if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
/* For PCIE read HEADER_TYPE to identify controler mode */
@@ -766,8 +807,8 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
if (ret)
goto err0;
} else {
- setup_indirect_pci(hose, rsrc_cfg.start,
- rsrc_cfg.start + 4, 0);
+ fsl_setup_indirect_pci(hose, rsrc_cfg.start,
+ rsrc_cfg.start + 4, 0);
}

printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "

#4 lore raw
Quoted 14 lines · Rojhalat Ibrahim
Quoted 5 lines · On Monday 11 March 2013 12:17:42 Kumar Gala wrote:

=20
Rather than do it this way, we should do something like:
=20
fsl_indirect_read_config() {
link check

Quoted 8 lines

if (link)
indirect_read_config()
}
=20
and just add fsl_indirect_{r,w}_config into fsl_pci.c =20
- k
=20

Quoted 3 lines

=20
Ok, how about this:
=20

I'd rather we just export indirect_read_config() & = indirect_write_config() from indirect_pci.c and call the functions = directly. Adding a global and call them via a function pointer seems = wrong to me.

- k

=

Quoted 8 lines

=20

---
arch/powerpc/sysdev/fsl_pci.c | 49=20
++++++++++++++++++++++++++++++++++++++----

1 file changed, 45 insertions(+), 4 deletions(-)

=20

= renamed from arch/powerpc/sysdev/fsl_pci.c +0 -0

b/arch/powerpc/sysdev/fsl_pci.c

arch/powerpc/sysdev/fsl_pci.c static int __init fsl_pcie_check_link(struct =

Quoted 13 lines

index 682084d..693db9f 100644

arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -36,6 +36,8 @@

=20
static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
=20

arch/powerpc/sysdev/fsl_pci.c +2 -0
+static struct pci_ops *indirect_pci_ops;
+

static void quirk_fsl_pcie_header(struct pci_dev *dev)

{
	u8 hdr_type;
arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -64,6 +66,45 @@ static int __init fsl_pcie_check_link(struct =

pci_controller=20

Quoted 5 lines
arch/powerpc/sysdev/fsl_pci.c +0 -0
 *hose)
 	return 0;
 }

=20
+static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int =

devfn,

arch/powerpc/sysdev/fsl_pci.c

Quoted 6 lines
arch/powerpc/sysdev/fsl_pci.c +6 -0
+				    int offset, int len, u32 *val)
+{
+	struct pci_controller *hose =3D pci_bus_to_host(bus);
+=09
+	// check the link status
+	if ((bus->number =3D=3D hose->first_busno) && (devfn =3D=3D 0)) =

{

arch/powerpc/sysdev/fsl_pci.c

Quoted 4 lines
arch/powerpc/sysdev/fsl_pci.c +4 -0
+		u32 ltssm =3D 0;
+		indirect_pci_ops->read(bus, 0, PCIE_LTSSM, 4, &ltssm);
+		if (ltssm < PCIE_LTSSM_L0) {
+			hose->indirect_type |=3D =

PPC_INDIRECT_TYPE_NO_PCIE_LINK;

arch/powerpc/sysdev/fsl_pci.c

Quoted 2 lines
arch/powerpc/sysdev/fsl_pci.c +2 -0
+		} else {
+			hose->indirect_type &=3D =

~PPC_INDIRECT_TYPE_NO_PCIE_LINK;

arch/powerpc/sysdev/fsl_pci.c

Quoted 6 lines
arch/powerpc/sysdev/fsl_pci.c +6 -0
+		}
+	}
+	return indirect_pci_ops->read(bus, devfn, offset, len, val);
+}
+
+static int fsl_indirect_write_config(struct pci_bus *bus, unsigned =

int devfn,

arch/powerpc/sysdev/fsl_pci.c

Quoted 12 lines
arch/powerpc/sysdev/fsl_pci.c +12 -0
+				     int offset, int len, u32 val)
+{
+	return indirect_pci_ops->write(bus, devfn, offset, len, val);
+}
+
+static struct pci_ops fsl_indirect_pci_ops =3D
+{
+	.read =3D fsl_indirect_read_config,
+	.write =3D fsl_indirect_write_config,
+};
+
+static void __init fsl_setup_indirect_pci(struct pci_controller* =

hose,

arch/powerpc/sysdev/fsl_pci.c

Quoted 2 lines
arch/powerpc/sysdev/fsl_pci.c +2 -0
+					  resource_size_t cfg_addr,
+					  resource_size_t cfg_data, u32 =

flags)

arch/powerpc/sysdev/fsl_pci.c int __init fsl_add_bridge(struct platform_device =

Quoted 10 lines
arch/powerpc/sysdev/fsl_pci.c +6 -0
+{
+	setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
+	indirect_pci_ops =3D hose->ops;
+	hose->ops =3D &fsl_indirect_pci_ops;
+}
+

#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) =20
#define MAX_PHYS_ADDR_BITS 40

arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -461,8 +502,8 @@ int __init fsl_add_bridge(struct platform_device =

*pdev,=20

Quoted 11 lines
arch/powerpc/sysdev/fsl_pci.c +0 -0
 int is_primary)
 	hose->first_busno =3D bus_range ? bus_range[0] : 0x0;
 	hose->last_busno =3D bus_range ? bus_range[1] : 0xff;

=20

arch/powerpc/sysdev/fsl_pci.c +2 -2
-	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
-		PPC_INDIRECT_TYPE_BIG_ENDIAN);
+	fsl_setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
+			       PPC_INDIRECT_TYPE_BIG_ENDIAN);

=20

	if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
		/* For PCIE read HEADER_TYPE to identify controler mode =

*/

arch/powerpc/sysdev/fsl_pci.c int __init mpc83xx_add_bridge(struct device_node =

Quoted 1 line
arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -766,8 +807,8 @@ int __init mpc83xx_add_bridge(struct device_node =

*dev)

Quoted 11 lines
arch/powerpc/sysdev/fsl_pci.c +2 -2
 		if (ret)
 			goto err0;
 	} else {
-		setup_indirect_pci(hose, rsrc_cfg.start,
-				   rsrc_cfg.start + 4, 0);
+		fsl_setup_indirect_pci(hose, rsrc_cfg.start,
+				       rsrc_cfg.start + 4, 0);
 	}

=20

	printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "

=20

#5 lore raw
Quoted 17 lines · Kumar Gala

=20
On Mar 12, 2013, at 4:23 AM, Rojhalat Ibrahim wrote:
=20

Quoted 14 lines · On Monday 11 March 2013 12:17:42 Kumar Gala wrote:
Quoted 13 lines

=20
Rather than do it this way, we should do something like:
=20
fsl_indirect_read_config() {
link check
if (link)
indirect_read_config()
}
=20
and just add fsl_indirect_{r,w}_config into fsl_pci.c =20
- k
=20

Quoted 5 lines
Quoted 3 lines

=20
Ok, how about this:
=20

=20
I'd rather we just export indirect_read_config() & =

indirect_write_config() from indirect_pci.c and call the functions = directly. Adding a global and call them via a function pointer seems = wrong to me.

Quoted 2 lines

=20
- k

Also, can you base this patch on my powerpc.git next branch as = fsl_pci.{c,h} have some changes in them.

- k

Quoted 9 lines

=20

=

Quoted 8 lines

=20

---
arch/powerpc/sysdev/fsl_pci.c | 49=20
++++++++++++++++++++++++++++++++++++++----

1 file changed, 45 insertions(+), 4 deletions(-)

=20

= renamed from arch/powerpc/sysdev/fsl_pci.c +0 -0

b/arch/powerpc/sysdev/fsl_pci.c

Quoted 5 lines
Quoted 5 lines

index 682084d..693db9f 100644

arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -36,6 +36,8 @@

=20

Quoted 8 lines

arch/powerpc/sysdev/fsl_pci.c static int __init fsl_pcie_check_link(struct =

Quoted 8 lines

static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
=20

arch/powerpc/sysdev/fsl_pci.c +2 -0
+static struct pci_ops *indirect_pci_ops;
+

static void quirk_fsl_pcie_header(struct pci_dev *dev)

{
	u8 hdr_type;
arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -64,6 +66,45 @@ static int __init fsl_pcie_check_link(struct =

pci_controller=20

Quoted 5 lines
Quoted 5 lines
arch/powerpc/sysdev/fsl_pci.c +0 -0
 *hose)
 	return 0;
 }

=20
+static int fsl_indirect_read_config(struct pci_bus *bus, unsigned =

int devfn,

Quoted 6 lines

arch/powerpc/sysdev/fsl_pci.c

Quoted 6 lines
arch/powerpc/sysdev/fsl_pci.c +6 -0
+				    int offset, int len, u32 *val)
+{
+	struct pci_controller *hose =3D pci_bus_to_host(bus);
+=09
+	// check the link status
+	if ((bus->number =3D=3D hose->first_busno) && (devfn =3D=3D 0)) =

{

Quoted 4 lines

arch/powerpc/sysdev/fsl_pci.c

Quoted 4 lines
arch/powerpc/sysdev/fsl_pci.c +4 -0
+		u32 ltssm =3D 0;
+		indirect_pci_ops->read(bus, 0, PCIE_LTSSM, 4, &ltssm);
+		if (ltssm < PCIE_LTSSM_L0) {
+			hose->indirect_type |=3D =

PPC_INDIRECT_TYPE_NO_PCIE_LINK;

Quoted 2 lines

arch/powerpc/sysdev/fsl_pci.c

Quoted 2 lines
arch/powerpc/sysdev/fsl_pci.c +2 -0
+		} else {
+			hose->indirect_type &=3D =

~PPC_INDIRECT_TYPE_NO_PCIE_LINK;

Quoted 6 lines

arch/powerpc/sysdev/fsl_pci.c

Quoted 6 lines
arch/powerpc/sysdev/fsl_pci.c +6 -0
+		}
+	}
+	return indirect_pci_ops->read(bus, devfn, offset, len, val);
+}
+
+static int fsl_indirect_write_config(struct pci_bus *bus, unsigned =

int devfn,

Quoted 4 lines
Quoted 4 lines
arch/powerpc/sysdev/fsl_pci.c +4 -0
+				     int offset, int len, u32 val)
+{
+	return indirect_pci_ops->write(bus, devfn, offset, len, val);
+}
Quoted 8 lines

arch/powerpc/sysdev/fsl_pci.c

Quoted 8 lines
arch/powerpc/sysdev/fsl_pci.c +8 -0
+
+static struct pci_ops fsl_indirect_pci_ops =3D
+{
+	.read =3D fsl_indirect_read_config,
+	.write =3D fsl_indirect_write_config,
+};
+
+static void __init fsl_setup_indirect_pci(struct pci_controller* =

hose,

Quoted 2 lines

arch/powerpc/sysdev/fsl_pci.c

Quoted 2 lines
arch/powerpc/sysdev/fsl_pci.c +2 -0
+					  resource_size_t cfg_addr,
+					  resource_size_t cfg_data, u32 =

flags)

Quoted 10 lines

arch/powerpc/sysdev/fsl_pci.c int __init fsl_add_bridge(struct platform_device =

Quoted 10 lines
arch/powerpc/sysdev/fsl_pci.c +6 -0
+{
+	setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
+	indirect_pci_ops =3D hose->ops;
+	hose->ops =3D &fsl_indirect_pci_ops;
+}
+

#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx) =20
#define MAX_PHYS_ADDR_BITS 40

arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -461,8 +502,8 @@ int __init fsl_add_bridge(struct platform_device =

*pdev,=20

Quoted 11 lines
Quoted 11 lines
arch/powerpc/sysdev/fsl_pci.c +0 -0
 int is_primary)
 	hose->first_busno =3D bus_range ? bus_range[0] : 0x0;
 	hose->last_busno =3D bus_range ? bus_range[1] : 0xff;

=20

arch/powerpc/sysdev/fsl_pci.c +2 -2
-	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
-		PPC_INDIRECT_TYPE_BIG_ENDIAN);
+	fsl_setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
+			       PPC_INDIRECT_TYPE_BIG_ENDIAN);

=20

	if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
		/* For PCIE read HEADER_TYPE to identify controler mode =

*/

Quoted 1 line

arch/powerpc/sysdev/fsl_pci.c int __init mpc83xx_add_bridge(struct device_node =

Quoted 1 line
arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -766,8 +807,8 @@ int __init mpc83xx_add_bridge(struct device_node =

*dev)

Quoted 16 lines
Quoted 11 lines
arch/powerpc/sysdev/fsl_pci.c +2 -2
 		if (ret)
 			goto err0;
 	} else {
-		setup_indirect_pci(hose, rsrc_cfg.start,
-				   rsrc_cfg.start + 4, 0);
+		fsl_setup_indirect_pci(hose, rsrc_cfg.start,
+				       rsrc_cfg.start + 4, 0);
 	}

=20

	printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "

=20

=20
_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

#6 lore raw
Quoted 11 lines · On Tuesday 12 March 2013 15:48:01 Kumar Gala wrote:
Quoted 5 lines

I'd rather we just export indirect_read_config() & indirect_write_config() from indirect_pci.c and call the functions directly. Adding a global and call them via a function pointer seems wrong to me.

- k

Also, can you base this patch on my powerpc.git next branch as fsl_pci.{c,h} have some changes in them.

- k

Hi Kumar,

here's the patch with direct calls to indirect_{r,w}_config based on your powerpc.git next branch.

This does not work for PCIe IP rev 3.0 as I'm not sure how to handle "struct resource" aside from making it global or changing "struct pci_controller".

Rojhalat

---
 arch/powerpc/sysdev/fsl_pci.c      |   51 ++++++++++++++++++++++++++++++++++---
 arch/powerpc/sysdev/indirect_pci.c |   10 ++-----
 2 files changed, 51 insertions(+), 10 deletions(-)
arch/powerpc/sysdev/fsl_pci.c +47 -4
index 3271177..1d7da9d 100644
@@ -36,6 +36,11 @@
 
 static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
 
+extern int indirect_read_config(struct pci_bus *bus, unsigned int devfn,
+				int offset, int len, u32 *val);
+extern int indirect_write_config(struct pci_bus *bus, unsigned int devfn,
+				 int offset, int len, u32 val);
+
 static void quirk_fsl_pcie_header(struct pci_dev *dev)
 {
 	u8 hdr_type;
@@ -86,6 +91,44 @@ static int __init fsl_pcie_check_link(struct pci_controller *hose,
 	return 0;
 }
 
+static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int devfn,
+				    int offset, int len, u32 *val)
+{
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	
+	// check the link status
+	if ((bus->number == hose->first_busno) && (devfn == 0)) {
+		u32 ltssm = 0;
+		indirect_read_config(bus, 0, PCIE_LTSSM, 4, &ltssm);
+		if (ltssm < PCIE_LTSSM_L0) {
+			hose->indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+		} else {
+			hose->indirect_type &= ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+		}
+	}
+	return indirect_read_config(bus, devfn, offset, len, val);
+}
+
+static int fsl_indirect_write_config(struct pci_bus *bus, unsigned int devfn,
+				     int offset, int len, u32 val)
+{
+	return indirect_write_config(bus, devfn, offset, len, val);
+}
+
+static struct pci_ops fsl_indirect_pci_ops =
+{
+	.read = fsl_indirect_read_config,
+	.write = fsl_indirect_write_config,
+};
+
+static void __init fsl_setup_indirect_pci(struct pci_controller* hose,
+					  resource_size_t cfg_addr,
+					  resource_size_t cfg_data, u32 flags)
+{
+	setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
+	hose->ops = &fsl_indirect_pci_ops;
+}
+
 #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
 
 #define MAX_PHYS_ADDR_BITS	40
@@ -483,8 +526,8 @@ int __init fsl_add_bridge(struct platform_device *pdev, int is_primary)
 	hose->first_busno = bus_range ? bus_range[0] : 0x0;
 	hose->last_busno = bus_range ? bus_range[1] : 0xff;
 
-	setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
-		PPC_INDIRECT_TYPE_BIG_ENDIAN);
+	fsl_setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
+			       PPC_INDIRECT_TYPE_BIG_ENDIAN);
 
 	if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
 		/* For PCIE read HEADER_TYPE to identify controler mode */
@@ -788,8 +831,8 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
 		if (ret)
 			goto err0;
 	} else {
-		setup_indirect_pci(hose, rsrc_cfg.start,
-				   rsrc_cfg.start + 4, 0);
+		fsl_setup_indirect_pci(hose, rsrc_cfg.start,
+				       rsrc_cfg.start + 4, 0);
 	}
 
 	printk(KERN_INFO "Found FSL PCI host bridge at 0x%016llx. "
arch/powerpc/sysdev/indirect_pci.c +4 -6
index 82fdad8..c6c8b52 100644
@@ -20,9 +20,8 @@
 #include <asm/pci-bridge.h>
 #include <asm/machdep.h>
 
-static int
-indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
-		     int len, u32 *val)
+int indirect_read_config(struct pci_bus *bus, unsigned int devfn,
+			 int offset, int len, u32 *val)
 {
 	struct pci_controller *hose = pci_bus_to_host(bus);
 	volatile void __iomem *cfg_data;
@@ -78,9 +77,8 @@ indirect_read_config(struct pci_bus *bus, unsigned int devfn, int offset,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static int
-indirect_write_config(struct pci_bus *bus, unsigned int devfn, int offset,
-		      int len, u32 val)
+int indirect_write_config(struct pci_bus *bus, unsigned int devfn,
+			  int offset, int len, u32 val)
 {
 	struct pci_controller *hose = pci_bus_to_host(bus);
 	volatile void __iomem *cfg_data;
#7 lore raw
Quoted 2 lines · Rojhalat Ibrahim
Quoted 1 line · On Tuesday 12 March 2013 15:48:01 Kumar Gala wrote:
Quoted 1 line

I'd rather we just export indirect_read_config() & =

indirect_write_config()

Quoted 1 line
Quoted 1 line
Quoted 1 line

from indirect_pci.c and call the functions directly. Adding a =

global and

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines

call them via a function pointer seems wrong to me. =20
- k

=20
Also, can you base this patch on my powerpc.git next branch as =

fsl_pci.{c,h}

Quoted 8 lines
Quoted 4 lines

have some changes in them.
=20
- k
=20

=20
Hi Kumar,
=20
here's the patch with direct calls to indirect_{r,w}_config based on =

your powerpc.git next branch.

Quoted 2 lines

=20
This does not work for PCIe IP rev 3.0 as I'm not sure how to handle =

"struct resource" aside from making it global or changing "struct = pci_controller".

Quoted 2 lines

=20
Rojhalat

I'll send a reworked version that hopefully deals with that issue that = you can test and make sure things work ok. As well as some other minor = style code cleanups.

- k=