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

On 19.03.2014 12:44, Stefan Roese wrote:

...

• 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.

You might be right. But I will put it into a separate "refacturing" patch. Stay tuned.

Matthias