The wol feature is controlled by the MMD3.8012 bit5,
need to set this bit when the wol function is enabled.
The reg18 bit0 is for enabling WOL interrupt, when wol
occurs, the wol interrupt status reg19 bit0 is set to 1.
Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/phy/at803x.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c5e145ff5ec8..2f7d96bd1be8 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -70,6 +70,8 @@
#define AT803X_CDT_STATUS_DELTA_TIME_MASK GENMASK(7, 0)
#define AT803X_LED_CONTROL 0x18
+#define AT803X_PHY_MMD3_WOL_CTRL 0x8012
+#define AT803X_WOL_EN BIT(5)
#define AT803X_LOC_MAC_ADDR_0_15_OFFSET 0x804C
#define AT803X_LOC_MAC_ADDR_16_31_OFFSET 0x804B
#define AT803X_LOC_MAC_ADDR_32_47_OFFSET 0x804A
@@ -327,7 +329,6 @@ static int at803x_set_wol(struct phy_device *phydev,
struct net_device *ndev = phydev->attached_dev;
const u8 *mac;
int ret;
- u32 value;
unsigned int i;
const unsigned int offsets[] = {
AT803X_LOC_MAC_ADDR_32_47_OFFSET,
@@ -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);
}
static void at803x_get_wol(struct phy_device *phydev,
@@ -370,8 +382,11 @@ static void at803x_get_wol(struct phy_device *phydev,
wol->supported = WAKE_MAGIC;
wol->wolopts = 0;
- value = phy_read(phydev, AT803X_INTR_ENABLE);
- if (value & AT803X_INTR_ENABLE_WOL)
+ value = phy_read_mmd(phydev, MDIO_MMD_PCS, AT803X_PHY_MMD3_WOL_CTRL);
+ if (value < 0)
+ return;
+
+ if (value & AT803X_WOL_EN)
wol->wolopts |= WAKE_MAGIC;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
> @@ -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?
Also, you are returning the contents of the interrupt status register?
You should probably be returning 0 if the read was successful.
Andrew
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
> Hi Andrew,
>
> when this register AT803X_INTR_STATUS bits are cleared after read, we can't
> clear only WOL interrupt here.
O.K. But you do have the value of the interrupt status register. So
you could call phy_trigger_machine(phydev) if there are any other
interrupt pending. They won't get lost that way.
Andrew
On 10/19/2021 8:29 PM, Andrew Lunn wrote:
>> Hi Andrew,
>>
>> when this register AT803X_INTR_STATUS bits are cleared after read, we can't
>> clear only WOL interrupt here.
> O.K. But you do have the value of the interrupt status register. So
> you could call phy_trigger_machine(phydev) if there are any other
> interrupt pending. They won't get lost that way.
>
> Andrew
This make sense, thanks for this comment, will add it in the next patch set.