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]
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

Powered by blists - more mailing lists