Re: [PATCH net-next v2] net: phy: micrel: Add support for lan9645x internal phy

[Andrew Lunn]

> > > > > > +static int lan9645x_suspend(struct phy_device *phydev)
> > > > > > +{
> > > > > > +    int ret, val;
> > > > > > +
> > > > > > +    /* Force link down before software power down (SPD), by doing software
> > > > > > +     * soft reset. This resets the PHY, but keeps all register configuration
> > > > > > +     * intact. The bit self clears.
> > > > > > +     *
> > > > > > +     * This is needed as a workaround for an issue where performing SPD on a
> > > > > > +     * port can bring adjacent ports down, when there is traffic flowing
> > > > > > +     * through the ports.
> > > > > > +     */
> > > > > > +    ret = phy_modify(phydev, LAN9645X_CONTROL_REGISTER,
> > > > > > +                     LAN9645X_CONTROL_REGISTER_SOFT_RESET, 1);
> > > > 
> > > > Any specific reason why you use a vendor-specific register here instead of BMCR
> > > > via genphy_soft_reset()?
> > > > 
> > > > 
> > > 
> > > Sorry I missed your mail. genphy_soft_reset() will do a software (hard) reset via
> > > bit 15 (BMCR_RESET). In particular it will reset the registers. We want to do
> > > a software soft reset, which resets the PHY without changing register values.
> > > 
> > No, BMCR_RESET usually doesn't reset configuration registers. That's why the
> > function is called genphy_*soft*_reset. In case your PHY behaves different,
> > which configuration registers does it change?
> > 
> 
> Ok I tought that was the usual behavior. The register spec says BMCR_RESET will reset
> the PHY and all its registers to their default state.

Most PHYs do a soft reset when this bit is set, but some do a much
harder reset, resetting all state including registers.

Does the documentation say anything about
LAN9645X_CONTROL_REGISTER_SOFT_RESET and self clearing? BMCR_RESET
should be cleared by the PHY when the reset is
complete. genphy_soft_reset() uses phy_poll_reset() to wait for the
bit to clear. If you don't wait, it could be your start writing to
registers while the reset is still happening, and those writes get
lost. You might have the same issue here.

      Andrew