[PATCH] ACPI / WAKEUP : enable wakeup power for physical child devices

[PATCH] ACPI / WAKEUP : enable wakeup power for physical child devices

[Andrey Skvortsov]

commit f2b56bc808addb908a5bf435d9b942c02af9a7c4
("ACPI / PM: Use device wakeup flags for handling ACPI wakeup devices")
broke wake-on-lan for Broadcom BCM4401 Ethernet card (b44) on Dell Vostro 1500 laptop.
device_may_wakeup for main ACPI device (PCIE) returns false, because it has
can_wakeup = 0. Therefore any physical devices connected to this parent
were not prepared for wakeup/suspend. But physical device is capable to
wakeup the system.

To fix this issue device_may_wakeup was replaced with acpi_device_may_wakeup.
acpi_device_may_wakeup was written based on function
acpi_system_wakeup_device_seq_show

Signed-off-by: Andrey Skvortsov <Andrej.Skvortzov@gmail.com>
---
 drivers/acpi/wakeup.c |   45 +++++++++++++++++++++++++++++++++++++--------
 1 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
index 1638401..0da7e70 100644
--- a/drivers/acpi/wakeup.c
+++ b/drivers/acpi/wakeup.c
@@ -19,6 +19,34 @@
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
 ACPI_MODULE_NAME("wakeup_devices")
 
+
+bool acpi_device_may_wakeup(struct acpi_device *dev)
+{
+	struct acpi_device_physical_node *entry;
+	bool may_wakeup = false;
+
+	mutex_lock(&dev->physical_node_lock);
+	if (!dev->physical_node_count)
+		may_wakeup = device_may_wakeup(&dev->dev);
+	else {
+		struct device *ldev;
+
+		list_for_each_entry(entry, &dev->physical_node_list, node) {
+			ldev = get_device(entry->dev);
+			if (!ldev)
+				continue;
+
+			may_wakeup = device_may_wakeup(&dev->dev) ||
+				device_may_wakeup(ldev);
+
+			put_device(ldev);
+		}
+	}
+
+	mutex_unlock(&dev->physical_node_lock);
+	return may_wakeup;
+}
+
 /**
  * acpi_enable_wakeup_devices - Enable wake-up device GPEs.
  * @sleep_state: ACPI system sleep state.
@@ -35,13 +63,14 @@ void acpi_enable_wakeup_devices(u8 sleep_state)
 		struct acpi_device *dev =
 			container_of(node, struct acpi_device, wakeup_list);
 
+
 		if (!dev->wakeup.flags.valid
-		    || sleep_state > (u32) dev->wakeup.sleep_state
-		    || !(device_may_wakeup(&dev->dev)
-		        || dev->wakeup.prepare_count))
+			|| sleep_state > (u32) dev->wakeup.sleep_state
+			|| !(acpi_device_may_wakeup(dev)
+			    || dev->wakeup.prepare_count))
 			continue;
 
-		if (device_may_wakeup(&dev->dev))
+		if (acpi_device_may_wakeup(dev))
 			acpi_enable_wakeup_device_power(dev, sleep_state);
 
 		/* The wake-up power should have been enabled already. */
@@ -63,15 +92,15 @@ void acpi_disable_wakeup_devices(u8 sleep_state)
 			container_of(node, struct acpi_device, wakeup_list);
 
 		if (!dev->wakeup.flags.valid
-		    || sleep_state > (u32) dev->wakeup.sleep_state
-		    || !(device_may_wakeup(&dev->dev)
-		        || dev->wakeup.prepare_count))
+			|| sleep_state > (u32) dev->wakeup.sleep_state
+			|| !(acpi_device_may_wakeup(dev)
+				|| dev->wakeup.prepare_count))
 			continue;
 
 		acpi_set_gpe_wake_mask(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
 				ACPI_GPE_DISABLE);
 
-		if (device_may_wakeup(&dev->dev))
+		if (acpi_device_may_wakeup(dev))
 			acpi_disable_wakeup_device_power(dev);
 	}
 }
-- 
1.7.2.5

Re: [PATCH] ACPI / WAKEUP : enable wakeup power for physical child devices

[Rafael J. Wysocki]

On Sunday, November 16, 2014 03:10:01 PM Andrey Skvortsov wrote:
> commit f2b56bc808addb908a5bf435d9b942c02af9a7c4
> ("ACPI / PM: Use device wakeup flags for handling ACPI wakeup devices")

That commit is from 2011.  Can you please tell me what makes you think
that it is the source of the problem?

> broke wake-on-lan for Broadcom BCM4401 Ethernet card (b44) on Dell Vostro 1500 laptop.
> device_may_wakeup for main ACPI device (PCIE) returns false, because it has
> can_wakeup = 0. Therefore any physical devices connected to this parent
> were not prepared for wakeup/suspend. But physical device is capable to
> wakeup the system.

If that is the case, then ->

> To fix this issue device_may_wakeup was replaced with acpi_device_may_wakeup.
> acpi_device_may_wakeup was written based on function
> acpi_system_wakeup_device_seq_show
> 
> Signed-off-by: Andrey Skvortsov <Andrej.Skvortzov@gmail.com>
> ---
>  drivers/acpi/wakeup.c |   45 +++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> index 1638401..0da7e70 100644
> --- a/drivers/acpi/wakeup.c
> +++ b/drivers/acpi/wakeup.c
> @@ -19,6 +19,34 @@
>  #define _COMPONENT		ACPI_SYSTEM_COMPONENT
>  ACPI_MODULE_NAME("wakeup_devices")
>  
> +
> +bool acpi_device_may_wakeup(struct acpi_device *dev)
> +{
> +	struct acpi_device_physical_node *entry;
> +	bool may_wakeup = false;
> +
> +	mutex_lock(&dev->physical_node_lock);
> +	if (!dev->physical_node_count)
> +		may_wakeup = device_may_wakeup(&dev->dev);
> +	else {
> +		struct device *ldev;
> +
> +		list_for_each_entry(entry, &dev->physical_node_list, node) {
> +			ldev = get_device(entry->dev);
> +			if (!ldev)
> +				continue;
> +
> +			may_wakeup = device_may_wakeup(&dev->dev) ||
> +				device_may_wakeup(ldev);
> +
> +			put_device(ldev);
> +		}
> +	}
> +
> +	mutex_unlock(&dev->physical_node_lock);
> +	return may_wakeup;
> +}
> +
>  /**
>   * acpi_enable_wakeup_devices - Enable wake-up device GPEs.
>   * @sleep_state: ACPI system sleep state.
> @@ -35,13 +63,14 @@ void acpi_enable_wakeup_devices(u8 sleep_state)
>  		struct acpi_device *dev =
>  			container_of(node, struct acpi_device, wakeup_list);
>  
> +
>  		if (!dev->wakeup.flags.valid
> -		    || sleep_state > (u32) dev->wakeup.sleep_state
> -		    || !(device_may_wakeup(&dev->dev)
> -		        || dev->wakeup.prepare_count))
> +			|| sleep_state > (u32) dev->wakeup.sleep_state
> +			|| !(acpi_device_may_wakeup(dev)
> +			    || dev->wakeup.prepare_count))

-> dev->wakeup.prepare_count here shold be different from 0 at this point.

>  			continue;
>  
> -		if (device_may_wakeup(&dev->dev))
> +		if (acpi_device_may_wakeup(dev))
>  			acpi_enable_wakeup_device_power(dev, sleep_state);
>  
>  		/* The wake-up power should have been enabled already. */
> @@ -63,15 +92,15 @@ void acpi_disable_wakeup_devices(u8 sleep_state)
>  			container_of(node, struct acpi_device, wakeup_list);
>  
>  		if (!dev->wakeup.flags.valid
> -		    || sleep_state > (u32) dev->wakeup.sleep_state
> -		    || !(device_may_wakeup(&dev->dev)
> -		        || dev->wakeup.prepare_count))
> +			|| sleep_state > (u32) dev->wakeup.sleep_state
> +			|| !(acpi_device_may_wakeup(dev)
> +				|| dev->wakeup.prepare_count))
>  			continue;
>  
>  		acpi_set_gpe_wake_mask(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
>  				ACPI_GPE_DISABLE);
>  
> -		if (device_may_wakeup(&dev->dev))
> +		if (acpi_device_may_wakeup(dev))
>  			acpi_disable_wakeup_device_power(dev);
>  	}
>  }

This is not the right fix.  The device_may_wakeup(&dev->dev) check in the
above code is specifically for ACPI device objects that do *not* have physical
devices assiciated with them (buttons etc.).

I'll have a look at the driver in question next week (please remind me in
case I get distracted).


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] ACPI / WAKEUP : enable wakeup power for physical child devices

[Andrey Skvortsov]

On Mon, Dec 01, 2014 at 02:15:36AM +0100, Rafael J. Wysocki wrote:

> > > 
> > > -> dev->wakeup.prepare_count here shold be different from 0 at this point.
> > 
> > Thank you very much for the tip. I checked prepare_count, it was 0.
> > I tried fix that and patched ssb and b44 drivers, that are used for my Ethernet card.
> > The code is below. Is this a better way to solve this problem? 
> 
> Having a cursory look at the changes only, I think so.

Thank you, then I will format the patch in a proper way and submit it.

> > diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
> > index 416620f..ebac411 100644
> > --- a/drivers/net/ethernet/broadcom/b44.c
> > +++ b/drivers/net/ethernet/broadcom/b44.c
> > @@ -2103,7 +2103,9 @@ static int b44_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> >  	else
> >  		bp->flags &= ~B44_FLAG_WOL_ENABLE;

-- 
Best regards,
Andrey Skvortsov

PGP Key ID: 0x57A3AEAD