| Message ID | 20250203191057.46351-4-gerhard@engleder-embedded.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Support loopback mode speed selection | expand |
On Mon, Feb 03, 2025 at 08:10:53PM +0100, Gerhard Engleder wrote: > +static int ksz9031_set_loopback(struct phy_device *phydev, bool enable, > + int speed) > +{ > + u16 ctl = BMCR_LOOPBACK; > + int ret, val; > + > + if (!enable) > + return genphy_loopback(phydev, enable, 0); > + > + if (speed == SPEED_10 || speed == SPEED_100 || speed == SPEED_1000) > + phydev->speed = speed; > + else if (speed) > + return -EINVAL; > + phydev->duplex = DUPLEX_FULL; > + > + ctl |= mii_bmcr_encode_fixed(phydev->speed, phydev->duplex); > + > + phy_modify(phydev, MII_BMCR, ~0, ctl); Why do you use phy_modify() here rather than phy_write() which is effectively what this is.
On 03.02.25 20:54, Russell King (Oracle) wrote: > On Mon, Feb 03, 2025 at 08:10:53PM +0100, Gerhard Engleder wrote: >> +static int ksz9031_set_loopback(struct phy_device *phydev, bool enable, >> + int speed) >> +{ >> + u16 ctl = BMCR_LOOPBACK; >> + int ret, val; >> + >> + if (!enable) >> + return genphy_loopback(phydev, enable, 0); >> + >> + if (speed == SPEED_10 || speed == SPEED_100 || speed == SPEED_1000) >> + phydev->speed = speed; >> + else if (speed) >> + return -EINVAL; >> + phydev->duplex = DUPLEX_FULL; >> + >> + ctl |= mii_bmcr_encode_fixed(phydev->speed, phydev->duplex); >> + >> + phy_modify(phydev, MII_BMCR, ~0, ctl); > > Why do you use phy_modify() here rather than phy_write() which is > effectively what this is. I copied that from genphy_loopback(). You are right, phy_modify() makes no sense with mask 0xffff. I will change it. Thanks! Gerhard
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index 9c0b1c229af6..f0cecd898312 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -1030,6 +1030,33 @@ static int ksz9021_config_init(struct phy_device *phydev) #define MII_KSZ9031RN_EDPD 0x23 #define MII_KSZ9031RN_EDPD_ENABLE BIT(0) +static int ksz9031_set_loopback(struct phy_device *phydev, bool enable, + int speed) +{ + u16 ctl = BMCR_LOOPBACK; + int ret, val; + + if (!enable) + return genphy_loopback(phydev, enable, 0); + + if (speed == SPEED_10 || speed == SPEED_100 || speed == SPEED_1000) + phydev->speed = speed; + else if (speed) + return -EINVAL; + phydev->duplex = DUPLEX_FULL; + + ctl |= mii_bmcr_encode_fixed(phydev->speed, phydev->duplex); + + phy_modify(phydev, MII_BMCR, ~0, ctl); + + ret = phy_read_poll_timeout(phydev, MII_BMSR, val, val & BMSR_LSTATUS, + 5000, 500000, true); + if (ret) + return ret; + + return 0; +} + static int ksz9031_of_load_skew_values(struct phy_device *phydev, const struct device_node *of_node, u16 reg, size_t field_sz, @@ -5564,6 +5591,7 @@ static struct phy_driver ksphy_driver[] = { .resume = kszphy_resume, .cable_test_start = ksz9x31_cable_test_start, .cable_test_get_status = ksz9x31_cable_test_get_status, + .set_loopback = ksz9031_set_loopback, }, { .phy_id = PHY_ID_LAN8814, .phy_id_mask = MICREL_PHY_ID_MASK,
The KSZ9031 PHYs requires full duplex for loopback mode. Add PHY specific set_loopback() to ensure this. Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> --- drivers/net/phy/micrel.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)