[PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Andrew Lunn]

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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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.

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Andrew Lunn]

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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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 ?

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Andrew Lunn]

> > 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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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?

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Andrew Lunn]

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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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.

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Richard Leitner]

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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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 !

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Richard Leitner]

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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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 ?

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Richard Leitner]

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

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Marek Vasut]

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.

Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

[Andrew Lunn]

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