[RFC PATCH v2 0/8] ACPI D3Cold state and SATA ZPODD support

[RFC PATCH v2 0/8] ACPI D3Cold state and SATA ZPODD support

[Lin Ming]

Hi all,

This is the v2 RFC patches to add ACPI D3Cold state and SATA ZPODD support.

v2:
- _PR3 indicates D3Cold support
- move can_power_off flag to pm_subsys_data
- allow all combinations of power resource and device
- split patches into smaller ones to make review easy

v1:
https://lkml.org/lkml/2012/2/13/86

[PATCH 1/8] ACPI: Introduce ACPI D3_COLD state support
[PATCH 2/8] ACPI: Add interface to register/unregister device to/from power resources
[PATCH 3/8] PCI: Move acpi_dev_run_wake to acpi core
[PATCH 4/8] libata-acpi: set acpi state for SATA port
[PATCH 5/8] libata-acpi: add ata port runtime D3Cold support
[PATCH 6/8] libata-acpi: register/unregister device to/from power resource
[PATCH 7/8] PM / Runtime: Add can_power_off flag to subsys data
[PATCH 8/8] [SCSI] sr: check and enable Zero-power ODD support

 drivers/acpi/power.c       |  166 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/acpi/scan.c        |    7 ++
 drivers/acpi/sleep.c       |   35 +++++++++
 drivers/ata/libata-acpi.c  |  137 +++++++++++++++++++++++++++++++++----
 drivers/ata/libata-scsi.c  |    6 +-
 drivers/ata/libata.h       |    8 +-
 drivers/pci/pci-acpi.c     |   40 +----------
 drivers/scsi/scsi_pm.c     |    8 ++
 drivers/scsi/sr.c          |   46 ++++++++++++
 drivers/scsi/sr.h          |    3 +
 include/acpi/acpi_bus.h    |    7 ++
 include/linux/pm.h         |    1 +
 include/scsi/scsi_device.h |    2 +
 13 files changed, 407 insertions(+), 59 deletions(-)

Thanks for any comment.
Lin Ming

[RFC PATCH v2 1/8] ACPI: Introduce ACPI D3_COLD state support

[Lin Ming]

From: Zhang Rui <rui.zhang@intel.com>

If a device has _PR3, it means the device supports D3_COLD.
Add the ability to validate and enter D3_COLD state in ACPI.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/acpi/power.c |    4 ++--
 drivers/acpi/scan.c  |    7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 9ac2a9f..0d681fb 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -500,14 +500,14 @@ int acpi_power_transition(struct acpi_device *device, int state)
 {
 	int result;
 
-	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
+	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD))
 		return -EINVAL;
 
 	if (device->power.state == state)
 		return 0;
 
 	if ((device->power.state < ACPI_STATE_D0)
-	    || (device->power.state > ACPI_STATE_D3))
+	    || (device->power.state > ACPI_STATE_D3_COLD))
 		return -ENODEV;
 
 	/* TBD: Resources must be ordered. */
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 8ab80ba..571396c 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -885,6 +885,13 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
 				acpi_bus_add_power_resource(ps->resources.handles[j]);
 		}
 
+		/* The exist of _PR3 indicates D3Cold support */
+		if (i == ACPI_STATE_D3) {
+			status = acpi_get_handle(device->handle, object_name, &handle);
+			if (ACPI_SUCCESS(status))
+				device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1;
+		}
+
 		/* Evaluate "_PSx" to see if we can do explicit sets */
 		object_name[2] = 'S';
 		status = acpi_get_handle(device->handle, object_name, &handle);
-- 
1.7.2.5

Re: [RFC PATCH v2 1/8] ACPI: Introduce ACPI D3_COLD state support

[Aaron Lu]

Hi,

On Thu, Mar 01, 2012 at 05:02:50PM +0800, Lin Ming wrote:
> From: Zhang Rui <rui.zhang@intel.com>
> 
> If a device has _PR3, it means the device supports D3_COLD.

This confused me...So this means if a device is put to D3, instead of
turning up the power resources referenced in _PR3, turning them all off
will make the device in D3 cold. This is understandable, but what about
_PR0? What power state this device is in when I turned off all the power
resources referenced in _PR0? Undefined?

I'm also confused with _PS3, what power state the device is in after its
_PS3 is executed? The spec said it will be in D3 cold or D3 hot, so I
guess there is no way to tell, right?

Thanks,
Aaron

> Add the ability to validate and enter D3_COLD state in ACPI.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  drivers/acpi/power.c |    4 ++--
>  drivers/acpi/scan.c  |    7 +++++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> index 9ac2a9f..0d681fb 100644
> --- a/drivers/acpi/power.c
> +++ b/drivers/acpi/power.c
> @@ -500,14 +500,14 @@ int acpi_power_transition(struct acpi_device *device, int state)
>  {
>  	int result;
>  
> -	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
> +	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD))
>  		return -EINVAL;
>  
>  	if (device->power.state == state)
>  		return 0;
>  
>  	if ((device->power.state < ACPI_STATE_D0)
> -	    || (device->power.state > ACPI_STATE_D3))
> +	    || (device->power.state > ACPI_STATE_D3_COLD))
>  		return -ENODEV;
>  
>  	/* TBD: Resources must be ordered. */
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 8ab80ba..571396c 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -885,6 +885,13 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
>  				acpi_bus_add_power_resource(ps->resources.handles[j]);
>  		}
>  
> +		/* The exist of _PR3 indicates D3Cold support */
> +		if (i == ACPI_STATE_D3) {
> +			status = acpi_get_handle(device->handle, object_name, &handle);
> +			if (ACPI_SUCCESS(status))
> +				device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1;
> +		}
> +
>  		/* Evaluate "_PSx" to see if we can do explicit sets */
>  		object_name[2] = 'S';
>  		status = acpi_get_handle(device->handle, object_name, &handle);
> -- 
> 1.7.2.5
> 
> 

Re: [RFC PATCH v2 1/8] ACPI: Introduce ACPI D3_COLD state support

[Lin Ming]

On Mon, 2012-03-12 at 10:00 +0800, Aaron Lu wrote:
> Hi,
> 
> On Thu, Mar 01, 2012 at 05:02:50PM +0800, Lin Ming wrote:
> > From: Zhang Rui <rui.zhang@intel.com>
> > 
> > If a device has _PR3, it means the device supports D3_COLD.
> 
> This confused me...So this means if a device is put to D3, instead of
> turning up the power resources referenced in _PR3, turning them all off
> will make the device in D3 cold. This is understandable, but what about
> _PR0? What power state this device is in when I turned off all the power
> resources referenced in _PR0? Undefined?

I understand your confusion...and even more confused because _PR0 and
_PR3 usually share the same power resource.

We are still contacting with the ACPI spec authors to get a clear
understanding of the D3/D3Hot/D3Cold definition.

> 
> I'm also confused with _PS3, what power state the device is in after its
> _PS3 is executed? The spec said it will be in D3 cold or D3 hot, so I
> guess there is no way to tell, right?

Yes,

The spec says,

"This control method is used to put the specific device into its D3hot
or D3 state"

I'm not sure what "D3 state" here means.
Probably not "D3 cold".

Lin Ming 

> 
> Thanks,
> Aaron
> 
> > Add the ability to validate and enter D3_COLD state in ACPI.
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> > ---
> >  drivers/acpi/power.c |    4 ++--
> >  drivers/acpi/scan.c  |    7 +++++++
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> > index 9ac2a9f..0d681fb 100644
> > --- a/drivers/acpi/power.c
> > +++ b/drivers/acpi/power.c
> > @@ -500,14 +500,14 @@ int acpi_power_transition(struct acpi_device *device, int state)
> >  {
> >  	int result;
> >  
> > -	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
> > +	if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD))
> >  		return -EINVAL;
> >  
> >  	if (device->power.state == state)
> >  		return 0;
> >  
> >  	if ((device->power.state < ACPI_STATE_D0)
> > -	    || (device->power.state > ACPI_STATE_D3))
> > +	    || (device->power.state > ACPI_STATE_D3_COLD))
> >  		return -ENODEV;
> >  
> >  	/* TBD: Resources must be ordered. */
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 8ab80ba..571396c 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -885,6 +885,13 @@ static int acpi_bus_get_power_flags(struct acpi_device *device)
> >  				acpi_bus_add_power_resource(ps->resources.handles[j]);
> >  		}
> >  
> > +		/* The exist of _PR3 indicates D3Cold support */
> > +		if (i == ACPI_STATE_D3) {
> > +			status = acpi_get_handle(device->handle, object_name, &handle);
> > +			if (ACPI_SUCCESS(status))
> > +				device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1;
> > +		}
> > +
> >  		/* Evaluate "_PSx" to see if we can do explicit sets */
> >  		object_name[2] = 'S';
> >  		status = acpi_get_handle(device->handle, object_name, &handle);
> > -- 
> > 1.7.2.5
> > 
> > 
> 

[RFC PATCH v2 2/8] ACPI: Add interface to register/unregister device to/from power resources

[Lin Ming]

Devices may share same list of power resources in _PR0, for example

Device(Dev0)
{
	Name (_PR0, Package (0x01)
	{
		P0PR,
		P1PR
	})
}

Device(Dev1)
{
	Name (_PR0, Package (0x01)
	{
		P0PR,
		P1PR
	}
}

Assume Dev0 and Dev1 were runtime suspended.
Then Dev0 is resumed first and it goes into D0 state.
But Dev1 is left in D0_Uninitialised state.

This is wrong. In this case, Dev1 must be resumed too.

In order to hand this case, each power resource maintains a list of
devices which relies on it.

When power resource is ON, it will check if the devices on its list
can be resumed. The device can only be resumed when all the power
resouces of its _PR0 are ON.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/acpi/power.c    |  162 +++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |    2 +
 2 files changed, 164 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 0d681fb..7049a7d 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -40,9 +40,11 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include "sleep.h"
+#include "internal.h"
 
 #define PREFIX "ACPI: "
 
@@ -77,6 +79,20 @@ static struct acpi_driver acpi_power_driver = {
 		},
 };
 
+/*
+ * A power managed device
+ * A device may rely on multiple power resources.
+ * */
+struct acpi_power_managed_device {
+	struct device *dev; /* The physical device */
+	acpi_handle *handle;
+};
+
+struct acpi_power_resource_device {
+	struct acpi_power_managed_device *device;
+	struct acpi_power_resource_device *next;
+};
+
 struct acpi_power_resource {
 	struct acpi_device * device;
 	acpi_bus_id name;
@@ -84,6 +100,9 @@ struct acpi_power_resource {
 	u32 order;
 	unsigned int ref_count;
 	struct mutex resource_lock;
+
+	/* List of devices relying on this power resource */
+	struct acpi_power_resource_device *devices;
 };
 
 static struct list_head acpi_power_resource_list;
@@ -183,8 +202,26 @@ static int acpi_power_get_list_state(struct acpi_handle_list *list, int *state)
 	return 0;
 }
 
+/* Resume the device when all power resources in _PR0 are on */
+static void acpi_power_on_device(struct acpi_power_managed_device *device)
+{
+	struct acpi_device *acpi_dev;
+	acpi_handle handle = device->handle;
+	int state;
+
+	if (acpi_bus_get_device(handle, &acpi_dev))
+		return;
+
+	if(acpi_power_get_inferred_state(acpi_dev, &state))
+		return;
+
+	if (state == ACPI_STATE_D0 && pm_runtime_suspended(device->dev))
+		pm_request_resume(device->dev);
+}
+
 static int __acpi_power_on(struct acpi_power_resource *resource)
 {
+	struct acpi_power_resource_device *device_list = resource->devices;
 	acpi_status status = AE_OK;
 
 	status = acpi_evaluate_object(resource->device->handle, "_ON", NULL, NULL);
@@ -197,6 +234,12 @@ static int __acpi_power_on(struct acpi_power_resource *resource)
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Power resource [%s] turned on\n",
 			  resource->name));
 
+	while (device_list) {
+		acpi_power_on_device(device_list->device);
+
+		device_list = device_list->next;
+	}
+
 	return 0;
 }
 
@@ -299,6 +342,125 @@ static int acpi_power_on_list(struct acpi_handle_list *list)
 	return result;
 }
 
+static void __acpi_power_resource_unregister_device(struct device *dev,
+		acpi_handle res_handle)
+{
+	struct acpi_power_resource *resource = NULL;
+	struct acpi_power_resource_device *prev, *curr;
+
+	if (acpi_power_get_context(res_handle, &resource))
+		return;
+
+	mutex_lock(&resource->resource_lock);
+	prev = NULL;
+	curr = resource->devices;
+	while (curr) {
+		if (curr->device->dev == dev) {
+			if (!prev)
+				resource->devices = curr->next;
+			else
+				prev->next = curr->next;
+
+			kfree(curr);
+			break;
+		}
+
+		prev = curr;
+		curr = curr->next;
+	}
+	mutex_unlock(&resource->resource_lock);
+}
+
+/* Unlink dev from all power resources in _PR0 */
+void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_handle_list *list;
+	int i;
+
+	if (!dev || !handle)
+		return;
+
+	if (acpi_bus_get_device(handle, &acpi_dev))
+		return;
+
+	list = &acpi_dev->power.states[ACPI_STATE_D0].resources;
+
+	for (i = 0; i < list->count; i++)
+		__acpi_power_resource_unregister_device(dev,
+			list->handles[i]);
+}
+
+static int __acpi_power_resource_register_device(
+	struct acpi_power_managed_device *powered_device, acpi_handle handle)
+{
+	struct acpi_power_resource *resource = NULL;
+	struct acpi_power_resource_device *power_resource_device;
+	int result;
+
+	result = acpi_power_get_context(handle, &resource);
+	if (result)
+		return result;
+
+	power_resource_device = kzalloc(
+		sizeof(*power_resource_device), GFP_KERNEL);
+	if (!power_resource_device)
+		return -ENOMEM;
+
+	power_resource_device->device = powered_device;
+
+	mutex_lock(&resource->resource_lock);
+	power_resource_device->next = resource->devices;
+	resource->devices = power_resource_device;
+	mutex_unlock(&resource->resource_lock);
+
+	return 0;
+}
+
+/* Link dev to all power resources in _PR0 */
+int acpi_power_resource_register_device(struct device *dev, acpi_handle handle)
+{
+	struct acpi_device *acpi_dev;
+	struct acpi_handle_list *list;
+	struct acpi_power_managed_device *powered_device;
+	int i, ret;
+
+	if (!dev || !handle)
+		return -ENODEV;
+
+	ret = acpi_bus_get_device(handle, &acpi_dev);
+	if (ret)
+		goto no_power_resource;
+
+	if (!acpi_dev->power.flags.power_resources)
+		goto no_power_resource;
+
+	powered_device = kzalloc(sizeof(*powered_device), GFP_KERNEL);
+	if (!powered_device)
+		return -ENOMEM;
+
+	powered_device->dev = dev;
+	powered_device->handle = handle;
+
+	list = &acpi_dev->power.states[ACPI_STATE_D0].resources;
+
+	for (i = 0; i < list->count; i++) {
+		ret = __acpi_power_resource_register_device(powered_device,
+			list->handles[i]);
+
+		if (ret) {
+			acpi_power_resource_unregister_device(dev, handle);
+			break;
+		}
+	}
+
+	return ret;
+
+no_power_resource:
+	printk(KERN_WARNING PREFIX "Invalid Power Resource to register!");
+	return -ENODEV;
+}
+
 /**
  * acpi_device_sleep_wake - execute _DSW (Device Sleep Wake) or (deprecated in
  *                          ACPI 3.0) _PSW (Power State Wake)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 6cd5b64..e168fff 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -323,6 +323,8 @@ int acpi_bus_set_power(acpi_handle handle, int state);
 int acpi_bus_update_power(acpi_handle handle, int *state_p);
 bool acpi_bus_power_manageable(acpi_handle handle);
 bool acpi_bus_can_wakeup(acpi_handle handle);
+int acpi_power_resource_register_device(struct device *dev, acpi_handle handle);
+void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle);
 #ifdef CONFIG_ACPI_PROC_EVENT
 int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
 int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);
-- 
1.7.2.5

Re: [RFC PATCH v2 2/8] ACPI: Add interface to register/unregister device to/from power resources

[Lin Ming]

On Thu, Mar 1, 2012 at 5:02 PM, Lin Ming <ming.m.lin@intel.com> wrote:
> Devices may share same list of power resources in _PR0, for example
>
> Device(Dev0)
> {
>        Name (_PR0, Package (0x01)
>        {
>                P0PR,
>                P1PR
>        })
> }
>
> Device(Dev1)
> {
>        Name (_PR0, Package (0x01)
>        {
>                P0PR,
>                P1PR
>        }
> }
>
> Assume Dev0 and Dev1 were runtime suspended.
> Then Dev0 is resumed first and it goes into D0 state.
> But Dev1 is left in D0_Uninitialised state.
>
> This is wrong. In this case, Dev1 must be resumed too.
>
> In order to hand this case, each power resource maintains a list of
> devices which relies on it.
>
> When power resource is ON, it will check if the devices on its list
> can be resumed. The device can only be resumed when all the power
> resouces of its _PR0 are ON.
>
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>

Hi Rafael,

This version addressed your comment at:
https://lkml.org/lkml/2012/2/13/355

Now we support all combinations of device/power resource.

Do you like it?

Thanks,
Lin Ming

> ---
>  drivers/acpi/power.c    |  162 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h |    2 +
>  2 files changed, 164 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> index 0d681fb..7049a7d 100644
> --- a/drivers/acpi/power.c
> +++ b/drivers/acpi/power.c
> @@ -40,9 +40,11 @@
>  #include <linux/init.h>
>  #include <linux/types.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
>  #include "sleep.h"
> +#include "internal.h"
>
>  #define PREFIX "ACPI: "
>
> @@ -77,6 +79,20 @@ static struct acpi_driver acpi_power_driver = {
>                },
>  };
>
> +/*
> + * A power managed device
> + * A device may rely on multiple power resources.
> + * */
> +struct acpi_power_managed_device {
> +       struct device *dev; /* The physical device */
> +       acpi_handle *handle;
> +};
> +
> +struct acpi_power_resource_device {
> +       struct acpi_power_managed_device *device;
> +       struct acpi_power_resource_device *next;
> +};
> +
>  struct acpi_power_resource {
>        struct acpi_device * device;
>        acpi_bus_id name;
> @@ -84,6 +100,9 @@ struct acpi_power_resource {
>        u32 order;
>        unsigned int ref_count;
>        struct mutex resource_lock;
> +
> +       /* List of devices relying on this power resource */
> +       struct acpi_power_resource_device *devices;
>  };
>
>  static struct list_head acpi_power_resource_list;
> @@ -183,8 +202,26 @@ static int acpi_power_get_list_state(struct acpi_handle_list *list, int *state)
>        return 0;
>  }
>
> +/* Resume the device when all power resources in _PR0 are on */
> +static void acpi_power_on_device(struct acpi_power_managed_device *device)
> +{
> +       struct acpi_device *acpi_dev;
> +       acpi_handle handle = device->handle;
> +       int state;
> +
> +       if (acpi_bus_get_device(handle, &acpi_dev))
> +               return;
> +
> +       if(acpi_power_get_inferred_state(acpi_dev, &state))
> +               return;
> +
> +       if (state == ACPI_STATE_D0 && pm_runtime_suspended(device->dev))
> +               pm_request_resume(device->dev);
> +}
> +
>  static int __acpi_power_on(struct acpi_power_resource *resource)
>  {
> +       struct acpi_power_resource_device *device_list = resource->devices;
>        acpi_status status = AE_OK;
>
>        status = acpi_evaluate_object(resource->device->handle, "_ON", NULL, NULL);
> @@ -197,6 +234,12 @@ static int __acpi_power_on(struct acpi_power_resource *resource)
>        ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Power resource [%s] turned on\n",
>                          resource->name));
>
> +       while (device_list) {
> +               acpi_power_on_device(device_list->device);
> +
> +               device_list = device_list->next;
> +       }
> +
>        return 0;
>  }
>
> @@ -299,6 +342,125 @@ static int acpi_power_on_list(struct acpi_handle_list *list)
>        return result;
>  }
>
> +static void __acpi_power_resource_unregister_device(struct device *dev,
> +               acpi_handle res_handle)
> +{
> +       struct acpi_power_resource *resource = NULL;
> +       struct acpi_power_resource_device *prev, *curr;
> +
> +       if (acpi_power_get_context(res_handle, &resource))
> +               return;
> +
> +       mutex_lock(&resource->resource_lock);
> +       prev = NULL;
> +       curr = resource->devices;
> +       while (curr) {
> +               if (curr->device->dev == dev) {
> +                       if (!prev)
> +                               resource->devices = curr->next;
> +                       else
> +                               prev->next = curr->next;
> +
> +                       kfree(curr);
> +                       break;
> +               }
> +
> +               prev = curr;
> +               curr = curr->next;
> +       }
> +       mutex_unlock(&resource->resource_lock);
> +}
> +
> +/* Unlink dev from all power resources in _PR0 */
> +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle)
> +{
> +       struct acpi_device *acpi_dev;
> +       struct acpi_handle_list *list;
> +       int i;
> +
> +       if (!dev || !handle)
> +               return;
> +
> +       if (acpi_bus_get_device(handle, &acpi_dev))
> +               return;
> +
> +       list = &acpi_dev->power.states[ACPI_STATE_D0].resources;
> +
> +       for (i = 0; i < list->count; i++)
> +               __acpi_power_resource_unregister_device(dev,
> +                       list->handles[i]);
> +}
> +
> +static int __acpi_power_resource_register_device(
> +       struct acpi_power_managed_device *powered_device, acpi_handle handle)
> +{
> +       struct acpi_power_resource *resource = NULL;
> +       struct acpi_power_resource_device *power_resource_device;
> +       int result;
> +
> +       result = acpi_power_get_context(handle, &resource);
> +       if (result)
> +               return result;
> +
> +       power_resource_device = kzalloc(
> +               sizeof(*power_resource_device), GFP_KERNEL);
> +       if (!power_resource_device)
> +               return -ENOMEM;
> +
> +       power_resource_device->device = powered_device;
> +
> +       mutex_lock(&resource->resource_lock);
> +       power_resource_device->next = resource->devices;
> +       resource->devices = power_resource_device;
> +       mutex_unlock(&resource->resource_lock);
> +
> +       return 0;
> +}
> +
> +/* Link dev to all power resources in _PR0 */
> +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle)
> +{
> +       struct acpi_device *acpi_dev;
> +       struct acpi_handle_list *list;
> +       struct acpi_power_managed_device *powered_device;
> +       int i, ret;
> +
> +       if (!dev || !handle)
> +               return -ENODEV;
> +
> +       ret = acpi_bus_get_device(handle, &acpi_dev);
> +       if (ret)
> +               goto no_power_resource;
> +
> +       if (!acpi_dev->power.flags.power_resources)
> +               goto no_power_resource;
> +
> +       powered_device = kzalloc(sizeof(*powered_device), GFP_KERNEL);
> +       if (!powered_device)
> +               return -ENOMEM;
> +
> +       powered_device->dev = dev;
> +       powered_device->handle = handle;
> +
> +       list = &acpi_dev->power.states[ACPI_STATE_D0].resources;
> +
> +       for (i = 0; i < list->count; i++) {
> +               ret = __acpi_power_resource_register_device(powered_device,
> +                       list->handles[i]);
> +
> +               if (ret) {
> +                       acpi_power_resource_unregister_device(dev, handle);
> +                       break;
> +               }
> +       }
> +
> +       return ret;
> +
> +no_power_resource:
> +       printk(KERN_WARNING PREFIX "Invalid Power Resource to register!");
> +       return -ENODEV;
> +}
> +
>  /**
>  * acpi_device_sleep_wake - execute _DSW (Device Sleep Wake) or (deprecated in
>  *                          ACPI 3.0) _PSW (Power State Wake)
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 6cd5b64..e168fff 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -323,6 +323,8 @@ int acpi_bus_set_power(acpi_handle handle, int state);
>  int acpi_bus_update_power(acpi_handle handle, int *state_p);
>  bool acpi_bus_power_manageable(acpi_handle handle);
>  bool acpi_bus_can_wakeup(acpi_handle handle);
> +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle);
> +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle);
>  #ifdef CONFIG_ACPI_PROC_EVENT
>  int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);
>  int acpi_bus_generate_proc_event4(const char *class, const char *bid, u8 type, int data);
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH v2 3/8] PCI: Move acpi_dev_run_wake to acpi core

[Lin Ming]

acpi_dev_run_wake is a generic function which can be used by
other subsystem too. Rename it to acpi_pm_device_run_wake, to be
consistent with acpi_pm_device_sleep_wake.

Then move it to acpi core.

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/acpi/sleep.c    |   35 +++++++++++++++++++++++++++++++++++
 drivers/pci/pci-acpi.c  |   40 +++-------------------------------------
 include/acpi/acpi_bus.h |    5 +++++
 3 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index ca191ff..00fa664 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -17,6 +17,7 @@
 #include <linux/suspend.h>
 #include <linux/reboot.h>
 #include <linux/acpi.h>
+#include <linux/pm_runtime.h>
 
 #include <asm/io.h>
 
@@ -730,6 +731,40 @@ int acpi_pm_device_sleep_state(struct device *dev, int *d_min_p)
 
 #ifdef CONFIG_PM_SLEEP
 /**
+ * acpi_pm_device_run_wake - Enable/disable wake-up for given device.
+ * @phys_dev: Device to enable/disable the platform to wake-up the system for.
+ * @enable: Whether enable or disable the wake-up functionality.
+ *
+ * Find the ACPI device object corresponding to @pci_dev and try to
+ * enable/disable the GPE associated with it.
+ */
+int acpi_pm_device_run_wake(struct device *phys_dev, bool enable)
+{
+	struct acpi_device *dev;
+	acpi_handle handle;
+
+	if (!device_run_wake(phys_dev))
+		return -EINVAL;
+
+	handle = DEVICE_ACPI_HANDLE(phys_dev);
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
+		dev_dbg(phys_dev, "ACPI handle has no context in %s!\n",
+			__func__);
+		return -ENODEV;
+	}
+
+	if (enable) {
+		acpi_enable_wakeup_device_power(dev, ACPI_STATE_S0);
+		acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
+	} else {
+		acpi_disable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
+		acpi_disable_wakeup_device_power(dev);
+	}
+
+	return 0;
+}
+
+/**
  *	acpi_pm_device_sleep_wake - enable or disable the system wake-up
  *                                  capability of given device
  *	@dev: device to handle
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 060fd22..0f150f2 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -277,40 +277,6 @@ static int acpi_pci_sleep_wake(struct pci_dev *dev, bool enable)
 	return 0;
 }
 
-/**
- * acpi_dev_run_wake - Enable/disable wake-up for given device.
- * @phys_dev: Device to enable/disable the platform to wake-up the system for.
- * @enable: Whether enable or disable the wake-up functionality.
- *
- * Find the ACPI device object corresponding to @pci_dev and try to
- * enable/disable the GPE associated with it.
- */
-static int acpi_dev_run_wake(struct device *phys_dev, bool enable)
-{
-	struct acpi_device *dev;
-	acpi_handle handle;
-
-	if (!device_run_wake(phys_dev))
-		return -EINVAL;
-
-	handle = DEVICE_ACPI_HANDLE(phys_dev);
-	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
-		dev_dbg(phys_dev, "ACPI handle has no context in %s!\n",
-			__func__);
-		return -ENODEV;
-	}
-
-	if (enable) {
-		acpi_enable_wakeup_device_power(dev, ACPI_STATE_S0);
-		acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
-	} else {
-		acpi_disable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number);
-		acpi_disable_wakeup_device_power(dev);
-	}
-
-	return 0;
-}
-
 static void acpi_pci_propagate_run_wake(struct pci_bus *bus, bool enable)
 {
 	while (bus->parent) {
@@ -318,14 +284,14 @@ static void acpi_pci_propagate_run_wake(struct pci_bus *bus, bool enable)
 
 		if (bridge->pme_interrupt)
 			return;
-		if (!acpi_dev_run_wake(&bridge->dev, enable))
+		if (!acpi_pm_device_run_wake(&bridge->dev, enable))
 			return;
 		bus = bus->parent;
 	}
 
 	/* We have reached the root bus. */
 	if (bus->bridge)
-		acpi_dev_run_wake(bus->bridge, enable);
+		acpi_pm_device_run_wake(bus->bridge, enable);
 }
 
 static int acpi_pci_run_wake(struct pci_dev *dev, bool enable)
@@ -333,7 +299,7 @@ static int acpi_pci_run_wake(struct pci_dev *dev, bool enable)
 	if (dev->pme_interrupt)
 		return 0;
 
-	if (!acpi_dev_run_wake(&dev->dev, enable))
+	if (!acpi_pm_device_run_wake(&dev->dev, enable))
 		return 0;
 
 	acpi_pci_propagate_run_wake(dev->bus, enable);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e168fff..f1c8ca6 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -394,8 +394,13 @@ static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
 #endif
 
 #ifdef CONFIG_PM_SLEEP
+int acpi_pm_device_run_wake(struct device *, bool);
 int acpi_pm_device_sleep_wake(struct device *, bool);
 #else
+static inline int acpi_pm_device_run_wake(struct device *dev, bool enable)
+{
+	return -ENODEV;
+}
 static inline int acpi_pm_device_sleep_wake(struct device *dev, bool enable)
 {
 	return -ENODEV;
-- 
1.7.2.5

[RFC PATCH v2 4/8] libata-acpi: set acpi state for SATA port

[Lin Ming]

Currently, ata_acpi_set_state() only sets acpi sate for IDE port.
Remove this limitation.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/ata/libata-acpi.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index b03e468..104c1d0 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -841,23 +841,26 @@ void ata_acpi_on_resume(struct ata_port *ap)
 void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
 {
 	struct ata_device *dev;
-
-	if (!ata_ap_acpi_handle(ap) || (ap->flags & ATA_FLAG_ACPI_SATA))
-		return;
+	acpi_handle handle;
+	int acpi_state;
 
 	/* channel first and then drives for power on and vica versa
 	   for power off */
-	if (state.event == PM_EVENT_ON)
-		acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D0);
+	handle = ata_ap_acpi_handle(ap);
+	if (handle && state.event == PM_EVENT_ON)
+		acpi_bus_set_power(handle, ACPI_STATE_D0);
 
 	ata_for_each_dev(dev, &ap->link, ENABLED) {
-		if (ata_dev_acpi_handle(dev))
-			acpi_bus_set_power(ata_dev_acpi_handle(dev),
+		handle = ata_dev_acpi_handle(dev);
+		if (handle)
+			acpi_bus_set_power(handle,
 				state.event == PM_EVENT_ON ?
 					ACPI_STATE_D0 : ACPI_STATE_D3);
 	}
-	if (state.event != PM_EVENT_ON)
-		acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D3);
+
+	handle = ata_ap_acpi_handle(ap);
+	if (handle && state.event != PM_EVENT_ON)
+		acpi_bus_set_power(handle, ACPI_STATE_D3);
 }
 
 /**
-- 
1.7.2.5

Re: [RFC PATCH v2 4/8] libata-acpi: set acpi state for SATA port

[Aaron Lu]

Hi,

On Thu, Mar 01, 2012 at 05:02:53PM +0800, Lin Ming wrote:
> Currently, ata_acpi_set_state() only sets acpi sate for IDE port.
> Remove this limitation.
> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  drivers/ata/libata-acpi.c |   21 ++++++++++++---------
>  1 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index b03e468..104c1d0 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -841,23 +841,26 @@ void ata_acpi_on_resume(struct ata_port *ap)
>  void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
>  {
>  	struct ata_device *dev;
> -
> -	if (!ata_ap_acpi_handle(ap) || (ap->flags & ATA_FLAG_ACPI_SATA))
> -		return;
> +	acpi_handle handle;
> +	int acpi_state;
>  
>  	/* channel first and then drives for power on and vica versa
>  	   for power off */
> -	if (state.event == PM_EVENT_ON)
> -		acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D0);
> +	handle = ata_ap_acpi_handle(ap);
> +	if (handle && state.event == PM_EVENT_ON)
> +		acpi_bus_set_power(handle, ACPI_STATE_D0);
>  
>  	ata_for_each_dev(dev, &ap->link, ENABLED) {
> -		if (ata_dev_acpi_handle(dev))
> -			acpi_bus_set_power(ata_dev_acpi_handle(dev),
> +		handle = ata_dev_acpi_handle(dev);
> +		if (handle)
> +			acpi_bus_set_power(handle,
>  				state.event == PM_EVENT_ON ?
>  					ACPI_STATE_D0 : ACPI_STATE_D3);
>  	}
> -	if (state.event != PM_EVENT_ON)
> -		acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D3);
> +
> +	handle = ata_ap_acpi_handle(ap);
> +	if (handle && state.event != PM_EVENT_ON)
> +		acpi_bus_set_power(handle, ACPI_STATE_D3);
>  }
>  
>  /**

Acked-by: Aaron Lu <aaron.lu@amd.com>

> -- 
> 1.7.2.5
> 
> 

[RFC PATCH v2 5/8] libata-acpi: add ata port runtime D3Cold support

[Lin Ming]

ATA port may support runtime D3Cold state, for example, Zero-power ODD case.
This patch adds wakeup notifier and enable/disable run_wake during
supend/resume.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/ata/libata-acpi.c |   82 +++++++++++++++++++++++++++++++++++++++++---
 drivers/ata/libata-scsi.c |    6 ++--
 drivers/ata/libata.h      |    8 ++--
 3 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 104c1d0..c2a4db6 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -16,6 +16,7 @@
 #include <linux/libata.h>
 #include <linux/pci.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
 #include <scsi/scsi_device.h>
 #include "libata.h"
 
@@ -852,10 +853,23 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
 
 	ata_for_each_dev(dev, &ap->link, ENABLED) {
 		handle = ata_dev_acpi_handle(dev);
-		if (handle)
-			acpi_bus_set_power(handle,
-				state.event == PM_EVENT_ON ?
-					ACPI_STATE_D0 : ACPI_STATE_D3);
+		if (!handle)
+			continue;
+
+		if (state.event != PM_EVENT_ON) {
+			acpi_state = acpi_pm_device_sleep_state(
+				&dev->sdev->sdev_gendev, NULL);
+			if (acpi_state > 0)
+				acpi_bus_set_power(handle, acpi_state);
+			if (ap->tdev.power.request == RPM_REQ_SUSPEND)
+				acpi_pm_device_run_wake(
+					&dev->sdev->sdev_gendev, true);
+		} else {
+			if (ap->tdev.power.request == RPM_REQ_RESUME)
+				acpi_pm_device_run_wake(
+					&dev->sdev->sdev_gendev, false);
+			acpi_bus_set_power(handle, ACPI_STATE_D0);
+		}
 	}
 
 	handle = ata_ap_acpi_handle(ap);
@@ -955,7 +969,7 @@ void ata_acpi_on_disable(struct ata_device *dev)
 	ata_acpi_clear_gtf(dev);
 }
 
-void ata_acpi_bind_dock(struct ata_device *dev)
+static void ata_acpi_bind_dock(struct ata_device *dev)
 {
 	struct device **docks;
 
@@ -965,7 +979,7 @@ void ata_acpi_bind_dock(struct ata_device *dev)
 	kfree(docks);
 }
 
-void ata_acpi_unbind_dock(struct ata_device *dev)
+static void ata_acpi_unbind_dock(struct ata_device *dev)
 {
 	struct device **docks;
 
@@ -975,6 +989,62 @@ void ata_acpi_unbind_dock(struct ata_device *dev)
 	kfree(docks);
 }
 
+static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
+{
+	struct ata_device *ata_dev = context;
+
+	if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev)
+		scsi_autopm_get_device(ata_dev->sdev);
+}
+
+static void ata_acpi_add_pm_notifier(struct ata_device *dev)
+{
+	struct acpi_device *acpi_dev;
+	acpi_handle handle;
+	acpi_status status;
+
+	handle = ata_dev_acpi_handle(dev);
+	if (!handle)
+		return;
+
+	status = acpi_bus_get_device(handle, &acpi_dev);
+	if (ACPI_SUCCESS(status) && acpi_dev->wakeup.flags.run_wake) {
+		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+			ata_acpi_wake_dev, dev);
+		device_set_run_wake(&dev->sdev->sdev_gendev, true);
+	}
+}
+
+static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
+{
+	struct acpi_device *acpi_dev;
+	acpi_handle handle;
+	acpi_status status;
+
+	handle = ata_dev_acpi_handle(dev);
+	if (!handle)
+		return;
+
+	status = acpi_bus_get_device(handle, &acpi_dev);
+	if (ACPI_SUCCESS(status) && acpi_dev->wakeup.flags.run_wake) {
+		device_set_run_wake(&dev->sdev->sdev_gendev, false);
+		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+			ata_acpi_wake_dev);
+	}
+}
+
+void ata_acpi_bind(struct ata_device *dev)
+{
+	ata_acpi_bind_dock(dev);
+	ata_acpi_add_pm_notifier(dev);
+}
+
+void ata_acpi_unbind(struct ata_device *dev)
+{
+	ata_acpi_unbind_dock(dev);
+	ata_acpi_remove_pm_notifier(dev);
+}
+
 static int is_pci_ata(struct device *dev)
 {
 	struct pci_dev *pdev;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index f08c447..231c3ec 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3444,7 +3444,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			if (!IS_ERR(sdev)) {
 				dev->sdev = sdev;
 				scsi_device_put(sdev);
-				ata_acpi_bind_dock(dev);
+				ata_acpi_bind(dev);
 			} else {
 				dev->sdev = NULL;
 			}
@@ -3541,12 +3541,12 @@ static void ata_scsi_remove_dev(struct ata_device *dev)
 	mutex_lock(&ap->scsi_host->scan_mutex);
 	spin_lock_irqsave(ap->lock, flags);
 
+	ata_acpi_unbind(dev);
+
 	/* clearing dev->sdev is protected by host lock */
 	sdev = dev->sdev;
 	dev->sdev = NULL;
 
-	ata_acpi_unbind_dock(dev);
-
 	if (sdev) {
 		/* If user initiated unplug races with us, sdev can go
 		 * away underneath us after the host lock and
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index e61ba10..2ec7c38 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -117,8 +117,8 @@ extern void ata_acpi_on_disable(struct ata_device *dev);
 extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state);
 extern int ata_acpi_register(void);
 extern void ata_acpi_unregister(void);
-extern void ata_acpi_bind_dock(struct ata_device *dev);
-extern void ata_acpi_unbind_dock(struct ata_device *dev);
+extern void ata_acpi_bind(struct ata_device *dev);
+extern void ata_acpi_unbind(struct ata_device *dev);
 #else
 static inline void ata_acpi_dissociate(struct ata_host *host) { }
 static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; }
@@ -129,8 +129,8 @@ static inline void ata_acpi_set_state(struct ata_port *ap,
 				      pm_message_t state) { }
 static inline int ata_acpi_register(void) { return 0; }
 static void ata_acpi_unregister(void) { }
-static void ata_acpi_bind_dock(struct ata_device *dev) { }
-static void ata_acpi_unbind_dock(struct ata_device *dev) { }
+static void ata_acpi_bind(struct ata_device *dev) { }
+static void ata_acpi_unbind(struct ata_device *dev) { }
 #endif
 
 /* libata-scsi.c */
-- 
1.7.2.5

Re: [RFC PATCH v2 5/8] libata-acpi: add ata port runtime D3Cold support

[Aaron Lu]

Hi,

On Thu, Mar 01, 2012 at 05:02:54PM +0800, Lin Ming wrote:
> ATA port may support runtime D3Cold state, for example, Zero-power ODD case.
> This patch adds wakeup notifier and enable/disable run_wake during
> supend/resume.

I've been thinking this for some time and realized that it would be
impossible for AMD's platform to work with this patch, the reason:
There is no _PRW in AMD's acpi implementation. And no _PRW would mean
the device is not wake up capable in current Linux ACPI implementation.

I've checked the ACPI spec and it said: 'the _PRW is only required for
devices that have the ability to wake the system from a system sleeping
state.'
So I'm not sure if _PRW fits here, since the most common use case for
zpodd would be: odd put to D3 cold and system is at S0, and odd is back
to D0 when necessary without affecting the system sleep state.

I suggest we install the acpi pm notifier on the handle based on the
following two criteria:
1 This ata device is DA capable;
2 Its acpi device indicates it is able to wake up itself in S0(_S0W
evaluates 4).

Does this make sense and will this work for your platform?

Another problem is how to place this ODD device into D3 cold state,
since our platform uses _PS3 control method to power off it. Do you have
any suggestions?

Thanks,
Aaron

> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  drivers/ata/libata-acpi.c |   82 +++++++++++++++++++++++++++++++++++++++++---
>  drivers/ata/libata-scsi.c |    6 ++--
>  drivers/ata/libata.h      |    8 ++--
>  3 files changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index 104c1d0..c2a4db6 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -16,6 +16,7 @@
>  #include <linux/libata.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  #include <scsi/scsi_device.h>
>  #include "libata.h"
>  
> @@ -852,10 +853,23 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
>  
>  	ata_for_each_dev(dev, &ap->link, ENABLED) {
>  		handle = ata_dev_acpi_handle(dev);
> -		if (handle)
> -			acpi_bus_set_power(handle,
> -				state.event == PM_EVENT_ON ?
> -					ACPI_STATE_D0 : ACPI_STATE_D3);
> +		if (!handle)
> +			continue;
> +
> +		if (state.event != PM_EVENT_ON) {
> +			acpi_state = acpi_pm_device_sleep_state(
> +				&dev->sdev->sdev_gendev, NULL);
> +			if (acpi_state > 0)
> +				acpi_bus_set_power(handle, acpi_state);
> +			if (ap->tdev.power.request == RPM_REQ_SUSPEND)
> +				acpi_pm_device_run_wake(
> +					&dev->sdev->sdev_gendev, true);
> +		} else {
> +			if (ap->tdev.power.request == RPM_REQ_RESUME)
> +				acpi_pm_device_run_wake(
> +					&dev->sdev->sdev_gendev, false);
> +			acpi_bus_set_power(handle, ACPI_STATE_D0);
> +		}
>  	}
>  
>  	handle = ata_ap_acpi_handle(ap);
> @@ -955,7 +969,7 @@ void ata_acpi_on_disable(struct ata_device *dev)
>  	ata_acpi_clear_gtf(dev);
>  }
>  
> -void ata_acpi_bind_dock(struct ata_device *dev)
> +static void ata_acpi_bind_dock(struct ata_device *dev)
>  {
>  	struct device **docks;
>  
> @@ -965,7 +979,7 @@ void ata_acpi_bind_dock(struct ata_device *dev)
>  	kfree(docks);
>  }
>  
> -void ata_acpi_unbind_dock(struct ata_device *dev)
> +static void ata_acpi_unbind_dock(struct ata_device *dev)
>  {
>  	struct device **docks;
>  
> @@ -975,6 +989,62 @@ void ata_acpi_unbind_dock(struct ata_device *dev)
>  	kfree(docks);
>  }
>  
> +static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
> +{
> +	struct ata_device *ata_dev = context;
> +
> +	if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev)
> +		scsi_autopm_get_device(ata_dev->sdev);
> +}
> +
> +static void ata_acpi_add_pm_notifier(struct ata_device *dev)
> +{
> +	struct acpi_device *acpi_dev;
> +	acpi_handle handle;
> +	acpi_status status;
> +
> +	handle = ata_dev_acpi_handle(dev);
> +	if (!handle)
> +		return;
> +
> +	status = acpi_bus_get_device(handle, &acpi_dev);
> +	if (ACPI_SUCCESS(status) && acpi_dev->wakeup.flags.run_wake) {
> +		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> +			ata_acpi_wake_dev, dev);
> +		device_set_run_wake(&dev->sdev->sdev_gendev, true);
> +	}
> +}
> +
> +static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
> +{
> +	struct acpi_device *acpi_dev;
> +	acpi_handle handle;
> +	acpi_status status;
> +
> +	handle = ata_dev_acpi_handle(dev);
> +	if (!handle)
> +		return;
> +
> +	status = acpi_bus_get_device(handle, &acpi_dev);
> +	if (ACPI_SUCCESS(status) && acpi_dev->wakeup.flags.run_wake) {
> +		device_set_run_wake(&dev->sdev->sdev_gendev, false);
> +		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> +			ata_acpi_wake_dev);
> +	}
> +}
> +
> +void ata_acpi_bind(struct ata_device *dev)
> +{
> +	ata_acpi_bind_dock(dev);
> +	ata_acpi_add_pm_notifier(dev);
> +}
> +
> +void ata_acpi_unbind(struct ata_device *dev)
> +{
> +	ata_acpi_unbind_dock(dev);
> +	ata_acpi_remove_pm_notifier(dev);
> +}
> +
>  static int is_pci_ata(struct device *dev)
>  {
>  	struct pci_dev *pdev;
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index f08c447..231c3ec 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3444,7 +3444,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  			if (!IS_ERR(sdev)) {
>  				dev->sdev = sdev;
>  				scsi_device_put(sdev);
> -				ata_acpi_bind_dock(dev);
> +				ata_acpi_bind(dev);
>  			} else {
>  				dev->sdev = NULL;
>  			}
> @@ -3541,12 +3541,12 @@ static void ata_scsi_remove_dev(struct ata_device *dev)
>  	mutex_lock(&ap->scsi_host->scan_mutex);
>  	spin_lock_irqsave(ap->lock, flags);
>  
> +	ata_acpi_unbind(dev);
> +
>  	/* clearing dev->sdev is protected by host lock */
>  	sdev = dev->sdev;
>  	dev->sdev = NULL;
>  
> -	ata_acpi_unbind_dock(dev);
> -
>  	if (sdev) {
>  		/* If user initiated unplug races with us, sdev can go
>  		 * away underneath us after the host lock and
> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> index e61ba10..2ec7c38 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -117,8 +117,8 @@ extern void ata_acpi_on_disable(struct ata_device *dev);
>  extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state);
>  extern int ata_acpi_register(void);
>  extern void ata_acpi_unregister(void);
> -extern void ata_acpi_bind_dock(struct ata_device *dev);
> -extern void ata_acpi_unbind_dock(struct ata_device *dev);
> +extern void ata_acpi_bind(struct ata_device *dev);
> +extern void ata_acpi_unbind(struct ata_device *dev);
>  #else
>  static inline void ata_acpi_dissociate(struct ata_host *host) { }
>  static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; }
> @@ -129,8 +129,8 @@ static inline void ata_acpi_set_state(struct ata_port *ap,
>  				      pm_message_t state) { }
>  static inline int ata_acpi_register(void) { return 0; }
>  static void ata_acpi_unregister(void) { }
> -static void ata_acpi_bind_dock(struct ata_device *dev) { }
> -static void ata_acpi_unbind_dock(struct ata_device *dev) { }
> +static void ata_acpi_bind(struct ata_device *dev) { }
> +static void ata_acpi_unbind(struct ata_device *dev) { }
>  #endif
>  
>  /* libata-scsi.c */
> -- 
> 1.7.2.5
> 
> 

Re: [RFC PATCH v2 5/8] libata-acpi: add ata port runtime D3Cold support

[Lin Ming]

On Mon, 2012-03-19 at 11:36 +0800, Aaron Lu wrote:
> Hi,
> 
> On Thu, Mar 01, 2012 at 05:02:54PM +0800, Lin Ming wrote:
> > ATA port may support runtime D3Cold state, for example, Zero-power ODD case.
> > This patch adds wakeup notifier and enable/disable run_wake during
> > supend/resume.
> 
> I've been thinking this for some time and realized that it would be
> impossible for AMD's platform to work with this patch, the reason:
> There is no _PRW in AMD's acpi implementation. And no _PRW would mean
> the device is not wake up capable in current Linux ACPI implementation.
> 
> I've checked the ACPI spec and it said: 'the _PRW is only required for
> devices that have the ability to wake the system from a system sleeping
> state.'
> So I'm not sure if _PRW fits here, since the most common use case for
> zpodd would be: odd put to D3 cold and system is at S0, and odd is back
> to D0 when necessary without affecting the system sleep state.
> 
> I suggest we install the acpi pm notifier on the handle based on the
> following two criteria:
> 1 This ata device is DA capable;
> 2 Its acpi device indicates it is able to wake up itself in S0(_S0W
> evaluates 4).

OK, reasonable to me.

> 
> Does this make sense and will this work for your platform?

Yes.

> 
> Another problem is how to place this ODD device into D3 cold state,
> since our platform uses _PS3 control method to power off it. Do you have
> any suggestions?

acpi_bus_set_power will execute _PS3 too.

So I think no problem for AMD's platform.

Thanks,
Lin Ming

> 
> Thanks,
> Aaron
> 
> > 
> > Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> > ---
> >  drivers/ata/libata-acpi.c |   82 +++++++++++++++++++++++++++++++++++++++++---
> >  drivers/ata/libata-scsi.c |    6 ++--
> >  drivers/ata/libata.h      |    8 ++--
> >  3 files changed, 83 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> > index 104c1d0..c2a4db6 100644
> > --- a/drivers/ata/libata-acpi.c
> > +++ b/drivers/ata/libata-acpi.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/libata.h>
> >  #include <linux/pci.h>
> >  #include <linux/slab.h>
> > +#include <linux/pm_runtime.h>
> >  #include <scsi/scsi_device.h>
> >  #include "libata.h"
> >  
> > @@ -852,10 +853,23 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
> >  
> >  	ata_for_each_dev(dev, &ap->link, ENABLED) {
> >  		handle = ata_dev_acpi_handle(dev);
> > -		if (handle)
> > -			acpi_bus_set_power(handle,
> > -				state.event == PM_EVENT_ON ?
> > -					ACPI_STATE_D0 : ACPI_STATE_D3);
> > +		if (!handle)
> > +			continue;
> > +
> > +		if (state.event != PM_EVENT_ON) {
> > +			acpi_state = acpi_pm_device_sleep_state(
> > +				&dev->sdev->sdev_gendev, NULL);
> > +			if (acpi_state > 0)
> > +				acpi_bus_set_power(handle, acpi_state);
> > +			if (ap->tdev.power.request == RPM_REQ_SUSPEND)
> > +				acpi_pm_device_run_wake(
> > +					&dev->sdev->sdev_gendev, true);
> > +		} else {
> > +			if (ap->tdev.power.request == RPM_REQ_RESUME)
> > +				acpi_pm_device_run_wake(
> > +					&dev->sdev->sdev_gendev, false);
> > +			acpi_bus_set_power(handle, ACPI_STATE_D0);
> > +		}
> >  	}
> >  
> >  	handle = ata_ap_acpi_handle(ap);
> > @@ -955,7 +969,7 @@ void ata_acpi_on_disable(struct ata_device *dev)
> >  	ata_acpi_clear_gtf(dev);
> >  }
> >  
> > -void ata_acpi_bind_dock(struct ata_device *dev)
> > +static void ata_acpi_bind_dock(struct ata_device *dev)
> >  {
> >  	struct device **docks;
> >  
> > @@ -965,7 +979,7 @@ void ata_acpi_bind_dock(struct ata_device *dev)
> >  	kfree(docks);
> >  }
> >  
> > -void ata_acpi_unbind_dock(struct ata_device *dev)
> > +static void ata_acpi_unbind_dock(struct ata_device *dev)
> >  {
> >  	struct device **docks;
> >  
> > @@ -975,6 +989,62 @@ void ata_acpi_unbind_dock(struct ata_device *dev)
> >  	kfree(docks);
> >  }
> >  
> > +static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
> > +{
> > +	struct ata_device *ata_dev = context;
> > +
> > +	if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev)
> > +		scsi_autopm_get_device(ata_dev->sdev);
> > +}
> > +
> > +static void ata_acpi_add_pm_notifier(struct ata_device *dev)
> > +{
> > +	struct acpi_device *acpi_dev;
> > +	acpi_handle handle;
> > +	acpi_status status;
> > +
> > +	handle = ata_dev_acpi_handle(dev);
> > +	if (!handle)
> > +		return;
> > +
> > +	status = acpi_bus_get_device(handle, &acpi_dev);
> > +	if (ACPI_SUCCESS(status) && acpi_dev->wakeup.flags.run_wake) {
> > +		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> > +			ata_acpi_wake_dev, dev);
> > +		device_set_run_wake(&dev->sdev->sdev_gendev, true);
> > +	}
> > +}
> > +
> > +static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
> > +{
> > +	struct acpi_device *acpi_dev;
> > +	acpi_handle handle;
> > +	acpi_status status;
> > +
> > +	handle = ata_dev_acpi_handle(dev);
> > +	if (!handle)
> > +		return;
> > +
> > +	status = acpi_bus_get_device(handle, &acpi_dev);
> > +	if (ACPI_SUCCESS(status) && acpi_dev->wakeup.flags.run_wake) {
> > +		device_set_run_wake(&dev->sdev->sdev_gendev, false);
> > +		acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
> > +			ata_acpi_wake_dev);
> > +	}
> > +}
> > +
> > +void ata_acpi_bind(struct ata_device *dev)
> > +{
> > +	ata_acpi_bind_dock(dev);
> > +	ata_acpi_add_pm_notifier(dev);
> > +}
> > +
> > +void ata_acpi_unbind(struct ata_device *dev)
> > +{
> > +	ata_acpi_unbind_dock(dev);
> > +	ata_acpi_remove_pm_notifier(dev);
> > +}
> > +
> >  static int is_pci_ata(struct device *dev)
> >  {
> >  	struct pci_dev *pdev;
> > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> > index f08c447..231c3ec 100644
> > --- a/drivers/ata/libata-scsi.c
> > +++ b/drivers/ata/libata-scsi.c
> > @@ -3444,7 +3444,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
> >  			if (!IS_ERR(sdev)) {
> >  				dev->sdev = sdev;
> >  				scsi_device_put(sdev);
> > -				ata_acpi_bind_dock(dev);
> > +				ata_acpi_bind(dev);
> >  			} else {
> >  				dev->sdev = NULL;
> >  			}
> > @@ -3541,12 +3541,12 @@ static void ata_scsi_remove_dev(struct ata_device *dev)
> >  	mutex_lock(&ap->scsi_host->scan_mutex);
> >  	spin_lock_irqsave(ap->lock, flags);
> >  
> > +	ata_acpi_unbind(dev);
> > +
> >  	/* clearing dev->sdev is protected by host lock */
> >  	sdev = dev->sdev;
> >  	dev->sdev = NULL;
> >  
> > -	ata_acpi_unbind_dock(dev);
> > -
> >  	if (sdev) {
> >  		/* If user initiated unplug races with us, sdev can go
> >  		 * away underneath us after the host lock and
> > diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> > index e61ba10..2ec7c38 100644
> > --- a/drivers/ata/libata.h
> > +++ b/drivers/ata/libata.h
> > @@ -117,8 +117,8 @@ extern void ata_acpi_on_disable(struct ata_device *dev);
> >  extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state);
> >  extern int ata_acpi_register(void);
> >  extern void ata_acpi_unregister(void);
> > -extern void ata_acpi_bind_dock(struct ata_device *dev);
> > -extern void ata_acpi_unbind_dock(struct ata_device *dev);
> > +extern void ata_acpi_bind(struct ata_device *dev);
> > +extern void ata_acpi_unbind(struct ata_device *dev);
> >  #else
> >  static inline void ata_acpi_dissociate(struct ata_host *host) { }
> >  static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; }
> > @@ -129,8 +129,8 @@ static inline void ata_acpi_set_state(struct ata_port *ap,
> >  				      pm_message_t state) { }
> >  static inline int ata_acpi_register(void) { return 0; }
> >  static void ata_acpi_unregister(void) { }
> > -static void ata_acpi_bind_dock(struct ata_device *dev) { }
> > -static void ata_acpi_unbind_dock(struct ata_device *dev) { }
> > +static void ata_acpi_bind(struct ata_device *dev) { }
> > +static void ata_acpi_unbind(struct ata_device *dev) { }
> >  #endif
> >  
> >  /* libata-scsi.c */
> > -- 
> > 1.7.2.5
> > 
> > 
> 

Re: [RFC PATCH v2 5/8] libata-acpi: add ata port runtime D3Cold support

[Aaron Lu]

Hi,

On Mon, Mar 19, 2012 at 01:27:00PM +0800, Lin Ming wrote:
> On Mon, 2012-03-19 at 11:36 +0800, Aaron Lu wrote:
> > Hi,
> > 
> > On Thu, Mar 01, 2012 at 05:02:54PM +0800, Lin Ming wrote:
> > > ATA port may support runtime D3Cold state, for example, Zero-power ODD case.
> > > This patch adds wakeup notifier and enable/disable run_wake during
> > > supend/resume.
> > 
> > I've been thinking this for some time and realized that it would be
> > impossible for AMD's platform to work with this patch, the reason:
> > There is no _PRW in AMD's acpi implementation. And no _PRW would mean
> > the device is not wake up capable in current Linux ACPI implementation.
> > 
> > I've checked the ACPI spec and it said: 'the _PRW is only required for
> > devices that have the ability to wake the system from a system sleeping
> > state.'
> > So I'm not sure if _PRW fits here, since the most common use case for
> > zpodd would be: odd put to D3 cold and system is at S0, and odd is back
> > to D0 when necessary without affecting the system sleep state.
> > 
> > I suggest we install the acpi pm notifier on the handle based on the
> > following two criteria:
> > 1 This ata device is DA capable;
> > 2 Its acpi device indicates it is able to wake up itself in S0(_S0W
> > evaluates 4).
> 
> OK, reasonable to me.
> 
> > 
> > Does this make sense and will this work for your platform?
> 
> Yes.
> 

Great ;-)

> > 
> > Another problem is how to place this ODD device into D3 cold state,
> > since our platform uses _PS3 control method to power off it. Do you have
> > any suggestions?
> 
> acpi_bus_set_power will execute _PS3 too.
> 
> So I think no problem for AMD's platform.
> 

Sorry I didn't make it clear.
The problem here is, we are going to set the device power state to D3
cold, and current OSPM has no support for it.

Another patch of yours solved this problem by defining:
1 Device supports D3 cold if it has _PR3;
2 For a device to be put to D3 cold, power off all the power resources
referenced in its _PR3.

Since this can't work for AMD's platform(there is no _PR3 for the sata
acpi device), I would like to change this a little bit:
1 Device supports D3 cold if it has _PR3 or _PS3;
2 For a device to be put to D3 cold, execute _PS3 first if available and
then deal with _PR3 as above.

What do you think of this?
If you are OK to the above idea, I can change the corresponding patch for
you to review.

Thanks,
Aaron

Re: [RFC PATCH v2 5/8] libata-acpi: add ata port runtime D3Cold support

[Lin Ming]

On Mon, 2012-03-19 at 14:35 +0800, Aaron Lu wrote:
> Hi,
> 
> On Mon, Mar 19, 2012 at 01:27:00PM +0800, Lin Ming wrote:
> > On Mon, 2012-03-19 at 11:36 +0800, Aaron Lu wrote:
> > > Hi,
> > > 
> > > On Thu, Mar 01, 2012 at 05:02:54PM +0800, Lin Ming wrote:
> > > > ATA port may support runtime D3Cold state, for example, Zero-power ODD case.
> > > > This patch adds wakeup notifier and enable/disable run_wake during
> > > > supend/resume.
> > > 
> > > I've been thinking this for some time and realized that it would be
> > > impossible for AMD's platform to work with this patch, the reason:
> > > There is no _PRW in AMD's acpi implementation. And no _PRW would mean
> > > the device is not wake up capable in current Linux ACPI implementation.
> > > 
> > > I've checked the ACPI spec and it said: 'the _PRW is only required for
> > > devices that have the ability to wake the system from a system sleeping
> > > state.'
> > > So I'm not sure if _PRW fits here, since the most common use case for
> > > zpodd would be: odd put to D3 cold and system is at S0, and odd is back
> > > to D0 when necessary without affecting the system sleep state.
> > > 
> > > I suggest we install the acpi pm notifier on the handle based on the
> > > following two criteria:
> > > 1 This ata device is DA capable;
> > > 2 Its acpi device indicates it is able to wake up itself in S0(_S0W
> > > evaluates 4).
> > 
> > OK, reasonable to me.
> > 
> > > 
> > > Does this make sense and will this work for your platform?
> > 
> > Yes.
> > 
> 
> Great ;-)
> 
> > > 
> > > Another problem is how to place this ODD device into D3 cold state,
> > > since our platform uses _PS3 control method to power off it. Do you have
> > > any suggestions?
> > 
> > acpi_bus_set_power will execute _PS3 too.
> > 
> > So I think no problem for AMD's platform.
> > 
> 
> Sorry I didn't make it clear.
> The problem here is, we are going to set the device power state to D3
> cold, and current OSPM has no support for it.
> 
> Another patch of yours solved this problem by defining:
> 1 Device supports D3 cold if it has _PR3;
> 2 For a device to be put to D3 cold, power off all the power resources
> referenced in its _PR3.
> 
> Since this can't work for AMD's platform(there is no _PR3 for the sata
> acpi device), I would like to change this a little bit:
> 1 Device supports D3 cold if it has _PR3 or _PS3;

_PS3 may only mean D3Hot support for other device.

You mentioned that AMD platform defined a special device, named ODDZ.

How about device supports D3 cold if

_PR3 or (is ODDZ and ODDZ._PS3)?

> 2 For a device to be put to D3 cold, execute _PS3 first if available and
> then deal with _PR3 as above.

__acpi_bus_set_power(...) has done this.


> 
> What do you think of this?
> If you are OK to the above idea, I can change the corresponding patch for
> you to review.

OK.

Thanks,
Lin Ming

> 
> Thanks,
> Aaron
> 
> 

Re: [RFC PATCH v2 5/8] libata-acpi: add ata port runtime D3Cold support

[Aaron Lu]

On Mon, Mar 19, 2012 at 03:03:40PM +0800, Lin Ming wrote:
> > 
> > Sorry I didn't make it clear.
> > The problem here is, we are going to set the device power state to D3
> > cold, and current OSPM has no support for it.
> > 
> > Another patch of yours solved this problem by defining:
> > 1 Device supports D3 cold if it has _PR3;
> > 2 For a device to be put to D3 cold, power off all the power resources
> > referenced in its _PR3.
> > 
> > Since this can't work for AMD's platform(there is no _PR3 for the sata
> > acpi device), I would like to change this a little bit:
> > 1 Device supports D3 cold if it has _PR3 or _PS3;
> 
> _PS3 may only mean D3Hot support for other device.
> 
> You mentioned that AMD platform defined a special device, named ODDZ.
> 
> How about device supports D3 cold if
> 
> _PR3 or (is ODDZ and ODDZ._PS3)?

Sounds good, I'll do this, thanks.

> 
> > 2 For a device to be put to D3 cold, execute _PS3 first if available and
> > then deal with _PR3 as above.
> 
> __acpi_bus_set_power(...) has done this.
> 

Actually not, current implementation of __acpi_bus_set_power will call
_PS4 ;-)
I'll need to think of a way to handle the D3 cold case.

-Aaron

[RFC PATCH v2 6/8] libata-acpi: register/unregister device to/from power resource

[Lin Ming]

Uses the interface introduced in earlier patch.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/ata/libata-acpi.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index c2a4db6..ec2a5a4 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1033,16 +1033,48 @@ static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
 	}
 }
 
+static void ata_acpi_register_power_resource(struct ata_device *dev)
+{
+	struct scsi_device *sdev = dev->sdev;
+	acpi_handle handle;
+	struct device *device;
+
+	handle = ata_dev_acpi_handle(dev);
+	if (!handle)
+		return;
+
+	device = &sdev->sdev_gendev;
+
+	acpi_power_resource_register_device(device, handle);
+}
+
+static void ata_acpi_unregister_power_resource(struct ata_device *dev)
+{
+	struct scsi_device *sdev = dev->sdev;
+	acpi_handle handle;
+	struct device *device;
+
+	handle = ata_dev_acpi_handle(dev);
+	if (!handle)
+		return;
+
+	device = &sdev->sdev_gendev;
+
+	acpi_power_resource_unregister_device(device, handle);
+}
+
 void ata_acpi_bind(struct ata_device *dev)
 {
 	ata_acpi_bind_dock(dev);
 	ata_acpi_add_pm_notifier(dev);
+	ata_acpi_register_power_resource(dev);
 }
 
 void ata_acpi_unbind(struct ata_device *dev)
 {
 	ata_acpi_unbind_dock(dev);
 	ata_acpi_remove_pm_notifier(dev);
+	ata_acpi_unregister_power_resource(dev);
 }
 
 static int is_pci_ata(struct device *dev)
-- 
1.7.2.5

[RFC PATCH v2 7/8] PM / Runtime: Add can_power_off flag to subsys data

[Lin Ming]

Parent device will check child's can_power_off flag to see if child device can
be powered off.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 include/linux/pm.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index e4982ac..2e74531 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -465,6 +465,7 @@ struct pm_subsys_data {
 #ifdef CONFIG_PM_GENERIC_DOMAINS
 	struct pm_domain_data *domain_data;
 #endif
+	bool can_power_off;
 };
 
 struct dev_pm_info {
-- 
1.7.2.5

Re: [RFC PATCH v2 7/8] PM / Runtime: Add can_power_off flag to subsys data

[Lin Ming]

On Thu, Mar 1, 2012 at 5:02 PM, Lin Ming <ming.m.lin@intel.com> wrote:
> Parent device will check child's can_power_off flag to see if child device can
> be powered off.
>
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> ---
>  include/linux/pm.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index e4982ac..2e74531 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -465,6 +465,7 @@ struct pm_subsys_data {
>  #ifdef CONFIG_PM_GENERIC_DOMAINS
>        struct pm_domain_data *domain_data;
>  #endif
> +       bool can_power_off;

Hi Rafael,

I moved can_power_off flag to pm_subsys_daa.
SCSI/SATA will use it.

Is it OK to you?

Thanks,
Lin Ming

>  };
>
>  struct dev_pm_info {
> --
> 1.7.2.5

[RFC PATCH v2 8/8] [SCSI] sr: check and enable Zero-power ODD support

[Lin Ming]

ZPODD(Zero Power Optical Disk Drive) is a new feature in
SATA 3.1 specification. It provides a way to power off unused ODD.

ZPODD support is checked in in sr_probe().
can_power_off flag is set during suspend if ZPODD is supported.

ATA port's runtime suspend callback will actually power off the ODD
and its runtime resume callback will actually power on the ODD.

When ODD is powered off(D3Cold state), inserting disk will trigger a
wakeup event(GPE). GPE AML handler notifies the associated device. Then
ODD is resumed in the notify handler.

Signed-off-by: Lin Ming <ming.m.lin@intel.com
---
 drivers/ata/libata-acpi.c  |    8 ++++++-
 drivers/scsi/scsi_pm.c     |    8 +++++++
 drivers/scsi/sr.c          |   46 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/sr.h          |    3 ++
 include/scsi/scsi_device.h |    2 +
 5 files changed, 66 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index ec2a5a4..66b13d4 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -859,8 +859,14 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
 		if (state.event != PM_EVENT_ON) {
 			acpi_state = acpi_pm_device_sleep_state(
 				&dev->sdev->sdev_gendev, NULL);
-			if (acpi_state > 0)
+			if (acpi_state > 0) {
+				/* Check if the device is ready for power off */
+				if (acpi_state == ACPI_STATE_D3_COLD &&
+					!scsi_device_can_power_off(dev->sdev))
+					acpi_state = ACPI_STATE_D3;
+
 				acpi_bus_set_power(handle, acpi_state);
+			}
 			if (ap->tdev.power.request == RPM_REQ_SUSPEND)
 				acpi_pm_device_run_wake(
 					&dev->sdev->sdev_gendev, true);
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index bf8bf79..24a0724 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -209,6 +209,14 @@ void scsi_autopm_put_host(struct Scsi_Host *shost)
 	pm_runtime_put_sync(&shost->shost_gendev);
 }
 
+bool scsi_device_can_power_off(struct scsi_device *sdev)
+{
+	struct device *dev = &sdev->sdev_gendev;
+
+	return !dev->power.subsys_data ? false :
+		dev->power.subsys_data->can_power_off;
+}
+
 #else
 
 #define scsi_runtime_suspend	NULL
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..7cd0489 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -45,6 +45,8 @@
 #include <linux/blkdev.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/acpi.h>
 #include <asm/uaccess.h>
 
 #include <scsi/scsi.h>
@@ -80,12 +82,38 @@ static int sr_probe(struct device *);
 static int sr_remove(struct device *);
 static int sr_done(struct scsi_cmnd *);
 
+static int sr_suspend(struct device *dev, pm_message_t mesg)
+{
+	struct scsi_cd *cd;
+
+	cd = dev_get_drvdata(dev);
+	if (cd->zpodd)
+		dev->power.subsys_data->can_power_off = true;
+
+	return 0;
+}
+
+static int sr_resume(struct device *dev)
+{
+	struct scsi_cd *cd;
+
+	cd = dev_get_drvdata(dev);
+	if (cd->zpodd) {
+		dev->power.subsys_data->can_power_off = false;
+		cd->zpodd_event = 0;
+	}
+
+	return 0;
+}
+
 static struct scsi_driver sr_template = {
 	.owner			= THIS_MODULE,
 	.gendrv = {
 		.name   	= "sr",
 		.probe		= sr_probe,
 		.remove		= sr_remove,
+		.suspend	= sr_suspend,
+		.resume		= sr_resume,
 	},
 	.done			= sr_done,
 };
@@ -216,6 +244,10 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	unsigned int events;
 	int ret;
 
+	/* Not necessary to check events if enter ZPODD state */
+	if (cd->zpodd && pm_runtime_suspended(&cd->device->sdev_gendev))
+		return 0;
+
 	/* no changer support */
 	if (CDSL_CURRENT != slot)
 		return 0;
@@ -260,6 +292,11 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	cd->media_present = scsi_status_is_good(ret) ||
 		(scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
 
+	if (!cd->media_present && cd->zpodd && !cd->zpodd_event) {
+		scsi_autopm_put_device(cd->device);
+		cd->zpodd_event = 1;
+	}
+
 	if (last_present != cd->media_present)
 		cd->device->changed = 1;
 
@@ -716,6 +753,12 @@ static int sr_probe(struct device *dev)
 	disk->flags |= GENHD_FL_REMOVABLE;
 	add_disk(disk);
 
+	/* TODO: add device attention bit check for ZPODD */
+	if (device_run_wake(dev)) {
+		cd->zpodd = 1;
+		dev_pm_get_subsys_data(dev);
+	}
+
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
 	return 0;
@@ -972,6 +1015,9 @@ static int sr_remove(struct device *dev)
 	kref_put(&cd->kref, sr_kref_release);
 	mutex_unlock(&sr_ref_mutex);
 
+	if (cd->zpodd)
+		dev_pm_put_subsys_data(dev);
+
 	return 0;
 }
 
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 37c8f6b..39b3d8c 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -42,6 +42,9 @@ typedef struct scsi_cd {
 	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
 	unsigned media_present:1;	/* media is present */
 
+	unsigned zpodd:1;	/* is ZPODD supported */
+	unsigned zpodd_event:1;
+
 	/* GET_EVENT spurious event handling, blk layer guarantees exclusion */
 	int tur_mismatch;		/* nr of get_event TUR mismatches */
 	bool tur_changed:1;		/* changed according to TUR */
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 77273f2..8b6c247 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -388,9 +388,11 @@ extern int scsi_execute_req(struct scsi_device *sdev, const unsigned char *cmd,
 #ifdef CONFIG_PM_RUNTIME
 extern int scsi_autopm_get_device(struct scsi_device *);
 extern void scsi_autopm_put_device(struct scsi_device *);
+extern bool scsi_device_can_power_off(struct scsi_device *);
 #else
 static inline int scsi_autopm_get_device(struct scsi_device *d) { return 0; }
 static inline void scsi_autopm_put_device(struct scsi_device *d) {}
+static inline bool scsi_device_can_power_off(struct scsi_device *d) { return false; };
 #endif /* CONFIG_PM_RUNTIME */
 
 static inline int __must_check scsi_device_reprobe(struct scsi_device *sdev)
-- 
1.7.2.5

Re: [RFC PATCH v2 8/8] [SCSI] sr: check and enable Zero-power ODD support

[Alan Stern]

On Thu, 1 Mar 2012, Lin Ming wrote:

> ZPODD(Zero Power Optical Disk Drive) is a new feature in
> SATA 3.1 specification. It provides a way to power off unused ODD.
> 
> ZPODD support is checked in in sr_probe().
> can_power_off flag is set during suspend if ZPODD is supported.
> 
> ATA port's runtime suspend callback will actually power off the ODD
> and its runtime resume callback will actually power on the ODD.
> 
> When ODD is powered off(D3Cold state), inserting disk will trigger a
> wakeup event(GPE). GPE AML handler notifies the associated device. Then
> ODD is resumed in the notify handler.

I have one stylistic comment on this patch...

> diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
> index 37c8f6b..39b3d8c 100644
> --- a/drivers/scsi/sr.h
> +++ b/drivers/scsi/sr.h
> @@ -42,6 +42,9 @@ typedef struct scsi_cd {
>  	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
>  	unsigned media_present:1;	/* media is present */
>  
> +	unsigned zpodd:1;	/* is ZPODD supported */
> +	unsigned zpodd_event:1;
> +

You should not expect your readers to understand what "ZPODD" means.  
drivers/scsi/sr.h is used by lots of different people, many of whom 
will have no idea what it refers to, especially since it is part of 
the SATA spec and not the SCSI spec.  You should provide a brief 
explanation.

Alan Stern

Re: [RFC PATCH v2 8/8] [SCSI] sr: check and enable Zero-power ODD support

[Lin Ming]

On Thu, 2012-03-01 at 11:02 -0500, Alan Stern wrote:
> On Thu, 1 Mar 2012, Lin Ming wrote:
> 
> > ZPODD(Zero Power Optical Disk Drive) is a new feature in
> > SATA 3.1 specification. It provides a way to power off unused ODD.
> > 
> > ZPODD support is checked in in sr_probe().
> > can_power_off flag is set during suspend if ZPODD is supported.
> > 
> > ATA port's runtime suspend callback will actually power off the ODD
> > and its runtime resume callback will actually power on the ODD.
> > 
> > When ODD is powered off(D3Cold state), inserting disk will trigger a
> > wakeup event(GPE). GPE AML handler notifies the associated device. Then
> > ODD is resumed in the notify handler.
> 
> I have one stylistic comment on this patch...
> 
> > diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
> > index 37c8f6b..39b3d8c 100644
> > --- a/drivers/scsi/sr.h
> > +++ b/drivers/scsi/sr.h
> > @@ -42,6 +42,9 @@ typedef struct scsi_cd {
> >  	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
> >  	unsigned media_present:1;	/* media is present */
> >  
> > +	unsigned zpodd:1;	/* is ZPODD supported */
> > +	unsigned zpodd_event:1;
> > +
> 
> You should not expect your readers to understand what "ZPODD" means.  
> drivers/scsi/sr.h is used by lots of different people, many of whom 
> will have no idea what it refers to, especially since it is part of 
> the SATA spec and not the SCSI spec.  You should provide a brief 
> explanation.

I'll add some explanation.

But I'm thinking maybe it's better to move this flag to ata layer, for
example, adding a flag to libata.h

ATA_DFLAG_ZPODD

sr runtime pm is only enabled when ZPODD(or more general, power off) is
supported.
So the problem is how will sr driver know this flag?


> 
> Alan Stern
> 

Re: [RFC PATCH v2 8/8] [SCSI] sr: check and enable Zero-power ODD support

[Aaron Lu]

Hi,

On Fri, Mar 2, 2012 at 3:02 PM, Lin Ming <ming.m.lin@intel.com> wrote:
> On Thu, 2012-03-01 at 11:02 -0500, Alan Stern wrote:
>> On Thu, 1 Mar 2012, Lin Ming wrote:
>>
>> > ZPODD(Zero Power Optical Disk Drive) is a new feature in
>> > SATA 3.1 specification. It provides a way to power off unused ODD.
>> >
>> > ZPODD support is checked in in sr_probe().
>> > can_power_off flag is set during suspend if ZPODD is supported.
>> >
>> > ATA port's runtime suspend callback will actually power off the ODD
>> > and its runtime resume callback will actually power on the ODD.
>> >
>> > When ODD is powered off(D3Cold state), inserting disk will trigger a
>> > wakeup event(GPE). GPE AML handler notifies the associated device. Then
>> > ODD is resumed in the notify handler.
>>
>> I have one stylistic comment on this patch...
>>
>> > diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
>> > index 37c8f6b..39b3d8c 100644
>> > --- a/drivers/scsi/sr.h
>> > +++ b/drivers/scsi/sr.h
>> > @@ -42,6 +42,9 @@ typedef struct scsi_cd {
>> >     unsigned readcd_cdda:1; /* reading audio data using READ_CD */
>> >     unsigned media_present:1;       /* media is present */
>> >
>> > +   unsigned zpodd:1;       /* is ZPODD supported */
>> > +   unsigned zpodd_event:1;
>> > +
>>
>> You should not expect your readers to understand what "ZPODD" means.
>> drivers/scsi/sr.h is used by lots of different people, many of whom
>> will have no idea what it refers to, especially since it is part of
>> the SATA spec and not the SCSI spec.  You should provide a brief
>> explanation.
>
> I'll add some explanation.
>
> But I'm thinking maybe it's better to move this flag to ata layer, for

Agree.

> example, adding a flag to libata.h
>
> ATA_DFLAG_ZPODD
>

How about ATA_DFLAG_DA(device attention)?
It follows the name used in the sata spec and this feature might have
other uses in the future in addition to zero power odd(just my wild guess).

> sr runtime pm is only enabled when ZPODD(or more general, power off) is
> supported.
> So the problem is how will sr driver know this flag?
>

What about adding a new field to scsi_device to reflect this capability of
the underlying sata device? We can then set this field in ata_scsi_scan_host.

-Aaron

Re: [RFC PATCH v2 8/8] [SCSI] sr: check and enable Zero-power ODD support

[Lin Ming]

On Fri, 2012-03-02 at 23:08 +0800, Aaron Lu wrote:
> Hi,
> 
> On Fri, Mar 2, 2012 at 3:02 PM, Lin Ming <ming.m.lin@intel.com> wrote:
> > On Thu, 2012-03-01 at 11:02 -0500, Alan Stern wrote:
> >> On Thu, 1 Mar 2012, Lin Ming wrote:
> >>
> >> > ZPODD(Zero Power Optical Disk Drive) is a new feature in
> >> > SATA 3.1 specification. It provides a way to power off unused ODD.
> >> >
> >> > ZPODD support is checked in in sr_probe().
> >> > can_power_off flag is set during suspend if ZPODD is supported.
> >> >
> >> > ATA port's runtime suspend callback will actually power off the ODD
> >> > and its runtime resume callback will actually power on the ODD.
> >> >
> >> > When ODD is powered off(D3Cold state), inserting disk will trigger a
> >> > wakeup event(GPE). GPE AML handler notifies the associated device. Then
> >> > ODD is resumed in the notify handler.
> >>
> >> I have one stylistic comment on this patch...
> >>
> >> > diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
> >> > index 37c8f6b..39b3d8c 100644
> >> > --- a/drivers/scsi/sr.h
> >> > +++ b/drivers/scsi/sr.h
> >> > @@ -42,6 +42,9 @@ typedef struct scsi_cd {
> >> >     unsigned readcd_cdda:1; /* reading audio data using READ_CD */
> >> >     unsigned media_present:1;       /* media is present */
> >> >
> >> > +   unsigned zpodd:1;       /* is ZPODD supported */
> >> > +   unsigned zpodd_event:1;
> >> > +
> >>
> >> You should not expect your readers to understand what "ZPODD" means.
> >> drivers/scsi/sr.h is used by lots of different people, many of whom
> >> will have no idea what it refers to, especially since it is part of
> >> the SATA spec and not the SCSI spec.  You should provide a brief
> >> explanation.
> >
> > I'll add some explanation.
> >
> > But I'm thinking maybe it's better to move this flag to ata layer, for
> 
> Agree.
> 
> > example, adding a flag to libata.h
> >
> > ATA_DFLAG_ZPODD
> >
> 
> How about ATA_DFLAG_DA(device attention)?

Yes, it's better.

> It follows the name used in the sata spec and this feature might have
> other uses in the future in addition to zero power odd(just my wild guess).
> 
> > sr runtime pm is only enabled when ZPODD(or more general, power off) is
> > supported.
> > So the problem is how will sr driver know this flag?
> >
> 
> What about adding a new field to scsi_device to reflect this capability of
> the underlying sata device? We can then set this field in ata_scsi_scan_host.

Nice idea.

I'll refresh this patch.

Thanks,
Lin Ming

Re: [RFC PATCH v2 8/8] [SCSI] sr: check and enable Zero-power ODD support

[Lin Ming]

On Sat, 2012-03-03 at 11:05 +0800, Lin Ming wrote:
> On Fri, 2012-03-02 at 23:08 +0800, Aaron Lu wrote:
> > Hi,
> > 
> > On Fri, Mar 2, 2012 at 3:02 PM, Lin Ming <ming.m.lin@intel.com> wrote:
> > > On Thu, 2012-03-01 at 11:02 -0500, Alan Stern wrote:
> > >> On Thu, 1 Mar 2012, Lin Ming wrote:
> > >>
> > >> > ZPODD(Zero Power Optical Disk Drive) is a new feature in
> > >> > SATA 3.1 specification. It provides a way to power off unused ODD.
> > >> >
> > >> > ZPODD support is checked in in sr_probe().
> > >> > can_power_off flag is set during suspend if ZPODD is supported.
> > >> >
> > >> > ATA port's runtime suspend callback will actually power off the ODD
> > >> > and its runtime resume callback will actually power on the ODD.
> > >> >
> > >> > When ODD is powered off(D3Cold state), inserting disk will trigger a
> > >> > wakeup event(GPE). GPE AML handler notifies the associated device. Then
> > >> > ODD is resumed in the notify handler.
> > >>
> > >> I have one stylistic comment on this patch...
> > >>
> > >> > diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
> > >> > index 37c8f6b..39b3d8c 100644
> > >> > --- a/drivers/scsi/sr.h
> > >> > +++ b/drivers/scsi/sr.h
> > >> > @@ -42,6 +42,9 @@ typedef struct scsi_cd {
> > >> >     unsigned readcd_cdda:1; /* reading audio data using READ_CD */
> > >> >     unsigned media_present:1;       /* media is present */
> > >> >
> > >> > +   unsigned zpodd:1;       /* is ZPODD supported */
> > >> > +   unsigned zpodd_event:1;
> > >> > +
> > >>
> > >> You should not expect your readers to understand what "ZPODD" means.
> > >> drivers/scsi/sr.h is used by lots of different people, many of whom
> > >> will have no idea what it refers to, especially since it is part of
> > >> the SATA spec and not the SCSI spec.  You should provide a brief
> > >> explanation.
> > >
> > > I'll add some explanation.
> > >
> > > But I'm thinking maybe it's better to move this flag to ata layer, for
> > 
> > Agree.
> > 
> > > example, adding a flag to libata.h
> > >
> > > ATA_DFLAG_ZPODD
> > >
> > 
> > How about ATA_DFLAG_DA(device attention)?
> 
> Yes, it's better.
> 
> > It follows the name used in the sata spec and this feature might have
> > other uses in the future in addition to zero power odd(just my wild guess).
> > 
> > > sr runtime pm is only enabled when ZPODD(or more general, power off) is
> > > supported.
> > > So the problem is how will sr driver know this flag?
> > >
> > 
> > What about adding a new field to scsi_device to reflect this capability of
> > the underlying sata device? We can then set this field in ata_scsi_scan_host.
> 
> Nice idea.
> 
> I'll refresh this patch.

Would you like the draft incremental patch below?
I have not tested it yet.

---
 drivers/ata/libata-core.c  |    3 +++
 drivers/ata/libata-scsi.c  |    2 ++
 drivers/scsi/sr.c          |   16 +++++++---------
 drivers/scsi/sr.h          |    1 -
 include/linux/ata.h        |    1 +
 include/linux/libata.h     |    2 ++
 include/scsi/scsi_device.h |    1 +
 7 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 6742cc4..2a10701 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2368,6 +2368,9 @@ int ata_dev_configure(struct ata_device *dev)
 			dma_dir_string = ", DMADIR";
 		}
 
+		if (atapi_id_has_da(dev->id))
+			dev->flags |= ATA_DFLAG_DA;
+
 		/* print device info to dmesg */
 		if (ata_msg_drv(ap) && print_info)
 			ata_dev_info(dev,
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 231c3ec..0bfbe41 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3445,6 +3445,8 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 				dev->sdev = sdev;
 				scsi_device_put(sdev);
 				ata_acpi_bind(dev);
+				if (dev->flags & ATA_DFLAG_DA)
+					sdev->power_off = 1;
 			} else {
 				dev->sdev = NULL;
 			}
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 7cd0489..0a75613 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -87,7 +87,7 @@ static int sr_suspend(struct device *dev, pm_message_t mesg)
 	struct scsi_cd *cd;
 
 	cd = dev_get_drvdata(dev);
-	if (cd->zpodd)
+	if (cd->device->power_off)
 		dev->power.subsys_data->can_power_off = true;
 
 	return 0;
@@ -98,7 +98,7 @@ static int sr_resume(struct device *dev)
 	struct scsi_cd *cd;
 
 	cd = dev_get_drvdata(dev);
-	if (cd->zpodd) {
+	if (cd->device->power_off) {
 		dev->power.subsys_data->can_power_off = false;
 		cd->zpodd_event = 0;
 	}
@@ -245,7 +245,8 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	int ret;
 
 	/* Not necessary to check events if enter ZPODD state */
-	if (cd->zpodd && pm_runtime_suspended(&cd->device->sdev_gendev))
+	if (cd->device->power_off &&
+			pm_runtime_suspended(&cd->device->sdev_gendev))
 		return 0;
 
 	/* no changer support */
@@ -292,7 +293,7 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
 	cd->media_present = scsi_status_is_good(ret) ||
 		(scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
 
-	if (!cd->media_present && cd->zpodd && !cd->zpodd_event) {
+	if (!cd->media_present && cd->device->power_off && !cd->zpodd_event) {
 		scsi_autopm_put_device(cd->device);
 		cd->zpodd_event = 1;
 	}
@@ -753,11 +754,8 @@ static int sr_probe(struct device *dev)
 	disk->flags |= GENHD_FL_REMOVABLE;
 	add_disk(disk);
 
-	/* TODO: add device attention bit check for ZPODD */
-	if (device_run_wake(dev)) {
-		cd->zpodd = 1;
+	if (sdev->power_off)
 		dev_pm_get_subsys_data(dev);
-	}
 
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
@@ -1015,7 +1013,7 @@ static int sr_remove(struct device *dev)
 	kref_put(&cd->kref, sr_kref_release);
 	mutex_unlock(&sr_ref_mutex);
 
-	if (cd->zpodd)
+	if (cd->device->power_off)
 		dev_pm_put_subsys_data(dev);
 
 	return 0;
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 39b3d8c..e81da64 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -42,7 +42,6 @@ typedef struct scsi_cd {
 	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
 	unsigned media_present:1;	/* media is present */
 
-	unsigned zpodd:1;	/* is ZPODD supported */
 	unsigned zpodd_event:1;
 
 	/* GET_EVENT spurious event handling, blk layer guarantees exclusion */
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 32df2b6..6ee41cc 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -578,6 +578,7 @@ static inline int ata_is_data(u8 prot)
 	  ((u64) (id)[(n) + 0]) )
 
 #define ata_id_cdb_intr(id)	(((id)[ATA_ID_CONFIG] & 0x60) == 0x20)
+#define atapi_id_has_da(id)	((id)[77] & (1 << 4))
 
 static inline bool ata_id_has_hipm(const u16 *id)
 {
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 36970c6..3085bdc 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -161,6 +161,8 @@ enum {
 	ATA_DFLAG_DETACH	= (1 << 24),
 	ATA_DFLAG_DETACHED	= (1 << 25),
 
+	ATA_DFLAG_DA		= (1 << 26), /* device supports Device Attention */
+
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
 	ATA_DEV_ATA_UNSUP	= 2,	/* ATA device (unsupported) */
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 8b6c247..4805d5a 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -151,6 +151,7 @@ struct scsi_device {
 	unsigned no_read_disc_info:1;	/* Avoid READ_DISC_INFO cmds */
 	unsigned no_read_capacity_16:1; /* Avoid READ_CAPACITY_16 cmds */
 	unsigned is_visible:1;	/* is the device visible in sysfs */
+	unsigned power_off:1; /* Device supports runtime power off */
 
 	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
 	struct list_head event_list;	/* asserted events */

Re: [RFC PATCH v2 8/8] [SCSI] sr: check and enable Zero-power ODD support

[Aaron Lu]

On Mon, Mar 12, 2012 at 10:49:58AM +0800, Lin Ming wrote:
> 
> Would you like the draft incremental patch below?
> I have not tested it yet.

Yes, this patch looks good to me, feel free to add my acked-by next time
you post it, thanks.

-Aaron

> 
> ---
>  drivers/ata/libata-core.c  |    3 +++
>  drivers/ata/libata-scsi.c  |    2 ++
>  drivers/scsi/sr.c          |   16 +++++++---------
>  drivers/scsi/sr.h          |    1 -
>  include/linux/ata.h        |    1 +
>  include/linux/libata.h     |    2 ++
>  include/scsi/scsi_device.h |    1 +
>  7 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 6742cc4..2a10701 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2368,6 +2368,9 @@ int ata_dev_configure(struct ata_device *dev)
>  			dma_dir_string = ", DMADIR";
>  		}
>  
> +		if (atapi_id_has_da(dev->id))
> +			dev->flags |= ATA_DFLAG_DA;
> +
>  		/* print device info to dmesg */
>  		if (ata_msg_drv(ap) && print_info)
>  			ata_dev_info(dev,
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 231c3ec..0bfbe41 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3445,6 +3445,8 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  				dev->sdev = sdev;
>  				scsi_device_put(sdev);
>  				ata_acpi_bind(dev);
> +				if (dev->flags & ATA_DFLAG_DA)
> +					sdev->power_off = 1;
>  			} else {
>  				dev->sdev = NULL;
>  			}
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 7cd0489..0a75613 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -87,7 +87,7 @@ static int sr_suspend(struct device *dev, pm_message_t mesg)
>  	struct scsi_cd *cd;
>  
>  	cd = dev_get_drvdata(dev);
> -	if (cd->zpodd)
> +	if (cd->device->power_off)
>  		dev->power.subsys_data->can_power_off = true;
>  
>  	return 0;
> @@ -98,7 +98,7 @@ static int sr_resume(struct device *dev)
>  	struct scsi_cd *cd;
>  
>  	cd = dev_get_drvdata(dev);
> -	if (cd->zpodd) {
> +	if (cd->device->power_off) {
>  		dev->power.subsys_data->can_power_off = false;
>  		cd->zpodd_event = 0;
>  	}
> @@ -245,7 +245,8 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>  	int ret;
>  
>  	/* Not necessary to check events if enter ZPODD state */
> -	if (cd->zpodd && pm_runtime_suspended(&cd->device->sdev_gendev))
> +	if (cd->device->power_off &&
> +			pm_runtime_suspended(&cd->device->sdev_gendev))
>  		return 0;
>  
>  	/* no changer support */
> @@ -292,7 +293,7 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
>  	cd->media_present = scsi_status_is_good(ret) ||
>  		(scsi_sense_valid(&sshdr) && sshdr.asc != 0x3a);
>  
> -	if (!cd->media_present && cd->zpodd && !cd->zpodd_event) {
> +	if (!cd->media_present && cd->device->power_off && !cd->zpodd_event) {
>  		scsi_autopm_put_device(cd->device);
>  		cd->zpodd_event = 1;
>  	}
> @@ -753,11 +754,8 @@ static int sr_probe(struct device *dev)
>  	disk->flags |= GENHD_FL_REMOVABLE;
>  	add_disk(disk);
>  
> -	/* TODO: add device attention bit check for ZPODD */
> -	if (device_run_wake(dev)) {
> -		cd->zpodd = 1;
> +	if (sdev->power_off)
>  		dev_pm_get_subsys_data(dev);
> -	}
>  
>  	sdev_printk(KERN_DEBUG, sdev,
>  		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
> @@ -1015,7 +1013,7 @@ static int sr_remove(struct device *dev)
>  	kref_put(&cd->kref, sr_kref_release);
>  	mutex_unlock(&sr_ref_mutex);
>  
> -	if (cd->zpodd)
> +	if (cd->device->power_off)
>  		dev_pm_put_subsys_data(dev);
>  
>  	return 0;
> diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
> index 39b3d8c..e81da64 100644
> --- a/drivers/scsi/sr.h
> +++ b/drivers/scsi/sr.h
> @@ -42,7 +42,6 @@ typedef struct scsi_cd {
>  	unsigned readcd_cdda:1;	/* reading audio data using READ_CD */
>  	unsigned media_present:1;	/* media is present */
>  
> -	unsigned zpodd:1;	/* is ZPODD supported */
>  	unsigned zpodd_event:1;
>  
>  	/* GET_EVENT spurious event handling, blk layer guarantees exclusion */
> diff --git a/include/linux/ata.h b/include/linux/ata.h
> index 32df2b6..6ee41cc 100644
> --- a/include/linux/ata.h
> +++ b/include/linux/ata.h
> @@ -578,6 +578,7 @@ static inline int ata_is_data(u8 prot)
>  	  ((u64) (id)[(n) + 0]) )
>  
>  #define ata_id_cdb_intr(id)	(((id)[ATA_ID_CONFIG] & 0x60) == 0x20)
> +#define atapi_id_has_da(id)	((id)[77] & (1 << 4))
>  
>  static inline bool ata_id_has_hipm(const u16 *id)
>  {
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 36970c6..3085bdc 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -161,6 +161,8 @@ enum {
>  	ATA_DFLAG_DETACH	= (1 << 24),
>  	ATA_DFLAG_DETACHED	= (1 << 25),
>  
> +	ATA_DFLAG_DA		= (1 << 26), /* device supports Device Attention */
> +
>  	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
>  	ATA_DEV_ATA		= 1,	/* ATA device */
>  	ATA_DEV_ATA_UNSUP	= 2,	/* ATA device (unsupported) */
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 8b6c247..4805d5a 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -151,6 +151,7 @@ struct scsi_device {
>  	unsigned no_read_disc_info:1;	/* Avoid READ_DISC_INFO cmds */
>  	unsigned no_read_capacity_16:1; /* Avoid READ_CAPACITY_16 cmds */
>  	unsigned is_visible:1;	/* is the device visible in sysfs */
> +	unsigned power_off:1; /* Device supports runtime power off */
>  
>  	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
>  	struct list_head event_list;	/* asserted events */
> 
> 
> 
>