[PATCH v2] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead

[PATCH v2] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead

[Yoshihiro Shimoda]

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(-)

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);
 
-- 
2.7.4

Re: [PATCH v2] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead

[David Miller]

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.

RE: [PATCH v2] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead

[Yoshihiro Shimoda]

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.

Re: [PATCH v2] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead

[Andrew Lunn]

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

RE: [PATCH v2] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead

[Yoshihiro Shimoda]

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