netdev - RE: [PATCH net-next 2/2] net: dsa: microchip: Add SGMII port support to KSZ9477 switch

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:
 <DM3PR11MB87360F9E39097E535416838EEC592@DM3PR11MB8736.namprd11.prod.outlook.com>
Date: Tue, 12 Nov 2024 02:55:29 +0000
From: <Tristram.Ha@...rochip.com>
To: <andrew@...n.ch>
CC: <Woojung.Huh@...rochip.com>, <olteanv@...il.com>, <robh@...nel.org>,
	<krzk+dt@...nel.org>, <conor+dt@...nel.org>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<marex@...x.de>, <UNGLinuxDriver@...rochip.com>,
	<devicetree@...r.kernel.org>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next 2/2] net: dsa: microchip: Add SGMII port support
 to KSZ9477 switch

> On Fri, Nov 08, 2024 at 05:56:33PM -0800, Tristram.Ha@...rochip.com wrote:
> > From: Tristram Ha <tristram.ha@...rochip.com>
> >
> > The SGMII module of KSZ9477 switch can be setup in 3 ways: 0 for direct
> > connect, 1 for 1000BaseT/1000BaseX SFP, and 2 for 10/100/1000BaseT SFP.
> 
> This naming is rather odd. First off, i would drop 'SFP'. It does not
> have to be an SFP on the other end, it could be another switch for
> example. 1 is PHY_INTERFACE_MODE_1000BASEX and 2 is
> PHY_INTERFACE_MODE_SGMII.
> 
> > SFP is typically used so the default is 1.  The driver can detect
> > 10/100/1000BaseT SFP and change the mode to 2.
> 
> phylink will tell you want mode to use. I would ignore what the
> hardware detects, so this driver is just the same as every other
> driver, making it easier to maintain.

There are some issues I found that will need your advises.

The phylink SFP code categorizes SFP using fiber cable as
PHY_INTERFACE_MODE_1000BASEX and SFP using a regular RJ45 connector as 
PHY_INTERFACE_MODE_SGMII, which has a PHY that can be accessed through
I2C connection with a PHY driver.  Now when SGMII SFP is used the phylink
cannot be created because it fails the validation in
phylink_sfp_config_phy().  The reason is the phydev has empty supported
and advertising data fields as it is just created.  Even if the provided
PCS driver can change the supported field to pass the initial validation
it cannot change the advertising field as it is readonly to the driver,
and this fails the second validation with phylink_sfp_select_interface().
This problem is so obvious I thought I must be doing something wrong.
This problem occurs in Linux 6.1 so it is not a new problem and nobody
encountered it?

This problem does not happen with SFP using fiber cable as a different
function is used to initialize the phylink.  As the SFP code already
retrieves the basic support information from the SFP EEPROM and stores
it in sfp_support it can be copied to supported and advertising to pass
the validation.

I mentioned the SGMII module operates differently for two types of SFP:
SGMII and 1000BASEX.  The 1000Base-T SFP operates the same as 1000Base-SX
fiber SFP, and the driver would like it to be assigned
PHY_INTERFACE_MODE_1000BASEX, but it is always assigned
PHY_INTERFACE_MODE_SGMII in sfp_select_interface because 1000baseT_Full
is compared before 1000baseX_Full.

Now I am not sure if those SFPs I tested have correct EEPROM.  Some
no-brand ones return 0xff value when the PHY driver reads the link status
from them and so that driver cannot tell when the link is down.  Other
than that those SFPs operate correctly in forwarding traffic.

It seems there is no way to assign an interupt to that PHY and so polling
is always used.

The SFP using fiber cable does not have the above issues but has its own
issue.  The SFP cage can detect the cable is being plugged in as the
light is detected.  The PCS driver is then consulted to confirm the link.
However as the cable is not completely plugged in the driver can report
the link is not up.  After two calls are done the port can be left into
unconnected state forever even after the cable is plugged in.  The driver
can detect there is something different about the link value and can try
to read it later to get a firm reading.  This just means the driver needs
to poll anyway.  But if interrupt is used and the driver uses it to
report link this issue is moot.

I just feel the SFP code offered in the phylink model does not help to
operate SFP well in the KSZ9477 switch, and it does not need that code to
provide basic SGMII port connection.