netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
@ 2020-09-03 20:27 Marek Vasut
  2020-09-03 21:00 ` Andrew Lunn
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Vasut @ 2020-09-03 20:27 UTC (permalink / raw)
  To: netdev
  Cc: Marek Vasut, Christoph Niedermaier, David S . Miller,
	NXP Linux Team, Richard Leitner, Shawn Guo

The phy_reset_after_clk_enable() does a PHY reset, which means the PHY
loses its register settings. The fec_enet_mii_probe() starts the PHY
and does the necessary calls to configure the PHY via PHY framework,
and loads the correct register settings into the PHY. Therefore,
fec_enet_mii_probe() should be called only after the PHY has been
reset, not before as it is now.

Fixes: 1b0a83ac04e3 ("net: fec: add phy_reset_after_clk_enable() support")
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Christoph Niedermaier <cniedermaier@dh-electronics.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Richard Leitner <richard.leitner@skidata.com>
Cc: Shawn Guo <shawnguo@kernel.org>
---
 drivers/net/ethernet/freescale/fec_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index fb37816a74db..23abe7f6cad0 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2984,17 +2984,17 @@ fec_enet_open(struct net_device *ndev)
 	/* Init MAC prior to mii bus probe */
 	fec_restart(ndev);
 
-	/* Probe and connect to PHY when open the interface */
-	ret = fec_enet_mii_probe(ndev);
-	if (ret)
-		goto err_enet_mii_probe;
-
 	/* Call phy_reset_after_clk_enable() again if it failed during
 	 * phy_reset_after_clk_enable() before because the PHY wasn't probed.
 	 */
 	if (reset_again)
 		phy_reset_after_clk_enable(ndev->phydev);
 
+	/* Probe and connect to PHY when open the interface */
+	ret = fec_enet_mii_probe(ndev);
+	if (ret)
+		goto err_enet_mii_probe;
+
 	if (fep->quirks & FEC_QUIRK_ERR006687)
 		imx6q_cpuidle_fec_irqs_used();
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 20:27 [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable() Marek Vasut
@ 2020-09-03 21:00 ` Andrew Lunn
  2020-09-03 21:36   ` Marek Vasut
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2020-09-03 21:00 UTC (permalink / raw)
  To: Marek Vasut
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On Thu, Sep 03, 2020 at 10:27:12PM +0200, Marek Vasut wrote:
> The phy_reset_after_clk_enable() does a PHY reset, which means the PHY
> loses its register settings. The fec_enet_mii_probe() starts the PHY
> and does the necessary calls to configure the PHY via PHY framework,
> and loads the correct register settings into the PHY. Therefore,
> fec_enet_mii_probe() should be called only after the PHY has been
> reset, not before as it is now.

I think this patch is related to what Florian is currently doing with
PHY clocks.

I think a better fix for the original problem is for the SMSC PHY
driver to control the clock itself. If it clk_prepare_enables() the
clock, it knows it will not be shut off again by the FEC run time
power management.

All this phy_reset_after_clk_enable() can then go away.

    Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 21:00 ` Andrew Lunn
@ 2020-09-03 21:36   ` Marek Vasut
  2020-09-03 21:53     ` Andrew Lunn
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Vasut @ 2020-09-03 21:36 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On 9/3/20 11:00 PM, Andrew Lunn wrote:
> On Thu, Sep 03, 2020 at 10:27:12PM +0200, Marek Vasut wrote:
>> The phy_reset_after_clk_enable() does a PHY reset, which means the PHY
>> loses its register settings. The fec_enet_mii_probe() starts the PHY
>> and does the necessary calls to configure the PHY via PHY framework,
>> and loads the correct register settings into the PHY. Therefore,
>> fec_enet_mii_probe() should be called only after the PHY has been
>> reset, not before as it is now.
> 
> I think this patch is related to what Florian is currently doing with
> PHY clocks.

Which is what ? Details please.

> I think a better fix for the original problem is for the SMSC PHY
> driver to control the clock itself. If it clk_prepare_enables() the
> clock, it knows it will not be shut off again by the FEC run time
> power management.

The FEC MAC is responsible for generating the clock, the PHY clock are
not part of the clock framework as far as I can tell.

> All this phy_reset_after_clk_enable() can then go away.

I'm not sure about that. Also, this is a much simpler fix which can be
backported easily.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 21:36   ` Marek Vasut
@ 2020-09-03 21:53     ` Andrew Lunn
  2020-09-03 22:03       ` Marek Vasut
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2020-09-03 21:53 UTC (permalink / raw)
  To: Marek Vasut
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On Thu, Sep 03, 2020 at 11:36:39PM +0200, Marek Vasut wrote:
> On 9/3/20 11:00 PM, Andrew Lunn wrote:
> > On Thu, Sep 03, 2020 at 10:27:12PM +0200, Marek Vasut wrote:
> >> The phy_reset_after_clk_enable() does a PHY reset, which means the PHY
> >> loses its register settings. The fec_enet_mii_probe() starts the PHY
> >> and does the necessary calls to configure the PHY via PHY framework,
> >> and loads the correct register settings into the PHY. Therefore,
> >> fec_enet_mii_probe() should be called only after the PHY has been
> >> reset, not before as it is now.
> > 
> > I think this patch is related to what Florian is currently doing with
> > PHY clocks.
> 
> Which is what ? Details please.

Have you used b4 before?

b4 am 20200903043947.3272453-1-f.fainelli@gmail.com

> > I think a better fix for the original problem is for the SMSC PHY
> > driver to control the clock itself. If it clk_prepare_enables() the
> > clock, it knows it will not be shut off again by the FEC run time
> > power management.
> 
> The FEC MAC is responsible for generating the clock, the PHY clock are
> not part of the clock framework as far as I can tell.

I'm not sure this is true. At least:

https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi#L123

and there are a few more examples:

imx6ul-14x14-evk.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
imx6ul-kontron-n6x1x-s.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
imx6ul-kontron-n6x1x-som-common.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
imx6ull-myir-mys-6ulx.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
imx6ul-phytec-phycore-som.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;

Maybe it is just IMX6?

      Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 21:53     ` Andrew Lunn
@ 2020-09-03 22:03       ` Marek Vasut
  2020-09-03 22:08         ` Andrew Lunn
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Vasut @ 2020-09-03 22:03 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On 9/3/20 11:53 PM, Andrew Lunn wrote:
> On Thu, Sep 03, 2020 at 11:36:39PM +0200, Marek Vasut wrote:
>> On 9/3/20 11:00 PM, Andrew Lunn wrote:
>>> On Thu, Sep 03, 2020 at 10:27:12PM +0200, Marek Vasut wrote:
>>>> The phy_reset_after_clk_enable() does a PHY reset, which means the PHY
>>>> loses its register settings. The fec_enet_mii_probe() starts the PHY
>>>> and does the necessary calls to configure the PHY via PHY framework,
>>>> and loads the correct register settings into the PHY. Therefore,
>>>> fec_enet_mii_probe() should be called only after the PHY has been
>>>> reset, not before as it is now.
>>>
>>> I think this patch is related to what Florian is currently doing with
>>> PHY clocks.
>>
>> Which is what ? Details please.
> 
> Have you used b4 before?
> 
> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com

That might be a fix for the long run, but I doubt there's any chance to
backport it all to stable, is there ?

>>> I think a better fix for the original problem is for the SMSC PHY
>>> driver to control the clock itself. If it clk_prepare_enables() the
>>> clock, it knows it will not be shut off again by the FEC run time
>>> power management.
>>
>> The FEC MAC is responsible for generating the clock, the PHY clock are
>> not part of the clock framework as far as I can tell.
> 
> I'm not sure this is true. At least:
> 
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi#L123
> 
> and there are a few more examples:
> 
> imx6ul-14x14-evk.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ul-kontron-n6x1x-s.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ul-kontron-n6x1x-som-common.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ull-myir-mys-6ulx.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ul-phytec-phycore-som.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> 
> Maybe it is just IMX6?

This is reference clock for the FEC inside the SoC, you probably want to
control the clock going out of the SoC and into the PHY, which is
different clock than the one described in the DT, right ?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 22:03       ` Marek Vasut
@ 2020-09-03 22:08         ` Andrew Lunn
  2020-09-03 22:45           ` Marek Vasut
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2020-09-03 22:08 UTC (permalink / raw)
  To: Marek Vasut
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

> > b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
> 
> That might be a fix for the long run, but I doubt there's any chance to
> backport it all to stable, is there ?

No. For stable we need something simpler.

> >>> I think a better fix for the original problem is for the SMSC PHY
> >>> driver to control the clock itself. If it clk_prepare_enables() the
> >>> clock, it knows it will not be shut off again by the FEC run time
> >>> power management.
> >>
> >> The FEC MAC is responsible for generating the clock, the PHY clock are
> >> not part of the clock framework as far as I can tell.
> > 
> > I'm not sure this is true. At least:
> > 
> > https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi#L123
> > 
> > and there are a few more examples:
> > 
> > imx6ul-14x14-evk.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> > imx6ul-kontron-n6x1x-s.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> > imx6ul-kontron-n6x1x-som-common.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> > imx6ull-myir-mys-6ulx.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> > imx6ul-phytec-phycore-som.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> > 
> > Maybe it is just IMX6?
> 
> This is reference clock for the FEC inside the SoC, you probably want to
> control the clock going out of the SoC and into the PHY, which is
> different clock than the one described in the DT, right ?

I _think_ this is the external clock which is feed to the PHY. Why
else put it in the phy node in DT? And it has the name "rmii-ref"
which again suggests it is the RMII clock, not something internal to
the FEC.

To be sure, we would need to check the datasheet.

   Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 22:08         ` Andrew Lunn
@ 2020-09-03 22:45           ` Marek Vasut
  2020-09-04 14:02             ` Andrew Lunn
  2020-09-09 12:24             ` Andrew Lunn
  0 siblings, 2 replies; 16+ messages in thread
From: Marek Vasut @ 2020-09-03 22:45 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On 9/4/20 12:08 AM, Andrew Lunn wrote:
>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
>>
>> That might be a fix for the long run, but I doubt there's any chance to
>> backport it all to stable, is there ?
> 
> No. For stable we need something simpler.

Like this patch ?

>>>>> I think a better fix for the original problem is for the SMSC PHY
>>>>> driver to control the clock itself. If it clk_prepare_enables() the
>>>>> clock, it knows it will not be shut off again by the FEC run time
>>>>> power management.
>>>>
>>>> The FEC MAC is responsible for generating the clock, the PHY clock are
>>>> not part of the clock framework as far as I can tell.
>>>
>>> I'm not sure this is true. At least:
>>>
>>> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi#L123
>>>
>>> and there are a few more examples:
>>>
>>> imx6ul-14x14-evk.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
>>> imx6ul-kontron-n6x1x-s.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
>>> imx6ul-kontron-n6x1x-som-common.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
>>> imx6ull-myir-mys-6ulx.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
>>> imx6ul-phytec-phycore-som.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
>>>
>>> Maybe it is just IMX6?
>>
>> This is reference clock for the FEC inside the SoC, you probably want to
>> control the clock going out of the SoC and into the PHY, which is
>> different clock than the one described in the DT, right ?
> 
> I _think_ this is the external clock which is feed to the PHY. Why
> else put it in the phy node in DT? And it has the name "rmii-ref"
> which again suggests it is the RMII clock, not something internal to
> the FEC.
> 
> To be sure, we would need to check the datasheet.

On iMX6Q where I have this issue (which btw is a very different SoC than
iMX6UL), this is not part of the PHY node. See
arch/arm/boot/dts/imx6qdl.dtsi . The SoC generates the clock and feeds
it into both the FEC and the PHY there.

Either way, this seems way out of scope for a bugfix which just corrects
the order of PHY reset/init, doesn't it?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 22:45           ` Marek Vasut
@ 2020-09-04 14:02             ` Andrew Lunn
  2020-09-04 15:26               ` Marek Vasut
  2020-09-09 12:24             ` Andrew Lunn
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2020-09-04 14:02 UTC (permalink / raw)
  To: Marek Vasut
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
> On 9/4/20 12:08 AM, Andrew Lunn wrote:
> >>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
> >>
> >> That might be a fix for the long run, but I doubt there's any chance to
> >> backport it all to stable, is there ?
> > 
> > No. For stable we need something simpler.
> 
> Like this patch ?

Yes.

But i would like to see a Tested-By: or similar from Richard
Leitner. Why does the current code work for his system? Does your
change break it?

       Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-04 14:02             ` Andrew Lunn
@ 2020-09-04 15:26               ` Marek Vasut
  2020-09-04 19:02                 ` Richard Leitner
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Vasut @ 2020-09-04 15:26 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On 9/4/20 4:02 PM, Andrew Lunn wrote:
> On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
>> On 9/4/20 12:08 AM, Andrew Lunn wrote:
>>>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
>>>>
>>>> That might be a fix for the long run, but I doubt there's any chance to
>>>> backport it all to stable, is there ?
>>>
>>> No. For stable we need something simpler.
>>
>> Like this patch ?
> 
> Yes.
> 
> But i would like to see a Tested-By: or similar from Richard
> Leitner. Why does the current code work for his system? Does your
> change break it?

I have the IRQ line connected and described in DT. The reset clears the
IRQ settings done by the SMSC PHY driver. The PHY works fine if I use
polling, because then even if no IRQs are generated by the PHY, the PHY
framework reads the status updates from the PHY periodically and the
default settings of the PHY somehow work (even if they are slightly
incorrect). I suspect that's how Richard had it working.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-04 15:26               ` Marek Vasut
@ 2020-09-04 19:02                 ` Richard Leitner
  2020-09-04 19:23                   ` Marek Vasut
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Leitner @ 2020-09-04 19:02 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Andrew Lunn, netdev, Christoph Niedermaier, David S . Miller,
	NXP Linux Team, Shawn Guo

On Fri, Sep 04, 2020 at 05:26:14PM +0200, Marek Vasut wrote:
> On 9/4/20 4:02 PM, Andrew Lunn wrote:
> > On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
> >> On 9/4/20 12:08 AM, Andrew Lunn wrote:
> >>>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
> >>>>
> >>>> That might be a fix for the long run, but I doubt there's any chance to
> >>>> backport it all to stable, is there ?
> >>>
> >>> No. For stable we need something simpler.
> >>
> >> Like this patch ?
> > 
> > Yes.
> > 
> > But i would like to see a Tested-By: or similar from Richard
> > Leitner. Why does the current code work for his system? Does your
> > change break it?
> 
> I have the IRQ line connected and described in DT. The reset clears the
> IRQ settings done by the SMSC PHY driver. The PHY works fine if I use
> polling, because then even if no IRQs are generated by the PHY, the PHY
> framework reads the status updates from the PHY periodically and the
> default settings of the PHY somehow work (even if they are slightly
> incorrect). I suspect that's how Richard had it working.

I have different PHYs on different PCBs in use, but IIRC none of them
has the IRQ line defined in the DT.
I will take a look at it, test your patch and give feedback ASAP.
Unfortunately it's unlikely that this will be before monday 😕
Hope that's ok.

regards;rl

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-04 19:02                 ` Richard Leitner
@ 2020-09-04 19:23                   ` Marek Vasut
  2020-09-09  8:38                     ` Richard Leitner
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Vasut @ 2020-09-04 19:23 UTC (permalink / raw)
  To: Richard Leitner
  Cc: Andrew Lunn, netdev, Christoph Niedermaier, David S . Miller,
	NXP Linux Team, Shawn Guo

On 9/4/20 9:02 PM, Richard Leitner wrote:
> On Fri, Sep 04, 2020 at 05:26:14PM +0200, Marek Vasut wrote:
>> On 9/4/20 4:02 PM, Andrew Lunn wrote:
>>> On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
>>>> On 9/4/20 12:08 AM, Andrew Lunn wrote:
>>>>>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
>>>>>>
>>>>>> That might be a fix for the long run, but I doubt there's any chance to
>>>>>> backport it all to stable, is there ?
>>>>>
>>>>> No. For stable we need something simpler.
>>>>
>>>> Like this patch ?
>>>
>>> Yes.
>>>
>>> But i would like to see a Tested-By: or similar from Richard
>>> Leitner. Why does the current code work for his system? Does your
>>> change break it?
>>
>> I have the IRQ line connected and described in DT. The reset clears the
>> IRQ settings done by the SMSC PHY driver. The PHY works fine if I use
>> polling, because then even if no IRQs are generated by the PHY, the PHY
>> framework reads the status updates from the PHY periodically and the
>> default settings of the PHY somehow work (even if they are slightly
>> incorrect). I suspect that's how Richard had it working.
> 
> I have different PHYs on different PCBs in use, but IIRC none of them
> has the IRQ line defined in the DT.
> I will take a look at it, test your patch and give feedback ASAP.
> Unfortunately it's unlikely that this will be before monday 😕
> Hope that's ok.

That's totally fine, thanks !

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-04 19:23                   ` Marek Vasut
@ 2020-09-09  8:38                     ` Richard Leitner
  2020-09-26 18:52                       ` Marek Vasut
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Leitner @ 2020-09-09  8:38 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Andrew Lunn, netdev, Christoph Niedermaier, David S . Miller,
	NXP Linux Team, Shawn Guo

On Fri, Sep 04, 2020 at 09:23:26PM +0200, Marek Vasut wrote:
> On 9/4/20 9:02 PM, Richard Leitner wrote:
> > On Fri, Sep 04, 2020 at 05:26:14PM +0200, Marek Vasut wrote:
> >> On 9/4/20 4:02 PM, Andrew Lunn wrote:
> >>> On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
> >>>> On 9/4/20 12:08 AM, Andrew Lunn wrote:
> >>>>>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
> >>>>>>
> >>>>>> That might be a fix for the long run, but I doubt there's any chance to
> >>>>>> backport it all to stable, is there ?
> >>>>>
> >>>>> No. For stable we need something simpler.
> >>>>
> >>>> Like this patch ?
> >>>
> >>> Yes.
> >>>
> >>> But i would like to see a Tested-By: or similar from Richard
> >>> Leitner. Why does the current code work for his system? Does your
> >>> change break it?
> >>
> >> I have the IRQ line connected and described in DT. The reset clears the
> >> IRQ settings done by the SMSC PHY driver. The PHY works fine if I use
> >> polling, because then even if no IRQs are generated by the PHY, the PHY
> >> framework reads the status updates from the PHY periodically and the
> >> default settings of the PHY somehow work (even if they are slightly
> >> incorrect). I suspect that's how Richard had it working.
> > 
> > I have different PHYs on different PCBs in use, but IIRC none of them
> > has the IRQ line defined in the DT.
> > I will take a look at it, test your patch and give feedback ASAP.
> > Unfortunately it's unlikely that this will be before monday 😕
> > Hope that's ok.
> 
> That's totally fine, thanks !

Hi, sorry for the delay.
I've applied the patch to our kernel and did some basic tests on
different custom imx6 PCBs. As everything seems to work fine for our
"non-irq configuration" please feel free to add

Tested-by: Richard Leitner <richard.leitner@skidata.com>

regards;rl

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-03 22:45           ` Marek Vasut
  2020-09-04 14:02             ` Andrew Lunn
@ 2020-09-09 12:24             ` Andrew Lunn
  1 sibling, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2020-09-09 12:24 UTC (permalink / raw)
  To: Marek Vasut
  Cc: netdev, Christoph Niedermaier, David S . Miller, NXP Linux Team,
	Richard Leitner, Shawn Guo

On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
> On 9/4/20 12:08 AM, Andrew Lunn wrote:
> >>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
> >>
> >> That might be a fix for the long run, but I doubt there's any chance to
> >> backport it all to stable, is there ?
> > 
> > No. For stable we need something simpler.
> 
> Like this patch ?

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-09  8:38                     ` Richard Leitner
@ 2020-09-26 18:52                       ` Marek Vasut
  2020-09-28 13:03                         ` Richard Leitner
  0 siblings, 1 reply; 16+ messages in thread
From: Marek Vasut @ 2020-09-26 18:52 UTC (permalink / raw)
  To: Richard Leitner
  Cc: Andrew Lunn, netdev, Christoph Niedermaier, David S . Miller,
	NXP Linux Team, Shawn Guo

On 9/9/20 10:38 AM, Richard Leitner wrote:
> On Fri, Sep 04, 2020 at 09:23:26PM +0200, Marek Vasut wrote:
>> On 9/4/20 9:02 PM, Richard Leitner wrote:
>>> On Fri, Sep 04, 2020 at 05:26:14PM +0200, Marek Vasut wrote:
>>>> On 9/4/20 4:02 PM, Andrew Lunn wrote:
>>>>> On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
>>>>>> On 9/4/20 12:08 AM, Andrew Lunn wrote:
>>>>>>>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
>>>>>>>>
>>>>>>>> That might be a fix for the long run, but I doubt there's any chance to
>>>>>>>> backport it all to stable, is there ?
>>>>>>>
>>>>>>> No. For stable we need something simpler.
>>>>>>
>>>>>> Like this patch ?
>>>>>
>>>>> Yes.
>>>>>
>>>>> But i would like to see a Tested-By: or similar from Richard
>>>>> Leitner. Why does the current code work for his system? Does your
>>>>> change break it?
>>>>
>>>> I have the IRQ line connected and described in DT. The reset clears the
>>>> IRQ settings done by the SMSC PHY driver. The PHY works fine if I use
>>>> polling, because then even if no IRQs are generated by the PHY, the PHY
>>>> framework reads the status updates from the PHY periodically and the
>>>> default settings of the PHY somehow work (even if they are slightly
>>>> incorrect). I suspect that's how Richard had it working.
>>>
>>> I have different PHYs on different PCBs in use, but IIRC none of them
>>> has the IRQ line defined in the DT.
>>> I will take a look at it, test your patch and give feedback ASAP.
>>> Unfortunately it's unlikely that this will be before monday 😕
>>> Hope that's ok.
>>
>> That's totally fine, thanks !
> 
> Hi, sorry for the delay.
> I've applied the patch to our kernel and did some basic tests on
> different custom imx6 PCBs. As everything seems to work fine for our
> "non-irq configuration" please feel free to add
> 
> Tested-by: Richard Leitner <richard.leitner@skidata.com>

So can this fix be applied ?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-26 18:52                       ` Marek Vasut
@ 2020-09-28 13:03                         ` Richard Leitner
  2020-10-06  9:15                           ` Marek Vasut
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Leitner @ 2020-09-28 13:03 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Andrew Lunn, netdev, Christoph Niedermaier, David S . Miller,
	NXP Linux Team, Shawn Guo

On Sat, Sep 26, 2020 at 08:52:17PM +0200, Marek Vasut wrote:
> On 9/9/20 10:38 AM, Richard Leitner wrote:
> > On Fri, Sep 04, 2020 at 09:23:26PM +0200, Marek Vasut wrote:
> >> On 9/4/20 9:02 PM, Richard Leitner wrote:
> >>> On Fri, Sep 04, 2020 at 05:26:14PM +0200, Marek Vasut wrote:
> >>>> On 9/4/20 4:02 PM, Andrew Lunn wrote:
> >>>>> On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
> >>>>>> On 9/4/20 12:08 AM, Andrew Lunn wrote:
> >>>>>>>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
> >>>>>>>>
> >>>>>>>> That might be a fix for the long run, but I doubt there's any chance to
> >>>>>>>> backport it all to stable, is there ?
> >>>>>>>
> >>>>>>> No. For stable we need something simpler.
> >>>>>>
> >>>>>> Like this patch ?
> >>>>>
> >>>>> Yes.
> >>>>>
> >>>>> But i would like to see a Tested-By: or similar from Richard
> >>>>> Leitner. Why does the current code work for his system? Does your
> >>>>> change break it?
> >>>>
> >>>> I have the IRQ line connected and described in DT. The reset clears the
> >>>> IRQ settings done by the SMSC PHY driver. The PHY works fine if I use
> >>>> polling, because then even if no IRQs are generated by the PHY, the PHY
> >>>> framework reads the status updates from the PHY periodically and the
> >>>> default settings of the PHY somehow work (even if they are slightly
> >>>> incorrect). I suspect that's how Richard had it working.
> >>>
> >>> I have different PHYs on different PCBs in use, but IIRC none of them
> >>> has the IRQ line defined in the DT.
> >>> I will take a look at it, test your patch and give feedback ASAP.
> >>> Unfortunately it's unlikely that this will be before monday 😕
> >>> Hope that's ok.
> >>
> >> That's totally fine, thanks !
> > 
> > Hi, sorry for the delay.
> > I've applied the patch to our kernel and did some basic tests on
> > different custom imx6 PCBs. As everything seems to work fine for our
> > "non-irq configuration" please feel free to add
> > 
> > Tested-by: Richard Leitner <richard.leitner@skidata.com>
> 
> So can this fix be applied ?

In case this question was aimed at me:
From my side there are no objections.

regards;rl

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()
  2020-09-28 13:03                         ` Richard Leitner
@ 2020-10-06  9:15                           ` Marek Vasut
  0 siblings, 0 replies; 16+ messages in thread
From: Marek Vasut @ 2020-10-06  9:15 UTC (permalink / raw)
  To: Richard Leitner
  Cc: Andrew Lunn, netdev, Christoph Niedermaier, David S . Miller,
	NXP Linux Team, Shawn Guo

On 9/28/20 3:03 PM, Richard Leitner wrote:
> On Sat, Sep 26, 2020 at 08:52:17PM +0200, Marek Vasut wrote:
>> On 9/9/20 10:38 AM, Richard Leitner wrote:
>>> On Fri, Sep 04, 2020 at 09:23:26PM +0200, Marek Vasut wrote:
>>>> On 9/4/20 9:02 PM, Richard Leitner wrote:
>>>>> On Fri, Sep 04, 2020 at 05:26:14PM +0200, Marek Vasut wrote:
>>>>>> On 9/4/20 4:02 PM, Andrew Lunn wrote:
>>>>>>> On Fri, Sep 04, 2020 at 12:45:44AM +0200, Marek Vasut wrote:
>>>>>>>> On 9/4/20 12:08 AM, Andrew Lunn wrote:
>>>>>>>>>>> b4 am 20200903043947.3272453-1-f.fainelli@gmail.com
>>>>>>>>>>
>>>>>>>>>> That might be a fix for the long run, but I doubt there's any chance to
>>>>>>>>>> backport it all to stable, is there ?
>>>>>>>>>
>>>>>>>>> No. For stable we need something simpler.
>>>>>>>>
>>>>>>>> Like this patch ?
>>>>>>>
>>>>>>> Yes.
>>>>>>>
>>>>>>> But i would like to see a Tested-By: or similar from Richard
>>>>>>> Leitner. Why does the current code work for his system? Does your
>>>>>>> change break it?
>>>>>>
>>>>>> I have the IRQ line connected and described in DT. The reset clears the
>>>>>> IRQ settings done by the SMSC PHY driver. The PHY works fine if I use
>>>>>> polling, because then even if no IRQs are generated by the PHY, the PHY
>>>>>> framework reads the status updates from the PHY periodically and the
>>>>>> default settings of the PHY somehow work (even if they are slightly
>>>>>> incorrect). I suspect that's how Richard had it working.
>>>>>
>>>>> I have different PHYs on different PCBs in use, but IIRC none of them
>>>>> has the IRQ line defined in the DT.
>>>>> I will take a look at it, test your patch and give feedback ASAP.
>>>>> Unfortunately it's unlikely that this will be before monday 😕
>>>>> Hope that's ok.
>>>>
>>>> That's totally fine, thanks !
>>>
>>> Hi, sorry for the delay.
>>> I've applied the patch to our kernel and did some basic tests on
>>> different custom imx6 PCBs. As everything seems to work fine for our
>>> "non-irq configuration" please feel free to add
>>>
>>> Tested-by: Richard Leitner <richard.leitner@skidata.com>
>>
>> So can this fix be applied ?
> 
> In case this question was aimed at me:
>>From my side there are no objections.

Thanks. It would be nice if this bugfix was applied upstream soon.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-10-06  9:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-03 20:27 [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable() Marek Vasut
2020-09-03 21:00 ` Andrew Lunn
2020-09-03 21:36   ` Marek Vasut
2020-09-03 21:53     ` Andrew Lunn
2020-09-03 22:03       ` Marek Vasut
2020-09-03 22:08         ` Andrew Lunn
2020-09-03 22:45           ` Marek Vasut
2020-09-04 14:02             ` Andrew Lunn
2020-09-04 15:26               ` Marek Vasut
2020-09-04 19:02                 ` Richard Leitner
2020-09-04 19:23                   ` Marek Vasut
2020-09-09  8:38                     ` Richard Leitner
2020-09-26 18:52                       ` Marek Vasut
2020-09-28 13:03                         ` Richard Leitner
2020-10-06  9:15                           ` Marek Vasut
2020-09-09 12:24             ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).