[PATCH 0/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[PATCH 0/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Ayman El-Khashab]

Old code didn't check the proper registers for the 460sx and so the link check
would always fail.  This changes the link check to use the config space reg and
adds the definition for the register.  Also changes the vc0 to a pll check.  
Sets up the port for gen2 speeds.

This uses the check_link function to specialize the link detect for the 460sx.
Tested on an eiger and custom board.  Note that is swaps the order of the link
check and dcr initialization since the config space needs the DCRs setup before
it can be mapped and used to check the link.


Ayman El-Khashab (1):
  powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

 arch/powerpc/sysdev/ppc4xx_pci.c |   83 +++++++++++++++++++++++++++++--------
 arch/powerpc/sysdev/ppc4xx_pci.h |    3 +
 2 files changed, 68 insertions(+), 18 deletions(-)

-- 
1.7.4.3

[PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Ayman El-Khashab]

Adds a register to the config space for the 460sx.  Changes the vc0
detect to a pll detect.  maps configuration space to test the link
status.  changes the setup to enable gen2 devices to operate at gen2
speeds.  fixes mapping that was not correct for the 460sx.

tested on the 460sx eiger and custom board

Signed-off-by: Ayman El-Khashab <ayman@elkhashab.com>
---
 arch/powerpc/sysdev/ppc4xx_pci.c |   83 +++++++++++++++++++++++++++++--------
 arch/powerpc/sysdev/ppc4xx_pci.h |    3 +
 2 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
index ad330fe..273963b 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -1092,6 +1092,10 @@ static int __init ppc460sx_pciex_core_init(struct device_node *np)
 	mtdcri(SDR0, PESDR1_460SX_HSSSLEW, 0xFFFF0000);
 	mtdcri(SDR0, PESDR2_460SX_HSSSLEW, 0xFFFF0000);
 
+	/* Set HSS PRBS enabled */
+	mtdcri(SDR0, PESDR0_460SX_HSSCTLSET, 0x00001130);
+	mtdcri(SDR0, PESDR2_460SX_HSSCTLSET, 0x00001130);
+
 	udelay(100);
 
 	/* De-assert PLLRESET */
@@ -1132,9 +1136,6 @@ static int ppc460sx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
 		dcri_clrset(SDR0, port->sdr_base + PESDRn_UTLSET2,
 				0, 0x01000000);
 
-	/*Gen-1*/
-	mtdcri(SDR0, port->sdr_base + PESDRn_460SX_RCEI, 0x08000000);
-
 	dcri_clrset(SDR0, port->sdr_base + PESDRn_RCSSET,
 			(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL),
 			PESDRx_RCSSET_RSTPYN);
@@ -1148,14 +1149,42 @@ static int ppc460sx_pciex_init_utl(struct ppc4xx_pciex_port *port)
 {
 	/* Max 128 Bytes */
 	out_be32 (port->utl_base + PEUTL_PBBSZ,   0x00000000);
+	/* Assert VRB and TXE - per datasheet turn off addr validation */
+	out_be32(port->utl_base + PEUTL_PCTL,  0x80800000);
 	return 0;
 }
 
+static void __init ppc460sx_pciex_check_link(struct ppc4xx_pciex_port *port)
+{
+	void __iomem *mbase;
+	int attempt = 50;
+
+	port->link = 0;
+
+	mbase = ioremap(port->cfg_space.start + 0x00000000, 0x1000);
+	if (mbase == NULL) {
+		printk(KERN_ERR "%s: Can't map internal config space !",
+			port->node->full_name);
+		goto done;
+	}
+
+	while (attempt && (0 == (in_le32(mbase + PECFG_460SX_DLLSTA)
+			& 0x00000010))) {
+		attempt--;
+		mdelay(10);
+	}
+	if (attempt)
+		port->link = 1;
+done:
+	iounmap(mbase);
+
+}
+
 static struct ppc4xx_pciex_hwops ppc460sx_pcie_hwops __initdata = {
 	.core_init	= ppc460sx_pciex_core_init,
 	.port_init_hw	= ppc460sx_pciex_init_port_hw,
 	.setup_utl	= ppc460sx_pciex_init_utl,
-	.check_link	= ppc4xx_pciex_check_link_sdr,
+	.check_link	= ppc460sx_pciex_check_link,
 };
 
 #endif /* CONFIG_44x */
@@ -1338,15 +1367,15 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
 	if (rc != 0)
 		return rc;
 
-	if (ppc4xx_pciex_hwops->check_link)
-		ppc4xx_pciex_hwops->check_link(port);
-
 	/*
 	 * Initialize mapping: disable all regions and configure
 	 * CFG and REG regions based on resources in the device tree
 	 */
 	ppc4xx_pciex_port_init_mapping(port);
 
+	if (ppc4xx_pciex_hwops->check_link)
+		ppc4xx_pciex_hwops->check_link(port);
+
 	/*
 	 * Map UTL
 	 */
@@ -1360,13 +1389,23 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
 		ppc4xx_pciex_hwops->setup_utl(port);
 
 	/*
-	 * Check for VC0 active and assert RDY.
+	 * Check for VC0 active or PLL Locked and assert RDY.
 	 */
 	if (port->sdr_base) {
-		if (port->link &&
-		    ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
-					     1 << 16, 1 << 16, 5000)) {
-			printk(KERN_INFO "PCIE%d: VC0 not active\n", port->index);
+		if (of_device_is_compatible(port->node,
+				"ibm,plb-pciex-460sx")){
+			if (port->link && ppc4xx_pciex_wait_on_sdr(port,
+					PESDRn_RCSSTS,
+					1 << 12, 1 << 12, 5000)) {
+				printk(KERN_INFO "PCIE%d: PLL not locked\n",
+						port->index);
+				port->link = 0;
+			}
+		} else if (port->link &&
+			ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
+				1 << 16, 1 << 16, 5000)) {
+			printk(KERN_INFO "PCIE%d: VC0 not active\n",
+					port->index);
 			port->link = 0;
 		}
 
@@ -1565,6 +1604,10 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 	pcial = RES_TO_U32_LOW(pci_addr);
 	sa = (0xffffffffu << ilog2(size)) | 0x1;
 
+	/* Enabled and single region */
+	sa |= (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx")) ?
+			0x5 : 0x3;
+
 	/* Program register values */
 	switch (index) {
 	case 0:
@@ -1573,8 +1616,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAH, lah);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKH, 0x7fffffff);
-		/* Note that 3 here means enabled | single region */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL, sa | 3);
+		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL, sa);
 		break;
 	case 1:
 		out_le32(mbase + PECFG_POM1LAH, pciah);
@@ -1582,8 +1624,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAH, lah);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKH, 0x7fffffff);
-		/* Note that 3 here means enabled | single region */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa | 3);
+		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa);
 		break;
 	case 2:
 		out_le32(mbase + PECFG_POM2LAH, pciah);
@@ -1591,8 +1632,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAH, lah);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKH, 0x7fffffff);
-		/* Note that 3 here means enabled | IO space !!! */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa | 3);
+		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa);
 		break;
 	}
 
@@ -1693,6 +1733,9 @@ static void __init ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
 		if (res->flags & IORESOURCE_PREFETCH)
 			sa |= 0x8;
 
+		if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+			sa |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+
 		out_le32(mbase + PECFG_BAR0HMPA, RES_TO_U32_HIGH(sa));
 		out_le32(mbase + PECFG_BAR0LMPA, RES_TO_U32_LOW(sa));
 
@@ -1854,6 +1897,10 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	}
 	out_le16(mbase + 0x202, val);
 
+	/* Enable Bus master, memory, and io space */
+	if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+		out_le16(mbase + 0x204, 0x7);
+
 	if (!port->endpoint) {
 		/* Set Class Code to PCI-PCI bridge and Revision Id to 1 */
 		out_le32(mbase + 0x208, 0x06040001);
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h b/arch/powerpc/sysdev/ppc4xx_pci.h
index 56d9e5d..28f0bb0 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.h
+++ b/arch/powerpc/sysdev/ppc4xx_pci.h
@@ -464,6 +464,9 @@
 #define PECFG_POM2LAL		0x390
 #define PECFG_POM2LAH		0x394
 
+/* 460sx only */
+#define PECFG_460SX_DLLSTA     0x3f8
+
 /* SDR Bit Mappings */
 #define PESDRx_RCSSET_HLDPLB	0x10000000
 #define PESDRx_RCSSET_RSTGU	0x01000000
-- 
1.7.4.3

Re: [PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Tony Breeds]

On Wed, Jul 13, 2011 at 07:33:31PM -0500, Ayman El-Khashab wrote:
> Adds a register to the config space for the 460sx.  Changes the vc0
> detect to a pll detect.  maps configuration space to test the link
> status.  changes the setup to enable gen2 devices to operate at gen2
> speeds.  fixes mapping that was not correct for the 460sx.
> 
> tested on the 460sx eiger and custom board

Hi Ayman.
	Just a few comments.
 
<snip>

> +static void __init ppc460sx_pciex_check_link(struct ppc4xx_pciex_port *port)
> +{
> +	void __iomem *mbase;
> +	int attempt = 50;
> +
> +	port->link = 0;
> +
> +	mbase = ioremap(port->cfg_space.start + 0x00000000, 0x1000);

Why + 0x00000000 ? ppc4xx_pciex_port_setup_hose() does:
mbase = ioremap(port->cfg_space.start + 0x10000000, 0x1000);
so isn't one of these statements is wrong?

> +	if (mbase == NULL) {
> +		printk(KERN_ERR "%s: Can't map internal config space !",
> +			port->node->full_name);
> +		goto done;
> +	}
> +
> +	while (attempt && (0 == (in_le32(mbase + PECFG_460SX_DLLSTA)
> +			& 0x00000010))) {

Nitpicking, I think it'd be nice if there was #define for 0x00000010
perhaps: #define PECFG_460SX_DLLSTA_LINKUP 0x00000010

> +		attempt--;
> +		mdelay(10);
> +	}
> +	if (attempt)
> +		port->link = 1;
> +done:
> +	iounmap(mbase);
> +
> +}
> +
>  static struct ppc4xx_pciex_hwops ppc460sx_pcie_hwops __initdata = {
>  	.core_init	= ppc460sx_pciex_core_init,
>  	.port_init_hw	= ppc460sx_pciex_init_port_hw,
>  	.setup_utl	= ppc460sx_pciex_init_utl,
> -	.check_link	= ppc4xx_pciex_check_link_sdr,
> +	.check_link	= ppc460sx_pciex_check_link,
>  };
>  
>  #endif /* CONFIG_44x */
> @@ -1338,15 +1367,15 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
>  	if (rc != 0)
>  		return rc;
>  
> -	if (ppc4xx_pciex_hwops->check_link)
> -		ppc4xx_pciex_hwops->check_link(port);
> -
>  	/*
>  	 * Initialize mapping: disable all regions and configure
>  	 * CFG and REG regions based on resources in the device tree
>  	 */
>  	ppc4xx_pciex_port_init_mapping(port);
>  
> +	if (ppc4xx_pciex_hwops->check_link)
> +		ppc4xx_pciex_hwops->check_link(port);
> +

Why move this?  You already iorempat the cfg space.
>  	/*
>  	 * Map UTL
>  	 */
> @@ -1360,13 +1389,23 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
>  		ppc4xx_pciex_hwops->setup_utl(port);
>  
>  	/*
> -	 * Check for VC0 active and assert RDY.
> +	 * Check for VC0 active or PLL Locked and assert RDY.
>  	 */
>  	if (port->sdr_base) {
> -		if (port->link &&
> -		    ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
> -					     1 << 16, 1 << 16, 5000)) {
> -			printk(KERN_INFO "PCIE%d: VC0 not active\n", port->index);
> +		if (of_device_is_compatible(port->node,
> +				"ibm,plb-pciex-460sx")){
> +			if (port->link && ppc4xx_pciex_wait_on_sdr(port,
> +					PESDRn_RCSSTS,
> +					1 << 12, 1 << 12, 5000)) {
> +				printk(KERN_INFO "PCIE%d: PLL not locked\n",
> +						port->index);
> +				port->link = 0;
> +			}
> +		} else if (port->link &&
> +			ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
> +				1 << 16, 1 << 16, 5000)) {
> +			printk(KERN_INFO "PCIE%d: VC0 not active\n",
> +					port->index);
>  			port->link = 0;
>  		}
>  
> @@ -1565,6 +1604,10 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
>  	pcial = RES_TO_U32_LOW(pci_addr);
>  	sa = (0xffffffffu << ilog2(size)) | 0x1;
>  
> +	/* Enabled and single region */
> +	sa |= (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx")) ?
> +			0x5 : 0x3;
> +
>  	/* Program register values */
>  	switch (index) {
>  	case 0:
> @@ -1573,8 +1616,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAH, lah);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAL, lal);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKH, 0x7fffffff);
> -		/* Note that 3 here means enabled | single region */
> -		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL, sa | 3);
> +		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL, sa);
>  		break;
>  	case 1:
>  		out_le32(mbase + PECFG_POM1LAH, pciah);
> @@ -1582,8 +1624,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAH, lah);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAL, lal);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKH, 0x7fffffff);
> -		/* Note that 3 here means enabled | single region */
> -		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa | 3);
> +		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa);
>  		break;
>  	case 2:
>  		out_le32(mbase + PECFG_POM2LAH, pciah);
> @@ -1591,8 +1632,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAH, lah);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAL, lal);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKH, 0x7fffffff);
> -		/* Note that 3 here means enabled | IO space !!! */
> -		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa | 3);
> +		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa);
>  		break;
>  	}

I think you really want to check the definitions for OMRs 2 and 3 to verify that this is right.
  
Yours Tony

Re: [PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Ayman El-Khashab]

Thanks Tony, some comments below.

On Thu, Jul 14, 2011 at 11:16:27AM +1000, Tony Breeds wrote:
> 
> > +static void __init ppc460sx_pciex_check_link(struct ppc4xx_pciex_port *port)
> > +{
> > +	void __iomem *mbase;
> > +	int attempt = 50;
> > +
> > +	port->link = 0;
> > +
> > +	mbase = ioremap(port->cfg_space.start + 0x00000000, 0x1000);
> 
> Why + 0x00000000 ? ppc4xx_pciex_port_setup_hose() does:
> mbase = ioremap(port->cfg_space.start + 0x10000000, 0x1000);
> so isn't one of these statements is wrong?

yes, that doesn't look right.   I'll verify that and make
sure that it works correctly and resubmit the patch.

> 
> > +	if (mbase == NULL) {
> > +		printk(KERN_ERR "%s: Can't map internal config space !",
> > +			port->node->full_name);
> > +		goto done;
> > +	}
> > +
> > +	while (attempt && (0 == (in_le32(mbase + PECFG_460SX_DLLSTA)
> > +			& 0x00000010))) {
> 
> Nitpicking, I think it'd be nice if there was #define for 0x00000010
> perhaps: #define PECFG_460SX_DLLSTA_LINKUP 0x00000010

ok.

> >  
> > -	if (ppc4xx_pciex_hwops->check_link)
> > -		ppc4xx_pciex_hwops->check_link(port);
> > -
> >  	/*
> >  	 * Initialize mapping: disable all regions and configure
> >  	 * CFG and REG regions based on resources in the device tree
> >  	 */
> >  	ppc4xx_pciex_port_init_mapping(port);
> >  
> > +	if (ppc4xx_pciex_hwops->check_link)
> > +		ppc4xx_pciex_hwops->check_link(port);
> > +
> 
> Why move this?  You already iorempat the cfg space.

This was what I was asking about before.  The reason that I
swapped the order of the init_mapping and check_link is
because the init_mapping currently sets up the cfgbax
registers.  Those setup the base address of the
configuration space on the PLB side of the bus.  As far as I
could determine, I cannot access the config space until
those registers are configured.  I need to touch the config
space in order to do the check_link b/c the 460sx uses the
extended config space to keep track of the link status.  I
looked at init mapping and based on what it did I did not
see any potential adverse effects.


> >  		out_le32(mbase + PECFG_POM2LAH, pciah);
> > @@ -1591,8 +1632,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
> >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAH, lah);
> >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAL, lal);
> >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKH, 0x7fffffff);
> > -		/* Note that 3 here means enabled | IO space !!! */
> > -		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa | 3);
> > +		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa);
> >  		break;
> >  	}
> 
> I think you really want to check the definitions for OMRs 2 and 3 to verify that this is right.

Thanks, good catch.  I'll change the first case block to
include a switch on the 460sx.  The first case statement
needs to be | 0x5, while the others need to stay 0x3.

Ayman

Re: [PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Benjamin Herrenschmidt]

On Thu, 2011-07-14 at 11:04 -0500, Ayman El-Khashab wrote:
> Thanks Tony, some comments below.
> 
> On Thu, Jul 14, 2011 at 11:16:27AM +1000, Tony Breeds wrote:
> > 
> > > +static void __init ppc460sx_pciex_check_link(struct ppc4xx_pciex_port *port)
> > > +{
> > > +	void __iomem *mbase;
> > > +	int attempt = 50;
> > > +
> > > +	port->link = 0;
> > > +
> > > +	mbase = ioremap(port->cfg_space.start + 0x00000000, 0x1000);
> > 
> > Why + 0x00000000 ? ppc4xx_pciex_port_setup_hose() does:
> > mbase = ioremap(port->cfg_space.start + 0x10000000, 0x1000);
> > so isn't one of these statements is wrong?
> 
> yes, that doesn't look right.   I'll verify that and make
> sure that it works correctly and resubmit the patch.

The state of that top bit is obscure. I couldn't figure out from docs
whether it's actually useful or not (it doesn't seem to make a
difference on the HW we've played with here).

It's possible that some parts need it to access the RC config space vs
emitting type 1 cycles, and some don't, in which case the address
decoding just wraps and the bit is ignored ?

> > > +	if (mbase == NULL) {
> > > +		printk(KERN_ERR "%s: Can't map internal config space !",
> > > +			port->node->full_name);
> > > +		goto done;
> > > +	}
> > > +
> > > +	while (attempt && (0 == (in_le32(mbase + PECFG_460SX_DLLSTA)
> > > +			& 0x00000010))) {
> > 
> > Nitpicking, I think it'd be nice if there was #define for 0x00000010
> > perhaps: #define PECFG_460SX_DLLSTA_LINKUP 0x00000010
> 
> ok.
> 
> > >  
> > > -	if (ppc4xx_pciex_hwops->check_link)
> > > -		ppc4xx_pciex_hwops->check_link(port);
> > > -
> > >  	/*
> > >  	 * Initialize mapping: disable all regions and configure
> > >  	 * CFG and REG regions based on resources in the device tree
> > >  	 */
> > >  	ppc4xx_pciex_port_init_mapping(port);
> > >  
> > > +	if (ppc4xx_pciex_hwops->check_link)
> > > +		ppc4xx_pciex_hwops->check_link(port);
> > > +
> > 
> > Why move this?  You already iorempat the cfg space.
> 
> This was what I was asking about before.  The reason that I
> swapped the order of the init_mapping and check_link is
> because the init_mapping currently sets up the cfgbax
> registers.  Those setup the base address of the
> configuration space on the PLB side of the bus.  As far as I
> could determine, I cannot access the config space until
> those registers are configured.  I need to touch the config
> space in order to do the check_link b/c the 460sx uses the
> extended config space to keep track of the link status.  I
> looked at init mapping and based on what it did I did not
> see any potential adverse effects.

I think it makes sense to setup mappings before checking the link. There
may be a couple of mechanical issues in the code with this, I haven't
looked, but I agree in principle.

> > >  		out_le32(mbase + PECFG_POM2LAH, pciah);
> > > @@ -1591,8 +1632,7 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
> > >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAH, lah);
> > >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAL, lal);
> > >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKH, 0x7fffffff);
> > > -		/* Note that 3 here means enabled | IO space !!! */
> > > -		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa | 3);
> > > +		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa);
> > >  		break;
> > >  	}
> > 
> > I think you really want to check the definitions for OMRs 2 and 3 to verify that this is right.
> 
> Thanks, good catch.  I'll change the first case block to
> include a switch on the 460sx.  The first case statement
> needs to be | 0x5, while the others need to stay 0x3.

Please make it constants using #define or something... those bits tend
to subtly change between ASICs too I noticed.

Cheers,
Ben.

> Ayman
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[v2 PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Ayman Elkhashab]

From: Ayman El-Khashab <ayman@elkhashab.com>

Adds a register to the config space for the 460sx.  Changes the vc0
detect to a pll detect.  maps configuration space to test the link
status.  changes the setup to enable gen2 devices to operate at gen2
speeds.  fixes mapping that was not correct for the 460sx.  added
bit definitions for the OMRxMSKL registers

tested on the 460sx eiger and custom board

Signed-off-by: Ayman El-Khashab <ayman@elkhashab.com>
---
 arch/powerpc/sysdev/ppc4xx_pci.c |   89 ++++++++++++++++++++++++++++++-------
 arch/powerpc/sysdev/ppc4xx_pci.h |   12 +++++
 2 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
index ad330fe..eeeeb12 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -1092,6 +1092,10 @@ static int __init ppc460sx_pciex_core_init(struct device_node *np)
 	mtdcri(SDR0, PESDR1_460SX_HSSSLEW, 0xFFFF0000);
 	mtdcri(SDR0, PESDR2_460SX_HSSSLEW, 0xFFFF0000);
 
+	/* Set HSS PRBS enabled */
+	mtdcri(SDR0, PESDR0_460SX_HSSCTLSET, 0x00001130);
+	mtdcri(SDR0, PESDR2_460SX_HSSCTLSET, 0x00001130);
+
 	udelay(100);
 
 	/* De-assert PLLRESET */
@@ -1132,9 +1136,6 @@ static int ppc460sx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
 		dcri_clrset(SDR0, port->sdr_base + PESDRn_UTLSET2,
 				0, 0x01000000);
 
-	/*Gen-1*/
-	mtdcri(SDR0, port->sdr_base + PESDRn_460SX_RCEI, 0x08000000);
-
 	dcri_clrset(SDR0, port->sdr_base + PESDRn_RCSSET,
 			(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL),
 			PESDRx_RCSSET_RSTPYN);
@@ -1148,14 +1149,42 @@ static int ppc460sx_pciex_init_utl(struct ppc4xx_pciex_port *port)
 {
 	/* Max 128 Bytes */
 	out_be32 (port->utl_base + PEUTL_PBBSZ,   0x00000000);
+	/* Assert VRB and TXE - per datasheet turn off addr validation */
+	out_be32(port->utl_base + PEUTL_PCTL,  0x80800000);
 	return 0;
 }
 
+static void __init ppc460sx_pciex_check_link(struct ppc4xx_pciex_port *port)
+{
+	void __iomem *mbase;
+	int attempt = 50;
+
+	port->link = 0;
+
+	mbase = ioremap(port->cfg_space.start + 0x10000000, 0x1000);
+	if (mbase == NULL) {
+		printk(KERN_ERR "%s: Can't map internal config space !",
+			port->node->full_name);
+		goto done;
+	}
+
+	while (attempt && (0 == (in_le32(mbase + PECFG_460SX_DLLSTA)
+			& PECFG_460SX_DLLSTA_LINKUP))) {
+		attempt--;
+		mdelay(10);
+	}
+	if (attempt)
+		port->link = 1;
+done:
+	iounmap(mbase);
+
+}
+
 static struct ppc4xx_pciex_hwops ppc460sx_pcie_hwops __initdata = {
 	.core_init	= ppc460sx_pciex_core_init,
 	.port_init_hw	= ppc460sx_pciex_init_port_hw,
 	.setup_utl	= ppc460sx_pciex_init_utl,
-	.check_link	= ppc4xx_pciex_check_link_sdr,
+	.check_link	= ppc460sx_pciex_check_link,
 };
 
 #endif /* CONFIG_44x */
@@ -1338,15 +1367,15 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
 	if (rc != 0)
 		return rc;
 
-	if (ppc4xx_pciex_hwops->check_link)
-		ppc4xx_pciex_hwops->check_link(port);
-
 	/*
 	 * Initialize mapping: disable all regions and configure
 	 * CFG and REG regions based on resources in the device tree
 	 */
 	ppc4xx_pciex_port_init_mapping(port);
 
+	if (ppc4xx_pciex_hwops->check_link)
+		ppc4xx_pciex_hwops->check_link(port);
+
 	/*
 	 * Map UTL
 	 */
@@ -1360,13 +1389,23 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
 		ppc4xx_pciex_hwops->setup_utl(port);
 
 	/*
-	 * Check for VC0 active and assert RDY.
+	 * Check for VC0 active or PLL Locked and assert RDY.
 	 */
 	if (port->sdr_base) {
-		if (port->link &&
-		    ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
-					     1 << 16, 1 << 16, 5000)) {
-			printk(KERN_INFO "PCIE%d: VC0 not active\n", port->index);
+		if (of_device_is_compatible(port->node,
+				"ibm,plb-pciex-460sx")){
+			if (port->link && ppc4xx_pciex_wait_on_sdr(port,
+					PESDRn_RCSSTS,
+					1 << 12, 1 << 12, 5000)) {
+				printk(KERN_INFO "PCIE%d: PLL not locked\n",
+						port->index);
+				port->link = 0;
+			}
+		} else if (port->link &&
+			ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
+				1 << 16, 1 << 16, 5000)) {
+			printk(KERN_INFO "PCIE%d: VC0 not active\n",
+					port->index);
 			port->link = 0;
 		}
 
@@ -1573,8 +1612,15 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAH, lah);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKH, 0x7fffffff);
-		/* Note that 3 here means enabled | single region */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL, sa | 3);
+		/*Enabled and single region */
+		if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+			dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL,
+				sa | DCRO_PEGPL_460SX_OMR1MSKL_UOT
+					| DCRO_PEGPL_OMRxMSKL_VAL);
+		else
+			dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL,
+				sa | DCRO_PEGPL_OMR1MSKL_UOT
+					| DCRO_PEGPL_OMRxMSKL_VAL);
 		break;
 	case 1:
 		out_le32(mbase + PECFG_POM1LAH, pciah);
@@ -1582,8 +1628,8 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAH, lah);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKH, 0x7fffffff);
-		/* Note that 3 here means enabled | single region */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa | 3);
+		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL,
+				sa | DCRO_PEGPL_OMRxMSKL_VAL);
 		break;
 	case 2:
 		out_le32(mbase + PECFG_POM2LAH, pciah);
@@ -1592,7 +1638,9 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKH, 0x7fffffff);
 		/* Note that 3 here means enabled | IO space !!! */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa | 3);
+		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL,
+				sa | DCRO_PEGPL_OMR3MSKL_IO
+					| DCRO_PEGPL_OMRxMSKL_VAL);
 		break;
 	}
 
@@ -1693,6 +1741,9 @@ static void __init ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
 		if (res->flags & IORESOURCE_PREFETCH)
 			sa |= 0x8;
 
+		if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+			sa |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+
 		out_le32(mbase + PECFG_BAR0HMPA, RES_TO_U32_HIGH(sa));
 		out_le32(mbase + PECFG_BAR0LMPA, RES_TO_U32_LOW(sa));
 
@@ -1854,6 +1905,10 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	}
 	out_le16(mbase + 0x202, val);
 
+	/* Enable Bus master, memory, and io space */
+	if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+		out_le16(mbase + 0x204, 0x7);
+
 	if (!port->endpoint) {
 		/* Set Class Code to PCI-PCI bridge and Revision Id to 1 */
 		out_le32(mbase + 0x208, 0x06040001);
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h b/arch/powerpc/sysdev/ppc4xx_pci.h
index 56d9e5d..61b3659 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.h
+++ b/arch/powerpc/sysdev/ppc4xx_pci.h
@@ -464,6 +464,18 @@
 #define PECFG_POM2LAL		0x390
 #define PECFG_POM2LAH		0x394
 
+/* 460sx only */
+#define PECFG_460SX_DLLSTA     0x3f8
+
+/* 460sx Bit Mappings */
+#define PECFG_460SX_DLLSTA_LINKUP	 0x00000010
+#define DCRO_PEGPL_460SX_OMR1MSKL_UOT	 0x00000004
+
+/* PEGPL Bit Mappings */
+#define DCRO_PEGPL_OMRxMSKL_VAL	 0x00000001
+#define DCRO_PEGPL_OMR1MSKL_UOT	 0x00000002
+#define DCRO_PEGPL_OMR3MSKL_IO	 0x00000002
+
 /* SDR Bit Mappings */
 #define PESDRx_RCSSET_HLDPLB	0x10000000
 #define PESDRx_RCSSET_RSTGU	0x01000000
-- 
1.7.4.3

Re: [v2 PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Tony Breeds]

On Fri, Jul 15, 2011 at 11:40:27AM -0500, Ayman Elkhashab wrote:

> @@ -1582,8 +1628,8 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAH, lah);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAL, lal);
>  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKH, 0x7fffffff);
> -		/* Note that 3 here means enabled | single region */
> -		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa | 3);
> +		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL,
> +				sa | DCRO_PEGPL_OMRxMSKL_VAL);

Didn't you just change "sa | 3" to "sa | 1" ?

<snip>

> index 56d9e5d..61b3659 100644
> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
> @@ -464,6 +464,18 @@
>  #define PECFG_POM2LAL		0x390
>  #define PECFG_POM2LAH		0x394
>  
> +/* 460sx only */
> +#define PECFG_460SX_DLLSTA     0x3f8
> +
> +/* 460sx Bit Mappings */
> +#define PECFG_460SX_DLLSTA_LINKUP	 0x00000010
> +#define DCRO_PEGPL_460SX_OMR1MSKL_UOT	 0x00000004
> +
> +/* PEGPL Bit Mappings */
> +#define DCRO_PEGPL_OMRxMSKL_VAL	 0x00000001
> +#define DCRO_PEGPL_OMR1MSKL_UOT	 0x00000002
> +#define DCRO_PEGPL_OMR3MSKL_IO	 0x00000002
> +
>  /* SDR Bit Mappings */
>  #define PESDRx_RCSSET_HLDPLB	0x10000000
>  #define PESDRx_RCSSET_RSTGU	0x01000000

Yours Tony

Re: [v2 PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Ayman El-Khashab]

On Mon, Jul 18, 2011 at 02:01:15PM +1000, Tony Breeds wrote:
> On Fri, Jul 15, 2011 at 11:40:27AM -0500, Ayman Elkhashab wrote:
> 
> > @@ -1582,8 +1628,8 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
> >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAH, lah);
> >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAL, lal);
> >  		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKH, 0x7fffffff);
> > -		/* Note that 3 here means enabled | single region */
> > -		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa | 3);
> > +		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL,
> > +				sa | DCRO_PEGPL_OMRxMSKL_VAL);
> 
> Didn't you just change "sa | 3" to "sa | 1" ?
> 

Yes, but I think that is correct for it to be "1".  The data
sheets for these parts that I checked had bit 1 marked as
reserved.  Only OMR1MSKL and OMR3MSKL had extra definitions
such as the _IO and _UOT.  The parts I checked which were
the sheets for the EX and SX (which cover another 6 or 7
parts) all had it with just a single bit defined on that
register.

Ayman

Re: [v2 PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Tony Breeds]

On Mon, Jul 18, 2011 at 08:31:01AM -0500, Ayman El-Khashab wrote:

> Yes, but I think that is correct for it to be "1".  The data
> sheets for these parts that I checked had bit 1 marked as
> reserved.  Only OMR1MSKL and OMR3MSKL had extra definitions
> such as the _IO and _UOT.  The parts I checked which were
> the sheets for the EX and SX (which cover another 6 or 7
> parts) all had it with just a single bit defined on that
> register.

Ahh okay.  I kind of think that this may need to be a seperate change.  At the
very least it needs to be explicitly mentioned in the change log.

Yours Tony

[v3 PATCH 0/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Ayman Elkhashab]

From: Ayman El-Khashab <ayman@elkhashab.com>

Changes from v1->v2
Added definitions for the bits in the OMRxMSKL registers
Refactored the setting of the OMRxMSKL registers for the 460SX
Added bit defines for the PECFG_460SX_DLLSTA register

Changes from v2->v3
Fixed commit message to be more clear as to what was done

Ayman El-Khashab (1):
  powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

 arch/powerpc/sysdev/ppc4xx_pci.c |   89 ++++++++++++++++++++++++++++++-------
 arch/powerpc/sysdev/ppc4xx_pci.h |   12 +++++
 2 files changed, 84 insertions(+), 17 deletions(-)

-- 
1.7.4.3

[v3 PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Ayman Elkhashab]

From: Ayman El-Khashab <ayman@elkhashab.com>

Adds a register to the config space for the 460sx.  Changes the vc0
detect to a pll detect.  maps configuration space to test the link
status.  changes the setup to enable gen2 devices to operate at gen2
speeds.  fixes mapping that was not correct for the 460sx.  added
bit definitions for the OMRxMSKL registers.  Removed reserved bit
that was set incorrectly in the OMR2MSKL register.

tested on the 460sx eiger and custom board

Signed-off-by: Ayman El-Khashab <ayman@elkhashab.com>
---
 arch/powerpc/sysdev/ppc4xx_pci.c |   89 ++++++++++++++++++++++++++++++-------
 arch/powerpc/sysdev/ppc4xx_pci.h |   12 +++++
 2 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
index ad330fe..eeeeb12 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -1092,6 +1092,10 @@ static int __init ppc460sx_pciex_core_init(struct device_node *np)
 	mtdcri(SDR0, PESDR1_460SX_HSSSLEW, 0xFFFF0000);
 	mtdcri(SDR0, PESDR2_460SX_HSSSLEW, 0xFFFF0000);
 
+	/* Set HSS PRBS enabled */
+	mtdcri(SDR0, PESDR0_460SX_HSSCTLSET, 0x00001130);
+	mtdcri(SDR0, PESDR2_460SX_HSSCTLSET, 0x00001130);
+
 	udelay(100);
 
 	/* De-assert PLLRESET */
@@ -1132,9 +1136,6 @@ static int ppc460sx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
 		dcri_clrset(SDR0, port->sdr_base + PESDRn_UTLSET2,
 				0, 0x01000000);
 
-	/*Gen-1*/
-	mtdcri(SDR0, port->sdr_base + PESDRn_460SX_RCEI, 0x08000000);
-
 	dcri_clrset(SDR0, port->sdr_base + PESDRn_RCSSET,
 			(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL),
 			PESDRx_RCSSET_RSTPYN);
@@ -1148,14 +1149,42 @@ static int ppc460sx_pciex_init_utl(struct ppc4xx_pciex_port *port)
 {
 	/* Max 128 Bytes */
 	out_be32 (port->utl_base + PEUTL_PBBSZ,   0x00000000);
+	/* Assert VRB and TXE - per datasheet turn off addr validation */
+	out_be32(port->utl_base + PEUTL_PCTL,  0x80800000);
 	return 0;
 }
 
+static void __init ppc460sx_pciex_check_link(struct ppc4xx_pciex_port *port)
+{
+	void __iomem *mbase;
+	int attempt = 50;
+
+	port->link = 0;
+
+	mbase = ioremap(port->cfg_space.start + 0x10000000, 0x1000);
+	if (mbase == NULL) {
+		printk(KERN_ERR "%s: Can't map internal config space !",
+			port->node->full_name);
+		goto done;
+	}
+
+	while (attempt && (0 == (in_le32(mbase + PECFG_460SX_DLLSTA)
+			& PECFG_460SX_DLLSTA_LINKUP))) {
+		attempt--;
+		mdelay(10);
+	}
+	if (attempt)
+		port->link = 1;
+done:
+	iounmap(mbase);
+
+}
+
 static struct ppc4xx_pciex_hwops ppc460sx_pcie_hwops __initdata = {
 	.core_init	= ppc460sx_pciex_core_init,
 	.port_init_hw	= ppc460sx_pciex_init_port_hw,
 	.setup_utl	= ppc460sx_pciex_init_utl,
-	.check_link	= ppc4xx_pciex_check_link_sdr,
+	.check_link	= ppc460sx_pciex_check_link,
 };
 
 #endif /* CONFIG_44x */
@@ -1338,15 +1367,15 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
 	if (rc != 0)
 		return rc;
 
-	if (ppc4xx_pciex_hwops->check_link)
-		ppc4xx_pciex_hwops->check_link(port);
-
 	/*
 	 * Initialize mapping: disable all regions and configure
 	 * CFG and REG regions based on resources in the device tree
 	 */
 	ppc4xx_pciex_port_init_mapping(port);
 
+	if (ppc4xx_pciex_hwops->check_link)
+		ppc4xx_pciex_hwops->check_link(port);
+
 	/*
 	 * Map UTL
 	 */
@@ -1360,13 +1389,23 @@ static int __init ppc4xx_pciex_port_init(struct ppc4xx_pciex_port *port)
 		ppc4xx_pciex_hwops->setup_utl(port);
 
 	/*
-	 * Check for VC0 active and assert RDY.
+	 * Check for VC0 active or PLL Locked and assert RDY.
 	 */
 	if (port->sdr_base) {
-		if (port->link &&
-		    ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
-					     1 << 16, 1 << 16, 5000)) {
-			printk(KERN_INFO "PCIE%d: VC0 not active\n", port->index);
+		if (of_device_is_compatible(port->node,
+				"ibm,plb-pciex-460sx")){
+			if (port->link && ppc4xx_pciex_wait_on_sdr(port,
+					PESDRn_RCSSTS,
+					1 << 12, 1 << 12, 5000)) {
+				printk(KERN_INFO "PCIE%d: PLL not locked\n",
+						port->index);
+				port->link = 0;
+			}
+		} else if (port->link &&
+			ppc4xx_pciex_wait_on_sdr(port, PESDRn_RCSSTS,
+				1 << 16, 1 << 16, 5000)) {
+			printk(KERN_INFO "PCIE%d: VC0 not active\n",
+					port->index);
 			port->link = 0;
 		}
 
@@ -1573,8 +1612,15 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAH, lah);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKH, 0x7fffffff);
-		/* Note that 3 here means enabled | single region */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL, sa | 3);
+		/*Enabled and single region */
+		if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+			dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL,
+				sa | DCRO_PEGPL_460SX_OMR1MSKL_UOT
+					| DCRO_PEGPL_OMRxMSKL_VAL);
+		else
+			dcr_write(port->dcrs, DCRO_PEGPL_OMR1MSKL,
+				sa | DCRO_PEGPL_OMR1MSKL_UOT
+					| DCRO_PEGPL_OMRxMSKL_VAL);
 		break;
 	case 1:
 		out_le32(mbase + PECFG_POM1LAH, pciah);
@@ -1582,8 +1628,8 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAH, lah);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKH, 0x7fffffff);
-		/* Note that 3 here means enabled | single region */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL, sa | 3);
+		dcr_write(port->dcrs, DCRO_PEGPL_OMR2MSKL,
+				sa | DCRO_PEGPL_OMRxMSKL_VAL);
 		break;
 	case 2:
 		out_le32(mbase + PECFG_POM2LAH, pciah);
@@ -1592,7 +1638,9 @@ static int __init ppc4xx_setup_one_pciex_POM(struct ppc4xx_pciex_port	*port,
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR3BAL, lal);
 		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKH, 0x7fffffff);
 		/* Note that 3 here means enabled | IO space !!! */
-		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL, sa | 3);
+		dcr_write(port->dcrs, DCRO_PEGPL_OMR3MSKL,
+				sa | DCRO_PEGPL_OMR3MSKL_IO
+					| DCRO_PEGPL_OMRxMSKL_VAL);
 		break;
 	}
 
@@ -1693,6 +1741,9 @@ static void __init ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
 		if (res->flags & IORESOURCE_PREFETCH)
 			sa |= 0x8;
 
+		if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+			sa |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+
 		out_le32(mbase + PECFG_BAR0HMPA, RES_TO_U32_HIGH(sa));
 		out_le32(mbase + PECFG_BAR0LMPA, RES_TO_U32_LOW(sa));
 
@@ -1854,6 +1905,10 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	}
 	out_le16(mbase + 0x202, val);
 
+	/* Enable Bus master, memory, and io space */
+	if (of_device_is_compatible(port->node, "ibm,plb-pciex-460sx"))
+		out_le16(mbase + 0x204, 0x7);
+
 	if (!port->endpoint) {
 		/* Set Class Code to PCI-PCI bridge and Revision Id to 1 */
 		out_le32(mbase + 0x208, 0x06040001);
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h b/arch/powerpc/sysdev/ppc4xx_pci.h
index 56d9e5d..61b3659 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.h
+++ b/arch/powerpc/sysdev/ppc4xx_pci.h
@@ -464,6 +464,18 @@
 #define PECFG_POM2LAL		0x390
 #define PECFG_POM2LAH		0x394
 
+/* 460sx only */
+#define PECFG_460SX_DLLSTA     0x3f8
+
+/* 460sx Bit Mappings */
+#define PECFG_460SX_DLLSTA_LINKUP	 0x00000010
+#define DCRO_PEGPL_460SX_OMR1MSKL_UOT	 0x00000004
+
+/* PEGPL Bit Mappings */
+#define DCRO_PEGPL_OMRxMSKL_VAL	 0x00000001
+#define DCRO_PEGPL_OMR1MSKL_UOT	 0x00000002
+#define DCRO_PEGPL_OMR3MSKL_IO	 0x00000002
+
 /* SDR Bit Mappings */
 #define PESDRx_RCSSET_HLDPLB	0x10000000
 #define PESDRx_RCSSET_RSTGU	0x01000000
-- 
1.7.4.3

Re: [v3 PATCH 1/1] powerpc/4xx: enable and fix pcie gen1/gen2 on the 460sx

[Tony Breeds]

On Wed, Jul 20, 2011 at 08:02:29AM -0500, Ayman Elkhashab wrote:
> From: Ayman El-Khashab <ayman@elkhashab.com>
> 
> Adds a register to the config space for the 460sx.  Changes the vc0
> detect to a pll detect.  maps configuration space to test the link
> status.  changes the setup to enable gen2 devices to operate at gen2
> speeds.  fixes mapping that was not correct for the 460sx.  added
> bit definitions for the OMRxMSKL registers.  Removed reserved bit
> that was set incorrectly in the OMR2MSKL register.

FWIW Looks good to me.

Yours Tony