From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jisheng Zhang Subject: Re: [PATCH] net: phy: marvell: clear wol event before setting it Date: Thu, 19 Apr 2018 17:09:32 +0800 Message-ID: <20180419170932.7a0b88fb@xhacker.debian> References: <20180419160232.519d15be@xhacker.debian> <96e77eac86794bef9a5b772147527c67@bgmail102.nvidia.com> <20180419165351.5388021e@xhacker.debian> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Andrew Lunn , Florian Fainelli , "David S. Miller" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Jingju Hou To: Bhadram Varka Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Thu, 19 Apr 2018 09:00:40 +0000 Bhadram Varka wrote: > Hi, > > > -----Original Message----- > > From: Jisheng Zhang > > Sent: Thursday, April 19, 2018 2:24 PM > > To: Bhadram Varka > > Cc: Andrew Lunn ; Florian Fainelli ; > > David S. Miller ; netdev@vger.kernel.org; linux- > > kernel@vger.kernel.org; Jingju Hou > > 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-owner@vger.kernel.org On > > > > Behalf Of Jisheng Zhang > > > > Sent: Thursday, April 19, 2018 1:33 PM > > > > To: Andrew Lunn ; Florian Fainelli > > > > ; David S. Miller > > > > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Jingju Hou > > > > > > > > Subject: [PATCH] net: phy: marvell: clear wol event before setting > > > > it > > > > > > > > From: Jingju Hou > > > > > > > > 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 > > > > Signed-off-by: Jisheng Zhang > > > > --- > > > > 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