| Message ID | 1400031616-19287-1-git-send-email-zhangfei.gao@linaro.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, 2014-05-14 at 09:40 +0800, Zhangfei Gao wrote: > With commit be9dad1f9f26604fb ("net: phy: suspend phydev when going > to HALTED"), an unused PHY device will be put in a low-power mode > using BMCR_PDOWN. Some Ethernet drivers might be calling phy_start() > and phy_stop() from ndo_open and ndo_close() respectively, while > calling phy_connect() and phy_disconnect() from probe and remove. > In such a case, the PHY will be powered down during the phy_stop() > call, but will fail to be powered up in phy_start(). > This patch fixes this scenario. trivial notes: > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c [] > @@ -715,7 +715,7 @@ void phy_state_machine(struct work_struct *work) > struct delayed_work *dwork = to_delayed_work(work); > struct phy_device *phydev = > container_of(dwork, struct phy_device, state_queue); > - int needs_aneg = 0, do_suspend = 0; > + int needs_aneg = 0, do_suspend = 0, do_resume = 0; Perhaps these 3 vars should bool > @@ -876,6 +878,9 @@ void phy_state_machine(struct work_struct *work) > if (do_suspend) > phy_suspend(phydev); > > + if (do_resume) > + phy_resume(phydev); > + and these should be else if if (needs_aneg) ... else if (do_suspend) ... else if (do_resume) ... -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/14/2014 09:57 AM, Joe Perches wrote: > On Wed, 2014-05-14 at 09:40 +0800, Zhangfei Gao wrote: >> With commit be9dad1f9f26604fb ("net: phy: suspend phydev when going >> to HALTED"), an unused PHY device will be put in a low-power mode >> using BMCR_PDOWN. Some Ethernet drivers might be calling phy_start() >> and phy_stop() from ndo_open and ndo_close() respectively, while >> calling phy_connect() and phy_disconnect() from probe and remove. >> In such a case, the PHY will be powered down during the phy_stop() >> call, but will fail to be powered up in phy_start(). >> This patch fixes this scenario. > > trivial notes: > >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > [] >> @@ -715,7 +715,7 @@ void phy_state_machine(struct work_struct *work) >> struct delayed_work *dwork = to_delayed_work(work); >> struct phy_device *phydev = >> container_of(dwork, struct phy_device, state_queue); >> - int needs_aneg = 0, do_suspend = 0; >> + int needs_aneg = 0, do_suspend = 0, do_resume = 0; > > Perhaps these 3 vars should bool Yes, agree. > >> @@ -876,6 +878,9 @@ void phy_state_machine(struct work_struct *work) >> if (do_suspend) >> phy_suspend(phydev); >> >> + if (do_resume) >> + phy_resume(phydev); >> + > > and these should be else if > > if (needs_aneg) > ... > else if (do_suspend) > ... > else if (do_resume) > ... Ok, since needs_aneg can not happen simultaneously with suspend/resume. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 1b6d09a..be5d52e 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -715,7 +715,7 @@ void phy_state_machine(struct work_struct *work) struct delayed_work *dwork = to_delayed_work(work); struct phy_device *phydev = container_of(dwork, struct phy_device, state_queue); - int needs_aneg = 0, do_suspend = 0; + int needs_aneg = 0, do_suspend = 0, do_resume = 0; int err = 0; mutex_lock(&phydev->lock); @@ -865,6 +865,8 @@ void phy_state_machine(struct work_struct *work) } phydev->adjust_link(phydev->attached_dev); } + + do_resume = 1; break; } @@ -876,6 +878,9 @@ void phy_state_machine(struct work_struct *work) if (do_suspend) phy_suspend(phydev); + if (do_resume) + phy_resume(phydev); + if (err < 0) phy_error(phydev);
With commit be9dad1f9f26604fb ("net: phy: suspend phydev when going to HALTED"), an unused PHY device will be put in a low-power mode using BMCR_PDOWN. Some Ethernet drivers might be calling phy_start() and phy_stop() from ndo_open and ndo_close() respectively, while calling phy_connect() and phy_disconnect() from probe and remove. In such a case, the PHY will be powered down during the phy_stop() call, but will fail to be powered up in phy_start(). This patch fixes this scenario. Signed-off-by: Jiancheng Xue <xuejiancheng@huawei.com> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> --- drivers/net/phy/phy.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)