[PATCH net-next 1/2] phy: add set_wol/get_wol functions

[PATCH net-next 1/2] phy: add set_wol/get_wol functions

[Michael Stapelberg]

This allows ethernet drivers (such as the mv643xx_eth) to support
Wake on LAN on platforms where PHY registers have to be configured
for Wake on LAN (e.g. the Marvell Kirkwood based qnap TS-119P II).

Signed-off-by: Michael Stapelberg <michael@stapelberg.de>
---
 drivers/net/phy/phy.c |   16 ++++++++++++++++
 include/linux/phy.h   |   10 ++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index ef9ea92..298b4c2 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1188,3 +1188,19 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
 	return 0;
 }
 EXPORT_SYMBOL(phy_ethtool_set_eee);
+
+int phy_ethtool_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+	if (phydev->drv->set_wol)
+		return phydev->drv->set_wol(phydev, wol);
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(phy_ethtool_set_wol);
+
+void phy_ethtool_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+	if (phydev->drv->get_wol)
+		phydev->drv->get_wol(phydev, wol);
+}
+EXPORT_SYMBOL(phy_ethtool_get_wol);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 33999ad..9e11039 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -455,6 +455,14 @@ struct phy_driver {
 	 */
 	void (*txtstamp)(struct phy_device *dev, struct sk_buff *skb, int type);
 
+	/* Some devices (e.g. qnap TS-119P II) require PHY register changes to
+	 * enable Wake on LAN, so set_wol is provided to be called in the
+	 * ethernet driver's set_wol function. */
+	int (*set_wol)(struct phy_device *dev, struct ethtool_wolinfo *wol);
+
+	/* See set_wol, but for checking whether Wake on LAN is enabled. */
+	void (*get_wol)(struct phy_device *dev, struct ethtool_wolinfo *wol);
+
 	struct device_driver driver;
 };
 #define to_phy_driver(d) container_of(d, struct phy_driver, driver)
@@ -560,6 +568,8 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable);
 int phy_get_eee_err(struct phy_device *phydev);
 int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data);
 int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data);
+int phy_ethtool_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol);
+void phy_ethtool_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol);
 
 int __init mdio_bus_init(void);
 void mdio_bus_exit(void);
-- 
1.7.10.4

[PATCH net-next 2/2] mv643xx_eth with 88E1318S: support Wake on LAN

[Michael Stapelberg]

This has been tested on a qnap TS-119P II. Note that enabling WOL with
"ethtool -s eth0 wol g" is not enough; you also need to tell the PIC
microcontroller inside the qnap that WOL should be enabled by sending
0xF2 with qcontrol(1) and you have to disable EUP ("Energy-using
Products", a European power-saving thing) by sending 0xF4.

Signed-off-by: Michael Stapelberg <michael@stapelberg.de>
---
 drivers/net/ethernet/marvell/mv643xx_eth.c |   32 +++++++
 drivers/net/phy/marvell.c                  |  127 ++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 2914050..64a405e 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -20,6 +20,8 @@
  * Copyright (C) 2007-2008 Marvell Semiconductor
  *			   Lennert Buytenhek <buytenh@marvell.com>
  *
+ * Copyright (C) 2013 Michael Stapelberg <michael@stapelberg.de>
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version 2
@@ -1484,6 +1486,34 @@ mv643xx_eth_get_settings_phyless(struct mv643xx_eth_private *mp,
 	return 0;
 }
 
+static void
+mv643xx_eth_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct mv643xx_eth_private *mp = netdev_priv(dev);
+	wol->supported = 0;
+	wol->wolopts = 0;
+	if (mp->phy)
+		phy_ethtool_get_wol(mp->phy, wol);
+}
+
+static int
+mv643xx_eth_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
+{
+	struct mv643xx_eth_private *mp = netdev_priv(dev);
+	int err;
+
+	if (mp->phy == NULL)
+		return -EOPNOTSUPP;
+
+	err = phy_ethtool_set_wol(mp->phy, wol);
+	/* Given that mv643xx_eth works without the marvell-specific PHY driver,
+	 * this debugging hint is useful to have.
+	 */
+	if (err == -EOPNOTSUPP)
+		netdev_info(dev, "The PHY does not support set_wol, was CONFIG_MARVELL_PHY enabled?\n");
+	return err;
+}
+
 static int
 mv643xx_eth_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
@@ -1665,6 +1695,8 @@ static const struct ethtool_ops mv643xx_eth_ethtool_ops = {
 	.get_ethtool_stats	= mv643xx_eth_get_ethtool_stats,
 	.get_sset_count		= mv643xx_eth_get_sset_count,
 	.get_ts_info		= ethtool_op_get_ts_info,
+	.get_wol                = mv643xx_eth_get_wol,
+	.set_wol                = mv643xx_eth_set_wol,
 };
 
 
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 22dec9c..b05ac6e 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -7,6 +7,8 @@
  *
  * Copyright (c) 2004 Freescale Semiconductor, Inc.
  *
+ * Copyright (c) 2013 Michael Stapelberg <michael@stapelberg.de>
+ *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
  * Free Software Foundation;  either version 2 of the  License, or (at your
@@ -80,6 +82,28 @@
 #define MII_88E1318S_PHY_MSCR1_REG	16
 #define MII_88E1318S_PHY_MSCR1_PAD_ODD	BIT(6)
 
+/* Copper Specific Interrupt Enable Register */
+#define MII_88E1318S_PHY_CSIER                              0x12
+/* WOL Event Interrupt Enable */
+#define MII_88E1318S_PHY_CSIER_WOL_EIE                      BIT(7)
+
+/* LED Timer Control Register */
+#define MII_88E1318S_PHY_LED_PAGE                           0x03
+#define MII_88E1318S_PHY_LED_TCR                            0x12
+#define MII_88E1318S_PHY_LED_TCR_FORCE_INT                  BIT(15)
+#define MII_88E1318S_PHY_LED_TCR_INTn_ENABLE                BIT(7)
+#define MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW             BIT(11)
+
+/* Magic Packet MAC address registers */
+#define MII_88E1318S_PHY_MAGIC_PACKET_WORD2                 0x17
+#define MII_88E1318S_PHY_MAGIC_PACKET_WORD1                 0x18
+#define MII_88E1318S_PHY_MAGIC_PACKET_WORD0                 0x19
+
+#define MII_88E1318S_PHY_WOL_PAGE                           0x11
+#define MII_88E1318S_PHY_WOL_CTRL                           0x10
+#define MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS          BIT(12)
+#define MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE BIT(14)
+
 #define MII_88E1121_PHY_LED_CTRL	16
 #define MII_88E1121_PHY_LED_PAGE	3
 #define MII_88E1121_PHY_LED_DEF		0x0030
@@ -696,6 +720,107 @@ static int m88e1121_did_interrupt(struct phy_device *phydev)
 	return 0;
 }
 
+static void m88e1318_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+	wol->supported = WAKE_MAGIC;
+	wol->wolopts = 0;
+
+	if (phy_write(phydev, MII_MARVELL_PHY_PAGE,
+		MII_88E1318S_PHY_WOL_PAGE) < 0)
+		return;
+
+	if (phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL) &
+		MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE)
+		wol->wolopts |= WAKE_MAGIC;
+
+	if (phy_write(phydev, MII_MARVELL_PHY_PAGE, 0x00) < 0)
+		return;
+}
+
+static int m88e1318_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
+{
+	int err, oldpage, temp;
+
+	oldpage = phy_read(phydev, MII_MARVELL_PHY_PAGE);
+
+	if (wol->wolopts & WAKE_MAGIC) {
+		/* Explicitly switch to page 0x00, just to be sure */
+		err = phy_write(phydev, MII_MARVELL_PHY_PAGE, 0x00);
+		if (err < 0)
+			return err;
+
+		/* Enable the WOL interrupt */
+		temp = phy_read(phydev, MII_88E1318S_PHY_CSIER);
+		temp |= MII_88E1318S_PHY_CSIER_WOL_EIE;
+		err = phy_write(phydev, MII_88E1318S_PHY_CSIER, temp);
+		if (err < 0)
+			return err;
+
+		err = phy_write(phydev, MII_MARVELL_PHY_PAGE,
+				MII_88E1318S_PHY_LED_PAGE);
+		if (err < 0)
+			return err;
+
+		/* Setup LED[2] as interrupt pin (active low) */
+		temp = phy_read(phydev, MII_88E1318S_PHY_LED_TCR);
+		temp &= ~MII_88E1318S_PHY_LED_TCR_FORCE_INT;
+		temp |= MII_88E1318S_PHY_LED_TCR_INTn_ENABLE;
+		temp |= MII_88E1318S_PHY_LED_TCR_INT_ACTIVE_LOW;
+		err = phy_write(phydev, MII_88E1318S_PHY_LED_TCR, temp);
+		if (err < 0)
+			return err;
+
+		err = phy_write(phydev, MII_MARVELL_PHY_PAGE,
+				MII_88E1318S_PHY_WOL_PAGE);
+		if (err < 0)
+			return err;
+
+		/* Store the device address for the magic packet */
+		err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD2,
+			  ((phydev->attached_dev->dev_addr[5] << 8) |
+			    phydev->attached_dev->dev_addr[4]));
+		if (err < 0)
+			return err;
+		err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD1,
+			  ((phydev->attached_dev->dev_addr[3] << 8) |
+			    phydev->attached_dev->dev_addr[2]));
+		if (err < 0)
+			return err;
+		err = phy_write(phydev, MII_88E1318S_PHY_MAGIC_PACKET_WORD0,
+			  ((phydev->attached_dev->dev_addr[1] << 8) |
+			    phydev->attached_dev->dev_addr[0]));
+		if (err < 0)
+			return err;
+
+		/* Clear WOL status and enable magic packet matching */
+		temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
+		temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
+		temp |= MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
+		err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+		if (err < 0)
+			return err;
+	} else {
+		err = phy_write(phydev, MII_MARVELL_PHY_PAGE,
+				MII_88E1318S_PHY_WOL_PAGE);
+		if (err < 0)
+			return err;
+
+		/* Clear WOL status and disable magic packet matching */
+		temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
+		temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
+		temp &= ~MII_88E1318S_PHY_WOL_CTRL_MAGIC_PACKET_MATCH_ENABLE;
+		err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
+		if (err < 0)
+			return err;
+	}
+
+	err = phy_write(phydev, MII_MARVELL_PHY_PAGE, oldpage);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 static struct phy_driver marvell_drivers[] = {
 	{
 		.phy_id = MARVELL_PHY_ID_88E1101,
@@ -772,6 +897,8 @@ static struct phy_driver marvell_drivers[] = {
 		.ack_interrupt = &marvell_ack_interrupt,
 		.config_intr = &marvell_config_intr,
 		.did_interrupt = &m88e1121_did_interrupt,
+		.get_wol = &m88e1318_get_wol,
+		.set_wol = &m88e1318_set_wol,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
-- 
1.7.10.4

Re: [PATCH net-next 2/2] mv643xx_eth with 88E1318S: support Wake on LAN

[Andrew Lunn]

On Tue, Mar 12, 2013 at 12:39:22AM +0100, Michael Stapelberg wrote:
> This has been tested on a qnap TS-119P II. Note that enabling WOL with
> "ethtool -s eth0 wol g" is not enough; you also need to tell the PIC
> microcontroller inside the qnap that WOL should be enabled by sending
> 0xF2 with qcontrol(1) and you have to disable EUP ("Energy-using
> Products", a European power-saving thing) by sending 0xF4.
> 
> Signed-off-by: Michael Stapelberg <michael@stapelberg.de>
> ---
>  drivers/net/ethernet/marvell/mv643xx_eth.c |   32 +++++++
>  drivers/net/phy/marvell.c                  |  127 ++++++++++++++++++++++++++++
>  2 files changed, 159 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index 2914050..64a405e 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -20,6 +20,8 @@
>   * Copyright (C) 2007-2008 Marvell Semiconductor
>   *			   Lennert Buytenhek <buytenh@marvell.com>
>   *
> + * Copyright (C) 2013 Michael Stapelberg <michael@stapelberg.de>
> + *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License
>   * as published by the Free Software Foundation; either version 2
> @@ -1484,6 +1486,34 @@ mv643xx_eth_get_settings_phyless(struct mv643xx_eth_private *mp,
>  	return 0;
>  }
>  
> +static void
> +mv643xx_eth_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> +{
> +	struct mv643xx_eth_private *mp = netdev_priv(dev);
> +	wol->supported = 0;
> +	wol->wolopts = 0;
> +	if (mp->phy)
> +		phy_ethtool_get_wol(mp->phy, wol);
> +}
> +
> +static int
> +mv643xx_eth_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> +{
> +	struct mv643xx_eth_private *mp = netdev_priv(dev);
> +	int err;
> +
> +	if (mp->phy == NULL)
> +		return -EOPNOTSUPP;
> +
> +	err = phy_ethtool_set_wol(mp->phy, wol);
> +	/* Given that mv643xx_eth works without the marvell-specific PHY driver,
> +	 * this debugging hint is useful to have.
> +	 */
> +	if (err == -EOPNOTSUPP)
> +		netdev_info(dev, "The PHY does not support set_wol, was CONFIG_MARVELL_PHY enabled?\n");
> +	return err;

Hi Michael

I would not have this netdev_info() message. Only a few boxes are
going to have a PHY with this capability, so printing when it does not
is rather annoying. Either just return the error code, or make it a
netdev_dbg().

      Andrew

Re: [PATCH net-next 1/2] phy: add set_wol/get_wol functions

[Florian Fainelli]

Hello Michael,

This is looking good from my perspective, just a minor nitpick with 
repsect to your comment below.

On 03/12/2013 12:56 AM, Michael Stapelberg wrote:
> This allows ethernet drivers (such as the mv643xx_eth) to support
> Wake on LAN on platforms where PHY registers have to be configured
> for Wake on LAN (e.g. the Marvell Kirkwood based qnap TS-119P II).
>
> Signed-off-by: Michael Stapelberg <michael@stapelberg.de>
> ---
>   drivers/net/phy/phy.c |   16 ++++++++++++++++
>   include/linux/phy.h   |   10 ++++++++++
>   2 files changed, 26 insertions(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index ef9ea92..298b4c2 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1188,3 +1188,19 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
>   	return 0;
>   }
>   EXPORT_SYMBOL(phy_ethtool_set_eee);
> +
> +int phy_ethtool_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
> +{
> +	if (phydev->drv->set_wol)
> +		return phydev->drv->set_wol(phydev, wol);
> +
> +	return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL(phy_ethtool_set_wol);
> +
> +void phy_ethtool_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
> +{
> +	if (phydev->drv->get_wol)
> +		phydev->drv->get_wol(phydev, wol);
> +}
> +EXPORT_SYMBOL(phy_ethtool_get_wol);
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 33999ad..9e11039 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -455,6 +455,14 @@ struct phy_driver {
>   	 */
>   	void (*txtstamp)(struct phy_device *dev, struct sk_buff *skb, int type);
>
> +	/* Some devices (e.g. qnap TS-119P II) require PHY register changes to
> +	 * enable Wake on LAN, so set_wol is provided to be called in the
> +	 * ethernet driver's set_wol function. */

I would just mention here that some PHY chip needs to be configured to 
enable WoL at the PHY level (such as the Marvell 88E1318S PHY).

> +	int (*set_wol)(struct phy_device *dev, struct ethtool_wolinfo *wol);
> +
> +	/* See set_wol, but for checking whether Wake on LAN is enabled. */
> +	void (*get_wol)(struct phy_device *dev, struct ethtool_wolinfo *wol);
> +
>   	struct device_driver driver;
>   };
>   #define to_phy_driver(d) container_of(d, struct phy_driver, driver)
> @@ -560,6 +568,8 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable);
>   int phy_get_eee_err(struct phy_device *phydev);
>   int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data);
>   int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data);
> +int phy_ethtool_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol);
> +void phy_ethtool_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol);
>
>   int __init mdio_bus_init(void);
>   void mdio_bus_exit(void);
>

Re: [PATCH net-next 1/2] phy: add set_wol/get_wol functions

[David Miller]

From: Michael Stapelberg <michael@stapelberg.de>
Date: Tue, 12 Mar 2013 00:56:44 +0100

> This allows ethernet drivers (such as the mv643xx_eth) to support
> Wake on LAN on platforms where PHY registers have to be configured
> for Wake on LAN (e.g. the Marvell Kirkwood based qnap TS-119P II).
> 
> Signed-off-by: Michael Stapelberg <michael@stapelberg.de>

Applied.

Re: [PATCH net-next 2/2] mv643xx_eth with 88E1318S: support Wake on LAN

[David Miller]

From: Michael Stapelberg <michael@stapelberg.de>
Date: Tue, 12 Mar 2013 00:56:45 +0100

> This has been tested on a qnap TS-119P II. Note that enabling WOL with
> "ethtool -s eth0 wol g" is not enough; you also need to tell the PIC
> microcontroller inside the qnap that WOL should be enabled by sending
> 0xF2 with qcontrol(1) and you have to disable EUP ("Energy-using
> Products", a European power-saving thing) by sending 0xF4.
> 
> Signed-off-by: Michael Stapelberg <michael@stapelberg.de>

Applied.