netdev - Re: [PATCH net-next v8 08/11] net: stmmac: dwmac-loongson: Fix MAC speed for GNET

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9e985b73-9ac6-4bc2-b102-18c0891a0d9b@loongson.cn>
Date: Thu, 28 Mar 2024 18:08:30 +0800
From: Yanteng Si <siyanteng@...ngson.cn>
To: Andrew Lunn <andrew@...n.ch>
Cc: Serge Semin <fancer.lancer@...il.com>, hkallweit1@...il.com,
 peppe.cavallaro@...com, alexandre.torgue@...s.st.com, joabreu@...opsys.com,
 Jose.Abreu@...opsys.com, chenhuacai@...ngson.cn, linux@...linux.org.uk,
 guyinggang@...ngson.cn, netdev@...r.kernel.org, chris.chenfeiyang@...il.com
Subject: Re: [PATCH net-next v8 08/11] net: stmmac: dwmac-loongson: Fix MAC
 speed for GNET

在 2024/3/27 20:47, Andrew Lunn 写道:
> On Wed, Mar 27, 2024 at 10:41:57AM +0800, Yanteng Si wrote:
>> 在 2024/3/26 20:21, Andrew Lunn 写道:
>>> On Tue, Mar 26, 2024 at 08:02:55PM +0800, Yanteng Si wrote:
>>>> 在 2024/3/21 23:02, Andrew Lunn 写道:
>>>>>> When switching speeds (from 100M to 1000M), the phy cannot output clocks,
>>>>>>
>>>>>> resulting in the unavailability of the network card.  At this time, a reset
>>>>>> of the
>>>>>>
>>>>>> phy is required.
>>>>> reset, or restart of autoneg?
>>>> reset.
>>> If you need a reset, why are you asking it to restart auto-neg?
>> Autoneg was discussed in patch v1, but we may have misunderstood the
>> description from our hardware engineers at the time. The root cause is that
>> there is an error in the connection between the MAC and PHY. After repeated
>> tests, we have found that
>>
>> auto-negcannot solve all problems and can only be reset. Thanks, Yanteng
>   
> So calling phylink_ethtool_nway_reset() does not fix your problem, and
> you need some other fix.
>
Sorry, it's my fault.  I was stupid. Let me explain the whole story:

First, here is the original code we got:

  if (speed == SPEED_1000) {
   if (readl(ptr->ioaddr + MAC_CTRL_REG) & (1 << 15) /* PS */) {
    /* reset phy */
    phy_set_bits(ndev->phydev, 0 /*MII_BMCR*/, 0x200 /*BMCR_ANRESTART*/);

Although the comment in the code is **reset phy**, the actual operation
is to modify the autoneg bit. This method fixes all problems.

V1:
if (phydev->speed == SPEED_1000)
  phydev->autoneg = AUTONEG_ENABLE

After discussing with you, see:
<https://lore.kernel.org/loongarch/be1874e517f4f4cc50906f18689a0add3594c2e0.1689215889.git.chenfeiyang@loongson.cn/>

v2 -> v8:
if (speed == SPEED_1000)
  if (readl(ptr->ioaddr + MAC_CTRL_REG) & (1 << 15) /* PS */)
   phy_restart_aneg(ndev->phydev);

Subsequent versions have been using this method. In recent days, I 
accidentally
conducted an erroneous test, possibly due to the network cable not being
firmly connected, leading me to mistakenly believe that this did not fix all
problems. After conducting sufficient testing today, I found that I was 
mistaken
and that calling phy_restart_aneg() directly fixed all problems.

At your suggestion, calling phylink_et Athletic_nway_reset() also works 
well, but
it will report an assertion failed at drivers/net/phy/phylink.c in the 
dmesg.
So let's continue to call phy_restart_aneg()?

Finally, why did I reply you with reset?  It's because I first checked 
the original
code, and the comment in the original code was /* reset phy */.  I 
thought it was
reset, but today I found out that it was actually autoneg. I apologize 
for my

carelessness.

Thanks,

Yanteng