* [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
@ 2011-12-20 12:44 Vinh Nguyen Huu Tuong
2011-12-20 15:31 ` Josh Boyer
0 siblings, 1 reply; 9+ messages in thread
From: Vinh Nguyen Huu Tuong @ 2011-12-20 12:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Josh Boyer, Matt Porter,
Kumar Gala, Paul Gortmaker, Anton Blanchard, Dave Kleikamp,
Grant Likely, Tony Breeds, Rob Herring, Jiri Kosina,
Lucas De Marchi, Ayman El-Khashab, linuxppc-dev, linux-kernel
Cc: Vinh Nguyen Huu Tuong
This patch extends PCI-E driver to support PCI-E for APM821xx SoC on Bluestone board.
Signed-off-by: Vinh Nguyen Huu Tuong <vhtnguyen@apm.com>
---
arch/powerpc/platforms/44x/Kconfig | 1 +
arch/powerpc/sysdev/ppc4xx_pci.c | 109 +++++++++++++++++++++++++++++++++++-
arch/powerpc/sysdev/ppc4xx_pci.h | 4 +
3 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index 762322c..cd62377 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -23,6 +23,7 @@ config BLUESTONE
default n
select PPC44x_SIMPLE
select APM821xx
+ select PPC4xx_PCI_EXPRESS
select IBM_EMAC_RGMII
help
This option enables support for the APM APM821xx Evaluation board.
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c
index 862f11b..4e866a5 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.c
+++ b/arch/powerpc/sysdev/ppc4xx_pci.c
@@ -1040,6 +1040,109 @@ static struct ppc4xx_pciex_hwops ppc460ex_pcie_hwops __initdata =
.check_link = ppc4xx_pciex_check_link_sdr,
};
+static int __init apm821xx_pciex_core_init(struct device_node *np)
+{
+ /* Return the number of pcie port */
+ return 1;
+}
+
+static int apm821xx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
+{
+ u32 val;
+ u32 utlset1;
+ u32 timeout;
+
+ /*
+ * Do a software reset on PCIe ports.
+ * This code is to fix the issue that pci drivers doesn't re-assign
+ * bus number for PCIE devices after Uboot
+ * scanned and configured all the buses (eg. PCIE NIC IntelPro/1000
+ * PT quad port, SAS LSI 1064E)
+ */
+
+ mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST + (port->index * 0x55), 0x0);
+ mdelay(10);
+
+ if (port->endpoint)
+ val = PTYPE_LEGACY_ENDPOINT << 20;
+ else
+ val = PTYPE_ROOT_PORT << 20;
+
+ if (port->index == 0) {
+ val |= LNKW_X1 << 12;
+ utlset1 = 0x00000000;
+ } else {
+ val |= LNKW_X4 << 12;
+ utlset1 = 0x20101101;
+ }
+
+ mtdcri(SDR0, port->sdr_base + PESDRn_DLPSET, val);
+ mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET1, utlset1);
+ mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET2, 0x01010000);
+
+ switch (port->index) {
+ case 0:
+ mtdcri(SDR0, PESDR0_460EX_L0CDRCTL, 0x00003230);
+ mtdcri(SDR0, PESDR0_460EX_L0DRV, 0x00000130);
+ mtdcri(SDR0, PESDR0_460EX_L0CLK, 0x00000006);
+
+ mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x10000000);
+ mdelay(50);
+ mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x30000000);
+ break;
+
+ case 1:
+ mtdcri(SDR0, PESDR1_460EX_L0CDRCTL, 0x00003230);
+ mtdcri(SDR0, PESDR1_460EX_L1CDRCTL, 0x00003230);
+ mtdcri(SDR0, PESDR1_460EX_L2CDRCTL, 0x00003230);
+ mtdcri(SDR0, PESDR1_460EX_L3CDRCTL, 0x00003230);
+ mtdcri(SDR0, PESDR1_460EX_L0DRV, 0x00000130);
+ mtdcri(SDR0, PESDR1_460EX_L1DRV, 0x00000130);
+ mtdcri(SDR0, PESDR1_460EX_L2DRV, 0x00000130);
+ mtdcri(SDR0, PESDR1_460EX_L3DRV, 0x00000130);
+ mtdcri(SDR0, PESDR1_460EX_L0CLK, 0x00000006);
+ mtdcri(SDR0, PESDR1_460EX_L1CLK, 0x00000006);
+ mtdcri(SDR0, PESDR1_460EX_L2CLK, 0x00000006);
+ mtdcri(SDR0, PESDR1_460EX_L3CLK, 0x00000006);
+
+ mtdcri(SDR0, PESDR1_460EX_PHY_CTL_RST, 0x10000000);
+ break;
+ }
+
+ mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
+ mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) |
+ (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN));
+
+ /* Poll for PHY reset */
+ timeout = 0;
+ while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
+ (port->index * 0x55)) & 0x1)) &&
+ (timeout < PCIE_PHY_RESET_TIMEOUT)) {
+ udelay(10);
+ timeout++;
+ }
+
+ if (timeout < PCIE_PHY_RESET_TIMEOUT) {
+ mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
+ (mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) &
+ ~(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL)) |
+ PESDRx_RCSSET_RSTPYN);
+
+ port->has_ibpre = 1;
+
+ return 0;
+ } else {
+ printk(KERN_INFO "PCIE: Can't reset PHY\n");
+ return -1;
+ }
+}
+
+static struct ppc4xx_pciex_hwops apm821xx_pcie_hwops __initdata = {
+ .core_init = apm821xx_pciex_core_init,
+ .port_init_hw = apm821xx_pciex_init_port_hw,
+ .setup_utl = ppc460ex_pciex_init_utl,
+};
+
static int __init ppc460sx_pciex_core_init(struct device_node *np)
{
/* HSS drive amplitude */
@@ -1304,6 +1407,8 @@ static int __init ppc4xx_pciex_check_core_init(struct device_node *np)
ppc4xx_pciex_hwops = &ppc460ex_pcie_hwops;
if (of_device_is_compatible(np, "ibm,plb-pciex-460sx"))
ppc4xx_pciex_hwops = &ppc460sx_pcie_hwops;
+ if (of_device_is_compatible(np, "ibm,plb-pciex-apm821xx"))
+ ppc4xx_pciex_hwops = &apm821xx_pcie_hwops;
#endif /* CONFIG_44x */
#ifdef CONFIG_40x
if (of_device_is_compatible(np, "ibm,plb-pciex-405ex"))
@@ -1751,9 +1856,9 @@ static void __init ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
* if it works
*/
out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
- out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
+ out_le32(mbase + PECFG_PIM0LAH, 0x00000008); /* Moving on HB */
out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
- out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
+ out_le32(mbase + PECFG_PIM1LAH, 0x0000000c); /* Moving on HB */
out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h b/arch/powerpc/sysdev/ppc4xx_pci.h
index 32ce763..faf3017 100644
--- a/arch/powerpc/sysdev/ppc4xx_pci.h
+++ b/arch/powerpc/sysdev/ppc4xx_pci.h
@@ -441,6 +441,7 @@
/*
* Config space register offsets
*/
+#define PECFG_ECDEVCTL 0x060
#define PECFG_ECRTCTL 0x074
#define PECFG_BAR0LMPA 0x210
@@ -448,6 +449,7 @@
#define PECFG_BAR1MPA 0x218
#define PECFG_BAR2LMPA 0x220
#define PECFG_BAR2HMPA 0x224
+#define PECFG_ECDEVCAPPA 0x25c
#define PECFG_PIMEN 0x33c
#define PECFG_PIM0LAL 0x340
@@ -494,5 +496,7 @@ enum
LNKW_X8 = 0x8
};
+/* Timout for reset phy */
+#define PCIE_PHY_RESET_TIMEOUT 10
#endif /* __PPC4XX_PCI_H__ */
--
1.7.2.5
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
2011-12-20 12:44 [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board Vinh Nguyen Huu Tuong
@ 2011-12-20 15:31 ` Josh Boyer
0 siblings, 0 replies; 9+ messages in thread
From: Josh Boyer @ 2011-12-20 15:31 UTC (permalink / raw)
To: Vinh Nguyen Huu Tuong
Cc: Ayman El-Khashab, Dave Kleikamp, Lucas De Marchi, Rob Herring,
Paul Gortmaker, Paul Mackerras, Anton Blanchard, Jiri Kosina,
linuxppc-dev, linux-kernel
On Tue, Dec 20, 2011 at 7:44 AM, Vinh Nguyen Huu Tuong
<vhtnguyen@apm.com> wrote:
> This patch extends PCI-E driver to support PCI-E for APM821xx SoC on Blue=
stone board.
>
> Signed-off-by: Vinh Nguyen Huu Tuong <vhtnguyen@apm.com>
> +static int apm821xx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
> +{
> + =A0 =A0 =A0 u32 val;
> + =A0 =A0 =A0 u32 utlset1;
> + =A0 =A0 =A0 u32 timeout;
> +
> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0* Do a software reset on PCIe ports.
> + =A0 =A0 =A0 =A0* This code is to fix the issue that pci drivers doesn't=
re-assign
> + =A0 =A0 =A0 =A0* bus number for PCIE devices after Uboot
> + =A0 =A0 =A0 =A0* scanned and configured all the buses (eg. PCIE NIC Int=
elPro/1000
> + =A0 =A0 =A0 =A0* PT quad port, SAS LSI 1064E)
> + =A0 =A0 =A0 =A0*/
> +
> + =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST + (port->index * 0x55=
), 0x0);
> + =A0 =A0 =A0 mdelay(10);
> +
> + =A0 =A0 =A0 if (port->endpoint)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val =3D PTYPE_LEGACY_ENDPOINT << 20;
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val =3D PTYPE_ROOT_PORT << 20;
> +
> + =A0 =A0 =A0 if (port->index =3D=3D 0) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val |=3D LNKW_X1 << 12;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 utlset1 =3D 0x00000000;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val |=3D LNKW_X4 << 12;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 utlset1 =3D 0x20101101;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_DLPSET, val);
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET1, utlset1);
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET2, 0x01010000);
> +
> + =A0 =A0 =A0 switch (port->index) {
> + =A0 =A0 =A0 case 0:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_L0CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_L0DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_L0CLK, 0x00000006=
);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x10=
000000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mdelay(50);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x30=
000000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 case 1:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L0CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L1CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L2CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L3CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L0DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L1DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L2DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L3DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L0CLK, 0x00000006=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L1CLK, 0x00000006=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L2CLK, 0x00000006=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L3CLK, 0x00000006=
);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_PHY_CTL_RST, 0x10=
000000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> + =A0 =A0 =A0 }
Do we need a default case here to catch oddness and exit the function?
> +
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET=
) |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN=
));
> +
> + =A0 =A0 =A0 /* Poll for PHY reset */
> + =A0 =A0 =A0 timeout =3D 0;
> + =A0 =A0 =A0 while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (port->index * 0x55)) & 0x1=
)) &&
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(timeout < PCIE_PHY_RESET_TIMEOUT)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(10);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 timeout++;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 if (timeout < PCIE_PHY_RESET_TIMEOUT) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET=
,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (mfdcri(SDR0, port->sdr_bas=
e + PESDRn_RCSSET) &
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ~(PESDRx_RCSSET_RSTGU | PES=
DRx_RCSSET_RSTDL)) |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PESDRx_RCSSET_RSTPYN);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 port->has_ibpre =3D 1;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_INFO "PCIE: Can't reset PHY\n")=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -1;
> + =A0 =A0 =A0 }
If we can't reset the PHY, does this whole function essentially fail?
Do the devices not get renumbered, etc? If so, you probably want to
make that KERN_ERR.
> @@ -1751,9 +1856,9 @@ static void __init ppc4xx_configure_pciex_PIMs(stru=
ct ppc4xx_pciex_port *port,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * if it works
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM0LAL, 0x00000000=
);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000000)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000008)=
; /* Moving on HB */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM1LAL, 0x00000000=
);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x00000000)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x0000000c)=
; /* Moving on HB */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAH, 0xffff000=
0);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAL, 0x0000000=
0);
Why are these values changed, and are those changes only needed on APM821xx=
?
> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h b/arch/powerpc/sysdev/ppc4x=
x_pci.h
> index 32ce763..faf3017 100644
> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
> @@ -441,6 +441,7 @@
> =A0/*
> =A0* Config space register offsets
> =A0*/
> +#define PECFG_ECDEVCTL =A0 =A0 =A0 =A0 0x060
> =A0#define PECFG_ECRTCTL =A0 =A0 =A0 =A0 =A00x074
>
> =A0#define PECFG_BAR0LMPA =A0 =A0 =A0 =A0 0x210
> @@ -448,6 +449,7 @@
> =A0#define PECFG_BAR1MPA =A0 =A0 =A0 =A0 =A00x218
> =A0#define PECFG_BAR2LMPA =A0 =A0 =A0 =A0 0x220
> =A0#define PECFG_BAR2HMPA =A0 =A0 =A0 =A0 0x224
> +#define PECFG_ECDEVCAPPA =A0 =A0 =A0 0x25c
>
> =A0#define PECFG_PIMEN =A0 =A0 =A0 =A0 =A0 =A00x33c
> =A0#define PECFG_PIM0LAL =A0 =A0 =A0 =A0 =A00x340
> @@ -494,5 +496,7 @@ enum
> =A0 =A0 =A0 =A0LNKW_X8 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x8
> =A0};
>
> +/* Timout for reset phy */
> +#define PCIE_PHY_RESET_TIMEOUT 10
Is this value applicable to all the 44x devices with PCI-e?
josh
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
@ 2011-12-20 15:31 ` Josh Boyer
0 siblings, 0 replies; 9+ messages in thread
From: Josh Boyer @ 2011-12-20 15:31 UTC (permalink / raw)
To: Vinh Nguyen Huu Tuong
Cc: Benjamin Herrenschmidt, Paul Mackerras, Matt Porter, Kumar Gala,
Paul Gortmaker, Anton Blanchard, Dave Kleikamp, Grant Likely,
Tony Breeds, Rob Herring, Jiri Kosina, Lucas De Marchi,
Ayman El-Khashab, linuxppc-dev, linux-kernel
On Tue, Dec 20, 2011 at 7:44 AM, Vinh Nguyen Huu Tuong
<vhtnguyen@apm.com> wrote:
> This patch extends PCI-E driver to support PCI-E for APM821xx SoC on Bluestone board.
>
> Signed-off-by: Vinh Nguyen Huu Tuong <vhtnguyen@apm.com>
> +static int apm821xx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
> +{
> + u32 val;
> + u32 utlset1;
> + u32 timeout;
> +
> + /*
> + * Do a software reset on PCIe ports.
> + * This code is to fix the issue that pci drivers doesn't re-assign
> + * bus number for PCIE devices after Uboot
> + * scanned and configured all the buses (eg. PCIE NIC IntelPro/1000
> + * PT quad port, SAS LSI 1064E)
> + */
> +
> + mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST + (port->index * 0x55), 0x0);
> + mdelay(10);
> +
> + if (port->endpoint)
> + val = PTYPE_LEGACY_ENDPOINT << 20;
> + else
> + val = PTYPE_ROOT_PORT << 20;
> +
> + if (port->index == 0) {
> + val |= LNKW_X1 << 12;
> + utlset1 = 0x00000000;
> + } else {
> + val |= LNKW_X4 << 12;
> + utlset1 = 0x20101101;
> + }
> +
> + mtdcri(SDR0, port->sdr_base + PESDRn_DLPSET, val);
> + mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET1, utlset1);
> + mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET2, 0x01010000);
> +
> + switch (port->index) {
> + case 0:
> + mtdcri(SDR0, PESDR0_460EX_L0CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR0_460EX_L0DRV, 0x00000130);
> + mtdcri(SDR0, PESDR0_460EX_L0CLK, 0x00000006);
> +
> + mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x10000000);
> + mdelay(50);
> + mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x30000000);
> + break;
> +
> + case 1:
> + mtdcri(SDR0, PESDR1_460EX_L0CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L1CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L2CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L3CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L0DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L1DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L2DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L3DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L0CLK, 0x00000006);
> + mtdcri(SDR0, PESDR1_460EX_L1CLK, 0x00000006);
> + mtdcri(SDR0, PESDR1_460EX_L2CLK, 0x00000006);
> + mtdcri(SDR0, PESDR1_460EX_L3CLK, 0x00000006);
> +
> + mtdcri(SDR0, PESDR1_460EX_PHY_CTL_RST, 0x10000000);
> + break;
> + }
Do we need a default case here to catch oddness and exit the function?
> +
> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
> + mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) |
> + (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN));
> +
> + /* Poll for PHY reset */
> + timeout = 0;
> + while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
> + (port->index * 0x55)) & 0x1)) &&
> + (timeout < PCIE_PHY_RESET_TIMEOUT)) {
> + udelay(10);
> + timeout++;
> + }
> +
> + if (timeout < PCIE_PHY_RESET_TIMEOUT) {
> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
> + (mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) &
> + ~(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL)) |
> + PESDRx_RCSSET_RSTPYN);
> +
> + port->has_ibpre = 1;
> +
> + return 0;
> + } else {
> + printk(KERN_INFO "PCIE: Can't reset PHY\n");
> + return -1;
> + }
If we can't reset the PHY, does this whole function essentially fail?
Do the devices not get renumbered, etc? If so, you probably want to
make that KERN_ERR.
> @@ -1751,9 +1856,9 @@ static void __init ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
> * if it works
> */
> out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
> - out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
> + out_le32(mbase + PECFG_PIM0LAH, 0x00000008); /* Moving on HB */
> out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
> - out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
> + out_le32(mbase + PECFG_PIM1LAH, 0x0000000c); /* Moving on HB */
> out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
> out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
Why are these values changed, and are those changes only needed on APM821xx?
> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h b/arch/powerpc/sysdev/ppc4xx_pci.h
> index 32ce763..faf3017 100644
> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
> @@ -441,6 +441,7 @@
> /*
> * Config space register offsets
> */
> +#define PECFG_ECDEVCTL 0x060
> #define PECFG_ECRTCTL 0x074
>
> #define PECFG_BAR0LMPA 0x210
> @@ -448,6 +449,7 @@
> #define PECFG_BAR1MPA 0x218
> #define PECFG_BAR2LMPA 0x220
> #define PECFG_BAR2HMPA 0x224
> +#define PECFG_ECDEVCAPPA 0x25c
>
> #define PECFG_PIMEN 0x33c
> #define PECFG_PIM0LAL 0x340
> @@ -494,5 +496,7 @@ enum
> LNKW_X8 = 0x8
> };
>
> +/* Timout for reset phy */
> +#define PCIE_PHY_RESET_TIMEOUT 10
Is this value applicable to all the 44x devices with PCI-e?
josh
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
2011-12-20 15:31 ` Josh Boyer
@ 2011-12-21 4:47 ` Vinh Huu Tuong Nguyen
-1 siblings, 0 replies; 9+ messages in thread
From: Vinh Huu Tuong Nguyen @ 2011-12-21 4:47 UTC (permalink / raw)
To: Josh Boyer
Cc: Ayman El-Khashab, Dave Kleikamp, Lucas De Marchi, Rob Herring,
Paul Gortmaker, Paul Mackerras, Anton Blanchard, Jiri Kosina,
linuxppc-dev, linux-kernel
Hi Josh,
Please see below for my comments. If you have any concerns or suggestion,
please let me know.
Best regards,
Vinh Nguyen.
-----Original Message-----
From: Josh Boyer [mailto:jwboyer@gmail.com]
Sent: Tuesday, December 20, 2011 10:32 PM
To: Vinh Nguyen Huu Tuong
Cc: Benjamin Herrenschmidt; Paul Mackerras; Matt Porter; Kumar Gala; Paul
Gortmaker; Anton Blanchard; Dave Kleikamp; Grant Likely; Tony Breeds; Rob
Herring; Jiri Kosina; Lucas De Marchi; Ayman El-Khashab;
linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC
and Bluestone board
On Tue, Dec 20, 2011 at 7:44 AM, Vinh Nguyen Huu Tuong
<vhtnguyen@apm.com> wrote:
> This patch extends PCI-E driver to support PCI-E for APM821xx SoC on
Bluestone board.
>
> Signed-off-by: Vinh Nguyen Huu Tuong <vhtnguyen@apm.com>
> +static int apm821xx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
> +{
> + =A0 =A0 =A0 u32 val;
> + =A0 =A0 =A0 u32 utlset1;
> + =A0 =A0 =A0 u32 timeout;
> +
> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0* Do a software reset on PCIe ports.
> + =A0 =A0 =A0 =A0* This code is to fix the issue that pci drivers doesn't
re-assign
> + =A0 =A0 =A0 =A0* bus number for PCIE devices after Uboot
> + =A0 =A0 =A0 =A0* scanned and configured all the buses (eg. PCIE NIC
IntelPro/1000
> + =A0 =A0 =A0 =A0* PT quad port, SAS LSI 1064E)
> + =A0 =A0 =A0 =A0*/
> +
> + =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST + (port->index * 0x55=
),
0x0);
> + =A0 =A0 =A0 mdelay(10);
> +
> + =A0 =A0 =A0 if (port->endpoint)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val =3D PTYPE_LEGACY_ENDPOINT << 20;
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val =3D PTYPE_ROOT_PORT << 20;
> +
> + =A0 =A0 =A0 if (port->index =3D=3D 0) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val |=3D LNKW_X1 << 12;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 utlset1 =3D 0x00000000;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 val |=3D LNKW_X4 << 12;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 utlset1 =3D 0x20101101;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_DLPSET, val);
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET1, utlset1);
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET2, 0x01010000);
> +
> + =A0 =A0 =A0 switch (port->index) {
> + =A0 =A0 =A0 case 0:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_L0CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_L0DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_L0CLK, 0x00000006=
);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x10=
000000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mdelay(50);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x30=
000000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> +
> + =A0 =A0 =A0 case 1:
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L0CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L1CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L2CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L3CDRCTL, 0x00003=
230);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L0DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L1DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L2DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L3DRV, 0x00000130=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L0CLK, 0x00000006=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L1CLK, 0x00000006=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L2CLK, 0x00000006=
);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_L3CLK, 0x00000006=
);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, PESDR1_460EX_PHY_CTL_RST, 0x10=
000000);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> + =A0 =A0 =A0 }
Do we need a default case here to catch oddness and exit the function?
[Vinh Nguyen] we've already checked the port->index before calling this
function, so we assume that the port->index never exceed the checked
values.
> +
> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET=
) |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN=
));
> +
> + =A0 =A0 =A0 /* Poll for PHY reset */
> + =A0 =A0 =A0 timeout =3D 0;
> + =A0 =A0 =A0 while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (port->index * 0x55)) & 0x1=
)) &&
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(timeout < PCIE_PHY_RESET_TIMEOUT)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(10);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 timeout++;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 if (timeout < PCIE_PHY_RESET_TIMEOUT) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET=
,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (mfdcri(SDR0, port->sdr_bas=
e + PESDRn_RCSSET) &
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ~(PESDRx_RCSSET_RSTGU | PES=
DRx_RCSSET_RSTDL)) |
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PESDRx_RCSSET_RSTPYN);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 port->has_ibpre =3D 1;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> + =A0 =A0 =A0 } else {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_INFO "PCIE: Can't reset PHY\n")=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -1;
> + =A0 =A0 =A0 }
If we can't reset the PHY, does this whole function essentially fail?
Do the devices not get renumbered, etc? If so, you probably want to
make that KERN_ERR.
[Vinh Nguyen] if we can't reset the PHY, maybe the device can't work
properly. I will update codes to return the error in case PHY can't reset.
> @@ -1751,9 +1856,9 @@ static void __init
ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * if it works
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM0LAL, 0x00000000=
);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000000)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000008)=
; /* Moving
on HB */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM1LAL, 0x00000000=
);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x00000000)=
;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x0000000c)=
; /* Moving
on HB */
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAH, 0xffff000=
0);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAL, 0x0000000=
0);
Why are these values changed, and are those changes only needed on
APM821xx?
[Vinh Nguyen] In system memory map of 460EX chip, we have an "alias DDR
SDRAM" area that is Local Memory Alias for HB(high bandwidth), so we tried
to initialize it for speedup. For APM821xx, although we didn't mention
about this area, this configuration still works well. So we keep it.
> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h
b/arch/powerpc/sysdev/ppc4xx_pci.h
> index 32ce763..faf3017 100644
> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
> @@ -441,6 +441,7 @@
> =A0/*
> =A0* Config space register offsets
> =A0*/
> +#define PECFG_ECDEVCTL =A0 =A0 =A0 =A0 0x060
> =A0#define PECFG_ECRTCTL =A0 =A0 =A0 =A0 =A00x074
>
> =A0#define PECFG_BAR0LMPA =A0 =A0 =A0 =A0 0x210
> @@ -448,6 +449,7 @@
> =A0#define PECFG_BAR1MPA =A0 =A0 =A0 =A0 =A00x218
> =A0#define PECFG_BAR2LMPA =A0 =A0 =A0 =A0 0x220
> =A0#define PECFG_BAR2HMPA =A0 =A0 =A0 =A0 0x224
> +#define PECFG_ECDEVCAPPA =A0 =A0 =A0 0x25c
>
> =A0#define PECFG_PIMEN =A0 =A0 =A0 =A0 =A0 =A00x33c
> =A0#define PECFG_PIM0LAL =A0 =A0 =A0 =A0 =A00x340
> @@ -494,5 +496,7 @@ enum
> =A0 =A0 =A0 =A0LNKW_X8 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x8
> =A0};
>
> +/* Timout for reset phy */
> +#define PCIE_PHY_RESET_TIMEOUT 10
Is this value applicable to all the 44x devices with PCI-e?
[Vinh Nguyen] At this time, we only checked this on APM821xx chips.
josh
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
@ 2011-12-21 4:47 ` Vinh Huu Tuong Nguyen
0 siblings, 0 replies; 9+ messages in thread
From: Vinh Huu Tuong Nguyen @ 2011-12-21 4:47 UTC (permalink / raw)
To: Josh Boyer
Cc: Benjamin Herrenschmidt, Paul Mackerras, Matt Porter, Kumar Gala,
Paul Gortmaker, Anton Blanchard, Dave Kleikamp, Grant Likely,
Tony Breeds, Rob Herring, Jiri Kosina, Lucas De Marchi,
Ayman El-Khashab, linuxppc-dev, linux-kernel
Hi Josh,
Please see below for my comments. If you have any concerns or suggestion,
please let me know.
Best regards,
Vinh Nguyen.
-----Original Message-----
From: Josh Boyer [mailto:jwboyer@gmail.com]
Sent: Tuesday, December 20, 2011 10:32 PM
To: Vinh Nguyen Huu Tuong
Cc: Benjamin Herrenschmidt; Paul Mackerras; Matt Porter; Kumar Gala; Paul
Gortmaker; Anton Blanchard; Dave Kleikamp; Grant Likely; Tony Breeds; Rob
Herring; Jiri Kosina; Lucas De Marchi; Ayman El-Khashab;
linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC
and Bluestone board
On Tue, Dec 20, 2011 at 7:44 AM, Vinh Nguyen Huu Tuong
<vhtnguyen@apm.com> wrote:
> This patch extends PCI-E driver to support PCI-E for APM821xx SoC on
Bluestone board.
>
> Signed-off-by: Vinh Nguyen Huu Tuong <vhtnguyen@apm.com>
> +static int apm821xx_pciex_init_port_hw(struct ppc4xx_pciex_port *port)
> +{
> + u32 val;
> + u32 utlset1;
> + u32 timeout;
> +
> + /*
> + * Do a software reset on PCIe ports.
> + * This code is to fix the issue that pci drivers doesn't
re-assign
> + * bus number for PCIE devices after Uboot
> + * scanned and configured all the buses (eg. PCIE NIC
IntelPro/1000
> + * PT quad port, SAS LSI 1064E)
> + */
> +
> + mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST + (port->index * 0x55),
0x0);
> + mdelay(10);
> +
> + if (port->endpoint)
> + val = PTYPE_LEGACY_ENDPOINT << 20;
> + else
> + val = PTYPE_ROOT_PORT << 20;
> +
> + if (port->index == 0) {
> + val |= LNKW_X1 << 12;
> + utlset1 = 0x00000000;
> + } else {
> + val |= LNKW_X4 << 12;
> + utlset1 = 0x20101101;
> + }
> +
> + mtdcri(SDR0, port->sdr_base + PESDRn_DLPSET, val);
> + mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET1, utlset1);
> + mtdcri(SDR0, port->sdr_base + PESDRn_UTLSET2, 0x01010000);
> +
> + switch (port->index) {
> + case 0:
> + mtdcri(SDR0, PESDR0_460EX_L0CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR0_460EX_L0DRV, 0x00000130);
> + mtdcri(SDR0, PESDR0_460EX_L0CLK, 0x00000006);
> +
> + mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x10000000);
> + mdelay(50);
> + mtdcri(SDR0, PESDR0_460EX_PHY_CTL_RST, 0x30000000);
> + break;
> +
> + case 1:
> + mtdcri(SDR0, PESDR1_460EX_L0CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L1CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L2CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L3CDRCTL, 0x00003230);
> + mtdcri(SDR0, PESDR1_460EX_L0DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L1DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L2DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L3DRV, 0x00000130);
> + mtdcri(SDR0, PESDR1_460EX_L0CLK, 0x00000006);
> + mtdcri(SDR0, PESDR1_460EX_L1CLK, 0x00000006);
> + mtdcri(SDR0, PESDR1_460EX_L2CLK, 0x00000006);
> + mtdcri(SDR0, PESDR1_460EX_L3CLK, 0x00000006);
> +
> + mtdcri(SDR0, PESDR1_460EX_PHY_CTL_RST, 0x10000000);
> + break;
> + }
Do we need a default case here to catch oddness and exit the function?
[Vinh Nguyen] we've already checked the port->index before calling this
function, so we assume that the port->index never exceed the checked
values.
> +
> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
> + mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) |
> + (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN));
> +
> + /* Poll for PHY reset */
> + timeout = 0;
> + while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
> + (port->index * 0x55)) & 0x1)) &&
> + (timeout < PCIE_PHY_RESET_TIMEOUT)) {
> + udelay(10);
> + timeout++;
> + }
> +
> + if (timeout < PCIE_PHY_RESET_TIMEOUT) {
> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
> + (mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) &
> + ~(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL)) |
> + PESDRx_RCSSET_RSTPYN);
> +
> + port->has_ibpre = 1;
> +
> + return 0;
> + } else {
> + printk(KERN_INFO "PCIE: Can't reset PHY\n");
> + return -1;
> + }
If we can't reset the PHY, does this whole function essentially fail?
Do the devices not get renumbered, etc? If so, you probably want to
make that KERN_ERR.
[Vinh Nguyen] if we can't reset the PHY, maybe the device can't work
properly. I will update codes to return the error in case PHY can't reset.
> @@ -1751,9 +1856,9 @@ static void __init
ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
> * if it works
> */
> out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
> - out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
> + out_le32(mbase + PECFG_PIM0LAH, 0x00000008); /* Moving
on HB */
> out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
> - out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
> + out_le32(mbase + PECFG_PIM1LAH, 0x0000000c); /* Moving
on HB */
> out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
> out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
Why are these values changed, and are those changes only needed on
APM821xx?
[Vinh Nguyen] In system memory map of 460EX chip, we have an "alias DDR
SDRAM" area that is Local Memory Alias for HB(high bandwidth), so we tried
to initialize it for speedup. For APM821xx, although we didn't mention
about this area, this configuration still works well. So we keep it.
> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h
b/arch/powerpc/sysdev/ppc4xx_pci.h
> index 32ce763..faf3017 100644
> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
> @@ -441,6 +441,7 @@
> /*
> * Config space register offsets
> */
> +#define PECFG_ECDEVCTL 0x060
> #define PECFG_ECRTCTL 0x074
>
> #define PECFG_BAR0LMPA 0x210
> @@ -448,6 +449,7 @@
> #define PECFG_BAR1MPA 0x218
> #define PECFG_BAR2LMPA 0x220
> #define PECFG_BAR2HMPA 0x224
> +#define PECFG_ECDEVCAPPA 0x25c
>
> #define PECFG_PIMEN 0x33c
> #define PECFG_PIM0LAL 0x340
> @@ -494,5 +496,7 @@ enum
> LNKW_X8 = 0x8
> };
>
> +/* Timout for reset phy */
> +#define PCIE_PHY_RESET_TIMEOUT 10
Is this value applicable to all the 44x devices with PCI-e?
[Vinh Nguyen] At this time, we only checked this on APM821xx chips.
josh
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
2011-12-21 4:47 ` Vinh Huu Tuong Nguyen
@ 2011-12-21 20:20 ` Josh Boyer
-1 siblings, 0 replies; 9+ messages in thread
From: Josh Boyer @ 2011-12-21 20:20 UTC (permalink / raw)
To: Vinh Huu Tuong Nguyen
Cc: Ayman El-Khashab, Dave Kleikamp, Lucas De Marchi, Rob Herring,
Paul Gortmaker, Paul Mackerras, Anton Blanchard, Jiri Kosina,
linuxppc-dev, linux-kernel
On Tue, Dec 20, 2011 at 11:47 PM, Vinh Huu Tuong Nguyen
<vhtnguyen@apm.com> wrote:
>> +
>> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mfdcri(SDR0, port->sdr_base + PESDRn_RCSSE=
T) |
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPY=
N));
>> +
>> + =A0 =A0 =A0 /* Poll for PHY reset */
>> + =A0 =A0 =A0 timeout =3D 0;
>> + =A0 =A0 =A0 while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (port->index * 0x55)) & 0x=
1)) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(timeout < PCIE_PHY_RESET_TIMEOUT)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(10);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 timeout++;
>> + =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 if (timeout < PCIE_PHY_RESET_TIMEOUT) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSE=
T,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (mfdcri(SDR0, port->sdr_ba=
se + PESDRn_RCSSET) &
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ~(PESDRx_RCSSET_RSTGU | PE=
SDRx_RCSSET_RSTDL)) |
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PESDRx_RCSSET_RSTPYN);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 port->has_ibpre =3D 1;
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
>> + =A0 =A0 =A0 } else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_INFO "PCIE: Can't reset PHY\n"=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -1;
>> + =A0 =A0 =A0 }
>
> If we can't reset the PHY, does this whole function essentially fail?
> Do the devices not get renumbered, etc? =A0If so, you probably want to
> make that KERN_ERR.
> [Vinh Nguyen] if we can't reset the PHY, maybe the device can't work
> properly. I will update codes to return the error in case PHY can't reset=
.
OK.
>> @@ -1751,9 +1856,9 @@ static void __init
> ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * if it works
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM0LAL, 0x0000000=
0);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000000=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000008=
); /* Moving
> on HB */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM1LAL, 0x0000000=
0);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x00000000=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x0000000c=
); /* Moving
> on HB */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAH, 0xffff00=
00);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAL, 0x000000=
00);
>
> Why are these values changed, and are those changes only needed on
> APM821xx?
> [Vinh Nguyen] In system memory map of 460EX chip, we have an "alias DDR
> SDRAM" area that is Local Memory Alias for HB(high bandwidth), so we trie=
d
> to initialize it for speedup. For APM821xx, although we didn't mention
> about this area, this configuration still works well. So we keep it.
This function isn't just called for 460EX or APM821xx SoCs. It's
called on any 4xx SoC with PCIe configured.
Is this change going to work on them all? If not, it needs to be
conditionalized.
>> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h
> b/arch/powerpc/sysdev/ppc4xx_pci.h
>> index 32ce763..faf3017 100644
>> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
>> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
>> @@ -441,6 +441,7 @@
>> =A0/*
>> =A0* Config space register offsets
>> =A0*/
>> +#define PECFG_ECDEVCTL =A0 =A0 =A0 =A0 0x060
>> =A0#define PECFG_ECRTCTL =A0 =A0 =A0 =A0 =A00x074
>>
>> =A0#define PECFG_BAR0LMPA =A0 =A0 =A0 =A0 0x210
>> @@ -448,6 +449,7 @@
>> =A0#define PECFG_BAR1MPA =A0 =A0 =A0 =A0 =A00x218
>> =A0#define PECFG_BAR2LMPA =A0 =A0 =A0 =A0 0x220
>> =A0#define PECFG_BAR2HMPA =A0 =A0 =A0 =A0 0x224
>> +#define PECFG_ECDEVCAPPA =A0 =A0 =A0 0x25c
>>
>> =A0#define PECFG_PIMEN =A0 =A0 =A0 =A0 =A0 =A00x33c
>> =A0#define PECFG_PIM0LAL =A0 =A0 =A0 =A0 =A00x340
>> @@ -494,5 +496,7 @@ enum
>> =A0 =A0 =A0 =A0LNKW_X8 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x8
>> =A0};
>>
>> +/* Timout for reset phy */
>> +#define PCIE_PHY_RESET_TIMEOUT 10
>
> Is this value applicable to all the 44x devices with PCI-e?
> [Vinh Nguyen] At this time, we only checked this on APM821xx chips.
Then it should probably be conditionalized somehow. Or put as a
worst-case value that will work for them all.
josh
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
@ 2011-12-21 20:20 ` Josh Boyer
0 siblings, 0 replies; 9+ messages in thread
From: Josh Boyer @ 2011-12-21 20:20 UTC (permalink / raw)
To: Vinh Huu Tuong Nguyen
Cc: Benjamin Herrenschmidt, Paul Mackerras, Matt Porter, Kumar Gala,
Paul Gortmaker, Anton Blanchard, Dave Kleikamp, Grant Likely,
Tony Breeds, Rob Herring, Jiri Kosina, Lucas De Marchi,
Ayman El-Khashab, linuxppc-dev, linux-kernel
On Tue, Dec 20, 2011 at 11:47 PM, Vinh Huu Tuong Nguyen
<vhtnguyen@apm.com> wrote:
>> +
>> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> + mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) |
>> + (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN));
>> +
>> + /* Poll for PHY reset */
>> + timeout = 0;
>> + while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
>> + (port->index * 0x55)) & 0x1)) &&
>> + (timeout < PCIE_PHY_RESET_TIMEOUT)) {
>> + udelay(10);
>> + timeout++;
>> + }
>> +
>> + if (timeout < PCIE_PHY_RESET_TIMEOUT) {
>> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> + (mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) &
>> + ~(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL)) |
>> + PESDRx_RCSSET_RSTPYN);
>> +
>> + port->has_ibpre = 1;
>> +
>> + return 0;
>> + } else {
>> + printk(KERN_INFO "PCIE: Can't reset PHY\n");
>> + return -1;
>> + }
>
> If we can't reset the PHY, does this whole function essentially fail?
> Do the devices not get renumbered, etc? If so, you probably want to
> make that KERN_ERR.
> [Vinh Nguyen] if we can't reset the PHY, maybe the device can't work
> properly. I will update codes to return the error in case PHY can't reset.
OK.
>> @@ -1751,9 +1856,9 @@ static void __init
> ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
>> * if it works
>> */
>> out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
>> - out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
>> + out_le32(mbase + PECFG_PIM0LAH, 0x00000008); /* Moving
> on HB */
>> out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
>> - out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
>> + out_le32(mbase + PECFG_PIM1LAH, 0x0000000c); /* Moving
> on HB */
>> out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
>> out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
>
> Why are these values changed, and are those changes only needed on
> APM821xx?
> [Vinh Nguyen] In system memory map of 460EX chip, we have an "alias DDR
> SDRAM" area that is Local Memory Alias for HB(high bandwidth), so we tried
> to initialize it for speedup. For APM821xx, although we didn't mention
> about this area, this configuration still works well. So we keep it.
This function isn't just called for 460EX or APM821xx SoCs. It's
called on any 4xx SoC with PCIe configured.
Is this change going to work on them all? If not, it needs to be
conditionalized.
>> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h
> b/arch/powerpc/sysdev/ppc4xx_pci.h
>> index 32ce763..faf3017 100644
>> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
>> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
>> @@ -441,6 +441,7 @@
>> /*
>> * Config space register offsets
>> */
>> +#define PECFG_ECDEVCTL 0x060
>> #define PECFG_ECRTCTL 0x074
>>
>> #define PECFG_BAR0LMPA 0x210
>> @@ -448,6 +449,7 @@
>> #define PECFG_BAR1MPA 0x218
>> #define PECFG_BAR2LMPA 0x220
>> #define PECFG_BAR2HMPA 0x224
>> +#define PECFG_ECDEVCAPPA 0x25c
>>
>> #define PECFG_PIMEN 0x33c
>> #define PECFG_PIM0LAL 0x340
>> @@ -494,5 +496,7 @@ enum
>> LNKW_X8 = 0x8
>> };
>>
>> +/* Timout for reset phy */
>> +#define PCIE_PHY_RESET_TIMEOUT 10
>
> Is this value applicable to all the 44x devices with PCI-e?
> [Vinh Nguyen] At this time, we only checked this on APM821xx chips.
Then it should probably be conditionalized somehow. Or put as a
worst-case value that will work for them all.
josh
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
2011-12-21 20:20 ` Josh Boyer
@ 2011-12-26 8:00 ` Vinh Huu Tuong Nguyen
-1 siblings, 0 replies; 9+ messages in thread
From: Vinh Huu Tuong Nguyen @ 2011-12-26 8:00 UTC (permalink / raw)
To: Josh Boyer
Cc: Ayman El-Khashab, Dave Kleikamp, Lucas De Marchi, Rob Herring,
Paul Gortmaker, Paul Mackerras, Anton Blanchard, Jiri Kosina,
linuxppc-dev, linux-kernel
-----Original Message-----
From: Josh Boyer [mailto:jwboyer@gmail.com]
Sent: Thursday, December 22, 2011 3:20 AM
To: Vinh Huu Tuong Nguyen
Cc: Benjamin Herrenschmidt; Paul Mackerras; Matt Porter; Kumar Gala; Paul
Gortmaker; Anton Blanchard; Dave Kleikamp; Grant Likely; Tony Breeds; Rob
Herring; Jiri Kosina; Lucas De Marchi; Ayman El-Khashab;
linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC
and Bluestone board
On Tue, Dec 20, 2011 at 11:47 PM, Vinh Huu Tuong Nguyen
<vhtnguyen@apm.com> wrote:
>> +
>> + =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mfdcri(SDR0, port->sdr_base + PESDRn_RCSSE=
T) |
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPY=
N));
>> +
>> + =A0 =A0 =A0 /* Poll for PHY reset */
>> + =A0 =A0 =A0 timeout =3D 0;
>> + =A0 =A0 =A0 while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (port->index * 0x55)) & 0x=
1)) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(timeout < PCIE_PHY_RESET_TIMEOUT)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(10);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 timeout++;
>> + =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 if (timeout < PCIE_PHY_RESET_TIMEOUT) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mtdcri(SDR0, port->sdr_base + PESDRn_RCSSE=
T,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (mfdcri(SDR0, port->sdr_ba=
se + PESDRn_RCSSET) &
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ~(PESDRx_RCSSET_RSTGU | PE=
SDRx_RCSSET_RSTDL)) |
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PESDRx_RCSSET_RSTPYN);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 port->has_ibpre =3D 1;
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
>> + =A0 =A0 =A0 } else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_INFO "PCIE: Can't reset PHY\n"=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -1;
>> + =A0 =A0 =A0 }
>
> If we can't reset the PHY, does this whole function essentially fail?
> Do the devices not get renumbered, etc? =A0If so, you probably want to
> make that KERN_ERR.
> [Vinh Nguyen] if we can't reset the PHY, maybe the device can't work
> properly. I will update codes to return the error in case PHY can't
reset.
OK.
>> @@ -1751,9 +1856,9 @@ static void __init
> ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * if it works
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM0LAL, 0x0000000=
0);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000000=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM0LAH, 0x00000008=
); /* Moving
> on HB */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM1LAL, 0x0000000=
0);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x00000000=
);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 out_le32(mbase + PECFG_PIM1LAH, 0x0000000c=
); /* Moving
> on HB */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAH, 0xffff00=
00);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0out_le32(mbase + PECFG_PIM01SAL, 0x000000=
00);
>
> Why are these values changed, and are those changes only needed on
> APM821xx?
> [Vinh Nguyen] In system memory map of 460EX chip, we have an "alias DDR
> SDRAM" area that is Local Memory Alias for HB(high bandwidth), so we
tried
> to initialize it for speedup. For APM821xx, although we didn't mention
> about this area, this configuration still works well. So we keep it.
This function isn't just called for 460EX or APM821xx SoCs. It's
called on any 4xx SoC with PCIe configured.
Is this change going to work on them all? If not, it needs to be
conditionalized.
[Vinh Nguyen] I will update my codes as your comments. Please see next
submitted codes.
>> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h
> b/arch/powerpc/sysdev/ppc4xx_pci.h
>> index 32ce763..faf3017 100644
>> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
>> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
>> @@ -441,6 +441,7 @@
>> =A0/*
>> =A0* Config space register offsets
>> =A0*/
>> +#define PECFG_ECDEVCTL =A0 =A0 =A0 =A0 0x060
>> =A0#define PECFG_ECRTCTL =A0 =A0 =A0 =A0 =A00x074
>>
>> =A0#define PECFG_BAR0LMPA =A0 =A0 =A0 =A0 0x210
>> @@ -448,6 +449,7 @@
>> =A0#define PECFG_BAR1MPA =A0 =A0 =A0 =A0 =A00x218
>> =A0#define PECFG_BAR2LMPA =A0 =A0 =A0 =A0 0x220
>> =A0#define PECFG_BAR2HMPA =A0 =A0 =A0 =A0 0x224
>> +#define PECFG_ECDEVCAPPA =A0 =A0 =A0 0x25c
>>
>> =A0#define PECFG_PIMEN =A0 =A0 =A0 =A0 =A0 =A00x33c
>> =A0#define PECFG_PIM0LAL =A0 =A0 =A0 =A0 =A00x340
>> @@ -494,5 +496,7 @@ enum
>> =A0 =A0 =A0 =A0LNKW_X8 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x8
>> =A0};
>>
>> +/* Timout for reset phy */
>> +#define PCIE_PHY_RESET_TIMEOUT 10
>
> Is this value applicable to all the 44x devices with PCI-e?
> [Vinh Nguyen] At this time, we only checked this on APM821xx chips.
Then it should probably be conditionalized somehow. Or put as a
worst-case value that will work for them all.
[Vinh Nguyen] At this time, this definition is called in
apm821xx_pciex_init_port_hw function that is used only for apm821xx chip.
So I think it's ok, but as your recommendation, I'll check with various
boards that I have to get the best value and update it in next submitted
codes.
Best regards,
Vinh Nguyen.
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board
@ 2011-12-26 8:00 ` Vinh Huu Tuong Nguyen
0 siblings, 0 replies; 9+ messages in thread
From: Vinh Huu Tuong Nguyen @ 2011-12-26 8:00 UTC (permalink / raw)
To: Josh Boyer
Cc: Benjamin Herrenschmidt, Paul Mackerras, Matt Porter, Kumar Gala,
Paul Gortmaker, Anton Blanchard, Dave Kleikamp, Grant Likely,
Tony Breeds, Rob Herring, Jiri Kosina, Lucas De Marchi,
Ayman El-Khashab, linuxppc-dev, linux-kernel
-----Original Message-----
From: Josh Boyer [mailto:jwboyer@gmail.com]
Sent: Thursday, December 22, 2011 3:20 AM
To: Vinh Huu Tuong Nguyen
Cc: Benjamin Herrenschmidt; Paul Mackerras; Matt Porter; Kumar Gala; Paul
Gortmaker; Anton Blanchard; Dave Kleikamp; Grant Likely; Tony Breeds; Rob
Herring; Jiri Kosina; Lucas De Marchi; Ayman El-Khashab;
linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC
and Bluestone board
On Tue, Dec 20, 2011 at 11:47 PM, Vinh Huu Tuong Nguyen
<vhtnguyen@apm.com> wrote:
>> +
>> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> + mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) |
>> + (PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTPYN));
>> +
>> + /* Poll for PHY reset */
>> + timeout = 0;
>> + while ((!(mfdcri(SDR0, PESDR0_460EX_RSTSTA +
>> + (port->index * 0x55)) & 0x1)) &&
>> + (timeout < PCIE_PHY_RESET_TIMEOUT)) {
>> + udelay(10);
>> + timeout++;
>> + }
>> +
>> + if (timeout < PCIE_PHY_RESET_TIMEOUT) {
>> + mtdcri(SDR0, port->sdr_base + PESDRn_RCSSET,
>> + (mfdcri(SDR0, port->sdr_base + PESDRn_RCSSET) &
>> + ~(PESDRx_RCSSET_RSTGU | PESDRx_RCSSET_RSTDL)) |
>> + PESDRx_RCSSET_RSTPYN);
>> +
>> + port->has_ibpre = 1;
>> +
>> + return 0;
>> + } else {
>> + printk(KERN_INFO "PCIE: Can't reset PHY\n");
>> + return -1;
>> + }
>
> If we can't reset the PHY, does this whole function essentially fail?
> Do the devices not get renumbered, etc? If so, you probably want to
> make that KERN_ERR.
> [Vinh Nguyen] if we can't reset the PHY, maybe the device can't work
> properly. I will update codes to return the error in case PHY can't
reset.
OK.
>> @@ -1751,9 +1856,9 @@ static void __init
> ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
>> * if it works
>> */
>> out_le32(mbase + PECFG_PIM0LAL, 0x00000000);
>> - out_le32(mbase + PECFG_PIM0LAH, 0x00000000);
>> + out_le32(mbase + PECFG_PIM0LAH, 0x00000008); /* Moving
> on HB */
>> out_le32(mbase + PECFG_PIM1LAL, 0x00000000);
>> - out_le32(mbase + PECFG_PIM1LAH, 0x00000000);
>> + out_le32(mbase + PECFG_PIM1LAH, 0x0000000c); /* Moving
> on HB */
>> out_le32(mbase + PECFG_PIM01SAH, 0xffff0000);
>> out_le32(mbase + PECFG_PIM01SAL, 0x00000000);
>
> Why are these values changed, and are those changes only needed on
> APM821xx?
> [Vinh Nguyen] In system memory map of 460EX chip, we have an "alias DDR
> SDRAM" area that is Local Memory Alias for HB(high bandwidth), so we
tried
> to initialize it for speedup. For APM821xx, although we didn't mention
> about this area, this configuration still works well. So we keep it.
This function isn't just called for 460EX or APM821xx SoCs. It's
called on any 4xx SoC with PCIe configured.
Is this change going to work on them all? If not, it needs to be
conditionalized.
[Vinh Nguyen] I will update my codes as your comments. Please see next
submitted codes.
>> diff --git a/arch/powerpc/sysdev/ppc4xx_pci.h
> b/arch/powerpc/sysdev/ppc4xx_pci.h
>> index 32ce763..faf3017 100644
>> --- a/arch/powerpc/sysdev/ppc4xx_pci.h
>> +++ b/arch/powerpc/sysdev/ppc4xx_pci.h
>> @@ -441,6 +441,7 @@
>> /*
>> * Config space register offsets
>> */
>> +#define PECFG_ECDEVCTL 0x060
>> #define PECFG_ECRTCTL 0x074
>>
>> #define PECFG_BAR0LMPA 0x210
>> @@ -448,6 +449,7 @@
>> #define PECFG_BAR1MPA 0x218
>> #define PECFG_BAR2LMPA 0x220
>> #define PECFG_BAR2HMPA 0x224
>> +#define PECFG_ECDEVCAPPA 0x25c
>>
>> #define PECFG_PIMEN 0x33c
>> #define PECFG_PIM0LAL 0x340
>> @@ -494,5 +496,7 @@ enum
>> LNKW_X8 = 0x8
>> };
>>
>> +/* Timout for reset phy */
>> +#define PCIE_PHY_RESET_TIMEOUT 10
>
> Is this value applicable to all the 44x devices with PCI-e?
> [Vinh Nguyen] At this time, we only checked this on APM821xx chips.
Then it should probably be conditionalized somehow. Or put as a
worst-case value that will work for them all.
[Vinh Nguyen] At this time, this definition is called in
apm821xx_pciex_init_port_hw function that is used only for apm821xx chip.
So I think it's ok, but as your recommendation, I'll check with various
boards that I have to get the best value and update it in next submitted
codes.
Best regards,
Vinh Nguyen.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-12-26 8:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-20 12:44 [PATCH 3/3] powerpc/44x: Add support PCI-E for APM821xx SoC and Bluestone board Vinh Nguyen Huu Tuong
2011-12-20 15:31 ` Josh Boyer
2011-12-20 15:31 ` Josh Boyer
2011-12-21 4:47 ` Vinh Huu Tuong Nguyen
2011-12-21 4:47 ` Vinh Huu Tuong Nguyen
2011-12-21 20:20 ` Josh Boyer
2011-12-21 20:20 ` Josh Boyer
2011-12-26 8:00 ` Vinh Huu Tuong Nguyen
2011-12-26 8:00 ` Vinh Huu Tuong Nguyen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.