From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Wed, 27 Jan 2016 18:28:43 +0100 Subject: [U-Boot] [PATCH v2 2/2] net: phy: Add PHY driver for mv88e61xx switches In-Reply-To: <56A8F077.60308@elecsyscorp.com> References: <1450734319-9515-1-git-send-email-kevin.smith@elecsyscorp.com> <1450734319-9515-4-git-send-email-kevin.smith@elecsyscorp.com> <56A8F077.60308@elecsyscorp.com> Message-ID: <20160127182843.7f7c13a2@lilith> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Kevin, On Wed, 27 Jan 2016 16:29:42 +0000, Kevin Smith 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.