Hi Yegor, On Tue, Jun 06, 2023 at 10:21:39AM +0200, Yegor Yefremov wrote: > Hi Sascha, > > > > + ret = phy_modify(phydev, YT8511_PAGE, YT8511_DELAY_FE_TX_EN, fe); > > + if (ret < 0) > > + goto err_restore_page; > > + > > + /* leave pll enabled in sleep */ > > + ret = phy_write(phydev, YT8511_PAGE_SELECT, YT8511_EXT_SLEEP_CTRL); > > + if (ret < 0) > > + goto err_restore_page; > > + > > + ret = phy_modify(phydev, YT8511_PAGE, 0, YT8511_PLLON_SLP); > > + if (ret < 0) > > + goto err_restore_page; > > + > > +err_restore_page: > > + return phy_restore_page(phydev, oldpage, ret); > > As for this approach, it is also used by some other drivers in the Linux kernel: > > drivers/net/phy/realtek.c > drivers/net/phy/icplus.c It's ok like this. I didn't realize phy_restore_page() takes ret as argument and returns it. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |