Messages in this thread | | | Subject | Re: [PATCH] net: phy: marvell: clear wol event before setting it | From | Bhadram Varka <> | Date | Thu, 26 Apr 2018 11:56:33 +0530 |
| |
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.
| |