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: 6d749428788b ("net: phy: DP83TC811: Introduce support for the
DP83TC811 phy")
Signed-off-by: Dan Murphy <[email protected]>
---
drivers/net/phy/dp83tc811.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index 06f08832ebcd..ff325fb748b9 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -139,16 +139,19 @@ static int dp83811_set_wol(struct phy_device *phydev,
value &= ~DP83811_WOL_SECURE_ON;
}
- value |= (DP83811_WOL_EN | DP83811_WOL_INDICATION_SEL |
- DP83811_WOL_CLR_INDICATION);
- phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
- value);
+ /* Clear any pending WoL interrupt */
+ phy_read(phydev, MII_DP83811_INT_STAT1);
+
+ value |= DP83811_WOL_EN | DP83811_WOL_INDICATION_SEL |
+ DP83811_WOL_CLR_INDICATION;
+
+ return phy_write_mmd(phydev, DP83822_DEVADDR,
+ MII_DP83811_WOL_CFG, value);
} else {
- phy_clear_bits_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
- DP83811_WOL_EN);
+ return phy_clear_bits_mmd(phydev, DP83811_DEVADDR,
+ MII_DP83811_WOL_CFG, DP83811_WOL_EN);
}
- return 0;
}
static void dp83811_get_wol(struct phy_device *phydev,
@@ -292,8 +295,8 @@ static int dp83811_config_init(struct phy_device *phydev)
value = DP83811_WOL_MAGIC_EN | DP83811_WOL_SECURE_ON | DP83811_WOL_EN;
- return phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
- value);
+ return phy_clear_bits_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
+ value);
}
static int dp83811_phy_reset(struct phy_device *phydev)
--
2.25.1
Hi Dan,
I love your patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on linus/master v5.7-rc3 next-20200428]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Dan-Murphy/WoL-fixes-for-DP83822-and-DP83tc811/20200428-130050
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 37255e7a8f470a7d9678be89c18ba15d6ebf43f7
config: x86_64-randconfig-c002-20200428 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project f30416fdde922eaa655934e050026930fefbd260)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/net/phy/dp83tc811.c:148:32: error: use of undeclared identifier 'DP83822_DEVADDR'
return phy_write_mmd(phydev, DP83822_DEVADDR,
^
1 error generated.
vim +/DP83822_DEVADDR +148 drivers/net/phy/dp83tc811.c
96
97 static int dp83811_set_wol(struct phy_device *phydev,
98 struct ethtool_wolinfo *wol)
99 {
100 struct net_device *ndev = phydev->attached_dev;
101 const u8 *mac;
102 u16 value;
103
104 if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
105 mac = (const u8 *)ndev->dev_addr;
106
107 if (!is_valid_ether_addr(mac))
108 return -EINVAL;
109
110 /* MAC addresses start with byte 5, but stored in mac[0].
111 * 811 PHYs store bytes 4|5, 2|3, 0|1
112 */
113 phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_DA1,
114 (mac[1] << 8) | mac[0]);
115 phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_DA2,
116 (mac[3] << 8) | mac[2]);
117 phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_DA3,
118 (mac[5] << 8) | mac[4]);
119
120 value = phy_read_mmd(phydev, DP83811_DEVADDR,
121 MII_DP83811_WOL_CFG);
122 if (wol->wolopts & WAKE_MAGIC)
123 value |= DP83811_WOL_MAGIC_EN;
124 else
125 value &= ~DP83811_WOL_MAGIC_EN;
126
127 if (wol->wolopts & WAKE_MAGICSECURE) {
128 phy_write_mmd(phydev, DP83811_DEVADDR,
129 MII_DP83811_RXSOP1,
130 (wol->sopass[1] << 8) | wol->sopass[0]);
131 phy_write_mmd(phydev, DP83811_DEVADDR,
132 MII_DP83811_RXSOP2,
133 (wol->sopass[3] << 8) | wol->sopass[2]);
134 phy_write_mmd(phydev, DP83811_DEVADDR,
135 MII_DP83811_RXSOP3,
136 (wol->sopass[5] << 8) | wol->sopass[4]);
137 value |= DP83811_WOL_SECURE_ON;
138 } else {
139 value &= ~DP83811_WOL_SECURE_ON;
140 }
141
142 /* Clear any pending WoL interrupt */
143 phy_read(phydev, MII_DP83811_INT_STAT1);
144
145 value |= DP83811_WOL_EN | DP83811_WOL_INDICATION_SEL |
146 DP83811_WOL_CLR_INDICATION;
147
> 148 return phy_write_mmd(phydev, DP83822_DEVADDR,
149 MII_DP83811_WOL_CFG, value);
150 } else {
151 return phy_clear_bits_mmd(phydev, DP83811_DEVADDR,
152 MII_DP83811_WOL_CFG, DP83811_WOL_EN);
153 }
154
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
From: Dan Murphy <[email protected]>
Date: Mon, 27 Apr 2020 16:21:12 -0500
> + return phy_write_mmd(phydev, DP83822_DEVADDR,
^^^^^^^^^^^^^^^^
Please don't submit patches that have not even had a conversation with
the compiler.
This register define only exists in dp83822.c and you are trying to use
it in dp83tc811.c
If this doesn't compile, how did you do functional testing of this
change?
If you compile tested these changes against a tree other than the 'net'
tree, please don't do that.
Thanks.
David
On 4/28/20 3:05 PM, David Miller wrote:
> From: Dan Murphy <[email protected]>
> Date: Mon, 27 Apr 2020 16:21:12 -0500
>
>> + return phy_write_mmd(phydev, DP83822_DEVADDR,
> ^^^^^^^^^^^^^^^^
>
> Please don't submit patches that have not even had a conversation with
> the compiler.
>
> This register define only exists in dp83822.c and you are trying to use
> it in dp83tc811.c
>
> If this doesn't compile, how did you do functional testing of this
> change?
My fault, I thought my defconfig had this turned on to compile as it did
in v1.
I functionally tested these changes on the DP83822 as they are register
and feature compatible for WoL
Dan
> If you compile tested these changes against a tree other than the 'net'
> tree, please don't do that.
>
> Thanks.