The 11/24/2021 16:04, Russell King (Oracle) wrote:

On Wed, Nov 24, 2021 at 04:43:23PM +0100, Horatiu Vultur wrote:

quoted

quoted

quoted

Actually, port->config.phy_mode will not get zeroed. Because right after
the memset it follows: 'config = port->config'.

Ah, missed that, thanks. However, why should portmode and phy_mode be
different?

Because the serdes knows only few modes(QSGMII, SGMII, GMII) and this
information will come from DT. So I would like to have one variable that
will configure the serdes ('phy_mode') and one will configure the MAC
('portmode').

I don't follow why you need this to be different.

Isn't the point of interfaces such as phy_set_mode_ext() such that we
can achieve independence of the details of what is behind that
interface - so, as it takes a PHY interface mode, if we're operating
in 1000BASE-X, we pass that to phy_set_mode_ext(). It is then the
responsibility of the Serdes PHY driver to decide that means "sgmii"
mode for the Serdes?

I have kept the responsability in the network driver to decide which
interface should for serdes, but I can change that as you suggested.

For example, the Marvell CP110 comphy driver does this:

        if (submode == PHY_INTERFACE_MODE_1000BASEX)
                submode = PHY_INTERFACE_MODE_SGMII;

because the serdes phy settings for PHY_INTERFACE_MODE_1000BASEX are
no different from PHY_INTERFACE_MODE_SGMII - and that detail is hidden
from the network driver.

Yes, I will add a similar check in the serdes driver.

The next question this brings up is... you're setting all the different
interface modes in phylink_config.supported_interfaces, which basically
means you're giving permission for phylink to switch between any of
those modes. So, what if the serdes is in QSGMII mode but phylink
requests SGMII mode. Doesn't your driver architecture mean that if
you're in QSGMII mode you can't use SGMII or GMII mode?

Is there some kind of restriction that you need to split this, or is
this purely down to the way this driver has been written?

It was just the way the driver has been written.

I don't see any other driver in the kernel making this kind of split.

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

-- 
/Horatiu