On Thu, Jan 11, 2018 at 05:29:22PM -0800, Florian Fainelli wrote:
> On 01/11/2018 12:55 PM, Andrew Lunn wrote:
> > __phy_modify would return the old value of the register before it was
> > modified. Thus on success, it does not return 0, but a positive value.
> > Thus functions using phy_modify, which is a wrapper around
> > __phy_modify, can start returning > 0 on success, rather than 0. As a
> > result, breakage has been noticed in various places, where 0 was
> > assumed.
> >
> > Code inspection does not find any current location where the return of
> > the old value is currently used.
>
> phy_restore_page() does actually use the old value returned by
> __phy_modify(),
Hi Florian
int phy_restore_page(struct phy_device *phydev, int oldpage, int ret)
{
int r;
if (oldpage >= 0) {
r = __phy_write_page(phydev, oldpage);
/* Propagate the operation return code if the page write
* was successful.
*/
if (ret >= 0 && r < 0)
ret = r;
} else {
/* Propagate the phy page selection error code */
ret = oldpage;
}
mutex_unlock(&phydev->mdio.bus->mdio_lock);
return ret;
}
Ah! I see it now. The value of ret parameter can be what phy_modify()
returned. If ret is not an error, and __phy_write_page() returned an
error, use r, the error from __phy_write_page().
As you say, the actual value is not used, just an indication of if it
represents an error. So this change is O.K.
Andrew
