linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* DP83867 ethernet PHY regression
@ 2023-05-22 14:58 Francesco Dolcini
  2023-05-22 15:15 ` Andrew Lunn
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Francesco Dolcini @ 2023-05-22 14:58 UTC (permalink / raw)
  To: Andrew Lunn, Praneeth Bajjuri, Geet Modi, David S. Miller
  Cc: Heiner Kallweit, Russell King, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel, Grygorii Strashko, Dan Murphy

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

Any suggestion?
Francesco


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

* Re: DP83867 ethernet PHY regression
  2023-05-22 14:58 DP83867 ethernet PHY regression Francesco Dolcini
@ 2023-05-22 15:15 ` Andrew Lunn
  2023-05-22 15:35   ` Francesco Dolcini
  2023-05-24 12:13 ` Andrew Lunn
  2023-05-24 12:51 ` Bagas Sanjaya
  2 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2023-05-22 15:15 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Praneeth Bajjuri, Geet Modi, David S. Miller, Heiner Kallweit,
	Russell King, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev,
	linux-kernel, Grygorii Strashko, Dan Murphy

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.

Do you have more details about how it does not work.

Please could you use:

mii-tool -vvv ethX

in both the good and bad state. The register values might give us a
clue.

Thanks
	Andrew

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

* Re: DP83867 ethernet PHY regression
  2023-05-22 15:15 ` Andrew Lunn
@ 2023-05-22 15:35   ` Francesco Dolcini
  2023-05-22 16:15     ` Andrew Lunn
  0 siblings, 1 reply; 11+ messages in thread
From: Francesco Dolcini @ 2023-05-22 15:35 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Francesco Dolcini, Praneeth Bajjuri, Geet Modi, David S. Miller,
	Heiner Kallweit, Russell King, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel, Grygorii Strashko, Dan Murphy

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On Mon, May 22, 2023 at 05:15:56PM +0200, Andrew Lunn 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.
> 
> Do you have more details about how it does not work.
> 
> Please could you use:
> 
> mii-tool -vvv ethX

please see the attached files:

working_da9ef50f545f_reverted.txt
  this is on a v6.4-rc, with da9ef50f545f reverted

not_working.txt
  v6.4-rc not working

working.txt
  v6.4-rc working


It looks like, even on cold boot, it's not working in a reliable way.
Not sure the exact difference when it's working and when it's not.

> in both the good and bad state. The register values might give us a
> clue.

Thanks for your help,
Francesco


[-- Attachment #2: not_working.txt --]
[-- Type: text/plain, Size: 676 bytes --]

Using SIOCGMIIPHY=0x8947
eth0: negotiated 1000baseT-FD flow-control, link ok
  registers for MII PHY 0: 
    1140 796d 2000 a231 05e1 c5e1 006f 2001
    5806 0200 3800 0000 0000 4007 0000 3000
    5048 ac02 ec10 0004 2bc7 0000 0000 0040
    6150 4444 0002 0000 0000 0000 0282 0000
  product info: vendor 08:00:28, model 35 rev 1
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control
  link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control

[-- Attachment #3: working_da9ef50f545f_reverted.txt --]
[-- Type: text/plain, Size: 676 bytes --]

Using SIOCGMIIPHY=0x8947
eth0: negotiated 1000baseT-FD flow-control, link ok
  registers for MII PHY 0: 
    1140 796d 2000 a231 05e1 c5e1 006d 2001
    5806 0200 3800 0000 0000 4007 0000 3000
    5048 af02 ec10 0000 2bc7 0000 0000 0040
    6150 4444 0002 0000 0000 0000 0282 0000
  product info: vendor 08:00:28, model 35 rev 1
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control
  link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control

[-- Attachment #4: working.txt --]
[-- Type: text/plain, Size: 676 bytes --]

Using SIOCGMIIPHY=0x8947
eth0: negotiated 1000baseT-FD flow-control, link ok
  registers for MII PHY 0: 
    1140 796d 2000 a231 05e1 c5e1 006f 2001
    5806 0200 3800 0000 0000 4007 0000 3000
    5048 ac02 ec10 0000 2bc7 0000 0000 0040
    6150 4444 0002 0000 0000 0000 0282 0000
  product info: vendor 08:00:28, model 35 rev 1
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control
  link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control

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

* Re: DP83867 ethernet PHY regression
  2023-05-22 15:35   ` Francesco Dolcini
@ 2023-05-22 16:15     ` Andrew Lunn
  2023-05-25  8:18       ` Alexander Stein
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2023-05-22 16:15 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Praneeth Bajjuri, Geet Modi, David S. Miller, Heiner Kallweit,
	Russell King, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev,
	linux-kernel, Grygorii Strashko, Dan Murphy

On Mon, May 22, 2023 at 05:35:33PM +0200, Francesco Dolcini wrote:
> On Mon, May 22, 2023 at 05:15:56PM +0200, Andrew Lunn 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.
> > 
> > Do you have more details about how it does not work.
> > 
> > Please could you use:
> > 
> > mii-tool -vvv ethX
> 
> please see the attached files:
> 
> working_da9ef50f545f_reverted.txt
>   this is on a v6.4-rc, with da9ef50f545f reverted
> 
> not_working.txt
>   v6.4-rc not working
> 
> working.txt
>   v6.4-rc working
> 
> 
> It looks like, even on cold boot, it's not working in a reliable way.
> Not sure the exact difference when it's working and when it's not.

> Using SIOCGMIIPHY=0x8947
> eth0: negotiated 1000baseT-FD flow-control, link ok
>   registers for MII PHY 0: 
>     1140 796d 2000 a231 05e1 c5e1 006f 2001
>     5806 0200 3800 0000 0000 4007 0000 3000
>     5048 ac02 ec10 0004 2bc7 0000 0000 0040
>     6150 4444 0002 0000 0000 0000 0282 0000

>     1140 796d 2000 a231 05e1 c5e1 006d 2001
>     5806 0200 3800 0000 0000 4007 0000 3000
>     5048 af02 ec10 0000 2bc7 0000 0000 0040
>     6150 4444 0002 0000 0000 0000 0282 0000

Register  6: 006f vs 006d
Register 17: ac02 vs 1f02
Register 19: 0004 vs 0000

Register 6 is MII_EXPANSION. Bit 1 is

#define EXPANSION_LCWP          0x0002  /* Got new RX page code word   */

So that is probably not relevant here.

Register 17 is MII_DP83867_PHYSTS, and bits 8 and 9 are not documented
in the driver. Do you have the datasheet?

Register 19 is MII_DP83867_ISR. The interrupt bits are not documented
in the driver either.

This driver also uses C45 registers, which are not shown here. At some
point, we might need to look at those. But first it would be good to
understand what these differences mean.

	Andrew

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

* Re: DP83867 ethernet PHY regression
  2023-05-22 14:58 DP83867 ethernet PHY regression Francesco Dolcini
  2023-05-22 15:15 ` Andrew Lunn
@ 2023-05-24 12:13 ` Andrew Lunn
  2023-05-24 12:51 ` Bagas Sanjaya
  2 siblings, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2023-05-24 12:13 UTC (permalink / raw)
  To: Francesco Dolcini, Praneeth Bajjuri
  Cc: Praneeth Bajjuri, Geet Modi, David S. Miller, Heiner Kallweit,
	Russell King, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev,
	linux-kernel, Grygorii Strashko, Dan Murphy

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.

Hi Praneeth

Do you have any suggestions? It was your patched which introduced the
regression. Before we revert it, i want to give you an opportunity to
fix the problem it introduced.

Thanks
	Andrew

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

* Re: DP83867 ethernet PHY regression
  2023-05-22 14:58 DP83867 ethernet PHY regression Francesco Dolcini
  2023-05-22 15:15 ` Andrew Lunn
  2023-05-24 12:13 ` Andrew Lunn
@ 2023-05-24 12:51 ` Bagas Sanjaya
  2023-05-25  6:31   ` Johannes Pointner
  2 siblings, 1 reply; 11+ messages in thread
From: Bagas Sanjaya @ 2023-05-24 12:51 UTC (permalink / raw)
  To: Francesco Dolcini, Andrew Lunn, Praneeth Bajjuri, Geet Modi,
	David S. Miller
  Cc: Heiner Kallweit, Russell King, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel, Grygorii Strashko, Dan Murphy

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

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

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: DP83867 ethernet PHY regression
  2023-05-24 12:51 ` Bagas Sanjaya
@ 2023-05-25  6:31   ` Johannes Pointner
  2023-05-25  7:33     ` Francesco Dolcini
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Pointner @ 2023-05-25  6:31 UTC (permalink / raw)
  To: Francesco Dolcini, Bagas Sanjaya, Andrew Lunn, Praneeth Bajjuri
  Cc: Heiner Kallweit, Russell King, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel, Grygorii Strashko, Dan Murphy

On Wed, May 24, 2023 at 3:22 PM Bagas Sanjaya <bagasdotme@gmail.com> 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@mail.gmail.com/
I also raised the question for the commit da9ef50f545f.
https://lore.kernel.org/lkml/CAHvQdo1U_L=pETmTJXjdzO+k7vNTxMyujn99Y3Ot9xAyQu=atQ@mail.gmail.com/

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.

Regards,
Hannes

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

* Re: DP83867 ethernet PHY regression
  2023-05-25  6:31   ` Johannes Pointner
@ 2023-05-25  7:33     ` Francesco Dolcini
  2023-05-25  8:18       ` Bajjuri, Praneeth
  0 siblings, 1 reply; 11+ messages in thread
From: Francesco Dolcini @ 2023-05-25  7:33 UTC (permalink / raw)
  To: Johannes Pointner
  Cc: Francesco Dolcini, Bagas Sanjaya, Andrew Lunn, Praneeth Bajjuri,
	Heiner Kallweit, Russell King, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel, Grygorii Strashko, Dan Murphy

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@gmail.com> 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@mail.gmail.com/
> I also raised the question for the commit da9ef50f545f.
> https://lore.kernel.org/lkml/CAHvQdo1U_L=pETmTJXjdzO+k7vNTxMyujn99Y3Ot9xAyQu=atQ@mail.gmail.com/
> 
> 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



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

* Re: DP83867 ethernet PHY regression
  2023-05-25  7:33     ` Francesco Dolcini
@ 2023-05-25  8:18       ` Bajjuri, Praneeth
  2023-06-15 20:46         ` Francesco Dolcini
  0 siblings, 1 reply; 11+ messages in thread
From: Bajjuri, Praneeth @ 2023-05-25  8:18 UTC (permalink / raw)
  To: Francesco Dolcini, Johannes Pointner, Siddharth Vadapalli, vikram.sharma
  Cc: Bagas Sanjaya, Andrew Lunn, Heiner Kallweit, Russell King,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel

Andrew,Francesco,

On 5/25/2023 2:33 AM, Francesco Dolcini wrote:
> 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@gmail.com> 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.

Thank you for bringing this issue to attention.
I have looped Siddharth and vikram to further investigate and provide input.


>>>>
>>>> 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@mail.gmail.com/
>> I also raised the question for the commit da9ef50f545f.
>> https://lore.kernel.org/lkml/CAHvQdo1U_L=pETmTJXjdzO+k7vNTxMyujn99Y3Ot9xAyQu=atQ@mail.gmail.com/
>>
>> 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
> 
> 

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

* Re: DP83867 ethernet PHY regression
  2023-05-22 16:15     ` Andrew Lunn
@ 2023-05-25  8:18       ` Alexander Stein
  0 siblings, 0 replies; 11+ messages in thread
From: Alexander Stein @ 2023-05-25  8:18 UTC (permalink / raw)
  To: Francesco Dolcini, Andrew Lunn
  Cc: Praneeth Bajjuri, Geet Modi, David S. Miller, Heiner Kallweit,
	Russell King, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev,
	linux-kernel, Grygorii Strashko, Dan Murphy

Hi,

Am Montag, 22. Mai 2023, 18:15:56 CEST schrieb Andrew Lunn:
> On Mon, May 22, 2023 at 05:35:33PM +0200, Francesco Dolcini wrote:
> > On Mon, May 22, 2023 at 05:15:56PM +0200, Andrew Lunn 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.
> > > 
> > > Do you have more details about how it does not work.
> > > 
> > > Please could you use:
> > > 
> > > mii-tool -vvv ethX
> > 
> > please see the attached files:
> > 
> > working_da9ef50f545f_reverted.txt
> > 
> >   this is on a v6.4-rc, with da9ef50f545f reverted
> > 
> > not_working.txt
> > 
> >   v6.4-rc not working
> > 
> > working.txt
> > 
> >   v6.4-rc working
> > 
> > It looks like, even on cold boot, it's not working in a reliable way.
> > Not sure the exact difference when it's working and when it's not.
> > 
> > Using SIOCGMIIPHY=0x8947
> > eth0: negotiated 1000baseT-FD flow-control, link ok
> > 
> >   registers for MII PHY 0:
> >     1140 796d 2000 a231 05e1 c5e1 006f 2001
> >     5806 0200 3800 0000 0000 4007 0000 3000
> >     5048 ac02 ec10 0004 2bc7 0000 0000 0040
> >     6150 4444 0002 0000 0000 0000 0282 0000
> >     
> >     1140 796d 2000 a231 05e1 c5e1 006d 2001
> >     5806 0200 3800 0000 0000 4007 0000 3000
> >     5048 af02 ec10 0000 2bc7 0000 0000 0040
> >     6150 4444 0002 0000 0000 0000 0282 0000
> 
> Register  6: 006f vs 006d
> Register 17: ac02 vs 1f02
> Register 19: 0004 vs 0000
> 
> Register 6 is MII_EXPANSION. Bit 1 is
> 
> #define EXPANSION_LCWP          0x0002  /* Got new RX page code word   */
> 
> So that is probably not relevant here.
> 
> Register 17 is MII_DP83867_PHYSTS, and bits 8 and 9 are not documented
> in the driver. Do you have the datasheet?

Bit 8 & 9 is indicating the MDI/MDIX resolution status for lines A/B and C/D.

> Register 19 is MII_DP83867_ISR. The interrupt bits are not documented
> in the driver either.

I guess that's the more interesting part. Bit 2 (0x4) indicates a xGMII 
(RGMII/SGMII) error interrupt.

Best regards,
Alexander

> This driver also uses C45 registers, which are not shown here. At some
> point, we might need to look at those. But first it would be good to
> understand what these differences mean.
> 
> 	Andrew


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



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

* Re: DP83867 ethernet PHY regression
  2023-05-25  8:18       ` Bajjuri, Praneeth
@ 2023-06-15 20:46         ` Francesco Dolcini
  0 siblings, 0 replies; 11+ messages in thread
From: Francesco Dolcini @ 2023-06-15 20:46 UTC (permalink / raw)
  To: Bajjuri, Praneeth, Siddharth Vadapalli, vikram.sharma
  Cc: Francesco Dolcini, Johannes Pointner, Bagas Sanjaya, Andrew Lunn,
	Heiner Kallweit, Russell King, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel

Hello Bajjuri, Siddharth and Vikram

On Thu, May 25, 2023 at 03:18:34AM -0500, Bajjuri, Praneeth wrote:
> On 5/25/2023 2:33 AM, Francesco Dolcini wrote:
> > 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@gmail.com> 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.
> 
> Thank you for bringing this issue to attention.
> I have looped Siddharth and vikram to further investigate and provide input.

Were you able to make any progress? If we are not getting anywhere in
the coming days I would be inclined to send a revert, what do you think?

Francesco



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

end of thread, other threads:[~2023-06-15 20:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-22 14:58 DP83867 ethernet PHY regression Francesco Dolcini
2023-05-22 15:15 ` Andrew Lunn
2023-05-22 15:35   ` Francesco Dolcini
2023-05-22 16:15     ` Andrew Lunn
2023-05-25  8:18       ` Alexander Stein
2023-05-24 12:13 ` Andrew Lunn
2023-05-24 12:51 ` Bagas Sanjaya
2023-05-25  6:31   ` Johannes Pointner
2023-05-25  7:33     ` Francesco Dolcini
2023-05-25  8:18       ` Bajjuri, Praneeth
2023-06-15 20:46         ` Francesco Dolcini

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