netdev - Re: [PATCH NET V4 1/2] net: phy: Add phy loopback support in net phy framework

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 24 Jun 2017 15:44:36 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Lin Yun Sheng <linyunsheng@...wei.com>
Cc:     davem@...emloft.net, f.fainelli@...il.com,
        huangdaode@...ilicon.com, xuwei5@...ilicon.com,
        liguozhu@...ilicon.com, Yisen.Zhuang@...wei.com,
        gabriele.paoloni@...wei.com, john.garry@...wei.com,
        linuxarm@...wei.com, salil.mehta@...wei.com, lipeng321@...wei.com,
        tremyfr@...il.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH NET V4 1/2] net: phy: Add phy loopback support in net phy
 framework

> @@ -1087,7 +1087,7 @@ int phy_suspend(struct phy_device *phydev)
>  {
>  	struct phy_driver *phydrv = to_phy_driver(phydev->mdio.dev.driver);
>  	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
> -	int ret = 0;
> +	int ret = -EOPNOTSUPP;
>  
>  	/* If the device has WOL enabled, we cannot suspend the PHY */
>  	phy_ethtool_get_wol(phydev, &wol);
> @@ -1109,7 +1109,7 @@ int phy_suspend(struct phy_device *phydev)
>  int phy_resume(struct phy_device *phydev)
>  {
>  	struct phy_driver *phydrv = to_phy_driver(phydev->mdio.dev.driver);
> -	int ret = 0;
> +	int ret = -EOPNOTSUPP;
>  
>  	if (phydev->drv && phydrv->resume)
>  		ret = phydrv->resume(phydev);
> @@ -1123,6 +1123,39 @@ int phy_resume(struct phy_device *phydev)
>  }

These changes should be in a separate patch, since they have nothing
to do with loopback. You want lots of small patches, making them
easier to both describe the why it is needed, and easier to review.

What are the implications of this change? Are you sure the power core
code does not understand EOPNOTSUPP as being a real error?

>  EXPORT_SYMBOL(phy_resume);
>  
> +int phy_loopback(struct phy_device *phydev, bool enable)
> +{
> +	struct phy_driver *phydrv = to_phy_driver(phydev->mdio.dev.driver);
> +	int ret = 0;
> +
> +	mutex_lock(&phydev->lock);
> +
> +	if (enable && phydev->loopback_enabled) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (!enable && !phydev->loopback_enabled) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (phydev->drv && phydrv->set_loopback)
> +		ret = phydrv->set_loopback(phydev, enable);
> +	else
> +		ret = -EOPNOTSUPP;
> +
> +	if (ret)
> +		goto out;
> +
> +	phydev->loopback_enabled = enable;
> +
> +out:
> +	mutex_unlock(&phydev->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(phy_loopback);

This looks good now.

     Andrew