Hi, Andrew
On 2017/6/24 21:44, Andrew Lunn wrote:
>> @@ -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.
