netdev - Re: [PATCH] net: phy: marvell: clear wol event before setting it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4273f766-a017-b336-7d14-a28901d274b9@nvidia.com>
Date:   Thu, 26 Apr 2018 11:10:21 +0530
From:   Bhadram Varka <vbhadram@...dia.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Jisheng Zhang <Jisheng.Zhang@...aptics.com>
CC:     Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Jingju Hou <Jingju.Hou@...aptics.com>
Subject: Re: [PATCH] net: phy: marvell: clear wol event before setting it

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@...aptics.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@...aptics.com>
>> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@...aptics.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 ?

Observed that once WoL event occurred for magic packet then for next 
magic packet WoL event is not asserted.
Need to explicitly clear WOL status so that WOL interrupt will be 
generated by the HW.

Thanks,
Bhadram.



Thanks,
Bhadram