Re: [PATCH net-next v2 2/4] net: phy: c45: add genphy_c45_config_master_slave()

[Andrew Lunn]

> > +/**
> > + * genphy_c45_config_master_slave - Configure Master/Slave setting for
> > C45 PHYs
> > + * @phydev: target phy_device struct
> > + *
> > + * Description: Configures the Master/Slave manual setting in the
> > + * 10GBASE-T control register (MMD 7, Register 0x0020) according to
> > + * IEEE 802.3 standards.
> > + */
> 
> Please document the return value (0 = unchanged, 1 = changed, <0 errno),
> like
> it is done for genphy_c45_an_config_aneg().
> 
> > +int genphy_c45_config_master_slave(struct phy_device *phydev)

We already have genphy_c45_pma_baset1_setup_master_slave(). This new
function does the same thing, but not for T1 PHYs. It probably should
use a similar name.

> > +{
> > +	u16 val = 0;
> > +
> > +	switch (phydev->master_slave_set) {
> > +	case MASTER_SLAVE_CFG_MASTER_FORCE:
> > +		val = MDIO_AN_10GBT_CTRL_MS_ENABLE | MDIO_AN_10GBT_CTRL_MS_VALUE;
> > +		break;
> > +	case MASTER_SLAVE_CFG_SLAVE_FORCE:
> > +		val = MDIO_AN_10GBT_CTRL_MS_ENABLE;
> > +		break;
> > +	case MASTER_SLAVE_CFG_UNKNOWN:
> > +	case MASTER_SLAVE_CFG_MASTER_PREFERRED:
> > +	case MASTER_SLAVE_CFG_SLAVE_PREFERRED:
> > +	default:
> > +		break;
> > +	}
> 
> Per IEEE 802.3 (45.2.7.10.3), 7.32.13 (port type) is a copy of 9.10 /
> CTL1000_PREFER_MASTER, like it is done in c22's genphy_setup_master_slave().
> 
> So set it for MASTER_PREFERRED, clear it for SLAVE_PREFERRED and leave
> UNKNOWN/UNSUPPORTED
> untouched (return 0). Use -EOPNOTSUPP only for default.

Agreed, making it the same as
genphy_c45_pma_baset1_setup_master_slave().

    Andrew

---
pw-bot: cr