| Message ID | 7c3c37c0-7c24-a63c-b441-b1e8085a3c96@gmx.at |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: manfred.schlaegl@gmx.at <manfred.schlaegl@gmx.at> Sent: Monday, October 24, 2016 5:26 PM > To: Andy Duan <fugang.duan@nxp.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH] net: fec: hard phy reset on open > > We have seen some problems with auto negotiation on i.MX6 using LAN8720, > after interface down/up. > > In our configuration, the ptp clock is used externally as reference clock for > the phy. Some phys (e.g. LAN8720) needs a stable clock while and after hard > reset. > Before this patch, the driver disabled the clock on close but did no hard reset > on open, after enabling the clocks again. > > A solution that prevents disabling the clocks on close was considered, but > discarded because of bad power saving behavior. > > This patch saves the reset dt properties on probe and does a reset on every > open after clocks where enabled, to make sure the clock is stable while and > after hard reset. > > Tested on i.MX6 and i.MX28, both using LAN8720. > > Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com> > --- This patch did hard reset to let phy stable. Firstly, you should do reset before clock enable. Secondly, we suggest to do phy reset in phy driver, not MAC driver. Regards, Andy > drivers/net/ethernet/freescale/fec.h | 4 ++ > drivers/net/ethernet/freescale/fec_main.c | 77 +++++++++++++++++------- > ------- > 2 files changed, 47 insertions(+), 34 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec.h > b/drivers/net/ethernet/freescale/fec.h > index c865135..379e619 100644 > --- a/drivers/net/ethernet/freescale/fec.h > +++ b/drivers/net/ethernet/freescale/fec.h > @@ -498,6 +498,10 @@ struct fec_enet_private { > struct clk *clk_enet_out; > struct clk *clk_ptp; > > + int phy_reset; > + bool phy_reset_active_high; > + int phy_reset_msec; > + > bool ptp_clk_on; > struct mutex ptp_clk_mutex; > unsigned int num_tx_queues; > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index 48a033e..8cc1ec5 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -2802,6 +2802,22 @@ static int fec_enet_alloc_buffers(struct > net_device *ndev) > return 0; > } > > +static void fec_reset_phy(struct fec_enet_private *fep) { > + if (!gpio_is_valid(fep->phy_reset)) > + return; > + > + gpio_set_value_cansleep(fep->phy_reset, !!fep- > >phy_reset_active_high); > + > + if (fep->phy_reset_msec > 20) > + msleep(fep->phy_reset_msec); > + else > + usleep_range(fep->phy_reset_msec * 1000, > + fep->phy_reset_msec * 1000 + 1000); > + > + gpio_set_value_cansleep(fep->phy_reset, !fep- > >phy_reset_active_high); > +} > + > static int > fec_enet_open(struct net_device *ndev) > { > @@ -2817,6 +2833,8 @@ fec_enet_open(struct net_device *ndev) > if (ret) > goto clk_enable; > > + fec_reset_phy(fep); > + > /* I should reset the ring buffers here, but I don't yet know > * a simple way to do that. > */ > @@ -3183,52 +3201,39 @@ static int fec_enet_init(struct net_device *ndev) > return 0; > } > > -#ifdef CONFIG_OF > -static void fec_reset_phy(struct platform_device *pdev) > +static int > +fec_get_reset_phy(struct platform_device *pdev, int *msec, int > *phy_reset, > + bool *active_high) > { > - int err, phy_reset; > - bool active_high = false; > - int msec = 1; > + int err; > struct device_node *np = pdev->dev.of_node; > > - if (!np) > - return; > + if (!np || !of_device_is_available(np)) > + return 0; > > - of_property_read_u32(np, "phy-reset-duration", &msec); > + of_property_read_u32(np, "phy-reset-duration", msec); > /* A sane reset duration should not be longer than 1s */ > - if (msec > 1000) > - msec = 1; > + if (*msec > 1000) > + *msec = 1; > > - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); > - if (!gpio_is_valid(phy_reset)) > - return; > + *phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); > + if (!gpio_is_valid(*phy_reset)) > + return 0; > > - active_high = of_property_read_bool(np, "phy-reset-active-high"); > + *active_high = of_property_read_bool(np, "phy-reset-active-high"); > > - err = devm_gpio_request_one(&pdev->dev, phy_reset, > - active_high ? GPIOF_OUT_INIT_HIGH : > GPIOF_OUT_INIT_LOW, > - "phy-reset"); > + err = devm_gpio_request_one(&pdev->dev, *phy_reset, > + *active_high ? > + GPIOF_OUT_INIT_HIGH : > + GPIOF_OUT_INIT_LOW, > + "phy-reset"); > if (err) { > dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", > err); > - return; > + return err; > } > > - if (msec > 20) > - msleep(msec); > - else > - usleep_range(msec * 1000, msec * 1000 + 1000); > - > - gpio_set_value_cansleep(phy_reset, !active_high); > -} > -#else /* CONFIG_OF */ > -static void fec_reset_phy(struct platform_device *pdev) -{ > - /* > - * In case of platform probe, the reset has been done > - * by machine code. > - */ > + return 0; > } > -#endif /* CONFIG_OF */ > > static void > fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, int > *num_rx) @@ -3409,7 +3414,10 @@ fec_probe(struct platform_device > *pdev) > pm_runtime_set_active(&pdev->dev); > pm_runtime_enable(&pdev->dev); > > - fec_reset_phy(pdev); > + ret = fec_get_reset_phy(pdev, &fep->phy_reset_msec, &fep- > >phy_reset, > + &fep->phy_reset_active_high); > + if (ret) > + goto failed_reset; > > if (fep->bufdesc_ex) > fec_ptp_init(pdev); > @@ -3467,6 +3475,7 @@ fec_probe(struct platform_device *pdev) > failed_mii_init: > failed_irq: > failed_init: > +failed_reset: > fec_ptp_stop(pdev); > if (fep->reg_phy) > regulator_disable(fep->reg_phy); > -- > 2.1.4
On 2016-10-24 16:03, Andy Duan wrote: > From: manfred.schlaegl@gmx.at <manfred.schlaegl@gmx.at> Sent: Monday, October 24, 2016 5:26 PM >> To: Andy Duan <fugang.duan@nxp.com> >> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org >> Subject: [PATCH] net: fec: hard phy reset on open >> >> We have seen some problems with auto negotiation on i.MX6 using LAN8720, >> after interface down/up. >> >> In our configuration, the ptp clock is used externally as reference clock for >> the phy. Some phys (e.g. LAN8720) needs a stable clock while and after hard >> reset. >> Before this patch, the driver disabled the clock on close but did no hard reset >> on open, after enabling the clocks again. >> >> A solution that prevents disabling the clocks on close was considered, but >> discarded because of bad power saving behavior. >> >> This patch saves the reset dt properties on probe and does a reset on every >> open after clocks where enabled, to make sure the clock is stable while and >> after hard reset. >> >> Tested on i.MX6 and i.MX28, both using LAN8720. >> >> Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com> >> --- > This patch did hard reset to let phy stable. > Firstly, you should do reset before clock enable. I have to disagree here. The phy demands(datasheet + tests) a stable clock at the time of (hard-)reset and after this. Therefore the clock has to be enabled before the hard reset. (This is exactly the reason for the patch.) Generally: The sense of a reset is to defer the start of digital circuit until the environment (power, clocks, ...) has stabilized. Furthermore: Before this patch the hard reset was done in fec_probe. And here also after the clocks were enabled. Whats was your argument to do it the other way in this special case? > Secondly, we suggest to do phy reset in phy driver, not MAC driver. I was not sure, if you meant a soft-, or hard-reset here. In case you are talking about soft reset: Yes, the phy drivers perform a soft reset. Sadly a soft reset is not sufficient in this case - The phy recovers only on a hard reset from lost clock. (datasheet + tests) In case you're talking about hard reset: Intuitively I would also think, that the (hard-)reset should be handled by the phy driver, but this is not reality in given implementations. Documentation/devicetree/bindings/net/fsl-fec.txt says - phy-reset-gpios : Should specify the gpio for phy reset It is explicitly talked about phy-reset here. And the (hard-)reset was handled by the fec driver also before this patch (once on probe). > > Regards, > Andy Thanks for your feedback! Best regards, Manfred > >> drivers/net/ethernet/freescale/fec.h | 4 ++ >> drivers/net/ethernet/freescale/fec_main.c | 77 +++++++++++++++++------- >> ------- >> 2 files changed, 47 insertions(+), 34 deletions(-) >> >> diff --git a/drivers/net/ethernet/freescale/fec.h >> b/drivers/net/ethernet/freescale/fec.h >> index c865135..379e619 100644 >> --- a/drivers/net/ethernet/freescale/fec.h >> +++ b/drivers/net/ethernet/freescale/fec.h >> @@ -498,6 +498,10 @@ struct fec_enet_private { >> struct clk *clk_enet_out; >> struct clk *clk_ptp; >> >> + int phy_reset; >> + bool phy_reset_active_high; >> + int phy_reset_msec; >> + >> bool ptp_clk_on; >> struct mutex ptp_clk_mutex; >> unsigned int num_tx_queues; >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> b/drivers/net/ethernet/freescale/fec_main.c >> index 48a033e..8cc1ec5 100644 >> --- a/drivers/net/ethernet/freescale/fec_main.c >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> @@ -2802,6 +2802,22 @@ static int fec_enet_alloc_buffers(struct >> net_device *ndev) >> return 0; >> } >> >> +static void fec_reset_phy(struct fec_enet_private *fep) { >> + if (!gpio_is_valid(fep->phy_reset)) >> + return; >> + >> + gpio_set_value_cansleep(fep->phy_reset, !!fep- >>> phy_reset_active_high); >> + >> + if (fep->phy_reset_msec > 20) >> + msleep(fep->phy_reset_msec); >> + else >> + usleep_range(fep->phy_reset_msec * 1000, >> + fep->phy_reset_msec * 1000 + 1000); >> + >> + gpio_set_value_cansleep(fep->phy_reset, !fep- >>> phy_reset_active_high); >> +} >> + >> static int >> fec_enet_open(struct net_device *ndev) >> { >> @@ -2817,6 +2833,8 @@ fec_enet_open(struct net_device *ndev) >> if (ret) >> goto clk_enable; >> >> + fec_reset_phy(fep); >> + >> /* I should reset the ring buffers here, but I don't yet know >> * a simple way to do that. >> */ >> @@ -3183,52 +3201,39 @@ static int fec_enet_init(struct net_device *ndev) >> return 0; >> } >> >> -#ifdef CONFIG_OF >> -static void fec_reset_phy(struct platform_device *pdev) >> +static int >> +fec_get_reset_phy(struct platform_device *pdev, int *msec, int >> *phy_reset, >> + bool *active_high) >> { >> - int err, phy_reset; >> - bool active_high = false; >> - int msec = 1; >> + int err; >> struct device_node *np = pdev->dev.of_node; >> >> - if (!np) >> - return; >> + if (!np || !of_device_is_available(np)) >> + return 0; >> >> - of_property_read_u32(np, "phy-reset-duration", &msec); >> + of_property_read_u32(np, "phy-reset-duration", msec); >> /* A sane reset duration should not be longer than 1s */ >> - if (msec > 1000) >> - msec = 1; >> + if (*msec > 1000) >> + *msec = 1; >> >> - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); >> - if (!gpio_is_valid(phy_reset)) >> - return; >> + *phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); >> + if (!gpio_is_valid(*phy_reset)) >> + return 0; >> >> - active_high = of_property_read_bool(np, "phy-reset-active-high"); >> + *active_high = of_property_read_bool(np, "phy-reset-active-high"); >> >> - err = devm_gpio_request_one(&pdev->dev, phy_reset, >> - active_high ? GPIOF_OUT_INIT_HIGH : >> GPIOF_OUT_INIT_LOW, >> - "phy-reset"); >> + err = devm_gpio_request_one(&pdev->dev, *phy_reset, >> + *active_high ? >> + GPIOF_OUT_INIT_HIGH : >> + GPIOF_OUT_INIT_LOW, >> + "phy-reset"); >> if (err) { >> dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", >> err); >> - return; >> + return err; >> } >> >> - if (msec > 20) >> - msleep(msec); >> - else >> - usleep_range(msec * 1000, msec * 1000 + 1000); >> - >> - gpio_set_value_cansleep(phy_reset, !active_high); >> -} >> -#else /* CONFIG_OF */ >> -static void fec_reset_phy(struct platform_device *pdev) -{ >> - /* >> - * In case of platform probe, the reset has been done >> - * by machine code. >> - */ >> + return 0; >> } >> -#endif /* CONFIG_OF */ >> >> static void >> fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, int >> *num_rx) @@ -3409,7 +3414,10 @@ fec_probe(struct platform_device >> *pdev) >> pm_runtime_set_active(&pdev->dev); >> pm_runtime_enable(&pdev->dev); >> >> - fec_reset_phy(pdev); >> + ret = fec_get_reset_phy(pdev, &fep->phy_reset_msec, &fep- >>> phy_reset, >> + &fep->phy_reset_active_high); >> + if (ret) >> + goto failed_reset; >> >> if (fep->bufdesc_ex) >> fec_ptp_init(pdev); >> @@ -3467,6 +3475,7 @@ fec_probe(struct platform_device *pdev) >> failed_mii_init: >> failed_irq: >> failed_init: >> +failed_reset: >> fec_ptp_stop(pdev); >> if (fep->reg_phy) >> regulator_disable(fep->reg_phy); >> -- >> 2.1.4
From: Manfred Schlaegl <manfred.schlaegl@ginzinger.com> Sent: Monday, October 24, 2016 10:43 PM > To: Andy Duan <fugang.duan@nxp.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] net: fec: hard phy reset on open > > On 2016-10-24 16:03, Andy Duan wrote: > > From: manfred.schlaegl@gmx.at <manfred.schlaegl@gmx.at> Sent: > Monday, > > October 24, 2016 5:26 PM > >> To: Andy Duan <fugang.duan@nxp.com> > >> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org > >> Subject: [PATCH] net: fec: hard phy reset on open > >> > >> We have seen some problems with auto negotiation on i.MX6 using > >> LAN8720, after interface down/up. > >> > >> In our configuration, the ptp clock is used externally as reference > >> clock for the phy. Some phys (e.g. LAN8720) needs a stable clock > >> while and after hard reset. > >> Before this patch, the driver disabled the clock on close but did no > >> hard reset on open, after enabling the clocks again. > >> > >> A solution that prevents disabling the clocks on close was > >> considered, but discarded because of bad power saving behavior. > >> > >> This patch saves the reset dt properties on probe and does a reset on > >> every open after clocks where enabled, to make sure the clock is > >> stable while and after hard reset. > >> > >> Tested on i.MX6 and i.MX28, both using LAN8720. > >> > >> Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com> > >> --- > > > This patch did hard reset to let phy stable. > > > Firstly, you should do reset before clock enable. > I have to disagree here. > The phy demands(datasheet + tests) a stable clock at the time of (hard- > )reset and after this. Therefore the clock has to be enabled before the hard > reset. > (This is exactly the reason for the patch.) > > Generally: The sense of a reset is to defer the start of digital circuit until the > environment (power, clocks, ...) has stabilized. > > Furthermore: Before this patch the hard reset was done in fec_probe. And > here also after the clocks were enabled. > > Whats was your argument to do it the other way in this special case? > I check some different vendor phy, hard reset assert after clock stable. But I still don't ensure all phys are this action. > > Secondly, we suggest to do phy reset in phy driver, not MAC driver. > I was not sure, if you meant a soft-, or hard-reset here. > > In case you are talking about soft reset: > Yes, the phy drivers perform a soft reset. Sadly a soft reset is not sufficient in > this case - The phy recovers only on a hard reset from lost clock. (datasheet + > tests) > > In case you're talking about hard reset: > Intuitively I would also think, that the (hard-)reset should be handled by the > phy driver, but this is not reality in given implementations. > > Documentation/devicetree/bindings/net/fsl-fec.txt says > > - phy-reset-gpios : Should specify the gpio for phy reset > > It is explicitly talked about phy-reset here. And the (hard-)reset was handled > by the fec driver also before this patch (once on probe). > I suggest to do phy hard reset in phy driver like: drivers/net/phy/spi_ks8995.c: and Uwe Kleine-König's patch "phy: add support for a reset-gpio specification" (I don't know why the patch is reverted now.) Regards, Andy > > > > Regards, > > Andy > > Thanks for your feedback! > > Best regards, > Manfred > > > > > > >> drivers/net/ethernet/freescale/fec.h | 4 ++ > >> drivers/net/ethernet/freescale/fec_main.c | 77 > >> +++++++++++++++++------- > >> ------- > >> 2 files changed, 47 insertions(+), 34 deletions(-) > >> > >> diff --git a/drivers/net/ethernet/freescale/fec.h > >> b/drivers/net/ethernet/freescale/fec.h > >> index c865135..379e619 100644 > >> --- a/drivers/net/ethernet/freescale/fec.h > >> +++ b/drivers/net/ethernet/freescale/fec.h > >> @@ -498,6 +498,10 @@ struct fec_enet_private { > >> struct clk *clk_enet_out; > >> struct clk *clk_ptp; > >> > >> + int phy_reset; > >> + bool phy_reset_active_high; > >> + int phy_reset_msec; > >> + > >> bool ptp_clk_on; > >> struct mutex ptp_clk_mutex; > >> unsigned int num_tx_queues; > >> diff --git a/drivers/net/ethernet/freescale/fec_main.c > >> b/drivers/net/ethernet/freescale/fec_main.c > >> index 48a033e..8cc1ec5 100644 > >> --- a/drivers/net/ethernet/freescale/fec_main.c > >> +++ b/drivers/net/ethernet/freescale/fec_main.c > >> @@ -2802,6 +2802,22 @@ static int fec_enet_alloc_buffers(struct > >> net_device *ndev) > >> return 0; > >> } > >> > >> +static void fec_reset_phy(struct fec_enet_private *fep) { > >> + if (!gpio_is_valid(fep->phy_reset)) > >> + return; > >> + > >> + gpio_set_value_cansleep(fep->phy_reset, !!fep- > >>> phy_reset_active_high); > >> + > >> + if (fep->phy_reset_msec > 20) > >> + msleep(fep->phy_reset_msec); > >> + else > >> + usleep_range(fep->phy_reset_msec * 1000, > >> + fep->phy_reset_msec * 1000 + 1000); > >> + > >> + gpio_set_value_cansleep(fep->phy_reset, !fep- > >>> phy_reset_active_high); > >> +} > >> + > >> static int > >> fec_enet_open(struct net_device *ndev) { @@ -2817,6 +2833,8 @@ > >> fec_enet_open(struct net_device *ndev) > >> if (ret) > >> goto clk_enable; > >> > >> + fec_reset_phy(fep); > >> + > >> /* I should reset the ring buffers here, but I don't yet know > >> * a simple way to do that. > >> */ > >> @@ -3183,52 +3201,39 @@ static int fec_enet_init(struct net_device > *ndev) > >> return 0; > >> } > >> > >> -#ifdef CONFIG_OF > >> -static void fec_reset_phy(struct platform_device *pdev) > >> +static int > >> +fec_get_reset_phy(struct platform_device *pdev, int *msec, int > >> *phy_reset, > >> + bool *active_high) > >> { > >> - int err, phy_reset; > >> - bool active_high = false; > >> - int msec = 1; > >> + int err; > >> struct device_node *np = pdev->dev.of_node; > >> > >> - if (!np) > >> - return; > >> + if (!np || !of_device_is_available(np)) > >> + return 0; > >> > >> - of_property_read_u32(np, "phy-reset-duration", &msec); > >> + of_property_read_u32(np, "phy-reset-duration", msec); > >> /* A sane reset duration should not be longer than 1s */ > >> - if (msec > 1000) > >> - msec = 1; > >> + if (*msec > 1000) > >> + *msec = 1; > >> > >> - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); > >> - if (!gpio_is_valid(phy_reset)) > >> - return; > >> + *phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); > >> + if (!gpio_is_valid(*phy_reset)) > >> + return 0; > >> > >> - active_high = of_property_read_bool(np, "phy-reset-active-high"); > >> + *active_high = of_property_read_bool(np, "phy-reset-active-high"); > >> > >> - err = devm_gpio_request_one(&pdev->dev, phy_reset, > >> - active_high ? GPIOF_OUT_INIT_HIGH : > >> GPIOF_OUT_INIT_LOW, > >> - "phy-reset"); > >> + err = devm_gpio_request_one(&pdev->dev, *phy_reset, > >> + *active_high ? > >> + GPIOF_OUT_INIT_HIGH : > >> + GPIOF_OUT_INIT_LOW, > >> + "phy-reset"); > >> if (err) { > >> dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", > err); > >> - return; > >> + return err; > >> } > >> > >> - if (msec > 20) > >> - msleep(msec); > >> - else > >> - usleep_range(msec * 1000, msec * 1000 + 1000); > >> - > >> - gpio_set_value_cansleep(phy_reset, !active_high); > >> -} > >> -#else /* CONFIG_OF */ > >> -static void fec_reset_phy(struct platform_device *pdev) -{ > >> - /* > >> - * In case of platform probe, the reset has been done > >> - * by machine code. > >> - */ > >> + return 0; > >> } > >> -#endif /* CONFIG_OF */ > >> > >> static void > >> fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, > >> int > >> *num_rx) @@ -3409,7 +3414,10 @@ fec_probe(struct platform_device > >> *pdev) > >> pm_runtime_set_active(&pdev->dev); > >> pm_runtime_enable(&pdev->dev); > >> > >> - fec_reset_phy(pdev); > >> + ret = fec_get_reset_phy(pdev, &fep->phy_reset_msec, &fep- > >>> phy_reset, > >> + &fep->phy_reset_active_high); > >> + if (ret) > >> + goto failed_reset; > >> > >> if (fep->bufdesc_ex) > >> fec_ptp_init(pdev); > >> @@ -3467,6 +3475,7 @@ fec_probe(struct platform_device *pdev) > >> failed_mii_init: > >> failed_irq: > >> failed_init: > >> +failed_reset: > >> fec_ptp_stop(pdev); > >> if (fep->reg_phy) > >> regulator_disable(fep->reg_phy); > >> -- > >> 2.1.4 >
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h index c865135..379e619 100644 --- a/drivers/net/ethernet/freescale/fec.h +++ b/drivers/net/ethernet/freescale/fec.h @@ -498,6 +498,10 @@ struct fec_enet_private { struct clk *clk_enet_out; struct clk *clk_ptp; + int phy_reset; + bool phy_reset_active_high; + int phy_reset_msec; + bool ptp_clk_on; struct mutex ptp_clk_mutex; unsigned int num_tx_queues; diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 48a033e..8cc1ec5 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2802,6 +2802,22 @@ static int fec_enet_alloc_buffers(struct net_device *ndev) return 0; } +static void fec_reset_phy(struct fec_enet_private *fep) +{ + if (!gpio_is_valid(fep->phy_reset)) + return; + + gpio_set_value_cansleep(fep->phy_reset, !!fep->phy_reset_active_high); + + if (fep->phy_reset_msec > 20) + msleep(fep->phy_reset_msec); + else + usleep_range(fep->phy_reset_msec * 1000, + fep->phy_reset_msec * 1000 + 1000); + + gpio_set_value_cansleep(fep->phy_reset, !fep->phy_reset_active_high); +} + static int fec_enet_open(struct net_device *ndev) { @@ -2817,6 +2833,8 @@ fec_enet_open(struct net_device *ndev) if (ret) goto clk_enable; + fec_reset_phy(fep); + /* I should reset the ring buffers here, but I don't yet know * a simple way to do that. */ @@ -3183,52 +3201,39 @@ static int fec_enet_init(struct net_device *ndev) return 0; } -#ifdef CONFIG_OF -static void fec_reset_phy(struct platform_device *pdev) +static int +fec_get_reset_phy(struct platform_device *pdev, int *msec, int *phy_reset, + bool *active_high) { - int err, phy_reset; - bool active_high = false; - int msec = 1; + int err; struct device_node *np = pdev->dev.of_node; - if (!np) - return; + if (!np || !of_device_is_available(np)) + return 0; - of_property_read_u32(np, "phy-reset-duration", &msec); + of_property_read_u32(np, "phy-reset-duration", msec); /* A sane reset duration should not be longer than 1s */ - if (msec > 1000) - msec = 1; + if (*msec > 1000) + *msec = 1; - phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); - if (!gpio_is_valid(phy_reset)) - return; + *phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); + if (!gpio_is_valid(*phy_reset)) + return 0; - active_high = of_property_read_bool(np, "phy-reset-active-high"); + *active_high = of_property_read_bool(np, "phy-reset-active-high"); - err = devm_gpio_request_one(&pdev->dev, phy_reset, - active_high ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, - "phy-reset"); + err = devm_gpio_request_one(&pdev->dev, *phy_reset, + *active_high ? + GPIOF_OUT_INIT_HIGH : + GPIOF_OUT_INIT_LOW, + "phy-reset"); if (err) { dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err); - return; + return err; } - if (msec > 20) - msleep(msec); - else - usleep_range(msec * 1000, msec * 1000 + 1000); - - gpio_set_value_cansleep(phy_reset, !active_high); -} -#else /* CONFIG_OF */ -static void fec_reset_phy(struct platform_device *pdev) -{ - /* - * In case of platform probe, the reset has been done - * by machine code. - */ + return 0; } -#endif /* CONFIG_OF */ static void fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, int *num_rx) @@ -3409,7 +3414,10 @@ fec_probe(struct platform_device *pdev) pm_runtime_set_active(&pdev->dev); pm_runtime_enable(&pdev->dev); - fec_reset_phy(pdev); + ret = fec_get_reset_phy(pdev, &fep->phy_reset_msec, &fep->phy_reset, + &fep->phy_reset_active_high); + if (ret) + goto failed_reset; if (fep->bufdesc_ex) fec_ptp_init(pdev); @@ -3467,6 +3475,7 @@ fec_probe(struct platform_device *pdev) failed_mii_init: failed_irq: failed_init: +failed_reset: fec_ptp_stop(pdev); if (fep->reg_phy) regulator_disable(fep->reg_phy);
We have seen some problems with auto negotiation on i.MX6 using LAN8720, after interface down/up. In our configuration, the ptp clock is used externally as reference clock for the phy. Some phys (e.g. LAN8720) needs a stable clock while and after hard reset. Before this patch, the driver disabled the clock on close but did no hard reset on open, after enabling the clocks again. A solution that prevents disabling the clocks on close was considered, but discarded because of bad power saving behavior. This patch saves the reset dt properties on probe and does a reset on every open after clocks where enabled, to make sure the clock is stable while and after hard reset. Tested on i.MX6 and i.MX28, both using LAN8720. Signed-off-by: Manfred Schlaegl <manfred.schlaegl@ginzinger.com> --- drivers/net/ethernet/freescale/fec.h | 4 ++ drivers/net/ethernet/freescale/fec_main.c | 77 +++++++++++++++++-------------- 2 files changed, 47 insertions(+), 34 deletions(-)