Re: [PATCH net-next v6 5/5] net: phy: add driver for built-in 2.5G ethernet PHY on MT7988

[Daniel Golle]

On Mon, Jun 03, 2024 at 02:41:44PM +0100, Russell King (Oracle) wrote:
> On Mon, Jun 03, 2024 at 02:31:46PM +0100, Daniel Golle wrote:
> > On Mon, Jun 03, 2024 at 02:25:01PM +0100, Russell King (Oracle) wrote:
> > > On Mon, Jun 03, 2024 at 08:18:34PM +0800, Sky Huang wrote:
> > > > Add support for internal 2.5Gphy on MT7988. This driver will load
> > > > necessary firmware, add appropriate time delay and figure out LED.
> > > > Also, certain control registers will be set to fix link-up issues.
> > > 
> > > Based on our previous discussion, it may be worth checking in the
> > > .config_init() method whether phydev->interface is one of the
> > > PHY interface modes that this PHY supports. As I understand from one
> > > of your previous emails, the possibilities are XGMII, USXGMII or
> > > INTERNAL. Thus:
> > > 
> > > > +static int mt798x_2p5ge_phy_config_init(struct phy_device *phydev)
> > > > +{
> > > > +	struct pinctrl *pinctrl;
> > > > +	int ret;
> > > 
> > > 	/* Check that the PHY interface type is compatible */
> > > 	if (phydev->interface != PHY_INTERFACE_MODE_INTERNAL &&
> > > 	    phydev->interface != PHY_INTERFACE_MODE_XGMII &&
> > > 	    phydev->interface != PHY_INTERFACE_MODE_USXGMII)
> > > 		return -ENODEV;
> > 
> > The PHY is built-into the SoC, and as such the connection type should
> > always be "internal". The PHY does not exist as dedicated IC, only
> > as built-in part of the MT7988 SoC.
> 
> That's not how it was described to me by Sky.
> 
> If what you say is correct, then the implementation of
> mt798x_2p5ge_phy_get_rate_matching() which checks for interface modes
> other than INTERNAL is not correct. Also it means that config_init()
> should not permit anything but INTERNAL.

The way the PHY is connected to the MAC *inside the chip* is XGMII
according the MediaTek. So call it "internal" or "xgmii", however, up to
my knowledge it's a fact that there is **only one way** this PHY is
connected and used, and that is being an internal part of the MT7988 SoC.

Imho, as there are no actual XGMII signals exposed anywhere I'd use
"internal" to describe the link between MAC and PHY (which are both
inside the same chip package).