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

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-next>] [day] [month] [year] [list]

Message-ID: <CO1PR11MB477189838CFCDB952D4B064BD5099@CO1PR11MB4771.namprd11.prod.outlook.com>
Date:   Tue, 22 Jun 2021 04:21:47 +0000
From:   "Ismail, Mohammad Athari" <mohammad.athari.ismail@...el.com>
To:     "andrew@...n.ch" <andrew@...n.ch>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "hmehrtens@...linear.com" <hmehrtens@...linear.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "tmohren@...linear.com" <tmohren@...linear.com>,
        "vee.khee.wong@...ux.intel.com" <vee.khee.wong@...ux.intel.com>,
        "lxu@...linear.com" <lxu@...linear.com>
Subject: Re: [PATCH v3] net: phy: add Maxlinear GPY115/21x/24x driver

> -----Original Message-----
> From: Ismail, Mohammad Athari <mohammad.athari.ismail@...el.com>
> Sent: Tuesday, June 22, 2021 12:15 PM
> To: Ismail, Mohammad Athari <mohammad.athari.ismail@...el.com>
> 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