Re: [PATCH] net: stmicro: implement basic Wake-On-LAN support

[Andrew Lunn <andrew@lunn.ch>]

> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -267,7 +267,14 @@ static void dwmac1000_flow_ctrl(struct mac_device_info *hw, unsigned int duplex,
>  static void dwmac1000_pmt(struct mac_device_info *hw, unsigned long mode)
>  {
>  	void __iomem *ioaddr = hw->pcsr;
> -	unsigned int pmt = 0;
> +	unsigned int pmt = 0, i = 0;
> +
> +	writel(pointer_reset, ioaddr + GMAC_PMT);
> +	mdelay(100);

That is quite a long delay. Is there a bit which can be polled to let
you know it is ready?

> +
> +	for (i = 0; i < WAKEUP_REG_LENGTH; i++)
> +		writel(*(stmmac_wakeup_filter_config + i),
> +		       ioaddr + GMAC_WAKEUP_FILTER);
>  
>  	if (mode & WAKE_MAGIC) {
>  		pr_debug("GMAC: WOL Magic frame\n");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 0a4d093adfc9..7866f3ec5ef6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4513,6 +4513,7 @@ int stmmac_suspend(struct device *dev)
>  
>  	/* Enable Power down mode by programming the PMT regs */
>  	if (device_may_wakeup(priv->device)) {
> +		priv->wolopts |= WAKE_MAGIC;
>  		stmmac_pmt(priv, priv->hw, priv->wolopts);
>  		priv->irq_wake = 1;
>  	} else {
> @@ -4598,6 +4599,7 @@ int stmmac_resume(struct device *dev)
>  			stmmac_mdio_reset(priv->mii);
>  	}
>  
> +	device_set_wakeup_enable(dev, 0);
>  	netif_device_attach(ndev);
>  
>  	mutex_lock(&priv->lock);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index cc1e887e47b5..ec69521f061c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -322,6 +322,7 @@ static int __maybe_unused stmmac_pci_suspend(struct device *dev)
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	int ret;
>  
> +	device_set_wakeup_enable(dev, 1);
>  	ret = stmmac_suspend(dev);
>  	if (ret)
>  		return ret;

This looks too minimum. I would expect there to be code in set_wol and
get_wol, to indicate WAKE_MAGIC is available, and to turn it
on/off. You also need to deal with when the PHY also implements
WAKE_MAGIC. Ideally, the PHY should implement it, since it can do it
with less power. But when the PHY does not have support, then the MAC
should implement it.

Maybe some of this code already exists, i've not looked.

       Andrew