netdev - Re: [PATCH net-next] net: tn40xx: add initial ethtool

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: <20240703.215728.957684876602887757.fujita.tomonori@gmail.com>
Date: Wed, 03 Jul 2024 21:57:28 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: linux@...linux.org.uk
Cc: andrew@...n.ch, fujita.tomonori@...il.com, netdev@...r.kernel.org,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com
Subject: Re: [PATCH net-next] net: tn40xx: add initial ethtool_ops support

On Fri, 28 Jun 2024 16:45:01 +0100
"Russell King (Oracle)" <linux@...linux.org.uk> wrote:

>> > +static int tn40_ethtool_get_link_ksettings(struct net_device *ndev,
>> > +					   struct ethtool_link_ksettings *cmd)
>> > +{
>> > +	struct tn40_priv *priv = netdev_priv(ndev);
>> > +
>> > +	return phylink_ethtool_ksettings_get(priv->phylink, cmd);
>> > +}
>> 
>> Have you tried implementing tn40_ethtool_set_link_ksettings() in the
>> same way?
> 
> I did think about commenting on that, and the [sg]et_pauseparam
> methods as well, but when one realises that the driver only supports
> one speed and duplex (10G FD) but no pause it didn't seem to make
> sense.

I have not been able to find register configuration to make
1000Base-SX work with QT2025 PHY.

> Not having pause effectively rules out pause-frame rate adaption
> by the PHY, so the PHY probably only supports 10G link speeds,
> and if I remember correctly, 10GBASE-T requires autoneg.

I suppose that the HW supports pause because a register named
regPAUSE_QUANT is set up during initialization. But the original
driver doesn't support [sg]et_pauseparam or give the details.

> The autonegotiation specification was improved in the 1998 release of
> IEEE 802.3. This was followed by the release of the IEEE 802.3ab Gigabit
> Ethernet standard in 1999 which specified mandatory autonegotiation for
> 1000BASE-T. Autonegotiation is also mandatory for 1000BASE-TX and
> 10GBASE-T implementations.
> 
> which is loose language - "mandatory autonegotiation" does that refer
> to support for auto-negotiation or require auto-negotiation to be
> always enabled?
> 
> We're already seeing some PHYs from some manufacturers that seem to be
> following the "require auto-negotiation to be always enabled".
> 
> So why have I gone down what seems to be an unrelated rabbit hole?
> 
> If tn40 is connected to a 10GBASE-T PHY, implementing the
> set_link_ksettings() method would give the user control over whether AN
> is used on the media side.
>
> If 802.3 requires AN to be supported but is not necessarily enabled,
> then there is use in exposing the set_link_ksettings() method.
> 
> If 802.3 requires AN to be supproted and always enabled, then
> implementing set_link_ksettings() in this case would not provide any
> value.

I've read your comment on stmmac patch:

https://lore.kernel.org/netdev/ZoQX1bqtJI2Zd9qH@shell.armlinux.org.uk/

When implementing BASE-T PHY support for tn40 driver, I'll check the
situation again.

Thanks a lot!