[U-Boot] [PATCH v2 2/2] net: phy: Add PHY driver for mv88e61xx switches

[Albert ARIBAUD]

Hello Kevin,

On Wed, 27 Jan 2016 16:29:42 +0000, Kevin Smith
<kevin.smith at elecsyscorp.com> wrote:
> Hi Joe,

> On 01/26/2016 06:11 PM, Joe Hershberger wrote:

> >> +       /* Replace the bus with the fake device */
> > Fake how? This is a confusing comment to me as written.
> The genphy functions assume that they can write to the PHY directly 
> using the MII bus, and the address it uses is the address of a 
> register.  This is not the case for this chip with multiple PHY 
> interfaces, which have to be accessed indirectly.  To handle this, I 
> have created a "fake" mii_dev whose read/write functions are the 
> indirection functions, and stored the actual mdio_bus in the private 
> data for the "fake" device, which is then used by the indirect 
> functions.  This allows this driver to make use of common genphy stuff 
> where appropriate.  Maybe "wrapper" or "indirect bus" is a better name 
> for it.  Let me know if you have a preference or better idea.

"Indirect bus" is better IMO, as it gives a clue about what actually
happens.

> >> +
> >> +       mac_addr = phydev->addr;
> >> +
> >> +       for (i = 0; i < PORT_COUNT; i++) {
> >> +               if ((1 << i) & CONFIG_MV88E61XX_PHY_PORTS) {
> >> +                       phydev->addr = i;
> >> +                       mv88e61xx_phy_enable(phydev, i);
> >> +                       mv88e61xx_phy_setup(phydev, i);
> >> +                       mv88e61xx_phy_config_port(phydev, i);
> > These all return status, but are ignored.
> Even if one fails, it seems appropriate to me to continue initializing 
> the others and not bail completely.  Should I catch the error, print a 
> warning and "continue" in the loop?  Or is completely bailing the right 
> thing to do?  If there are some errors, but other successes, what should 
> the function return?

Warn for each port switch initialization failure, bail out if no port
could be initialized?

> Thanks,
> Kevin

Amicalement,
-- 
Albert.