diff mbox

IMX6 FEC connection drops occasionally with 'MDIO read timeout'

Message ID CAOMZO5DnKfNF__HThRBMix-yr6X9-x0VOVm57oeCQ2iwyL8gzg@mail.gmail.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Fabio Estevam April 12, 2017, 4:26 p.m. UTC
Hi Tim,

On Wed, Apr 12, 2017 at 1:15 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>
> Andrew,
>
> Thanks for the reply. Your talking about suspend/resume power
> management right? The users reporting this were not using
> suspend/resume.
>
> With regards to clock are you talking about the IPG clock? Is there
> any other way that would get turned off other than fec suspend/resume?

Yes, through pm_runtime.

Can you check if this quick debug change help?



If you don't see the problem with it, then it means we need to fix the
pm runtime support in this driver.

Most likely pm runtime is turning off the clocks when it should not.

Comments

Tim Harvey April 12, 2017, 4:54 p.m. UTC | #1
On Wed, Apr 12, 2017 at 9:26 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Tim,
>
> On Wed, Apr 12, 2017 at 1:15 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>>
>> Andrew,
>>
>> Thanks for the reply. Your talking about suspend/resume power
>> management right? The users reporting this were not using
>> suspend/resume.
>>
>> With regards to clock are you talking about the IPG clock? Is there
>> any other way that would get turned off other than fec suspend/resume?
>
> Yes, through pm_runtime.
>
> Can you check if this quick debug change help?
>
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3606,8 +3606,6 @@ static int __maybe_unused
> fec_runtime_suspend(struct device *dev)
>         struct net_device *ndev = dev_get_drvdata(dev);
>         struct fec_enet_private *fep = netdev_priv(ndev);
>
> -       clk_disable_unprepare(fep->clk_ipg);
> -
>         return 0;
>  }
>
>
> If you don't see the problem with it, then it means we need to fix the
> pm runtime support in this driver.
>
> Most likely pm runtime is turning off the clocks when it should not.

Fabio,

Ok, I understand now. We will disable the IPG clock disable and see if
that makes a difference.

Tim
Andy Duan April 13, 2017, 5:45 a.m. UTC | #2
On 2017年04月13日 00:54, Tim Harvey wrote:
> On Wed, Apr 12, 2017 at 9:26 AM, Fabio Estevam <festevam@gmail.com> wrote:

>> Hi Tim,

>>

>> On Wed, Apr 12, 2017 at 1:15 PM, Tim Harvey <tharvey@gateworks.com> wrote:

>>> Andrew,

>>>

>>> Thanks for the reply. Your talking about suspend/resume power

>>> management right? The users reporting this were not using

>>> suspend/resume.

>>>

>>> With regards to clock are you talking about the IPG clock? Is there

>>> any other way that would get turned off other than fec suspend/resume?

>> Yes, through pm_runtime.

>>

>> Can you check if this quick debug change help?

>>

>> --- a/drivers/net/ethernet/freescale/fec_main.c

>> +++ b/drivers/net/ethernet/freescale/fec_main.c

>> @@ -3606,8 +3606,6 @@ static int __maybe_unused

>> fec_runtime_suspend(struct device *dev)

>>          struct net_device *ndev = dev_get_drvdata(dev);

>>          struct fec_enet_private *fep = netdev_priv(ndev);

>>

>> -       clk_disable_unprepare(fep->clk_ipg);

>> -

>>          return 0;

>>   }

>>

>>

>> If you don't see the problem with it, then it means we need to fix the

>> pm runtime support in this driver.

>>

>> Most likely pm runtime is turning off the clocks when it should not.

> Fabio,

>

> Ok, I understand now. We will disable the IPG clock disable and see if

> that makes a difference.

>

> Tim

Firstly, pls try the change suggested by Andrew.
I guess system enter to wait mode, and enet irq cannot wakeup system in 
real time that causes mii irq much latency. then mii bus access timeout.
If so, can you add below change to your dts file and try it ?

iomux pinctrl:
                 pinctrl_enet_irq: enetirqgrp {
                         fsl,pins = <
MX6QDL_PAD_GPIO_6__ENET_IRQ             0x000b1
                         >;

                 };


&fec {
         ...
         pinctrl-0 = <&pinctrl_enet &pinctrl_enet_irq>;
         interrupts-extended = <&gpio1 6 0x04>, <&gpc 0 119 0x04>;
         ...
};

&i2c3 {
         ...
         status = "disabled";
};


Regards,
Andy
diff mbox

Patch

--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3606,8 +3606,6 @@  static int __maybe_unused
fec_runtime_suspend(struct device *dev)
        struct net_device *ndev = dev_get_drvdata(dev);
        struct fec_enet_private *fep = netdev_priv(ndev);

-       clk_disable_unprepare(fep->clk_ipg);
-
        return 0;
 }