Hi Andrew,

Thank you for code review and valuable comments.

On Wed, Sep 28, 2016 at 06:27:05PM +0200, Andrew Lunn wrote:

EXTERNAL EMAIL

quoted

+#define MSCC_PHY_WOL_MAC_CONTROL       27
+#define EDGE_RATE_CNTL_POS             5
+#define EDGE_RATE_CNTL_MASK            0x00E0

This patch does not require these two #defines.

Please indicate in the cover note if the patches depends on other
patches in order to cleanly apply. Or if these patches are going to
conflict with some other patches.

Accepted. I will remove those 2 defines.

quoted

+     reg_val = phy_read(phydev, MSCC_PHY_WOL_MAC_CONTROL);
+     if (wol_conf->wolopts & WAKE_MAGICSECURE)
+             reg_val |= SECURE_ON_ENABLE;
+     else
+             reg_val &= ~SECURE_ON_ENABLE;
+     phy_write(phydev, MSCC_PHY_WOL_MAC_CONTROL, reg_val);
+
+     if (wol_conf->wolopts & WAKE_MAGICSECURE) {
+             reg_val = wol_conf->sopass[4] << 8;
+             reg_val |= wol_conf->sopass[5];
+             phy_write(phydev, MSCC_PHY_WOL_LOWER_PASSWD, reg_val);
+             reg_val = wol_conf->sopass[2] << 8;
+             reg_val |= wol_conf->sopass[3];
+             phy_write(phydev, MSCC_PHY_WOL_MID_PASSWD, reg_val);
+             reg_val = wol_conf->sopass[0] << 8;
+             reg_val |= wol_conf->sopass[1];
+             phy_write(phydev, MSCC_PHY_WOL_UPPER_PASSWD, reg_val);
+     } else {
+             phy_write(phydev, MSCC_PHY_WOL_LOWER_PASSWD, 0);
+             phy_write(phydev, MSCC_PHY_WOL_MID_PASSWD, 0);
+             phy_write(phydev, MSCC_PHY_WOL_UPPER_PASSWD, 0);
+     }

Wouldn't it be better to set the password, and then enable the
password feature?

Accepted. I will change.

I don't know much about WOL. Hopefully Florian will add further
comments.

         Andrew

---
Thanks,
Raju.