netdev - Re: [PATCH net] net: spacemit: Implement emac_set

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 for Android: free password hash cracker in your pocket

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

Message-ID: <729fc508-0682-41b0-8582-b1388f31e08d@iscas.ac.cn>
Date: Fri, 31 Oct 2025 21:29:23 +0800
From: Vivian Wang <wangruikang@...as.ac.cn>
To: Andrew Lunn <andrew@...n.ch>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Yixun Lan <dlan@...too.org>,
 Maxime Chevallier <maxime.chevallier@...tlin.com>,
 Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
 Troy Mitchell <troy.mitchell@...ux.spacemit.com>, netdev@...r.kernel.org,
 linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: spacemit: Implement emac_set_pauseparam properly

On 10/31/25 20:43, Andrew Lunn wrote:
> On Fri, Oct 31, 2025 at 03:22:56PM +0800, Vivian Wang wrote:
>> On 10/31/25 05:32, Andrew Lunn wrote:
>>>> [...]
>>>>
>>>> -		emac_set_fc(priv, fc);
>>>> -	}
>>>> +	phy_set_asym_pause(dev->phydev, pause->rx_pause, pause->tx_pause);
>>> It is hard to read what this patch is doing, but there are 3 use cases.
>> Yeah, I guess the patch doesn't look great. I'll reorganize it in the
>> next version to make it clearer what the new implementation is and also
>> fix it up per your other comments.
>>
>>> 1) general autoneg for link speed etc, and pause autoneg
>>> 2) general autoneg for link speed etc, and forced pause
>>> 3) forced link speed etc, and forced pause.
>> Thanks for the tip on the different cases. However, there's one bit I
>> don't really understand: Isn't this set_pauseparam thing only for
>> setting pause autoneg / force?
> Nope. You need to think about how it interacts with generic autoneg.
>
>        ethtool -A|--pause devname [autoneg on|off] [rx on|off] [tx on|off]
>
>        ethtool -s devname [speed N] [lanes N] [duplex half|full]
>               [port tp|aui|bnc|mii] [mdix auto|on|off] [autoneg on|off]
>
> These autoneg are different things. -s is about generic autoneg,
> speed, duplex, etc. However pause can also be negotiated, or not,
> using -A.
>
> You can only autoneg pause if you are doing generic autoneg. So there
> are three combinations you need to handle.

Oh, that is what I had missed. I hadn't understood this part before. Thanks.

> With pause autoneg off, you can set registers in the MAC immediately,
> but you need to be careful not to overwrite the values when generic
> autoneg completes and the adjust_link callback is called.
>
> If you have pause autoneg on, you have to wait for the adjust_link
> callback to be called with the results of the negotiation, including
> pause.
>
> phylink hides all this logic. There is a link_up callback, which tells
> you how to program the hardware. You just do it, no logic needed.

This makes sense. I'll look into using phylink.

Thanks,
Vivian "dramforever" Wang