Re: [PATCH 1/3] tg3: Avoid setting power.can_wakeup for devices that cannot wake up

["Matt Carlson" <mcarlson@broadcom.com>]

On Thu, Feb 10, 2011 at 08:53:09AM -0800, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The tg3 driver uses device_init_wakeup() in such a way that the
> device's power.can_wakeup flag may be set even though the PCI
> subsystem cleared it before, in which case the device cannot wake
> up the system from sleep states.  Modify the driver to only change
> the power.can_wakeup flag if the device is not capable of generating
> wakeup signals.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/net/tg3.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/net/tg3.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/tg3.c
> +++ linux-2.6/drivers/net/tg3.c
> @@ -12403,9 +12403,11 @@ static void __devinit tg3_get_eeprom_hw_
>  			tp->tg3_flags3 |= TG3_FLG3_RGMII_EXT_IBND_TX_EN;
>  	}
>  done:
> -	device_init_wakeup(&tp->pdev->dev, tp->tg3_flags & TG3_FLAG_WOL_CAP);
> -	device_set_wakeup_enable(&tp->pdev->dev,
> +	if (tp->tg3_flags & TG3_FLAG_WOL_CAP)
> +		device_set_wakeup_enable(&tp->pdev->dev,
>  				 tp->tg3_flags & TG3_FLAG_WOL_ENABLE);
> +	else
> +		device_set_wakeup_capable(&tp->pdev->dev, false);

I did this because I couldn't see where 'can_wakeup' gets set.  I don't
see a call to device_init_wakeup() that would be relevant to tg3
devices.  I do see a couple calls to device_set_wakeup_capable() in
acpi/glue.c and acpi/scan.c.  Is that the place?

>  }
>  
>  static int __devinit tg3_issue_otp_command(struct tg3 *tp, u32 cmd)

This is something I was always curious about too.  The TG3_FLAG_WOL_CAP
tracks whether or not the device can handle WOL.  Is it safe to do away
with this flag and lean on the 'can_wakeup' flag instead?  My concern is
that some other part of the system might enable that flag after
tg3_get_invariants() has run.  If that happens and the device isn't
really WOL capable bad things can occur.