> > +/* switch line-side interface between 10GBase-R and 1GBase-X
> > + * according to speed */
> > +static void mv2222_update_interface(struct phy_device *phydev)
> > +{
> > + struct mv2222_data *priv = phydev->priv;
> > +
> > + if (phydev->speed == SPEED_10000 &&
> > + priv->line_interface == PHY_INTERFACE_MODE_1000BASEX) {
> > + priv->line_interface = PHY_INTERFACE_MODE_10GBASER;
> > +
> > + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > + MV_PCS_HOST_XAUI | MV_PCS_LINE_10GBR);
> > + mv2222_soft_reset(phydev);
> > + }
> > +
> > + if (phydev->speed == SPEED_1000 &&
> > + priv->line_interface == PHY_INTERFACE_MODE_10GBASER) {
> > + priv->line_interface = PHY_INTERFACE_MODE_1000BASEX;
> > +
> > + phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_PCS_CONFIG,
> > + MV_PCS_HOST_XAUI | MV_PCS_LINE_1GBX_AN);
> > + mv2222_soft_reset(phydev);
> > + }
>
> Wouldn't it be better to have a single function to set the line
> interface, used by both this function and your sfp_module_insert
> function? I'm thinking something like:
>
> static int mv2222_set_line_interface(struct phy_device *phydev,
> phy_interface_t line_interface)
> {
> ...
> }
>
> and calling that from both these locations to configure the PHY for
> 10GBASE-R, 1000BASE-X and SGMII modes.
Agreed. This got me confused, wondering where the SGMII handling was.
Andrew
