lkml.org 
[lkml]   [2020]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net 1/2] net: phy: DP83822: Fix WoL in config init to be disabled
From
Date
Heiner

On 4/23/20 12:02 PM, Heiner Kallweit wrote:
> On 23.04.2020 18:39, Dan Murphy wrote:
>> The WoL feature should be disabled when config_init is called and the
>> feature should turned on or off when set_wol is called.
>>
>> In addition updated the calls to modify the registers to use the set_bit
>> and clear_bit function calls.
>>
>> Fixes: 3b427751a9d0 ("net: phy: DP83822 initial driver submission")
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> ---
>> drivers/net/phy/dp83822.c | 30 ++++++++++++++++--------------
>> 1 file changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
>> index fe9aa3ad52a7..40fdfd043947 100644
>> --- a/drivers/net/phy/dp83822.c
>> +++ b/drivers/net/phy/dp83822.c
>> @@ -137,16 +137,19 @@ static int dp83822_set_wol(struct phy_device *phydev,
>> value &= ~DP83822_WOL_SECURE_ON;
>> }
>>
>> - value |= (DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL |
>> - DP83822_WOL_CLR_INDICATION);
>> - phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
>> - value);
>> + /* Clear any pending WoL interrupt */
>> + phy_read(phydev, MII_DP83822_MISR2);
>> +
>> + value |= DP83822_WOL_EN | DP83822_WOL_INDICATION_SEL |
>> + DP83822_WOL_CLR_INDICATION;
>> +
>> + return phy_set_bits_mmd(phydev, DP83822_DEVADDR,
>> + MII_DP83822_WOL_CFG, value);
> The switch to phy_set_bits_mmd() doesn't seem to be correct here.
> So far bit DP83822_WOL_MAGIC_EN is cleared if WAKE_MAGIC isn't set.
> Similar for bit DP83822_WOL_SECURE_ON. With your change they don't
> get cleared any longer.


Thanks for the review.  I was not watching those bits during my testing
I will revert back to the original code.

>> } else {
>> - value = phy_read_mmd(phydev, DP83822_DEVADDR,
>> - MII_DP83822_WOL_CFG);
>> - value &= ~DP83822_WOL_EN;
>> - phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
>> - value);
>> + value = DP83822_WOL_EN | DP83822_WOL_CLR_INDICATION;
>> +
> You clear one bit more than before. The reason for this may be worth a note
> in the commit message.

Thanks this was an artifact from debugging.  I can remove the CLR as it
was cleared in set_wol enablement.

Dan

\
 
 \ /
  Last update: 2020-04-23 19:55    [W:0.041 / U:0.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site