Messages in this thread | | | Subject | Re: [PATCH v3 03/13] net: phy: at803x: improve the WOL feature | From | Jie Luo <> | Date | Tue, 19 Oct 2021 19:46:33 +0800 |
| |
On 10/19/2021 2:41 AM, Andrew Lunn wrote: >> @@ -348,18 +349,29 @@ static int at803x_set_wol(struct phy_device *phydev, >> phy_write_mmd(phydev, MDIO_MMD_PCS, offsets[i], >> mac[(i * 2) + 1] | (mac[(i * 2)] << 8)); >> >> + /* Enable WOL function */ >> + ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, AT803X_PHY_MMD3_WOL_CTRL, >> + 0, AT803X_WOL_EN); >> + if (ret) >> + return ret; >> + /* Enable WOL interrupt */ >> ret = phy_modify(phydev, AT803X_INTR_ENABLE, 0, AT803X_INTR_ENABLE_WOL); >> if (ret) >> return ret; >> - value = phy_read(phydev, AT803X_INTR_STATUS); >> } else { >> + /* Disable WoL function */ >> + ret = phy_modify_mmd(phydev, MDIO_MMD_PCS, AT803X_PHY_MMD3_WOL_CTRL, >> + AT803X_WOL_EN, 0); >> + if (ret) >> + return ret; >> + /* Disable WOL interrupt */ >> ret = phy_modify(phydev, AT803X_INTR_ENABLE, AT803X_INTR_ENABLE_WOL, 0); >> if (ret) >> return ret; >> - value = phy_read(phydev, AT803X_INTR_STATUS); >> } >> >> - return ret; >> + /* Clear WOL status */ >> + return phy_read(phydev, AT803X_INTR_STATUS); > It looks like you could be clearing other interrupt bits which have > not been serviced yet. Is it possible to clear just WoL?
Hi Andrew,
when this register AT803X_INTR_STATUS bits are cleared after read, we can't clear only WOL interrupt here.
> > Also, you are returning the contents of the interrupt status register? > You should probably be returning 0 if the read was successful. thanks for this comments, will correct it in the next patch set. > > Andrew
| |