> On Fri, Nov 08, 2024 at 05:56:33PM -0800, Tristram.Ha@xxxxxxxxxxxxx wrote: > > From: Tristram Ha <tristram.ha@xxxxxxxxxxxxx> > > > > 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.