> -----Original Message----- > From: Ben Warren [mailto:[email protected]] > Sent: Thursday, April 23, 2009 11:00 AM > To: Prafulla Wadaskar > Cc: [email protected]; Ashish Karkare; Prabhanjan Sarnaik; > Ronen Shitrit > Subject: Re: [U-Boot] [PATCH v6] Marvell MV88E61XX Switch > Driver support > > Hi Prafulla, > > So close... > > > +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. That's okay Ben, I am sorry I could not digest your suggestions properly I will re-release with patch with this change
> 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); These are static functions to be used internally by mv88e61xx driver I was trying to reduce code size by two line putting them in c file And associated macros. But any way putting them in header makes it more redable, I will do it. > > /* 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. Yes I agree, even I can further reduce some ammount of bytes from my patch :-) Regards.. Prafulla . . _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
