Received: by 10.192.165.148 with SMTP id m20csp1681821imm; Thu, 26 Apr 2018 00:12:01 -0700 (PDT) X-Google-Smtp-Source: AIpwx4+koTOzRtT7IOhkbqNmr5MLntKLEcGgJXL/FfwX02O0UmuWrX/lmtIdBGz7N2wr/ynRKojY X-Received: by 10.98.155.141 with SMTP id e13mr21742005pfk.157.1524726721805; Thu, 26 Apr 2018 00:12:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1524726721; cv=none; d=google.com; s=arc-20160816; b=Wikcv2ZyxJGjBWLD2rnPlImUdOY26ER1Xxi/WUjLuVGM248uang/ZdVRW/KARVoEyT wvxRRmIGM7cfnhxYcOKHas/J3yRuA0mKKkS9/PHhz1TQMJ1aiOpFQXv9fJaKuSSGgY75 5S5RV1vTZEXiZe3Zd3jVnytPFmPcH0WckIvc/sv9H4o707GgVjYy0JoV37zySJh/RXGy p/R7b+GGvx4u/uvPyc16o4jcEPpxrihlWWLjv+vlDFlQCZaUBSgkYn/WQp58quVW/RzM WO4S4GBCBF7Fjp29EsrALLxDbwz5rYWR0n1OpV2Hp6d07T6g8qWEmqS1gJevBlA3BvzF 4PLQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-language :content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:references:cc:to:from:subject:arc-authentication-results; bh=CxDop6NaMx70gBXpbR+XrDsjhrwwkKjZJMigbDZWpnU=; b=IB4QLn1GW+GRMfYpUIqvfj54uJhQa3N+cGv2WzGUaH5RsfUHTDEgJawj0ZFCGYVXEU mgPnbD8hmbQxJLaziYQnzYBJlykLM8XCRIZNUXgx7Ou6urtqamdcmOGEZyEcPKcF4TBJ H0hA61x/smkijd1XFSiQApwx3fjkf0sqCcfluOvIJheKC6nb3JHraQ3agj8nKd54EIzN EkAFBKFERkeZLvGfONa2yQtd1OihqX/SODvXA/aEIbXdZkNvQVa//Sbtp5H+yLQdVvCu zpMarzsyxq9PWfTEaFYQDd1T3ElrjQtN6fWnDaGR5OBzxoYlXBRgV81wWHq9VeYEBixY 2hIw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id s15-v6si18051338plp.243.2018.04.26.00.11.47; Thu, 26 Apr 2018 00:12:01 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753968AbeDZHKJ convert rfc822-to-8bit (ORCPT + 99 others); Thu, 26 Apr 2018 03:10:09 -0400 Received: from hqemgate16.nvidia.com ([216.228.121.65]:3363 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753860AbeDZHKG (ORCPT ); Thu, 26 Apr 2018 03:10:06 -0400 Received: from hqpgpgate102.nvidia.com (Not Verified[216.228.121.13]) by hqemgate16.nvidia.com id ; Thu, 26 Apr 2018 00:09:48 -0700 Received: from HQMAIL103.nvidia.com ([172.20.161.6]) by hqpgpgate102.nvidia.com (PGP Universal service); Thu, 26 Apr 2018 00:10:03 -0700 X-PGP-Universal: processed; by hqpgpgate102.nvidia.com on Thu, 26 Apr 2018 00:10:03 -0700 Received: from BGMAIL102.nvidia.com (10.25.59.11) by HQMAIL103.nvidia.com (172.20.187.11) with Microsoft SMTP Server (TLS) id 15.0.1347.2; Thu, 26 Apr 2018 07:10:04 +0000 Received: from [192.168.100.20] (10.19.65.191) by bgmail102.nvidia.com (10.25.59.11) with Microsoft SMTP Server (TLS) id 15.0.1347.2; Thu, 26 Apr 2018 07:10:00 +0000 Subject: Re: [PATCH] net: phy: marvell: clear wol event before setting it From: Bhadram Varka To: Jisheng Zhang CC: Andrew Lunn , Florian Fainelli , "David S. Miller" , , , Jingju Hou References: <20180419160232.519d15be@xhacker.debian> <20180419121801.GC17888@lunn.ch> <4273f766-a017-b336-7d14-a28901d274b9@nvidia.com> <20180426141508.6660a633@xhacker.debian> Message-ID: <73e21c83-f78a-8b22-a421-f179ef6adef1@nvidia.com> Date: Thu, 26 Apr 2018 12:39:59 +0530 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.19.65.191] X-ClientProxiedBy: BGMAIL104.nvidia.com (10.25.59.13) To bgmail102.nvidia.com (10.25.59.11) Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8BIT Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>>>> >>>>> 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 >>>>> + >>>>> /* 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; }