[net,v2,2/2] net: phy: DP83TC811: Fix WoL in config init to be disabled

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 <dmurphy@ti.com>
---
 drivers/net/phy/dp83tc811.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

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 <lkp@intel.com>

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/kbuild-all@lists.01.org

From: Dan Murphy <dmurphy@ti.com>
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 <dmurphy@ti.com>
> 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.