Re: [U-Boot] [PATCH] 4xx: add support for new PMC440 revision

[Stefan Roese]

Hi Matthias,

On 19.03.2014 10:25, Matthias Fuchs wrote:
This patch adds support for the new PMC440 hardware revision 1.4.
The board now uses Micrel KSZ9031 phys.
Add missing i2c initialization before reading bootstrap eeprom.

Signed-off-by: Matthias Fuchs <matthias.fu...@esd.eu>
---
  board/esd/pmc440/pmc440.c |   83 +++++++++++++++++++++++++++++++++------------
  1 file changed, 62 insertions(+), 21 deletions(-)

diff --git a/board/esd/pmc440/pmc440.c b/board/esd/pmc440/pmc440.c
index e86996c..7aee8e4 100644
--- a/board/esd/pmc440/pmc440.c
+++ b/board/esd/pmc440/pmc440.c
@@ -63,6 +63,8 @@ struct serial_device *default_serial_console(void)
                /* mark scratchreg valid */
                scratchreg = (scratchreg & 0xffffff00) | 0x80;

+               i2c_init_all();
+
                i = bootstrap_eeprom_read(CONFIG_SYS_I2C_BOOT_EEPROM_ADDR,
                                          0x10, buf, 4);
                if ((i != -1) && (buf[0] == 0x19) && (buf[1] == 0x75)) {
@@ -641,34 +643,73 @@ int is_pci_host(struct pci_controller *hose)
  #endif /* defined(CONFIG_PCI) */

  #ifdef CONFIG_RESET_PHY_R
+int pmc440_setup_ksz9031(char *devname, int phy_addr)
+{
+       unsigned short id1, id2;
+
+       if (miiphy_read(devname, phy_addr, 2, &id1) ||
+           miiphy_read(devname, phy_addr, 3, &id2)) {
+               printf("Phy%d: cannot read id\n", phy_addr);
+               return -1;
+       }
+
+       if ((id1 != 0x0022) || ((id2 & 0xfff0) != 0x1620)) {
+               printf("Phy%d: unexpected id\n", phy_addr);
+               return -1;
+       }
+
+       /* MMD 2.08: adjust tx_clk pad skew */
+       miiphy_write(devname, phy_addr, 0x0d, 2);
+       miiphy_write(devname, phy_addr, 0x0e, 8);
+       miiphy_write(devname, phy_addr, 0x0d, 0x4002);
+       miiphy_write(devname, phy_addr, 0x0e, 0xf | (0x17 << 5));
+
+       return 0;
+}
  void reset_phy(void)
  {
        char *s;
        unsigned short val_method, val_behavior;

-       /* special LED setup for NGCC/CANDES */
-       if ((s = getenv("bd_type")) &&
-           ((!strcmp(s, "ngcc")) || (!strcmp(s, "candes")))) {
-               val_method   = 0x0e0a;
-               val_behavior = 0x0cf2;
-       } else {
-               /* PMC440 standard type */
-               val_method   = 0x0e10;
-               val_behavior = 0x0cf0;
-       }
+       if (gd->board_type < 4) {
+               /* special LED setup for NGCC/CANDES */
+               s = getenv("bd_type");
+               if (s && ((!strcmp(s, "ngcc")) || (!strcmp(s, "candes")))) {
+                       val_method   = 0x0e0a;
+                       val_behavior = 0x0cf2;
+               } else {
+                       /* PMC440 standard type */
+                       val_method   = 0x0e10;
+                       val_behavior = 0x0cf0;
+               }

-       if (miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x1f, 0x0001) == 0) {
-               miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x11, 0x0010);
-               miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x11, 
val_behavior);
-               miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x10, val_method);
-               miiphy_write("ppc_4xx_eth0", CONFIG_PHY_ADDR, 0x1f, 0x0000);
-       }
+               if (miiphy_write("ppc_4xx_eth0",
+                                CONFIG_PHY_ADDR, 0x1f, 0x0001) == 0) {
+                       miiphy_write("ppc_4xx_eth0",
+                                    CONFIG_PHY_ADDR, 0x11, 0x0010);
+                       miiphy_write("ppc_4xx_eth0",
+                                    CONFIG_PHY_ADDR, 0x11, val_behavior);
+                       miiphy_write("ppc_4xx_eth0",
+                                    CONFIG_PHY_ADDR, 0x10, val_method);
+                       miiphy_write("ppc_4xx_eth0",
+                                    CONFIG_PHY_ADDR, 0x1f, 0x0000);
+               }

-       if (miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x1f, 0x0001) == 0) {
-               miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x11, 0x0010);
-               miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x11, 
val_behavior);
-               miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x10, 
val_method);
-               miiphy_write("ppc_4xx_eth1", CONFIG_PHY1_ADDR, 0x1f, 0x0000);
+               if (miiphy_write("ppc_4xx_eth1",
+                                CONFIG_PHY1_ADDR, 0x1f, 0x0001) == 0) {
+                       miiphy_write("ppc_4xx_eth1",
+                                    CONFIG_PHY1_ADDR, 0x11, 0x0010);
+                       miiphy_write("ppc_4xx_eth1",
+                                    CONFIG_PHY1_ADDR, 0x11, val_behavior);
+                       miiphy_write("ppc_4xx_eth1",
+                                    CONFIG_PHY1_ADDR, 0x10, val_method);
+                       miiphy_write("ppc_4xx_eth1",
+                                    CONFIG_PHY1_ADDR, 0x1f, 0x0000);
+               }

This if () section looks very similar to the one before in this patch. 
Only difference is the string "ppc_4xx_eth1". Can't you move this code 
into a function to reduce the code size?

I know this code duplication was not introduced with this patch. But it 
makes sense to simplify this now for my taste.

Thanks,
Stefan

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot