Messages in this thread Patches in this message | | | Subject | Re: [PATCH] net: phy: marvell: clear wol event before setting it | From | Bhadram Varka <> | Date | Thu, 26 Apr 2018 12:39:59 +0530 |
| |
Hi,
On 4/26/2018 11:56 AM, Bhadram Varka wrote: > Hi, > On 4/26/2018 11:45 AM, Jisheng Zhang wrote: >> Hi, >> >> On Thu, 26 Apr 2018 11:10:21 +0530 Bhadram Varka wrote: >> >>> Hi, >>> >>> On 4/19/2018 5:48 PM, Andrew Lunn wrote: >>>> On Thu, Apr 19, 2018 at 04:02:32PM +0800, Jisheng Zhang wrote: >>>>> From: Jingju Hou <Jingju.Hou@synaptics.com> >>>>> >>>>> If WOL event happened once, the LED[2] interrupt pin will not be >>>>> cleared unless reading the CSISR register. So clear the WOL event >>>>> before enabling it. >>>>> >>>>> Signed-off-by: Jingju Hou <Jingju.Hou@synaptics.com> >>>>> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com> >>>>> --- >>>>> drivers/net/phy/marvell.c | 9 +++++++++ >>>>> 1 file changed, 9 insertions(+) >>>>> >>>>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c >>>>> index c22e8e383247..b6abe1cbc84b 100644 >>>>> --- a/drivers/net/phy/marvell.c >>>>> +++ b/drivers/net/phy/marvell.c >>>>> @@ -115,6 +115,9 @@ >>>>> /* WOL Event Interrupt Enable */ >>>>> #define MII_88E1318S_PHY_CSIER_WOL_EIE BIT(7) >>>>> +/* Copper Specific Interrupt Status Register */ >>>>> +#define MII_88E1318S_PHY_CSISR 0x13 >>>>> + >>>>> /* LED Timer Control Register */ >>>>> #define MII_88E1318S_PHY_LED_TCR 0x12 >>>>> #define MII_88E1318S_PHY_LED_TCR_FORCE_INT BIT(15) >>>>> @@ -1393,6 +1396,12 @@ static int m88e1318_set_wol(struct >>>>> phy_device *phydev, >>>>> if (err < 0) >>>>> goto error; >>>>> + /* If WOL event happened once, the LED[2] interrupt pin >>>>> + * will not be cleared unless reading the CSISR register. >>>>> + * So clear the WOL event first before enabling it. >>>>> + */ >>>>> + phy_read(phydev, MII_88E1318S_PHY_CSISR); >>>>> + >>>> Hi Jisheng >>>> >>>> The problem with this is, you could be clearing a real interrupt, link >>>> down/up etc. If interrupts are in use, i think the normal interrupt >>>> handling will clear the WOL interrupt? So can you make this read >>>> conditional on !phy_interrupt_is_valid()? >>> So this will clear WoL interrupt bit from Copper Interrupt status >>> register. >>> >>> How about clearing WoL status (Page 17, register 17) for every WOL >>> event ? >>> >> This is already properly done by setting >> MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS >> in m88e1318_set_wol() > This part of the code executes only when we enable WOL through ethtool > (ethtool -s eth0 wol g) > > Lets say once WOL enabled through magic packet - HW generates WOL > interrupt once magic packet received. > The problem that I see here is that for the next immediate magic > packet I don't see WOL interrupt generated by the HW. > I need to explicitly clear WOL status for HW to generate WOL interrupt.
With the below patch I see WOL event interrupt for every magic packet that HW receives...
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index ed8a67d..5d3d138 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -55,6 +55,7 @@
#define MII_M1011_IEVENT 0x13 #define MII_M1011_IEVENT_CLEAR 0x0000 +#define MII_M1011_IEVENT_WOL_EVENT BIT(7)
#define MII_M1011_IMASK 0x12 - #define MII_M1011_IMASK_INIT 0x6400 + #define MII_M1011_IMASK_INIT 0x6480 @@ -195,13 +196,40 @@ struct marvell_priv { bool copper; };
+static int marvell_clear_wol_status(struct phy_device *phydev) +{ + int err, temp, oldpage; + + oldpage = phy_read(phydev, MII_MARVELL_PHY_PAGE); + if (oldpage < 0) + return oldpage; + + err = phy_write(phydev, MII_MARVELL_PHY_PAGE, + MII_88E1318S_PHY_WOL_PAGE); + if (err < 0) + return err; + + /* + * Clear WOL status so that for next WOL event + * interrupt will be generated by HW + */ + temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL); + temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS; + err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp); + if (err < 0) + return err; + + + phy_write(phydev, MII_MARVELL_PHY_PAGE, oldpage); + + return 0; +} + static int marvell_ack_interrupt(struct phy_device *phydev) { int err;
/* Clear the interrupts by reading the reg */ err = phy_read(phydev, MII_M1011_IEVENT); - if (err < 0) return err;
@@ -1454,12 +1482,18 @@ static int marvell_aneg_done(struct phy_device *phydev)
static int m88e1121_did_interrupt(struct phy_device *phydev) { - int imask; + int imask, err;
imask = phy_read(phydev, MII_M1011_IEVENT);
- if (imask & MII_M1011_IMASK_INIT) + if (imask & MII_M1011_IMASK_INIT) { + if (imask & MII_M1011_IEVENT_WOL_EVENT) { + err = marvell_clear_wol_status(phydev); + if (err < 0) + return 0; + } return 1; + }
return 0; }
| |