net: phy: icplus: call phy_restore_page() when phy_select_page() fails
Subsystems (4)
PATCH v2 1/1 net: phy: icplus: call phy_restore_page() when phy_select_page() fails
The comments to phy_select_page() say that "phy_restore_page() must always be called after this, irrespective of success or failure of this call." If we don't call phy_restore_page() then we are still holding the phy_lock_mdio_bus() so it eventually leads to a dead lock.
---
v2: fix a couple other instances I missed in v1
drivers/net/phy/icplus.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/net/phy/icplus.c +5 -4
@@ -239,7 +239,7 @@ static int ip101a_g_config_intr_pin(struct phy_device *phydev) oldpage =phy_select_page (phydev,IP101G_DEFAULT_PAGE );if (oldpage <0 ) -return oldpage; + goto out;/* configure the RXER/INTR_32 pin of the 32-pin IP101GR if needed: */ switch (priv->sel_intr32) {
@@ -314,7 +314,7 @@ static int ip101a_g_read_status(struct phy_device *phydev) oldpage =phy_select_page (phydev,IP101G_DEFAULT_PAGE );if (oldpage <0 ) -return oldpage; + goto out; ret =__phy_read (phydev,IP10XX_SPEC_CTRL_STATUS );if (ret <0 )
@@ -349,7 +349,8 @@ static int ip101a_g_read_status(struct phy_device *phydev)static int ip101a_g_config_mdix (struct phy_device *phydev) {u16 ctrl =0 , ctrl2 =0 ; -int oldpage, ret; +int oldpage; +int ret =0 ;switch (phydev->mdix_ctrl) {case ETH_TP_MDI :
@@ -367,7 +368,7 @@ static int ip101a_g_config_mdix(struct phy_device *phydev) oldpage =phy_select_page (phydev,IP101G_DEFAULT_PAGE );if (oldpage <0 ) -return oldpage; + goto out; ret =__phy_modify (phydev,IP10XX_SPEC_CTRL_STATUS ,IP101A_G_AUTO_MDIX_DIS , ctrl);
Signature · 1 line
2.30.0
Am 2021-02-19 11:10, schrieb Dan Carpenter:
Quoted 8 lines
The comments to
phy_select_page()say that "phy_restore_page()must always be called after this, irrespective of success or failure of this call." If we don't callphy_restore_page()then we are still holding thephy_lock_mdio_bus()so it eventually leads to a dead lock.Fixes 32ab60e53920 ("net: phy: icplus: add MDI/MDIX support for IP101A/G")Fixes f9bc51e6cce2 ("net: phy: icplus: fix paged register access")Signed-off-by Dan Carpenter <[email protected]>
I assume, this has to go through "net" if the merge window is closed, no?
-michael
Quoted 51 lines
---
v2: fix a couple other instances I missed in v1drivers/net/phy/icplus.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)drivers/net/phy/icplus.c +1
-1@@ -239,7 +239,7 @@ static int ip101a_g_config_intr_pin(struct phy_device *phydev) oldpage =phy_select_page (phydev,IP101G_DEFAULT_PAGE );if (oldpage <0 ) -return oldpage; + goto out;/* configure the RXER/INTR_32 pin of the 32-pin IP101GR if needed: */ drivers/net/phy/icplus.c +1
-1switch (priv->sel_intr32) {@@ -314,7 +314,7 @@ static int ip101a_g_read_status(struct phy_device *phydev) oldpage =phy_select_page (phydev,IP101G_DEFAULT_PAGE );if (oldpage <0 ) -return oldpage; + goto out; ret =__phy_read (phydev,IP10XX_SPEC_CTRL_STATUS );drivers/net/phy/icplus.c +2
-1if (ret <0 )@@ -349,7 +349,8 @@ static int ip101a_g_read_status(struct phy_device *phydev)static int ip101a_g_config_mdix (struct phy_device *phydev) {u16 ctrl =0 , ctrl2 =0 ; -int oldpage, ret; +int oldpage; +int ret =0 ;switch (phydev->mdix_ctrl) {drivers/net/phy/icplus.c +1
-1case ETH_TP_MDI :@@ -367,7 +368,7 @@ static int ip101a_g_config_mdix(struct phy_device *phydev) oldpage =phy_select_page (phydev,IP101G_DEFAULT_PAGE );if (oldpage <0 ) -return oldpage; + goto out; ret = __phy_modify(phydev, IP10XX_SPEC_CTRL_STATUS,
IP101A_G_AUTO_MDIX_DIS, ctrl);
Signature · 1 line
-michael
Quoted 6 lines · Michael Walle
Am 2021-02-19 11:10, schrieb Dan Carpenter:
Quoted 5 lines
The comments to
phy_select_page()say that "phy_restore_page()must always be called after this, irrespective of success or failure of this call." If we don't callphy_restore_page()then we are still holding thephy_lock_mdio_bus()so it eventually leads to a dead lock.
Quoted 8 lines
Quoted 4 lines
Fixes 32ab60e53920 ("net: phy: icplus: add MDI/MDIX support forIP101A/G")
Fixes f9bc51e6cce2 ("net: phy: icplus: fix paged register access")Signed-off-by Dan Carpenter <[email protected]>Reviewed-by Michael Walle <[email protected]>I assume, this has to go through "net" if the merge window is closed, no?
It applies to net-next and not to net. I expect that all of net-next will be merged into net soon enough, but as of a couple hours ago it only applied to net-next.
regards,
dan carpenter
Quoted 8 lines · Dan Carpenter
The comments to
phy_select_page()say that "phy_restore_page()must always be called after this, irrespective of success or failure of this call." If we don't callphy_restore_page()then we are still holding thephy_lock_mdio_bus()so it eventually leads to a dead lock.Fixes 32ab60e53920 ("net: phy: icplus: add MDI/MDIX support for IP101A/G")Fixes f9bc51e6cce2 ("net: phy: icplus: fix paged register access")Signed-off-by Dan Carpenter <[email protected]>
Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
This patch was applied to netdev/net.git (refs/heads/master):
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
Quoted 10 lines · On Fri, 19 Feb 2021 13:10:44 +0300 you wrote:
The comments to
phy_select_page()say that "phy_restore_page()must always be called after this, irrespective of success or failure of this call." If we don't callphy_restore_page()then we are still holding thephy_lock_mdio_bus()so it eventually leads to a dead lock.Fixes 32ab60e53920 ("net: phy: icplus: add MDI/MDIX support for IP101A/G")Fixes f9bc51e6cce2 ("net: phy: icplus: fix paged register access")Signed-off-by Dan Carpenter <[email protected]>[...]
Here is the summary with links:
- [v2,net-next] net: phy: icplus: call phy_restore_page() when phy_select_page() fails
https://git.kernel.org/netdev/net/c/4e9d9d1f4880
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html