
Hi Prafulla,
So close...
Prafulla Wadaskar wrote: <snip>
+/* Chip Address mode
- The Switch support two modes of operation
- single chip mode and
- Multi-chip mode
- Refer section 9.2 &9.3 in chip datasheet-02 for more details
- By default single chip mode is configured
- multichip mode operation can be configured in board header
- */
+#ifndef CONFIG_MV88E61XX_MULTICHIP_ADRMODE +#define mv88e61xx_wr_phy miiphy_write +#define mv88e61xx_rd_phy miiphy_read +#else
+static int mv88e61xx_busychk_multic(u32 devaddr) +{
- u32 reg = 0;
- u32 timeout = MV88E61XX_PHY_TIMEOUT;
- /* Poll till SMIBusy bit is clear */
- do {
miiphy_read(name, devaddr, 0x0, ®);
if (timeout-- == 0) {
printf("SMI busy timeout\n");
return -1;
}
- } while (reg & BIT15);
- return 0;
+}
+static void mv88e61xx_wr_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 data) +{
- u16 reg;
- u32 mii_dev_addr;
- /* command to read PHY dev address */
- if (!miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr)) {
printf("Error..could not read PHY dev address\n");
return;
- }
- mv88e61xx_busychk_multic(mii_dev_addr);
- /* Write data to Switch indirect data register */
- miiphy_write(name, mii_dev_addr, 0x1, data);
- /* Write command to Switch indirect command register (write) */
- miiphy_write(name, mii_dev_addr, 0x0,
reg_ofs | (phy_adr << 5) | BIT10 | BIT12 | BIT15);
+}
+static void mv88e61xx_rd_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 * data) +{
- u16 reg;
- u32 mii_dev_addr;
- /* command to read PHY dev address */
- if (!miiphy_read(name, 0xEE, 0xEE, &mii_dev_addr)) {
printf("Error..could not read PHY dev address\n");
return;
- }
- mv88e61xx_busychk_multic(mii_dev_addr);
- /* Write command to Switch indirect command register (read) */
- miiphy_write(name, mii_dev_addr, 0x0,
reg_ofs | (phy_adr << 5) | BIT10 | BIT12 | BIT15);
- mv88e61xx_busychk_multic(mii_dev_addr);
- /* Read data from Switch indirect data register */
- miiphy_read(name, mii_dev_addr, 0x1, (u16 *) & data);
+} +#endif /* CONFIG_MV88E61XX_MULTICHIP_ADRMODE */
I apologize for not being more clear here. Please don't put non-trivial functions in header files. What I was hoping you'd do was put something like this in the header file:
static void mv88e61xx_wr_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 data); static void mv88e61xx_rd_phy(char *name, u32 phy_adr, u32 reg_ofs, u16 * data);
/* Helpful comment here */ #ifndef CONFIG_MV88E61XX_MULTICHIP_ADRMODE #define WRITE_PHY miiphy_write #define READ_PHY miiphy_read #else #define WRITE_PHY mv88e61xx_wr_phy #define READ_PHY mv88e61xx_rd_phy #endif
and then use these macros throughout. You'll agree that it's much easier to follow.
+#endif /* _MV88E61XX_H */ diff --git a/include/netdev.h b/include/netdev.h index 2794ddd..e6832de 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -59,6 +59,7 @@ int mpc512x_fec_initialize(bd_t *bis); int mpc5xxx_fec_initialize(bd_t *bis); int mpc8220_fec_initialize(bd_t *bis); int mpc82xx_scc_enet_initialize(bd_t *bis); +int mv88e61xx_switch_initialize(struct mv88e61xx_config *swconfig); int natsemi_initialize(bd_t *bis); int npe_initialize(bd_t *bis); int ns8382x_initialize(bd_t *bis);
Apart from that, it looks really good. Thanks for all the hard work.
regards, Ben