Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code
Any direction on how these should be applied for testing?
snip
I've pushed a 'dev' branch on u-boot-85xx.git on denx.de with the current set
of patches applied.
Thanks. Things are much tidier now. I had a few comments. Let me know
if you'd like me to submit patches to address any of them.
* There's a large chunk of defines that are no longer needed in the X-ES
board code:
--- a/board/xes/common/fsl_8xxx_pci.c
+++ b/board/xes/common/fsl_8xxx_pci.c
@@ -35,29 +35,6 @@
static struct pci_controller pci1_hose;
#endif
-/*
- * 85xx and 86xx share naming conventions, but different layout.
- * Correlate names to CPU-specific values to share common
- * PCI code.
- */
-#if defined(CONFIG_MPC85xx)
-#define MPC8xxx_DEVDISR_PCIE1 MPC85xx_DEVDISR_PCIE
-#define MPC8xxx_DEVDISR_PCIE2 MPC85xx_DEVDISR_PCIE2
-#define MPC8xxx_DEVDISR_PCIE3 MPC85xx_DEVDISR_PCIE3
-#define MPC8xxx_PORDEVSR_IO_SELMPC85xx_PORDEVSR_IO_SEL
-#define MPC8xxx_PORDEVSR_IO_SEL_SHIFT MPC85xx_PORDEVSR_IO_SEL_SHIFT
-#define MPC8xxx_PORBMSR_HA MPC85xx_PORBMSR_HA
-#define MPC8xxx_PORBMSR_HA_SHIFT MPC85xx_PORBMSR_HA_SHIFT
-#elif defined(CONFIG_MPC86xx)
-#define MPC8xxx_DEVDISR_PCIE1 MPC86xx_DEVDISR_PCIEX1
-#define MPC8xxx_DEVDISR_PCIE2 MPC86xx_DEVDISR_PCIEX2
-#define MPC8xxx_DEVDISR_PCIE3 0 /* 8641 doesn't have PCIe3 */
-#define MPC8xxx_PORDEVSR_IO_SELMPC8641_PORDEVSR_IO_SEL
-#define MPC8xxx_PORDEVSR_IO_SEL_SHIFT MPC8641_PORDEVSR_IO_SEL_SHIFT
-#define MPC8xxx_PORBMSR_HA MPC8641_PORBMSR_HA
-#define MPC8xxx_PORBMSR_HA_SHIFT MPC8641_PORBMSR_HA_SHIFT
-#endif
-
void pci_init_board(void)
{
int first_free_busno = 0;
* It'd be nice if boards didn't have to define board_serdes_name(), or
at least a sane print statement was used if it wasn't. When I booted an
X-ES board the first time I got:
PCIE1: connected to NULL as Endpoint (base addr ef008000)
PCIE1: Bus 00 - 00
PCIE2: connected to NULL as Endpoint (base addr ef009000)
PCIE2: Bus 01 - 01
Other boards will have the same issue. Something like the following?:
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -526,8 +526,8 @@ int fsl_configure_pcie(struct fsl_pci_info *info,
set_next_law(info-mem_phys, law_size_bits(info-mem_size), info-law);
set_next_law(info-io_phys, law_size_bits(info-io_size), info-law);
is_endpoint = fsl_setup_hose(hose, info-regs);
- printf(PCIE%u: connected to %s as %s (base addr %lx)\n,
- info-pci_num, connected,
+ printf(PCIE%u: connected%s%s as %s (base addr %lx)\n,
+ info-pci_num, connected ? to : , connected ? connected :
,
is_endpoint ? Endpoint : Root Complex, info-regs);
return fsl_pci_init_port(info, hose, busno);
}
* Lastly, what about using a new define to specify the PCIe port's name
instead of making each board add a board_serdes_name() function? This
reduces each board directory's clutter and makes it so all PCIe-related
configuration and naming occurs in the board's config.h file. As an
example that uses the xpedite517x and mpc8572ds:
--- a/board/freescale/mpc8572ds/mpc8572ds.c
+++ b/board/freescale/mpc8572ds/mpc8572ds.c
@@ -149,17 +149,6 @@ phys_size_t fixed_sdram (void)
#endif
#ifdef CONFIG_PCI
-static const char *slot_names[] = {
- [PCIE1] = Slot 2,
- [PCIE2] = Slot 1,
- [PCIE3] = ULI,
-};
-
-const char *board_serdes_name(enum srds_prtcl device)
-{
- return slot_names[device];
-}
-
void pci_init_board(void)
{
struct pci_controller *hose;
--- a/drivers/pci/fsl_pci_init.c
+++ b/drivers/pci/fsl_pci_init.c
@@ -558,7 +558,26 @@ int fsl_configure_pcie(struct fsl_pci_info *info,
/* Implement a dummy function for those platforms w/o SERDES */
static const char *__board_serdes_name(enum srds_prtcl device)
{
- return NULL;
+ switch (device) {
+#ifdef CONFIG_SYS_PCIE1_NAME
+ case PCIE1:
+ return CONFIG_SYS_PCIE1_NAME;
+#endif
+#ifdef CONFIG_SYS_PCIE2_NAME
+ case PCIE2:
+ return CONFIG_SYS_PCIE2_NAME;
+#endif
+#ifdef CONFIG_SYS_PCIE3_NAME
+ case PCIE3:
+ return CONFIG_SYS_PCIE3_NAME;
+#endif
+#ifdef CONFIG_SYS_PCIE4_NAME
+ case PCIE4:
+ return CONFIG_SYS_PCIE4_NAME;
+#endif
+ default:
+ return NULL;
+ }
}
__attribute__((weak, alias(__board_serdes_name))) const char *
--- a/include/configs/xpedite517x.h
+++ b/include/configs/xpedite517x.h
@@ -347,6 +347,7 @@ extern unsigned long get_board_sys_clk(unsigned long dummy);
#define CONFIG_SYS_PCIE1_IO_BUS0x
#define CONFIG_SYS_PCIE1_IO_PHYS 0xe800
#define CONFIG_SYS_PCIE1_IO_SIZE 0x0080 /* 8M */
+#define CONFIG_SYS_PCIE1_NAME PEX8518 Switch
/* PCIE2 - VPX P1 */
#define CONFIG_SYS_PCIE2_MEM_BUS 0xc000
@@ -355,6 +356,7 @@ extern unsigned
Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code
On Dec 21, 2010, at 2:23 PM, Peter Tyser wrote: On Tue, 2010-12-21 at 11:49 -0600, Kumar Gala wrote: On Dec 20, 2010, at 10:49 AM, Peter Tyser wrote: Thanks for the cleanup. What branch should this series be applied to? And are there prerequisites? I'm having issues applying them to test and review. Any direction on how these should be applied for testing? snip I've pushed a 'dev' branch on u-boot-85xx.git on denx.de with the current set of patches applied. - k ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code
On Dec 21, 2010, at 2:23 PM, Peter Tyser wrote: Ah, OK. If we're removing the LAW entries for PCI1 in law.c below, how is a LAW being set for PCI1? It looks like PCIe laws are set in fsl_configure_pcie(), and PCI LAWs are set via set_next_law() in board-specific code? I'm not seeing the call to set_next_law() in X-ES board specific code after this change though. Best, Peter Yep, that was missing, will fix ;) - k ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code
On Dec 20, 2010, at 10:49 AM, Peter Tyser wrote:
Thanks for the cleanup. What branch should this series be applied to?
And are there prerequisites? I'm having issues applying them to test
and review.
On Fri, 2010-12-17 at 17:50 -0600, Kumar Gala wrote:
Remove duplicated code in MPC8572 DS board and utliize the common
fsl_pcie_init_board().
Looks like a copy/paste from the MPC8572.
On all the patches in the series s/utliize/utilize/.
snip
Will fix, oops ;)
--- a/board/xes/common/fsl_8xxx_pci.c
+++ b/board/xes/common/fsl_8xxx_pci.c
@@ -34,15 +34,6 @@
#ifdef CONFIG_PCI1
static struct pci_controller pci1_hose;
#endif
Is there a reason PCI1 wasn't changed over too? I see pci1_hose is
still referenced below, but other boards with a PCI1 don't use similar
code.
I was trying to limit how much clean up I did so left this to just PCIe
interfaces. Normal PCI and PCI-X is something I might get around to but one
thing at a time
-#ifdef CONFIG_PCIE1
-static struct pci_controller pcie1_hose;
-#endif
-#ifdef CONFIG_PCIE2
-static struct pci_controller pcie2_hose;
-#endif
-#ifdef CONFIG_PCIE3
-static struct pci_controller pcie3_hose;
-#endif
snip
diff --git a/board/xes/xpedite520x/law.c b/board/xes/xpedite520x/law.c
index bbfcb9d..3afb3ae 100644
--- a/board/xes/xpedite520x/law.c
+++ b/board/xes/xpedite520x/law.c
@@ -38,10 +38,6 @@ struct law_entry law_table[] = {
/* LBC window - maps 256M 0xf000 - 0x */
SET_LAW(CONFIG_SYS_FLASH_BASE2, LAW_SIZE_256M, LAW_TRGT_IF_LBC),
SET_LAW(CONFIG_SYS_NAND_BASE, LAW_SIZE_1M, LAW_TRGT_IF_LBC),
-#if CONFIG_SYS_PCI1_MEM_PHYS
-SET_LAW(CONFIG_SYS_PCI1_MEM_PHYS, LAW_SIZE_1G, LAW_TRGT_IF_PCI_1),
-SET_LAW(CONFIG_SYS_PCI1_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_1),
-#endif
#if CONFIG_SYS_PCI2_MEM_PHYS
SET_LAW(CONFIG_SYS_PCI2_MEM_PHYS, LAW_SIZE_256M, LAW_TRGT_IF_PCI_2),
SET_LAW(CONFIG_SYS_PCI2_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_2),
The PCI2 law can be removed too. Its not currently used on any boards
supported by mainline U-Boot.
Ok, will remove in updated patch
- k
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code
On Tue, 2010-12-21 at 11:49 -0600, Kumar Gala wrote:
On Dec 20, 2010, at 10:49 AM, Peter Tyser wrote:
Thanks for the cleanup. What branch should this series be applied to?
And are there prerequisites? I'm having issues applying them to test
and review.
Any direction on how these should be applied for testing?
snip
--- a/board/xes/common/fsl_8xxx_pci.c
+++ b/board/xes/common/fsl_8xxx_pci.c
@@ -34,15 +34,6 @@
#ifdef CONFIG_PCI1
static struct pci_controller pci1_hose;
#endif
Is there a reason PCI1 wasn't changed over too? I see pci1_hose is
still referenced below, but other boards with a PCI1 don't use similar
code.
I was trying to limit how much clean up I did so left this to just PCIe
interfaces. Normal PCI and PCI-X is something I might get around to but one
thing at a time
Ah, OK. If we're removing the LAW entries for PCI1 in law.c below, how
is a LAW being set for PCI1? It looks like PCIe laws are set in
fsl_configure_pcie(), and PCI LAWs are set via set_next_law() in
board-specific code? I'm not seeing the call to set_next_law() in X-ES
board specific code after this change though.
Best,
Peter
snip
diff --git a/board/xes/xpedite520x/law.c b/board/xes/xpedite520x/law.c
index bbfcb9d..3afb3ae 100644
--- a/board/xes/xpedite520x/law.c
+++ b/board/xes/xpedite520x/law.c
@@ -38,10 +38,6 @@ struct law_entry law_table[] = {
/* LBC window - maps 256M 0xf000 - 0x */
SET_LAW(CONFIG_SYS_FLASH_BASE2, LAW_SIZE_256M, LAW_TRGT_IF_LBC),
SET_LAW(CONFIG_SYS_NAND_BASE, LAW_SIZE_1M, LAW_TRGT_IF_LBC),
-#if CONFIG_SYS_PCI1_MEM_PHYS
- SET_LAW(CONFIG_SYS_PCI1_MEM_PHYS, LAW_SIZE_1G, LAW_TRGT_IF_PCI_1),
- SET_LAW(CONFIG_SYS_PCI1_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_1),
-#endif
#if CONFIG_SYS_PCI2_MEM_PHYS
SET_LAW(CONFIG_SYS_PCI2_MEM_PHYS, LAW_SIZE_256M, LAW_TRGT_IF_PCI_2),
SET_LAW(CONFIG_SYS_PCI2_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_2),
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code
Thanks for the cleanup. What branch should this series be applied to?
And are there prerequisites? I'm having issues applying them to test
and review.
On Fri, 2010-12-17 at 17:50 -0600, Kumar Gala wrote:
Remove duplicated code in MPC8572 DS board and utliize the common
fsl_pcie_init_board().
Looks like a copy/paste from the MPC8572.
On all the patches in the series s/utliize/utilize/.
snip
--- a/board/xes/common/fsl_8xxx_pci.c
+++ b/board/xes/common/fsl_8xxx_pci.c
@@ -34,15 +34,6 @@
#ifdef CONFIG_PCI1
static struct pci_controller pci1_hose;
#endif
Is there a reason PCI1 wasn't changed over too? I see pci1_hose is
still referenced below, but other boards with a PCI1 don't use similar
code.
-#ifdef CONFIG_PCIE1
-static struct pci_controller pcie1_hose;
-#endif
-#ifdef CONFIG_PCIE2
-static struct pci_controller pcie2_hose;
-#endif
-#ifdef CONFIG_PCIE3
-static struct pci_controller pcie3_hose;
-#endif
snip
diff --git a/board/xes/xpedite520x/law.c b/board/xes/xpedite520x/law.c
index bbfcb9d..3afb3ae 100644
--- a/board/xes/xpedite520x/law.c
+++ b/board/xes/xpedite520x/law.c
@@ -38,10 +38,6 @@ struct law_entry law_table[] = {
/* LBC window - maps 256M 0xf000 - 0x */
SET_LAW(CONFIG_SYS_FLASH_BASE2, LAW_SIZE_256M, LAW_TRGT_IF_LBC),
SET_LAW(CONFIG_SYS_NAND_BASE, LAW_SIZE_1M, LAW_TRGT_IF_LBC),
-#if CONFIG_SYS_PCI1_MEM_PHYS
- SET_LAW(CONFIG_SYS_PCI1_MEM_PHYS, LAW_SIZE_1G, LAW_TRGT_IF_PCI_1),
- SET_LAW(CONFIG_SYS_PCI1_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_1),
-#endif
#if CONFIG_SYS_PCI2_MEM_PHYS
SET_LAW(CONFIG_SYS_PCI2_MEM_PHYS, LAW_SIZE_256M, LAW_TRGT_IF_PCI_2),
SET_LAW(CONFIG_SYS_PCI2_IO_PHYS, LAW_SIZE_8M, LAW_TRGT_IF_PCI_2),
The PCI2 law can be removed too. Its not currently used on any boards
supported by mainline U-Boot.
Thanks,
Peter
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code
Remove duplicated code in MPC8572 DS board and utliize the common
fsl_pcie_init_board().
Signed-off-by: Kumar Gala ga...@kernel.crashing.org
CC: Peter Tyser pty...@xes-inc.com
---
board/xes/common/fsl_8xxx_pci.c | 82 ---
board/xes/xpedite517x/law.c |8
board/xes/xpedite520x/law.c |4 --
board/xes/xpedite537x/law.c | 12 --
board/xes/xpedite550x/law.c | 12 --
5 files changed, 8 insertions(+), 110 deletions(-)
diff --git a/board/xes/common/fsl_8xxx_pci.c b/board/xes/common/fsl_8xxx_pci.c
index 4135849..202520d 100644
--- a/board/xes/common/fsl_8xxx_pci.c
+++ b/board/xes/common/fsl_8xxx_pci.c
@@ -34,15 +34,6 @@
#ifdef CONFIG_PCI1
static struct pci_controller pci1_hose;
#endif
-#ifdef CONFIG_PCIE1
-static struct pci_controller pcie1_hose;
-#endif
-#ifdef CONFIG_PCIE2
-static struct pci_controller pcie2_hose;
-#endif
-#ifdef CONFIG_PCIE3
-static struct pci_controller pcie3_hose;
-#endif
/*
* 85xx and 86xx share naming conventions, but different layout.
@@ -69,22 +60,13 @@ static struct pci_controller pcie3_hose;
void pci_init_board(void)
{
- struct fsl_pci_info pci_info[3];
int first_free_busno = 0;
- int num = 0;
- int pcie_ep;
- __maybe_unused int pcie_configured;
-#if defined(CONFIG_MPC85xx)
+#ifdef CONFIG_PCI1
+ int pcie_ep;
+ struct fsl_pci_info pci_info;
volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
-#elif defined(CONFIG_MPC86xx)
- immap_t *immap = (immap_t *)CONFIG_SYS_IMMR;
- volatile ccsr_gur_t *gur = immap-im_gur;
-#endif
u32 devdisr = in_be32(gur-devdisr);
-
-#ifdef CONFIG_PCI1
- u32 pordevsr = in_be32(gur-pordevsr);
uint pci_spd_norm = in_be32(gur-pordevsr) MPC85xx_PORDEVSR_PCI1_SPD;
uint pci_32 = in_be32(gur-pordevsr) MPC85xx_PORDEVSR_PCI1_PCI32;
uint pci_arb = in_be32(gur-pordevsr) MPC85xx_PORDEVSR_PCI1_ARB;
@@ -92,8 +74,8 @@ void pci_init_board(void)
uint freq = CONFIG_SYS_CLK_FREQ / 1000 / 1000;
if (!(devdisr MPC85xx_DEVDISR_PCI1)) {
- SET_STD_PCI_INFO(pci_info[num], 1);
- pcie_ep = fsl_setup_hose(pci1_hose, pci_info[num].regs);
+ SET_STD_PCI_INFO(pci_info, 1);
+ pcie_ep = fsl_setup_hose(pci1_hose, pci_info.regs);
printf(PCI1: %d bit %s, %s %d MHz, %s, %s\n,
pci_32 ? 32 : 64,
pcix ? PCIX : PCI,
@@ -102,66 +84,18 @@ void pci_init_board(void)
pcie_ep ? agent : host,
pci_arb ? arbiter : external-arbiter);
- first_free_busno = fsl_pci_init_port(pci_info[num++],
+ first_free_busno = fsl_pci_init_port(pci_info,
pci1_hose, first_free_busno);
} else {
printf(PCI1: disabled\n);
}
#elif defined CONFIG_MPC8548
+ volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
/* PCI1 not present on MPC8572 */
setbits_be32(gur-devdisr, MPC85xx_DEVDISR_PCI1);
#endif
-#ifdef CONFIG_PCIE1
- pcie_configured = is_serdes_configured(PCIE1);
-
- if (pcie_configured !(devdisr MPC8xxx_DEVDISR_PCIE1)) {
- SET_STD_PCIE_INFO(pci_info[num], 1);
- pcie_ep = fsl_setup_hose(pcie1_hose, pci_info[num].regs);
- printf(PCIE1: connected as %s\n,
- pcie_ep ? Endpoint : Root Complex);
- first_free_busno = fsl_pci_init_port(pci_info[num++],
- pcie1_hose, first_free_busno);
- } else {
- printf(PCIE1: disabled\n);
- }
-#else
- setbits_be32(gur-devdisr, MPC8xxx_DEVDISR_PCIE1);
-#endif /* CONFIG_PCIE1 */
-
-#ifdef CONFIG_PCIE2
- pcie_configured = is_serdes_configured(PCIE2);
-
- if (pcie_configured !(devdisr MPC8xxx_DEVDISR_PCIE2)) {
- SET_STD_PCIE_INFO(pci_info[num], 2);
- pcie_ep = fsl_setup_hose(pcie2_hose, pci_info[num].regs);
- printf(PCIE2: connected as %s\n,
- pcie_ep ? Endpoint : Root Complex);
- first_free_busno = fsl_pci_init_port(pci_info[num++],
- pcie2_hose, first_free_busno);
- } else {
- printf(PCIE2: disabled\n);
- }
-#else
- setbits_be32(gur-devdisr, MPC8xxx_DEVDISR_PCIE2);
-#endif /* CONFIG_PCIE2 */
-
-#ifdef CONFIG_PCIE3
- pcie_configured = is_serdes_configured(PCIE3);
-
- if (pcie_configured !(devdisr MPC8xxx_DEVDISR_PCIE3)) {
- SET_STD_PCIE_INFO(pci_info[num], 3);
- pcie_ep = fsl_setup_hose(pcie3_hose, pci_info[num].regs);
- printf(PCIE3: connected as %s\n,
- pcie_ep ? Endpoint : Root Complex);
- first_free_busno = fsl_pci_init_port(pci_info[num++],
-
