[Regression] FEC: Panic on suspend in 3.16

Commit Message

Stefano Babic Aug. 6, 2014, 3:32 p.m. UTC

Hi Nathan, Martin,

On 06/08/2014 17:09, Nathan Lynch wrote:
> On 08/06/2014 09:52 AM, Martin Fuzzey wrote:
>> Hi all,
>>
>> I am using the fec ethernet driver on a i.MX53 SoC.
>>
>> All was working fine on 3.13 but, after upgrading to 3.16 I now get a
>> panic on suspend:
> 
> Same issue reported here:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/277728.html
> 
> a71e3c37960c (net: phy: Set the driver when registering an MDIO bus
> device) has been implicated; try reverting that.
> 

I confirm this issue (on a i.MX35). The ndev pointer is NULL in the
suspend function. I added the check for the pointer:

From 82af51e87b78000c0e038db6056c34f72a1f4045 Mon Sep 17 00:00:00 2001
From: Stefano Babic <[e-mail]>
Date: Wed, 6 Aug 2014 16:37:58 +0200
Subject: [PATCH] net: fec: check for pointer in suspend/resume

Check to get a valid structure to driver data
in suspend/resume functions before accessing.

Signed-off-by: Stefano Babic <[e-mail]>
---
 drivers/net/ethernet/freescale/fec_main.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Nathan Lynch Aug. 6, 2014, 3:43 p.m. UTC | #1

On 08/06/2014 10:32 AM, Stefano Babic wrote:
> Hi Nathan, Martin,
> 
> On 06/08/2014 17:09, Nathan Lynch wrote:
>> On 08/06/2014 09:52 AM, Martin Fuzzey wrote:
>>> Hi all,
>>>
>>> I am using the fec ethernet driver on a i.MX53 SoC.
>>>
>>> All was working fine on 3.13 but, after upgrading to 3.16 I now get a
>>> panic on suspend:
>>
>> Same issue reported here:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/277728.html
>>
>> a71e3c37960c (net: phy: Set the driver when registering an MDIO bus
>> device) has been implicated; try reverting that.
>>
> 
> I confirm this issue (on a i.MX35). The ndev pointer is NULL in the
> suspend function. I added the check for the pointer:

I'm not sure this is the right fix.  The fec driver's suspend function
is being called for a device it did not probe (I verified this on mx6q).

See Russell's explanation here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/277823.html

Russell King - ARM Linux Aug. 6, 2014, 3:47 p.m. UTC | #2

On Wed, Aug 06, 2014 at 05:32:41PM +0200, Stefano Babic wrote:
> Hi Nathan, Martin,
> 
> On 06/08/2014 17:09, Nathan Lynch wrote:
> > On 08/06/2014 09:52 AM, Martin Fuzzey wrote:
> >> Hi all,
> >>
> >> I am using the fec ethernet driver on a i.MX53 SoC.
> >>
> >> All was working fine on 3.13 but, after upgrading to 3.16 I now get a
> >> panic on suspend:
> > 
> > Same issue reported here:
> > 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/277728.html
> > 
> > a71e3c37960c (net: phy: Set the driver when registering an MDIO bus
> > device) has been implicated; try reverting that.
> > 
> 
> I confirm this issue (on a i.MX35). The ndev pointer is NULL in the
> suspend function. I added the check for the pointer:

As Nathan says, the problem is caused by a71e3c37960c and the fix which
will soon be merged into mainline and stable trees will be a revert of
that commit.

Thanks.

Stefano Babic Aug. 6, 2014, 3:52 p.m. UTC | #3

Hi Russel,

On 06/08/2014 17:47, Russell King - ARM Linux wrote:

> As Nathan says, the problem is caused by a71e3c37960c and the fix which
> will soon be merged into mainline and stable trees will be a revert of
> that commit.

I see. Thanks for explanation.

Stefano

Fabio Estevam Aug. 6, 2014, 4:01 p.m. UTC | #4

On Wed, Aug 6, 2014 at 12:47 PM, Russell King - ARM Linux
<[e-mail]> wrote:

> As Nathan says, the problem is caused by a71e3c37960c and the fix which
> will soon be merged into mainline and stable trees will be a revert of
> that commit.

Yes, Dave has already queued the revert for stable:
https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=ce7991e8198b80eb6b4441b6f6114bea4a665d66

Patch

diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index 77037fd..0f5f52f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2659,7 +2659,12 @@  static int
 fec_suspend(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
-	struct fec_enet_private *fep = netdev_priv(ndev);
+	struct fec_enet_private *fep;
+
+	if (!ndev)
+		return 0;
+
+	fep = netdev_priv(ndev);

 	if (netif_running(ndev)) {
 		fec_stop(ndev);
@@ -2678,9 +2683,13 @@  static int
 fec_resume(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
-	struct fec_enet_private *fep = netdev_priv(ndev);
+	struct fec_enet_private *fep;
 	int ret;

+	if (!ndev)
+		return 0;
+
+	fep = netdev_priv(ndev);
 	if (fep->reg_phy) {
 		ret = regulator_enable(fep->reg_phy);
 		if (ret)