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

login
register
mail settings
Submitter Ayman El-Khashab
Date July 14, 2011, 12:33 a.m.
Message ID <1310603611-8960-2-git-send-email-ayman@elkhashab.com>
Download mbox | patch
Permalink /patch/104613/
State Superseded
Headers show

Comments

Ayman El-Khashab - July 14, 2011, 12:33 a.m.
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(-)
Tony Breeds - July 14, 2011, 1:16 a.m.
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
Ayman El-Khashab - July 14, 2011, 4:04 p.m.
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
Benjamin Herrenschmidt - July 14, 2011, 10:36 p.m.
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/
Ayman Elkhashab - July 20, 2011, 1:02 p.m.
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(-)

Patch

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