Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jon Hunter]

On 11/06/2019 16:18, Jose Abreu wrote:
> Convert everything to phylink.
> 
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>

I am seeing a boot regression on -next for some of our boards that have
a synopsys ethernet controller that uses the dwmac-dwc-qos-ethernet
driver. Git bisect is pointing to this commit, but unfortunately this
cannot be cleanly reverted on top of -next to confirm. 

The bootlog shows the following bug is triggered ...

[   10.784989] ------------[ cut here ]------------
[   10.789597] kernel BUG at /home/jonathanh/workdir/tegra/mlt-linux_next/kernel/kernel/time/timer.c:952!
[   10.798881] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[   10.804351] Modules linked in:
[   10.807400] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G S                5.2.0-rc3-00940-g425b0fad9c7e #9
[   10.816682] Hardware name: NVIDIA Tegra186 P2771-0000 Development Board (DT)
[   10.823712] pstate: 20000005 (nzCv daif -PAN -UAO)
[   10.828496] pc : mod_timer+0x208/0x2d8
[   10.832235] lr : stmmac_napi_poll_tx+0x524/0x5a0
[   10.836839] sp : ffff000010003d00
[   10.840141] x29: ffff000010003d00 x28: ffff8001f42887c0 
[   10.845438] x27: ffff8001f42887c0 x26: ffff8001f55b7100 
[   10.850735] x25: 0000000000000000 x24: 0000000000000000 
[   10.856033] x23: ffff0000112e9000 x22: 0000000000000000 
[   10.861330] x21: 0000000000000001 x20: ffff0000121ad000 
[   10.866626] x19: ffff8001f47da000 x18: 0000000000000000 
[   10.871922] x17: 0000000000000000 x16: 0000000000000001 
[   10.877218] x15: 0000000000000009 x14: 0000000000001000 
[   10.882515] x13: 0000000080000000 x12: 0000000000000001 
[   10.887811] x11: 000000000000000c x10: 0000000000000000 
[   10.893107] x9 : 0000000000000000 x8 : 00000000fffee49c 
[   10.898403] x7 : 000000000000002a x6 : 000000000000002a 
[   10.903699] x5 : ffff8001f4189c80 x4 : 0000000000290000 
[   10.908995] x3 : 0000000000000000 x2 : 0000000000000000 
[   10.914291] x1 : 00000000fffee596 x0 : ffff8001f428b160 
[   10.919587] Call trace:
[   10.922024]  mod_timer+0x208/0x2d8
[   10.925415]  stmmac_napi_poll_tx+0x524/0x5a0
[   10.929674]  net_rx_action+0x220/0x318
[   10.933413]  __do_softirq+0x110/0x23c
[   10.937066]  irq_exit+0xcc/0xd8
[   10.940199]  __handle_domain_irq+0x60/0xb8
[   10.944282]  gic_handle_irq+0x58/0xb0
[   10.947931]  el1_irq+0xb8/0x180
[   10.951063]  arch_cpu_idle+0x10/0x18
[   10.954627]  do_idle+0x1dc/0x2a8
[   10.957845]  cpu_startup_entry+0x24/0x28
[   10.961758]  rest_init+0xd4/0xe0
[   10.964978]  arch_call_rest_init+0xc/0x14
[   10.968976]  start_kernel+0x44c/0x478
[   10.972626] Code: aa1503f4 aa1403f5 17ffffc5 d503201f (d4210000) 
[   10.978709] ---[ end trace 89626c50aaab321f ]---

I have not looked at this any further, but wanted to see if you have some
thoughts. 

Cheers
Jon

-- 
nvpublic

RE: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jose Abreu]

From: Jon Hunter <jonathanh@nvidia.com>

> I am seeing a boot regression on -next for some of our boards that have
> a synopsys ethernet controller that uses the dwmac-dwc-qos-ethernet
> driver. Git bisect is pointing to this commit, but unfortunately this
> cannot be cleanly reverted on top of -next to confirm. 

Thanks for reporting. Looks like the timer is not setup when 
stmmac_tx_clean() is called. When do you see this stacktrace ? After 
ifdown ?

Thanks,
Jose Miguel Abreu

Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jon Hunter]

On 18/06/2019 10:35, Jose Abreu wrote:
> From: Jon Hunter <jonathanh@nvidia.com>
> 
>> I am seeing a boot regression on -next for some of our boards that have
>> a synopsys ethernet controller that uses the dwmac-dwc-qos-ethernet
>> driver. Git bisect is pointing to this commit, but unfortunately this
>> cannot be cleanly reverted on top of -next to confirm. 
> 
> Thanks for reporting. Looks like the timer is not setup when 
> stmmac_tx_clean() is called. When do you see this stacktrace ? After 
> ifdown ?

I am not certain but I don't believe so. We are using a static IP address
and mounting the root file-system via NFS when we see this ...

[   10.607510] dwc-eth-dwmac 2490000.ethernet eth0: phy link up rgmii/1Gbps/Full
[   10.607536] dwc-eth-dwmac 2490000.ethernet eth0: phylink_mac_config: mode=phy/rgmii/1Gbps/Full adv=00,00000000,00000000 pause=0f link=1 an=0
[   10.608804] dwc-eth-dwmac 2490000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[   10.630979] IP-Config: Complete:
[   10.639046]      device=eth0, hwaddr=d2:e5:1c:57:26:4b, ipaddr=192.168.99.2, mask=255.255.255.0, gw=192.168.99.1
[   10.649201]      host=192.168.99.2, domain=, nis-domain=(none)
[   10.655022]      bootserver=192.168.0.1, rootserver=192.168.0.1, rootpath=
[   10.677531] VDD_1V8_AP_PLL: disabling
[   10.681194] VDD_RTC: disabling
[   10.684246] VDDIO_SDMMC3_AP: disabling
[   10.688132] VDD_HDMI_1V05: disabling
[   10.691704] SD_CARD_SW_PWR: disabling
[   10.695357] VDD_USB0: disabling
[   10.698488] VDD_USB1: disabling
[   10.701621] VDD_HDMI_5V0: disabling
[   10.705100] ALSA device list:
[   10.708063]   No soundcards found.
[   10.711914] Freeing unused kernel memory: 1472K
[   10.727005] Run /init as init process
[   10.784989] ------------[ cut here ]------------
[   10.789597] kernel BUG at /home/jonathanh/workdir/tegra/mlt-linux_next/kernel/kernel/time/timer.c:952!

Cheers
Jon

-- 
nvpublic

RE: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jose Abreu]

From: Jon Hunter <jonathanh@nvidia.com>

> I am not certain but I don't believe so. We are using a static IP address
> and mounting the root file-system via NFS when we see this ...

Can you please add a call to napi_synchronize() before every 
napi_disable() calls, like this:

if (queue < rx_queues_cnt) {
	napi_synchronize(&ch->rx_napi);
	napi_disable(&ch->rx_napi);
}

if (queue < tx_queues_cnt) {
	napi_synchronize(&ch->tx_napi);
	napi_disable(&ch->tx_napi);
}

[ I can send you a patch if you prefer ]

Thanks,
Jose Miguel Abreu

Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jon Hunter]

On 18/06/2019 10:46, Jose Abreu wrote:
> From: Jon Hunter <jonathanh@nvidia.com>
> 
>> I am not certain but I don't believe so. We are using a static IP address
>> and mounting the root file-system via NFS when we see this ...
> 
> Can you please add a call to napi_synchronize() before every 
> napi_disable() calls, like this:
> 
> if (queue < rx_queues_cnt) {
> 	napi_synchronize(&ch->rx_napi);
> 	napi_disable(&ch->rx_napi);
> }
> 
> if (queue < tx_queues_cnt) {
> 	napi_synchronize(&ch->tx_napi);
> 	napi_disable(&ch->tx_napi);
> }
> 
> [ I can send you a patch if you prefer ]

Yes I can try this and for completeness you mean ...

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4ca46289a742..d4a12cb64d8e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -146,10 +146,15 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
        for (queue = 0; queue < maxq; queue++) {
                struct stmmac_channel *ch = &priv->channel[queue];
 
-               if (queue < rx_queues_cnt)
+               if (queue < rx_queues_cnt) {
+                       napi_synchronize(&ch->rx_napi);
                        napi_disable(&ch->rx_napi);
-               if (queue < tx_queues_cnt)
+               }
+
+               if (queue < tx_queues_cnt) {
+                       napi_synchronize(&ch->tx_napi);
                        napi_disable(&ch->tx_napi);
+               }
        }
 }

Cheers
Jon

-- 
nvpublic

Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jon Hunter]

On 18/06/2019 11:18, Jon Hunter wrote:
> 
> On 18/06/2019 10:46, Jose Abreu wrote:
>> From: Jon Hunter <jonathanh@nvidia.com>
>>
>>> I am not certain but I don't believe so. We are using a static IP address
>>> and mounting the root file-system via NFS when we see this ...
>>
>> Can you please add a call to napi_synchronize() before every 
>> napi_disable() calls, like this:
>>
>> if (queue < rx_queues_cnt) {
>> 	napi_synchronize(&ch->rx_napi);
>> 	napi_disable(&ch->rx_napi);
>> }
>>
>> if (queue < tx_queues_cnt) {
>> 	napi_synchronize(&ch->tx_napi);
>> 	napi_disable(&ch->tx_napi);
>> }
>>
>> [ I can send you a patch if you prefer ]
> 
> Yes I can try this and for completeness you mean ...
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 4ca46289a742..d4a12cb64d8e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -146,10 +146,15 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>         for (queue = 0; queue < maxq; queue++) {
>                 struct stmmac_channel *ch = &priv->channel[queue];
>  
> -               if (queue < rx_queues_cnt)
> +               if (queue < rx_queues_cnt) {
> +                       napi_synchronize(&ch->rx_napi);
>                         napi_disable(&ch->rx_napi);
> -               if (queue < tx_queues_cnt)
> +               }
> +
> +               if (queue < tx_queues_cnt) {
> +                       napi_synchronize(&ch->tx_napi);
>                         napi_disable(&ch->tx_napi);
> +               }
>         }
>  }

So good news and bad news ...

The good news is that the above change does fix the initial crash
I am seeing. However, even with this change applied on top of
-next, it is still dying somewhere else and so there appears to
be a second issue. 

On a successful boot I see ...

[    6.150419] dwc-eth-dwmac 2490000.ethernet: Cannot get CSR clock

[    6.156441] dwc-eth-dwmac 2490000.ethernet: no reset control found

[    6.175866] dwc-eth-dwmac 2490000.ethernet: User ID: 0x10, Synopsys ID: 0x41

[    6.182912] dwc-eth-dwmac 2490000.ethernet: 	DWMAC4/5

[    6.187961] dwc-eth-dwmac 2490000.ethernet: DMA HW capability register supported

[    6.195351] dwc-eth-dwmac 2490000.ethernet: RX Checksum Offload Engine supported

[    6.202735] dwc-eth-dwmac 2490000.ethernet: TX Checksum insertion supported

[    6.209685] dwc-eth-dwmac 2490000.ethernet: Wake-Up On Lan supported

[    6.216041] dwc-eth-dwmac 2490000.ethernet: TSO supported

[    6.221433] dwc-eth-dwmac 2490000.ethernet: Enable RX Mitigation via HW Watchdog Timer

[    6.229342] dwc-eth-dwmac 2490000.ethernet: device MAC address 9a:9b:49:6f:a5:ee

[    6.236727] dwc-eth-dwmac 2490000.ethernet: TSO feature enabled

[    6.242689] libphy: stmmac: probed

On the latest -next with the patch applied I see ...

[    6.043529] dwc-eth-dwmac 2490000.ethernet: Cannot get CSR clock
[    6.049546] dwc-eth-dwmac 2490000.ethernet: no reset control found
[    6.068895] dwc-eth-dwmac 2490000.ethernet: User ID: 0x10, Synopsys ID: 0x41
[    6.075941] dwc-eth-dwmac 2490000.ethernet: 	DWMAC4/5
[    6.080989] dwc-eth-dwmac 2490000.ethernet: DMA HW capability register supported
[    6.088373] dwc-eth-dwmac 2490000.ethernet: RX Checksum Offload Engine supported
[    6.095756] dwc-eth-dwmac 2490000.ethernet: TX Checksum insertion supported
[    6.102708] dwc-eth-dwmac 2490000.ethernet: Wake-Up On Lan supported
[    6.109074] dwc-eth-dwmac 2490000.ethernet: TSO supported
[    6.114465] dwc-eth-dwmac 2490000.ethernet: Enable RX Mitigation via HW Watchdog Timer
[    6.122373] dwc-eth-dwmac 2490000.ethernet: device MAC address ee:3a:9a:b0:7e:34
[    6.129756] dwc-eth-dwmac 2490000.ethernet: TSO feature enabled

And it dies here. No more output is seen. I will try to figure
out which commit is causing this issue.

Cheers
Jon

-- 
nvpublic

Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jon Hunter]

On 18/06/2019 16:20, Jon Hunter wrote:
> 
> On 18/06/2019 11:18, Jon Hunter wrote:
>>
>> On 18/06/2019 10:46, Jose Abreu wrote:
>>> From: Jon Hunter <jonathanh@nvidia.com>
>>>
>>>> I am not certain but I don't believe so. We are using a static IP address
>>>> and mounting the root file-system via NFS when we see this ...
>>>
>>> Can you please add a call to napi_synchronize() before every 
>>> napi_disable() calls, like this:
>>>
>>> if (queue < rx_queues_cnt) {
>>> 	napi_synchronize(&ch->rx_napi);
>>> 	napi_disable(&ch->rx_napi);
>>> }
>>>
>>> if (queue < tx_queues_cnt) {
>>> 	napi_synchronize(&ch->tx_napi);
>>> 	napi_disable(&ch->tx_napi);
>>> }
>>>
>>> [ I can send you a patch if you prefer ]
>>
>> Yes I can try this and for completeness you mean ...
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 4ca46289a742..d4a12cb64d8e 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -146,10 +146,15 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>>         for (queue = 0; queue < maxq; queue++) {
>>                 struct stmmac_channel *ch = &priv->channel[queue];
>>  
>> -               if (queue < rx_queues_cnt)
>> +               if (queue < rx_queues_cnt) {
>> +                       napi_synchronize(&ch->rx_napi);
>>                         napi_disable(&ch->rx_napi);
>> -               if (queue < tx_queues_cnt)
>> +               }
>> +
>> +               if (queue < tx_queues_cnt) {
>> +                       napi_synchronize(&ch->tx_napi);
>>                         napi_disable(&ch->tx_napi);
>> +               }
>>         }
>>  }
> 
> So good news and bad news ...
> 
> The good news is that the above change does fix the initial crash
> I am seeing. However, even with this change applied on top of
> -next, it is still dying somewhere else and so there appears to
> be a second issue. 

Further testing has shown that actually this does NOT resolve the issue
and I am still seeing the crash. Sorry for the false-positive.

Jon

-- 
nvpublic

Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jon Hunter]

On 18/06/2019 20:44, Jon Hunter wrote:
> 
> On 18/06/2019 16:20, Jon Hunter wrote:
>>
>> On 18/06/2019 11:18, Jon Hunter wrote:
>>>
>>> On 18/06/2019 10:46, Jose Abreu wrote:
>>>> From: Jon Hunter <jonathanh@nvidia.com>
>>>>
>>>>> I am not certain but I don't believe so. We are using a static IP address
>>>>> and mounting the root file-system via NFS when we see this ...
>>>>
>>>> Can you please add a call to napi_synchronize() before every 
>>>> napi_disable() calls, like this:
>>>>
>>>> if (queue < rx_queues_cnt) {
>>>> 	napi_synchronize(&ch->rx_napi);
>>>> 	napi_disable(&ch->rx_napi);
>>>> }
>>>>
>>>> if (queue < tx_queues_cnt) {
>>>> 	napi_synchronize(&ch->tx_napi);
>>>> 	napi_disable(&ch->tx_napi);
>>>> }
>>>>
>>>> [ I can send you a patch if you prefer ]
>>>
>>> Yes I can try this and for completeness you mean ...
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index 4ca46289a742..d4a12cb64d8e 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -146,10 +146,15 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
>>>         for (queue = 0; queue < maxq; queue++) {
>>>                 struct stmmac_channel *ch = &priv->channel[queue];
>>>  
>>> -               if (queue < rx_queues_cnt)
>>> +               if (queue < rx_queues_cnt) {
>>> +                       napi_synchronize(&ch->rx_napi);
>>>                         napi_disable(&ch->rx_napi);
>>> -               if (queue < tx_queues_cnt)
>>> +               }
>>> +
>>> +               if (queue < tx_queues_cnt) {
>>> +                       napi_synchronize(&ch->tx_napi);
>>>                         napi_disable(&ch->tx_napi);
>>> +               }
>>>         }
>>>  }
>>
>> So good news and bad news ...
>>
>> The good news is that the above change does fix the initial crash
>> I am seeing. However, even with this change applied on top of
>> -next, it is still dying somewhere else and so there appears to
>> be a second issue. 
> 
> Further testing has shown that actually this does NOT resolve the issue
> and I am still seeing the crash. Sorry for the false-positive.

Any further feedback? I am still seeing this issue on today's -next.

Thanks
Jon

-- 
nvpublic

RE: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jose Abreu]

From: Jon Hunter <jonathanh@nvidia.com>

> Any further feedback? I am still seeing this issue on today's -next.

Apologies but I was in FTO.

Is there any possibility you can just disable the ethX configuration in 
the rootfs mount and manually configure it after rootfs is done ?

I just want to make sure in which conditions this is happening (if in 
ifdown or ifup).

Thanks,
Jose Miguel Abreu

Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jon Hunter]

On 25/06/2019 08:37, Jose Abreu wrote:
> From: Jon Hunter <jonathanh@nvidia.com>
> 
>> Any further feedback? I am still seeing this issue on today's -next.
> 
> Apologies but I was in FTO.
> 
> Is there any possibility you can just disable the ethX configuration in 
> the rootfs mount and manually configure it after rootfs is done ?
> 
> I just want to make sure in which conditions this is happening (if in 
> ifdown or ifup).

I have been looking at this a bit closer and I can see the problem. What
happens is that ...

1. stmmac_mac_link_up() is called and priv->eee_active is set to false
2. stmmac_eee_init() is called but because priv->eee_active is false,
   timer_setup() for eee_ctrl_timer is never called.
3. stmmac_eee_init() returns true and so then priv->eee_enabled is set 
   to true.
4. When stmmac_tx_clean() is called because priv->eee_enabled is set to    
   true, mod_timer() is called for the eee_ctrl_timer, but because 
   timer_setup() was never called, we hit the BUG defined at
   kernel/time/timer.c:952, because no function is defined for the 
   timer.

The following fixes it for me ...

--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -399,10 +399,13 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
        mutex_lock(&priv->lock);
 
        /* Check if it needs to be deactivated */
-       if (!priv->eee_active && priv->eee_enabled) {
-               netdev_dbg(priv->dev, "disable EEE\n");
-               del_timer_sync(&priv->eee_ctrl_timer);
-               stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+       if (!priv->eee_active) {
+               if (priv->eee_enabled) {
+                       netdev_dbg(priv->dev, "disable EEE\n");
+                       del_timer_sync(&priv->eee_ctrl_timer);
+                       stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+               }
+               mutex_unlock(&priv->lock);
                return false;
        }

It also looks like you have a potention deadlock in the current code
because in the case of if (!priv->eee_active && priv->eee_enabled)
you don't unlock the mutex. The above fixes this as well. I can send a
formal patch if this looks correct. 

Cheers
Jon

-- 
nvpublic

RE: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic

[Jose Abreu]

From: Jon Hunter <jonathanh@nvidia.com>

> I have been looking at this a bit closer and I can see the problem. What
> happens is that ...
> 
> 1. stmmac_mac_link_up() is called and priv->eee_active is set to false
> 2. stmmac_eee_init() is called but because priv->eee_active is false,
>    timer_setup() for eee_ctrl_timer is never called.
> 3. stmmac_eee_init() returns true and so then priv->eee_enabled is set 
>    to true.
> 4. When stmmac_tx_clean() is called because priv->eee_enabled is set to    
>    true, mod_timer() is called for the eee_ctrl_timer, but because 
>    timer_setup() was never called, we hit the BUG defined at
>    kernel/time/timer.c:952, because no function is defined for the 
>    timer.
> 
> The following fixes it for me ...
> 
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -399,10 +399,13 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
>         mutex_lock(&priv->lock);
>  
>         /* Check if it needs to be deactivated */
> -       if (!priv->eee_active && priv->eee_enabled) {
> -               netdev_dbg(priv->dev, "disable EEE\n");
> -               del_timer_sync(&priv->eee_ctrl_timer);
> -               stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
> +       if (!priv->eee_active) {
> +               if (priv->eee_enabled) {
> +                       netdev_dbg(priv->dev, "disable EEE\n");
> +                       del_timer_sync(&priv->eee_ctrl_timer);
> +                       stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
> +               }
> +               mutex_unlock(&priv->lock);
>                 return false;
>         }
> 
> It also looks like you have a potention deadlock in the current code
> because in the case of if (!priv->eee_active && priv->eee_enabled)
> you don't unlock the mutex. The above fixes this as well. I can send a
> formal patch if this looks correct. 

Thanks for looking into this! The fix looks correct so if you could 
submit a patch it would be great!

Thanks,
Jose Miguel Abreu