[U-Boot] [PATCH 08/15] powerpc/8xxx: Rework XES boards pci_init_board to use common FSL PCIe code

[Peter Tyser <ptyser@xes-inc.com>]

> > 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_SEL                MPC85xx_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_SEL                MPC8641_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_BUS                0x00000000
 #define CONFIG_SYS_PCIE1_IO_PHYS       0xe8000000
 #define CONFIG_SYS_PCIE1_IO_SIZE       0x00800000      /* 8M */
+#define CONFIG_SYS_PCIE1_NAME          "PEX8518 Switch"
 
 /* PCIE2 - VPX P1 */
 #define CONFIG_SYS_PCIE2_MEM_BUS       0xc0000000
@@ -355,6 +356,7 @@ extern unsigned long get_board_sys_clk(unsigned long dummy);
 #define CONFIG_SYS_PCIE2_IO_BUS                0x00000000
 #define CONFIG_SYS_PCIE2_IO_PHYS       0xe8800000
 #define CONFIG_SYS_PCIE2_IO_SIZE       0x00800000      /* 8M */
+#define CONFIG_SYS_PCIE2_NAME          "VPX Fabric A"
 
 /*
  * Networking options
--- a/include/configs/MPC8572DS.h
+++ b/include/configs/MPC8572DS.h
@@ -485,6 +485,7 @@
 #define CONFIG_SYS_PCIE3_IO_PHYS       0xffc00000
 #endif
 #define CONFIG_SYS_PCIE3_IO_SIZE       0x00010000      /* 64k */
+#define CONFIG_SYS_PCIE3_NAME          "ULI"
 
 /* controller 2, Slot 2, tgtid 2, Base address 9000 */
 #define CONFIG_SYS_PCIE2_MEM_VIRT      0xa0000000
@@ -504,6 +505,7 @@
 #define CONFIG_SYS_PCIE2_IO_PHYS       0xffc10000
 #endif
 #define CONFIG_SYS_PCIE2_IO_SIZE       0x00010000      /* 64k */
+#define CONFIG_SYS_PCIE2_NAME          "Slot 1"
 
 /* controller 1, Slot 1, tgtid 1, Base address a000 */
 #define CONFIG_SYS_PCIE1_MEM_VIRT      0xc0000000
@@ -523,6 +525,7 @@
 #define CONFIG_SYS_PCIE1_IO_PHYS       0xffc20000
 #endif
 #define CONFIG_SYS_PCIE1_IO_SIZE       0x00010000      /* 64k */
+#define CONFIG_SYS_PCIE1_NAME          "Slot 2"
 
 #if defined(CONFIG_PCI)
 


Best,
Peter