[U-Boot] [PATCH v6] Marvell MV88E61XX Switch Driver support

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ben Warren [mailto:biggerbadderben at gmail.com] 
> Sent: Thursday, April 23, 2009 11:00 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; 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 . .