Messages in this thread | | | Date | Sat, 11 Jun 2022 18:11:57 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH net-next v1 1/1] net: phy: add remote fault support |
| |
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c > index c67bf3060173..6c55c7f9b680 100644 > --- a/drivers/net/phy/phy-c45.c > +++ b/drivers/net/phy/phy-c45.c > @@ -205,7 +205,7 @@ static int genphy_c45_baset1_an_config_aneg(struct phy_device *phydev) > break; > case MASTER_SLAVE_CFG_UNKNOWN: > case MASTER_SLAVE_CFG_UNSUPPORTED: > - return 0; > + break; > default: > phydev_warn(phydev, "Unsupported Master/Slave mode\n"); > return -EOPNOTSUPP; > @@ -223,11 +223,16 @@ static int genphy_c45_baset1_an_config_aneg(struct phy_device *phydev) > break; > } > > + if (phydev->remote_fault_set >= REMOTE_FAULT_CFG_ERROR) > + adv_l |= MDIO_AN_T1_ADV_L_REMOTE_FAULT; > + > adv_l |= linkmode_adv_to_mii_t1_adv_l_t(phydev->advertising); > > ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_T1_ADV_L, > - (MDIO_AN_T1_ADV_L_FORCE_MS | MDIO_AN_T1_ADV_L_PAUSE_CAP > - | MDIO_AN_T1_ADV_L_PAUSE_ASYM), adv_l); > + (MDIO_AN_T1_ADV_L_FORCE_MS | > + MDIO_AN_T1_ADV_L_PAUSE_CAP | > + MDIO_AN_T1_ADV_L_PAUSE_ASYM | > + MDIO_AN_T1_ADV_L_REMOTE_FAULT), adv_l);
Since this is part of config_aneg, i assume you have to trigger an renegotiation, which will put the link down for a while. Is that actually required? Can the fault indicator be set at runtime without a full auto-neg? I suppose for a fault indicator, it probably does not matter, there is a fault... But i'm wondering about future extensions which might want to send values when the link is up. I've seen some PHYs indicate their make/model, etc. What sort of API would be needed for that?
It might also be useful if we could send an event to userspace when the receive state changes, so there is no need to poll. I thought something link a linkstate message was broadcast under some conditions? That again my suggest ksetting is maybe not the best place for this?
I see no problem in exposing this information, but i would like to be sure we get the API correct.
Andrew
| |