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: netdev@vger.kernel.org; linux-ker...@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. Thanks
