On Thu, 19 Apr 2018 09:00:40 +0000 Bhadram Varka wrote: > Hi, > > > -----Original Message----- > > From: Jisheng Zhang <jisheng.zh...@synaptics.com> > > Sent: Thursday, April 19, 2018 2:24 PM > > To: Bhadram Varka <vbhad...@nvidia.com> > > Cc: Andrew Lunn <and...@lunn.ch>; Florian Fainelli <f.faine...@gmail.com>; > > David S. Miller <da...@davemloft.net>; net...@vger.kernel.org; linux- > > ker...@vger.kernel.org; Jingju Hou <jingju....@synaptics.com> > > Subject: Re: [PATCH] net: phy: marvell: clear wol event before setting it > > > > Hi, > > > > On Thu, 19 Apr 2018 08:38:45 +0000 Bhadram Varka wrote: > > > > > Hi, > > > > > > > -----Original Message----- > > > > From: netdev-ow...@vger.kernel.org <netdev-ow...@vger.kernel.org> On > > > > Behalf Of Jisheng Zhang > > > > Sent: Thursday, April 19, 2018 1:33 PM > > > > To: Andrew Lunn <and...@lunn.ch>; Florian Fainelli > > > > <f.faine...@gmail.com>; David S. Miller <da...@davemloft.net> > > > > Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; Jingju Hou > > > > <jingju....@synaptics.com> > > > > Subject: [PATCH] net: phy: marvell: clear wol event before setting > > > > it > > > > > > > > From: Jingju Hou <jingju....@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....@synaptics.com> > > > > Signed-off-by: Jisheng Zhang <jisheng.zh...@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 > > > > + > > > > > > There is already macro to represent this register - MII_M1011_IEVENT. Do > > > we > > need this macro ? > > > > Good point. Will use MII_M1011_IEVENT instead in v2. > > > > > > > > > /* 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); > > > > > > This part of the operation already taken care by ack_interrupt and > > > did_interrupt [....] .ack_interrupt = &marvell_ack_interrupt, > > > .did_interrupt = &m88e1121_did_interrupt, [...] > > > > > > If at all WOL event occurred marvell_ack_interrupt will take care of > > > clearing the > > interrupt status register. > > > Am I missing anything here ? > > > > If there's no valid irq for phy, the ack_interrupt/did_interrupt won't be > > called. > > Which means that the PHY is not having Interrupt pin ?
No valid irq doesn't mean "not having interrupt pin". they are different > > Generally through PHY interrupt will wake up the system right. If there is no > interrupt pin then how the system will wake up the from suspend for the magic > packet.? > IIRC, the phy irq isn't necessary for WOL. The phy interrupt pin isn't necessarily taken as "interrupt" PS: Did you use outlook as your email client? it's not suitable for kernel mail list. Thanks
