Hi, Andrew

On 2017/6/24 21:44, Andrew Lunn wrote:

quoted

@@ -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.

Thanks for pointing out. Will remove it next version.

What are the implications of this change? Are you sure the power core
code does not understand EOPNOTSUPP as being a real error?
I was thinking phy_suspend/resume was making the mistake not returing error when

function point is null, as I did in phy_loopback. I am not sure if power core
understand EOPNOTSUPP.
But I think we should fix it if it is fixable, if not, then add a comment. because
it is not consitent with other phy_* . For anyone who is not familiar with phy core,
it is confusing.