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

Message-ID: <YMynL9c9MpfdC7Se@lunn.ch>
Date:   Fri, 18 Jun 2021 16:01:19 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Liang Xu <lxu@...linear.com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "vee.khee.wong@...ux.intel.com" <vee.khee.wong@...ux.intel.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        Hauke Mehrtens <hmehrtens@...linear.com>,
        Thomas Mohren <tmohren@...linear.com>
Subject: Re: [PATCH v3] net: phy: add Maxlinear GPY115/21x/24x driver

> 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