On Tue, Nov 12, 2024 at 02:55:29AM +0000, Tristram.Ha@microchip.com wrote:

quoted

On Fri, Nov 08, 2024 at 05:56:33PM -0800, Tristram.Ha@microchip.com wrote:

quoted

From: Tristram Ha <redacted>

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.

quoted

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.

Not quite correct, i think. If MDIO over I2C does not work, it will
still decide on 1000BaseX vs SGMII from the SFP eeprom contents. There
are some SFPs where the PHY is not accessible, and we have to live
with however it is configured.

Now when SGMII SFP is used the phylink
cannot be created because it fails the validation in
phylink_sfp_config_phy().

Please stop using 'SGMII SFP'. It should just be SGMII. The MAC should
not care what is on the other end, it could be a PHY, and SFP, or a
switch, all using Cisco SGMII.

The reason is the phydev has empty supported
and advertising data fields as it is just created.

Do you mean the phydev for the PHY in the SFP? Or do you have a second
phydev here? I'm confused.

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.

There is no standardisation of how you access the PHY in an SFP. So
each manufacture can do their own thing. However, there are a small
number of PHYs actually used inside SFPs, and we have support for
those common ones.

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

Correct, the interface between the SFP and the SFP cage does not have
an interrupt pin the PHY can use.

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.

Have you set pcs.poll? phylink will then poll the PCS every
second. You can report PCS status any time.

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.

We need to understand what is different about the KSZ9477 switch that
phylink does not work for it, yet phylink does work for mv88e6xxx,
sja1105, rzn1, qca, ocelot, and other MAC drivers.

	Andrew