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:

quoted

On Sun, Jun 09, 2024 at 02:40:03PM +0200, Hans-Frieder Vogt wrote:

quoted

--- 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.

quoted

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!