netdev - Re: [PATCH net-next v9 0/6] add ethernet driver for Tehuti Networks TN40xx chips

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>] [day] [month] [year] [list]

Date: Sun, 9 Jun 2024 20:12:42 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Hans-Frieder Vogt <hfdevel@....net>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
	andrew@...n.ch, horms@...nel.org, kuba@...nel.org, jiri@...nulli.us,
	pabeni@...hat.com, naveenm@...vell.com, jdamato@...tly.com
Subject: Re: [PATCH net-next v9 0/6] add ethernet driver for Tehuti Networks
 TN40xx chips

On Sun, Jun 09, 2024 at 08:36:42PM +0200, Hans-Frieder Vogt wrote:
> On 09.06.2024 15.48, Russell King (Oracle) wrote:
> 
> > On Sun, Jun 09, 2024 at 02:40:03PM +0200, Hans-Frieder Vogt wrote:
> > > --- a/drivers/net/ethernet/tehuti/tn40_phy.c    2024-06-06
> > > 06:43:40.865474664 +0200
> > > +++ b/drivers/net/ethernet/tehuti/tn40_phy.c    2024-06-06
> > > 18:57:01.978776712 +0200
> > > @@ -54,6 +54,8 @@ int tn40_phy_register(struct tn40_priv *
> > >                  return -1;
> > >          }
> > > 
> > > +       __set_bit(PHY_INTERFACE_MODE_XAUI, phydev->host_interfaces);
> > > +
> > >          config = &priv->phylink_config;
> > >          config->dev = &priv->ndev->dev;
> > >          config->type = PHYLINK_NETDEV;
> > This shouldn't be done - host_interfaces is really only for SFPs, and
> > it suggests that the 88x3310 isn't properly configured with pinstrapping
> > for the correct MAC type (which determines the interface mode to be used
> > to communicate with the MAC.)
> I already wondered why host_interfaces was used in the 88x3310, but not
> in the aqr105 phy driver. Makes sense because the 88x3310 supports both
> directly BASE-T and an SFP+ cage.
> > I'm not sure what to suggest here, other than further debug (e.g. what
> > interface mode is the 88x3310 trying to use without this?)
> 
> The message is:
> tn40xx 0000:10:00.0 enp16s0: PHY has no common interfaces

So... the 88x3310 supports SGMII, 2500BASE-X, 5GBASE-R, XAUI, RXAUI,
10GBASE-R and USXGMII on its host side interface.

When the phy is attached, the config_init method in the PHY driver
will be called, and it will fill in phydev->possible_interfaces to
reflect the interface modes that the PHY will _actually_ be using.

Phylink will notice phydev->possible_interfaces being non-empty and
check whether the union of the set of PHY possible_interfaces and
the set of MAC supported_interfaces is non-empty. If it's empty,
then the above message will be issued.

This suggests, as mentioned earlier, that the operating mode of the
88x3310 PHY doesn't match what this MAC can support, which makes me
wonder about the pinstrapping options for the 88x3310 on this
hardware. My guess is someone found a hardware design error and
decided "software can sort this out for us!"

> tn40xx 0000:10:00.0 enp16s0: validation of xaui with support
> 00,00000000,00018000,0000706f and advertisement
> 00,00000000,00018000,0000706f failed: -22

Basically, the PHY isn't operating in XAUI mode.

> You are probably right that the interfaceis not properly pinstrapped.
> bits 2:0 in 1f.f001 are initially 0, which means RXAUI, and the vendor
> driver just forces the bits to 1 (XAUI with rate matching). Maybe it is
> a quirk (or a design flaw of the Tehuti reference card). Just a thought:
> if it cannot be autodetected then maybe the simplest solution is adding
> a module parameter.

It may be that the best option is to set phydev->host_interfaces, but
I would like to see a comment giving details about why this is
necessary - essentially covering the information above please. It's
making use of that outside its original purpose, and I would like
such uses well documented so if stuff needs to change, we know what
and why these drivers are making use of it.

> I followed this discussion, but was hoping that the situation would change.
> I think I will give the card with AQR105 phy priority, assuming that the
> firmware topic is easier to solve for the Aquantia line of Marvell products.

Fingers crossed!

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!