Re: DP83867 ethernet PHY regression

[Francesco Dolcini]

Hello Johannes,

On Thu, May 25, 2023 at 08:31:00AM +0200, Johannes Pointner wrote:
> On Wed, May 24, 2023 at 3:22 PM Bagas Sanjaya <bagasdotme@xxxxxxxxx> wrote:
> >
> > On Mon, May 22, 2023 at 04:58:46PM +0200, Francesco Dolcini wrote:
> > > Hello all,
> > > commit da9ef50f545f ("net: phy: dp83867: perform soft reset and retain
> > > established link") introduces a regression on my TI AM62 based board.
> > >
> > > I have a working DTS with Linux TI 5.10 downstream kernel branch, while
> > > testing the DTS with v6.4-rc in preparation of sending it to the mailing
> > > list I noticed that ethernet is working only on a cold poweron.
> > >
> > > With da9ef50f545f reverted it always works.
> > >
> > > Here the DTS snippet for reference:
> > >
> > > &cpsw_port1 {
> > >       phy-handle = <&cpsw3g_phy0>;
> > >       phy-mode = "rgmii-rxid";
> > > };
> > >
> > > &cpsw3g_mdio {
> > >       assigned-clocks = <&k3_clks 157 20>;
> > >       assigned-clock-parents = <&k3_clks 157 22>;
> > >       assigned-clock-rates = <25000000>;
> > >
> > >       cpsw3g_phy0: ethernet-phy@0 {
> > >               compatible = "ethernet-phy-id2000.a231";
> > >               reg = <0>;
> > >               interrupt-parent = <&main_gpio0>;
> > >               interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> > >               reset-gpios = <&main_gpio0 17 GPIO_ACTIVE_LOW>;
> > >               reset-assert-us = <10>;
> > >               reset-deassert-us = <1000>;
> > >               ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> > >               ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> > >       };
> > > };
> > >
> >
> > Thanks for the regression report. I'm adding it to regzbot:
> >
> > #regzbot ^introduced: da9ef50f545f86
> > #regzbot title: TI AM62 DTS regression due to dp83867 soft reset
> 
> Hello Francesco,
> 
> I had a similar issue with a patch like this, but in my case it was the DP83822.
> https://lore.kernel.org/netdev/CAHvQdo2yzJC89K74c_CZFjPydDQ5i22w36XPR5tKVv_W8a2vcg@xxxxxxxxxxxxxx/
> I also raised the question for the commit da9ef50f545f.
> https://lore.kernel.org/lkml/CAHvQdo1U_L=pETmTJXjdzO+k7vNTxMyujn99Y3Ot9xAyQu=atQ@xxxxxxxxxxxxxx/
> 
> The problem was/is for me that the phy gets the clock from the CPU and
> the phy is already initialized in the u-boot.
> During the Linux kernel boot up there is a short amount of time where
> no clock is delivered to the phy.
> The phy didn't like this and was most of the time not usable anymore.
> The only thing that brought the phy/link back was resetting the phy
> using the phytool.

I had a look and it seems that is a different issue here, but I cannot
exclude that this is related.

First the link up/down, negotiation and mdio and related communication
is perfectly fine, what it looks like is not working is that no data is
flowing over RGMII.

Second, also in our case the clock is coming from the SoC, however this
clock is enabled way earlier in the boot, and at that time the phy is
even in reset.

 phy reset asserted
 . SPL on TI AM62 R5
   . enable clock
 . SPL on TI AM62 A
 . U-Boot proper on TI AM62 A
   .release phy reset

The phy reset is also configured in the DTS and used by the Linux driver.

In addition to that, as I already clarified in my second email, the
issue is happening also on a cold poweron. It happens most of the time,
but not always.

Francesco