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

- From: Ismail, Mohammad Athari \<hidden\>
- Date: 2021-06-22 04:21:51 
- List: netdev
- Page: https://ratatoskr.run/netdev/2021/06/11206004
- Thread: https://ratatoskr.run/netdev/2021/06/11206004/t.md

```text
> -----Original Message-----
> From: Ismail, Mohammad Athari <redacted>
> Sent: Tuesday, June 22, 2021 12:15 PM
> To: Ismail, Mohammad Athari <redacted>
> Subject:
> 
> > 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

Hi Andrew,

We also observe same issue on Marvell88E1510 PHY (C22 supported PHY) as well. It works with v5.12.11's genphy_loopback() but not net-next's.

Regards,
Athari

```