Subject: Re: [PATCH net-next 1/8] net: dsa: mv88e6xxx: Implement external MDIO bus on mv88e6390

From: Andrew Lunn <[email-masked]>
Date: Sat, 21 Jan 2017 17:15:32 +0100

On Fri, Jan 20, 2017 at 07:04:35PM -0500, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <[email-masked]> writes:
> 
> > The mv88e6390 has two MDIO busses. The internal MDIO bus is used for
> > the internal PHYs. The external MDIO can be used for external PHYs.
> > The external MDIO bus will be instantiated if there is an
> > "mdio-external" node in the device tree.
> 
> Thanks for pushing the 88E6390 support. Some comments below.
> 
> > +static int mv88e6xxx_read_phy(struct mv88e6xxx_chip *chip, int addr, int reg,
> > +static int mv88e6xxx_write_phy(struct mv88e6xxx_chip *chip, int addr, int reg,
> >  static int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
> >  static int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
> 
> Adding mv88e6xxx_read/write_phy() in addition to existing
> mv88e6xxx_phy_read/write() feels really confusing and hard to
> maintain. Can that be done the other way around maybe?

Yes, i agree. I didn't particularly like it either. What might be
simpler is to pass the struct mii_bus all the way down, where as we
only currently have chip.

     Andrew