Message ID | 1599609338-17732-1-git-send-email-yoshihiro.shimoda.uh@renesas.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [v2] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead | expand |
From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Date: Wed, 9 Sep 2020 08:55:38 +0900 > Changes from v1: > - Fix build error. When such a fundamental build failure is fixed (it could never have built for anyone, even you), I want it explained why this happened and how this was functionally tested if it did not even compile. I'm not applying this patch, you must resubmit it again after explaining what happened here instead of just quietly fixing the build failure. Thank you.
Hi David, > From: David Miller, Sent: Wednesday, September 9, 2020 12:25 PM > > From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Date: Wed, 9 Sep 2020 08:55:38 +0900 > > > Changes from v1: > > - Fix build error. > > When such a fundamental build failure is fixed (it could never have > built for anyone, even you), I want it explained why this happened > and how this was functionally tested if it did not even compile. I'm sorry about this. I used two PCs now: PC 1 = for testing at local PC 2 = for submitting patches at remote (because corporate network situation) I tested on the PC 1. But, after that, I modified the code on the PC 2 again. And, it seemed I didn't do a compile. Today, I got some emails from kernel test bot. So, I realized I had submitted a bad patch... > I'm not applying this patch, you must resubmit it again after > explaining what happened here instead of just quietly fixing > the build failure. Since the kernel test bot sent emails, I assumed I didn't need to reply by myself. I should have replied anyway... Best regards, Yoshihiro Shimoda > Thank you.
On Wed, Sep 09, 2020 at 04:18:56AM +0000, Yoshihiro Shimoda wrote: > Hi David, > > > From: David Miller, Sent: Wednesday, September 9, 2020 12:25 PM > > > > From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Date: Wed, 9 Sep 2020 08:55:38 +0900 > > > > > Changes from v1: > > > - Fix build error. > > > > When such a fundamental build failure is fixed (it could never have > > built for anyone, even you), I want it explained why this happened > > and how this was functionally tested if it did not even compile. > > I'm sorry about this. I used two PCs now: > PC 1 = for testing at local > PC 2 = for submitting patches at remote (because corporate network situation) > > I tested on the PC 1. > But, after that, I modified the code on the PC 2 again. And, it seemed > I didn't do a compile. This sort of split setup is always a bad idea. Always do the git format-patch on PC 1 and somehow get the patch files off it, and use PC 2 only for git send-email, never any development work. That way you will avoid issues like this. Andrew
Hi Andrew, > From: Andrew Lunn, Sent: Wednesday, September 9, 2020 10:47 PM > > On Wed, Sep 09, 2020 at 04:18:56AM +0000, Yoshihiro Shimoda wrote: > > Hi David, > > > > > From: David Miller, Sent: Wednesday, September 9, 2020 12:25 PM > > > > > > From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > Date: Wed, 9 Sep 2020 08:55:38 +0900 > > > > > > > Changes from v1: > > > > - Fix build error. > > > > > > When such a fundamental build failure is fixed (it could never have > > > built for anyone, even you), I want it explained why this happened > > > and how this was functionally tested if it did not even compile. > > > > I'm sorry about this. I used two PCs now: > > PC 1 = for testing at local > > PC 2 = for submitting patches at remote (because corporate network situation) > > > > I tested on the PC 1. > > But, after that, I modified the code on the PC 2 again. And, it seemed > > I didn't do a compile. > > This sort of split setup is always a bad idea. Always do the git > format-patch on PC 1 and somehow get the patch files off it, and use > PC 2 only for git send-email, never any development work. That way you > will avoid issues like this. Thank you for your comment! I agree with you. I'll use such setup. Best regards, Yoshihiro Shimoda
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 8adfbad..b93b40c 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1143,10 +1143,6 @@ int phy_init_hw(struct phy_device *phydev) if (ret < 0) return ret; - ret = phy_disable_interrupts(phydev); - if (ret) - return ret; - if (phydev->drv->config_init) ret = phydev->drv->config_init(phydev); @@ -1423,6 +1419,10 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, if (err) goto error; + err = phy_disable_interrupts(phydev); + if (err) + return err; + phy_resume(phydev); phy_led_triggers_register(phydev);
Since the micrel phy driver calls phy_init_hw() as a workaround, the commit 9886a4dbd2aa ("net: phy: call phy_disable_interrupts() in phy_init_hw()") disables the interrupt unexpectedly. So, call phy_disable_interrupts() in phy_attach_direct() instead. Otherwise, the phy cannot link up after the ethernet cable was disconnected. Note that other drivers (like at803x.c) also calls phy_init_hw(). So, perhaps, the driver caused a similar issue too. Fixes: 9886a4dbd2aa ("net: phy: call phy_disable_interrupts() in phy_init_hw()") Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- Changes from v1: - Fix build error. drivers/net/phy/phy_device.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)