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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <ee49cb12-2116-4f0d-8265-cd1c42b6037b@iscas.ac.cn>
Date: Fri, 31 Oct 2025 15:22:56 +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 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?

AFAICT from my testing, forcing link speed and duplex from ethtool still
works, so I'm not sure what I'm missing here.

> I don't see all these being handled. It gets much easier to get this
> right if you make use of phylink, since phylink handles all the
> business logic for you.

I'll look into calling phylink next version. Thanks.

Vivian "dramforever" Wang