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

[Florian Fainelli <florian@openwrt.org>]

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);
>