Re: [PATCH v3] net: phy: add Maxlinear GPY115/21x/24x driver

[Andrew Lunn <andrew@lunn.ch>]

> Net-next:
> 
> int genphy_loopback(struct phy_device *phydev, bool enable)
> {
>      if (enable) {
>          u16 val, ctl = BMCR_LOOPBACK;
>          int ret;
> 
>          if (phydev->speed == SPEED_1000)
>              ctl |= BMCR_SPEED1000;
>          else if (phydev->speed == SPEED_100)
>              ctl |= BMCR_SPEED100;
> 
>          if (phydev->duplex == DUPLEX_FULL)
>              ctl |= BMCR_FULLDPLX;
> 
>          phy_modify(phydev, MII_BMCR, ~0, ctl);
> 
>          ret = phy_read_poll_timeout(phydev, MII_BMSR, val,
>                          val & BMSR_LSTATUS,
>                      5000, 500000, true);
>          if (ret)
>              return ret;
>      } else {
>          phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, 0);
> 
>          phy_config_aneg(phydev);
>      }
> 
>      return 0;
> }
> 
> v5.12.11:
> 
> int genphy_loopback(struct phy_device *phydev, bool enable)
> {
>      return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
>                enable ? BMCR_LOOPBACK : 0);
> }
> 
> 
> Not sure whether anyone else reported similar issue.

The commit message says:

    net: phy: genphy_loopback: add link speed configuration
    
    In case of loopback, in most cases we need to disable autoneg support
    and force some speed configuration. Otherwise, depending on currently
    active auto negotiated link speed, the loopback may or may not work.

> Should I use phy_modify to set the LOOPBACK bit only in my driver 
> implementation as force speed with loopback enable does not work in our 
> device?

So you appear to have the exact opposite problem, you need to use
auto-neg, with yourself, in order to have link. So there are two
solutions:

1) As you say, implement it in your driver

2) Add a second generic implementation, which enables autoneg, if it
is not enabled, sets the loopback bit, and waits for the link to come
up.

Does your PHY driver error out when asked to do a forced mode? It
probably should, if your silicon does not support that part of C22.

	 Andrew