netdev - Re: [PATCH net-next v4 6/6] net: tn40xx: add PHYLIB support

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: <a7c9f272-9c5c-4aba-b7db-ae62e9cc8d0a@lunn.ch>
Date: Thu, 9 May 2024 15:37:55 +0200
From: Andrew Lunn <andrew@...n.ch>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, kuba@...nel.org, jiri@...nulli.us,
	horms@...nel.org
Subject: Re: [PATCH net-next v4 6/6] net: tn40xx: add PHYLIB support

On Thu, May 09, 2024 at 01:23:41PM +0900, FUJITA Tomonori wrote:
> Hi,
> 
> On Wed, 08 May 2024 22:18:51 +0900 (JST)
> FUJITA Tomonori <fujita.tomonori@...il.com> wrote:
> 
> >>>  		priv->link = 0;
> >>>  		if (priv->link_loop_cnt++ > TN40_LINK_LOOP_MAX) {
> >>>  			/* MAC reset */
> >>>  			tn40_set_link_speed(priv, 0);
> >>> +			tn40_set_link_speed(priv, priv->speed);
> >>>  			priv->link_loop_cnt = 0;
> >> 
> >> This should move into the link_down callback.
> > 
> > I'll try phylink callbacks to see if they would work. 
> 
> I found that the link_down callback doesn't work well for the MAC
> reset above.
> 
> Currently, when TN40_REG_MAC_LNK_STAT register tells that the link is
> off, the driver configures the MAC to generate an interrupt
> periodically; tn40_write_reg(priv, 0x5150, 1000000) is called in
> tn40_link_changed().
> 
> Eventually, the counter is over TN40_LINK_LOOP_MAX and then the driver
> executes the MAC reset. Without the MAC reset, the NIC will not work.
> 
> The link_down callback is called only when the link becomes down so it
> can't be used to trigger the MAC reset.

So this sounds like a hardware bug workaround.

But it might also be to do with auto-neg. The MAC PCS/SERDES and the
PHY PCS/SERDES, depending on the mode, should be performing auto-neg,
to indicate things like pause. For some hardware, you need to restart
autoneg when the line sides gets link. It could be this hardware has
no way to do that, other than hit the whole thing with a reset?

Take a look at struct phylink_pcs_ops and see if you can map bits of
the driver to this structure. It might be you can implement a PCS, and
have the pcs_an_restart do the MAC reset.

     Andrew