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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170121161532.GB7808@lunn.ch>
Date:   Sat, 21 Jan 2017 17:15:32 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:     David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/8] net: dsa: mv88e6xxx: Implement external
 MDIO bus on mv88e6390

On Fri, Jan 20, 2017 at 07:04:35PM -0500, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@...n.ch> 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