RE: [PATCH net-next v3 3/5] net: phy: mscc: Drop unnecessary phydev->lock

Biju Das
2026-04-14T18:45:21Z

Hi Russell King,

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 14 April 2026 18:48
> Subject: Re: [PATCH net-next v3 3/5] net: phy: mscc: Drop unnecessary phydev->lock
>
> On Sun, Apr 12, 2026 at 03:00:25PM +0100, Biju wrote:
> > @@ -486,15 +486,9 @@ static int vsc85xx_dt_led_modes_get(struct
> > phy_device *phydev,
> >
> > static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev, u8
> > edge_rate) {
> > - int rc;
> > -
> > - mutex_lock(&phydev->lock);
> > - rc = phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> > - MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
> > - edge_rate << EDGE_RATE_CNTL_POS);
> > - mutex_unlock(&phydev->lock);
> > -
> > - return rc;
> > + return phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_2,
> > + MSCC_PHY_WOL_MAC_CONTROL, EDGE_RATE_CNTL_MASK,
> > + edge_rate << EDGE_RATE_CNTL_POS);
>
> This one is fine.
>
> > @@ -503,7 +497,6 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
> > int rc;
> > u16 reg_val;
> >
> > - mutex_lock(&phydev->lock);
> > reg_val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
> > reg_val &= ~(MAC_IF_SELECTION_MASK);
> > switch (interface) {
> > @@ -522,17 +515,15 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
> > break;
> > default:
> > rc = -EINVAL;
> > - goto out_unlock;
> > + goto err;
> > }
> > rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
>
> I would much rather this was converted to use phy_modify() as well so that we ensure that the update is
> atomic.
>
> rc = phy_modify(phydev, MSCC_PHY_EXT_PHY_CNTL_1,
> MAC_IF_SELECTION_MASK, reg_val);

Agreed, will use phy_modify()

Cheers,
Biju