| Message ID | 1329124271-29464-3-git-send-email-ming.m.lin@intel.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Monday, February 13, 2012, Lin Ming wrote: > From: Zhang Rui <rui.zhang@intel.com> > > ACPI Power Resource can power on/off a couple of devices. > Introduce interfaces to register/unregister a device to/from > an ACPI Power Resource. > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/acpi/power.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++ > include/acpi/acpi_bus.h | 2 + > 2 files changed, 130 insertions(+), 0 deletions(-) > > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c > index 0d681fb..a7e2305 100644 > --- a/drivers/acpi/power.c > +++ b/drivers/acpi/power.c > @@ -84,10 +84,25 @@ struct acpi_power_resource { > u32 order; > unsigned int ref_count; > struct mutex resource_lock; > + struct list_head devices; /* list for devices powered by this PR */ > }; > > static struct list_head acpi_power_resource_list; > > +/* > + * When a power resource is turned on, all the devices are put to an uninitialized > + * state although their ACPI states are still D0. > + * To handle this, we need to keep a list of all devices powered by the power resource, > + * and resume all of them. > + * Currently, we only support this case: > + * 1) multiple devices share the same power resoruce, > + * 2) One device uses One Power Resource ONLY. This is incorrect. We actually support all combinations. However, we don't generally support checking what devices depend on the given power resource. > + */ > +struct acpi_powered_device { > + struct list_head node; > + struct device *dev; > +}; > + > /* -------------------------------------------------------------------------- > Power Resource Management > -------------------------------------------------------------------------- */ > @@ -455,6 +470,118 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev) > Device Power Management > -------------------------------------------------------------------------- */ > > +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle) > +{ > + struct acpi_power_resource *resource; > + struct acpi_device *acpi_dev, *res_dev; > + acpi_handle res_handle = NULL; > + struct acpi_powered_device *apd; > + int i; > + int ret; > + > + if (!handle || !dev) > + return -EINVAL; > + > + 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; > + > + for (i = 0; i <= ACPI_STATE_D3; i++) { > + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; > + > + if (!ps->flags.valid) > + continue; > + > + if (ps->resources.count > 1) > + return 0; > + > + if (!res_handle) > + res_handle = ps->resources.handles[0]; > + else if (res_handle != ps->resources.handles[0]) > + return 0; > + } I'm not sure what the above checks are needed for. It seems that this function will only be called from acpi_bus_add_power_resource() (which needs to be modified for this purpose, BTW), so it doesn't need to check all those things (those checks have been made already). > + > + ret = acpi_bus_get_device(res_handle, &res_dev); > + if (ret) > + goto no_power_resource; > + > + resource = res_dev->driver_data; > + if (!resource) > + goto no_power_resource; > + > + apd = kzalloc(sizeof(struct acpi_powered_device), GFP_KERNEL); > + if (!apd) > + return -ENOMEM; > + > + apd->dev = dev; > + mutex_lock(&resource->resource_lock); > + list_add_tail(&apd->node, &resource->devices); > + mutex_unlock(&resource->resource_lock); > + printk(KERN_INFO PREFIX "Device %s uses Power Resource %s\n", dev->kobj.name, acpi_device_name(res_dev)); > + > + return 0; > + > +no_power_resource: > + printk(KERN_WARNING PREFIX "Invalid Power Resource to register!"); > + return -ENODEV; > +} > + > +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle) > +{ Who is supposed to call this one? > + struct acpi_power_resource *resource; > + struct acpi_device *acpi_dev, *res_dev; > + acpi_handle res_handle = NULL; > + struct acpi_powered_device *apd, *tmp; > + int i; > + int ret; > + > + if (!handle || !dev) > + return; > + > + ret = acpi_bus_get_device(handle, &acpi_dev); > + if (ret) > + return; > + > + if (!acpi_dev->power.flags.power_resources) > + return; > + > + for (i = 0; i <= ACPI_STATE_D3; i++) { > + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; > + > + if (!ps->flags.valid) > + continue; > + > + if (ps->resources.count > 1) > + return; > + > + res_handle = ps->resources.handles[0]; > + break; > + } > + > + if (!res_handle) > + return; > + > + ret = acpi_bus_get_device(res_handle, &res_dev); > + if (ret) > + return; > + > + resource = res_dev->driver_data; > + if (!resource) > + return; > + > + mutex_lock(&resource->resource_lock); > + list_for_each_entry_safe(apd, tmp, &resource->devices, node) { > + if (apd->dev == dev) { > + list_del(&apd->node); > + kfree(apd); > + } > + } > + mutex_unlock(&resource->resource_lock); > +} > + > int acpi_power_get_inferred_state(struct acpi_device *device, int *state) > { > int result = 0; > @@ -550,6 +677,7 @@ static int acpi_power_add(struct acpi_device *device) > > resource->device = device; > mutex_init(&resource->resource_lock); > + INIT_LIST_HEAD(&resource->devices); > strcpy(resource->name, device->pnp.bus_id); > strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME); > strcpy(acpi_device_class(device), ACPI_POWER_CLASS); > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 6cd5b64..32fb899 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -322,6 +322,8 @@ int acpi_bus_get_status(struct acpi_device *device); > 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); > +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle); > +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle); > bool acpi_bus_can_wakeup(acpi_handle handle); > #ifdef CONFIG_ACPI_PROC_EVENT > int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data); Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 一, 2012-02-13 at 21:48 +0100, Rafael J. Wysocki wrote: > On Monday, February 13, 2012, Lin Ming wrote: > > From: Zhang Rui <rui.zhang@intel.com> > > > > ACPI Power Resource can power on/off a couple of devices. > > Introduce interfaces to register/unregister a device to/from > > an ACPI Power Resource. > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > --- > > drivers/acpi/power.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++ > > include/acpi/acpi_bus.h | 2 + > > 2 files changed, 130 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c > > index 0d681fb..a7e2305 100644 > > --- a/drivers/acpi/power.c > > +++ b/drivers/acpi/power.c > > @@ -84,10 +84,25 @@ struct acpi_power_resource { > > u32 order; > > unsigned int ref_count; > > struct mutex resource_lock; > > + struct list_head devices; /* list for devices powered by this PR */ > > }; > > > > static struct list_head acpi_power_resource_list; > > > > +/* > > + * When a power resource is turned on, all the devices are put to an uninitialized > > + * state although their ACPI states are still D0. > > + * To handle this, we need to keep a list of all devices powered by the power resource, > > + * and resume all of them. > > + * Currently, we only support this case: > > + * 1) multiple devices share the same power resoruce, > > + * 2) One device uses One Power Resource ONLY. > > This is incorrect. We actually support all combinations. Agreed. > However, we don't > generally support checking what devices depend on the given power resource. > Yes. But all the code in this patch is for runtime D3_COLD support only. At least for the BIOS code on hand, a device that support runtime D3_COLD uses one ACPI Power Resource only. We can add the support for all combinations if there are such kind of BIOS in the market, which I do not think there would be. Besides, as a RFC version, I do not want to make a over designed proposal at the beginning. > > + */ > > +struct acpi_powered_device { > > + struct list_head node; > > + struct device *dev; > > +}; > > + > > /* -------------------------------------------------------------------------- > > Power Resource Management > > -------------------------------------------------------------------------- */ > > @@ -455,6 +470,118 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev) > > Device Power Management > > -------------------------------------------------------------------------- */ > > > > +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle) > > +{ > > + struct acpi_power_resource *resource; > > + struct acpi_device *acpi_dev, *res_dev; > > + acpi_handle res_handle = NULL; > > + struct acpi_powered_device *apd; > > + int i; > > + int ret; > > + > > + if (!handle || !dev) > > + return -EINVAL; > > + > > + 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; > > + > > + for (i = 0; i <= ACPI_STATE_D3; i++) { > > + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; > > + > > + if (!ps->flags.valid) > > + continue; > > + > > + if (ps->resources.count > 1) > > + return 0; > > + > > + if (!res_handle) > > + res_handle = ps->resources.handles[0]; > > + else if (res_handle != ps->resources.handles[0]) > > + return 0; > > + } > > I'm not sure what the above checks are needed for. It seems that this function > will only be called from acpi_bus_add_power_resource() (which needs to be > modified for this purpose, BTW), so it doesn't need to check all those things > (those checks have been made already). > No. These two APIs are introduced to support the runtime D3_COLD remote wakeup. And they should be invoked by drivers, either in driver code or via bus layer code. Say, ATA port, who has _PR3 in its ACPI node, knows that it can enter D3_COLD at run time, and it supports remote wakeup in D3_COLD because it has _S0W (return value 4). When remote wakeup event is triggered, there is an ACPI event sent to the ATA controller/port, which sets the ATA controller/port back to D0 state. At this time, what we actually need is to resume the ZPODD, rather than the ATA controller/port. To follow the runtime PM model (runtime resume starts from leaf devices), ATA code can use these two APIs to tell ACPI to runtime resume ZPODD device directly, because ZPODD is powered by this Power Resource as well. thanks, rui > > + > > + ret = acpi_bus_get_device(res_handle, &res_dev); > > + if (ret) > > + goto no_power_resource; > > + > > + resource = res_dev->driver_data; > > + if (!resource) > > + goto no_power_resource; > > + > > + apd = kzalloc(sizeof(struct acpi_powered_device), GFP_KERNEL); > > + if (!apd) > > + return -ENOMEM; > > + > > + apd->dev = dev; > > + mutex_lock(&resource->resource_lock); > > + list_add_tail(&apd->node, &resource->devices); > > + mutex_unlock(&resource->resource_lock); > > + printk(KERN_INFO PREFIX "Device %s uses Power Resource %s\n", dev->kobj.name, acpi_device_name(res_dev)); > > + > > + return 0; > > + > > +no_power_resource: > > + printk(KERN_WARNING PREFIX "Invalid Power Resource to register!"); > > + return -ENODEV; > > +} > > + > > +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle) > > +{ > > Who is supposed to call this one? > > > + struct acpi_power_resource *resource; > > + struct acpi_device *acpi_dev, *res_dev; > > + acpi_handle res_handle = NULL; > > + struct acpi_powered_device *apd, *tmp; > > + int i; > > + int ret; > > + > > + if (!handle || !dev) > > + return; > > + > > + ret = acpi_bus_get_device(handle, &acpi_dev); > > + if (ret) > > + return; > > + > > + if (!acpi_dev->power.flags.power_resources) > > + return; > > + > > + for (i = 0; i <= ACPI_STATE_D3; i++) { > > + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; > > + > > + if (!ps->flags.valid) > > + continue; > > + > > + if (ps->resources.count > 1) > > + return; > > + > > + res_handle = ps->resources.handles[0]; > > + break; > > + } > > + > > + if (!res_handle) > > + return; > > + > > + ret = acpi_bus_get_device(res_handle, &res_dev); > > + if (ret) > > + return; > > + > > + resource = res_dev->driver_data; > > + if (!resource) > > + return; > > + > > + mutex_lock(&resource->resource_lock); > > + list_for_each_entry_safe(apd, tmp, &resource->devices, node) { > > + if (apd->dev == dev) { > > + list_del(&apd->node); > > + kfree(apd); > > + } > > + } > > + mutex_unlock(&resource->resource_lock); > > +} > > + > > int acpi_power_get_inferred_state(struct acpi_device *device, int *state) > > { > > int result = 0; > > @@ -550,6 +677,7 @@ static int acpi_power_add(struct acpi_device *device) > > > > resource->device = device; > > mutex_init(&resource->resource_lock); > > + INIT_LIST_HEAD(&resource->devices); > > strcpy(resource->name, device->pnp.bus_id); > > strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME); > > strcpy(acpi_device_class(device), ACPI_POWER_CLASS); > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > > index 6cd5b64..32fb899 100644 > > --- a/include/acpi/acpi_bus.h > > +++ b/include/acpi/acpi_bus.h > > @@ -322,6 +322,8 @@ int acpi_bus_get_status(struct acpi_device *device); > > 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); > > +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle); > > +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle); > > bool acpi_bus_can_wakeup(acpi_handle handle); > > #ifdef CONFIG_ACPI_PROC_EVENT > > int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data); > > Thanks, > Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, February 14, 2012, Zhang Rui wrote: > On 一, 2012-02-13 at 21:48 +0100, Rafael J. Wysocki wrote: > > On Monday, February 13, 2012, Lin Ming wrote: > > > From: Zhang Rui <rui.zhang@intel.com> > > > > > > ACPI Power Resource can power on/off a couple of devices. > > > Introduce interfaces to register/unregister a device to/from > > > an ACPI Power Resource. > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > --- > > > drivers/acpi/power.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++ > > > include/acpi/acpi_bus.h | 2 + > > > 2 files changed, 130 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c > > > index 0d681fb..a7e2305 100644 > > > --- a/drivers/acpi/power.c > > > +++ b/drivers/acpi/power.c > > > @@ -84,10 +84,25 @@ struct acpi_power_resource { > > > u32 order; > > > unsigned int ref_count; > > > struct mutex resource_lock; > > > + struct list_head devices; /* list for devices powered by this PR */ > > > }; > > > > > > static struct list_head acpi_power_resource_list; > > > > > > +/* > > > + * When a power resource is turned on, all the devices are put to an uninitialized > > > + * state although their ACPI states are still D0. > > > + * To handle this, we need to keep a list of all devices powered by the power resource, > > > + * and resume all of them. > > > + * Currently, we only support this case: > > > + * 1) multiple devices share the same power resoruce, > > > + * 2) One device uses One Power Resource ONLY. > > > > This is incorrect. We actually support all combinations. > > Agreed. > > > However, we don't > > generally support checking what devices depend on the given power resource. > > > Yes. > But all the code in this patch is for runtime D3_COLD support only. > At least for the BIOS code on hand, a device that support runtime > D3_COLD uses one ACPI Power Resource only. > We can add the support for all combinations if there are such kind of > BIOS in the market, which I do not think there would be. > > Besides, as a RFC version, I do not want to make a over designed > proposal at the beginning. > > > > + */ > > > +struct acpi_powered_device { > > > + struct list_head node; > > > + struct device *dev; > > > +}; > > > + > > > /* -------------------------------------------------------------------------- > > > Power Resource Management > > > -------------------------------------------------------------------------- */ > > > @@ -455,6 +470,118 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev) > > > Device Power Management > > > -------------------------------------------------------------------------- */ > > > > > > +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle) > > > +{ > > > + struct acpi_power_resource *resource; > > > + struct acpi_device *acpi_dev, *res_dev; > > > + acpi_handle res_handle = NULL; > > > + struct acpi_powered_device *apd; > > > + int i; > > > + int ret; > > > + > > > + if (!handle || !dev) > > > + return -EINVAL; > > > + > > > + 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; > > > + > > > + for (i = 0; i <= ACPI_STATE_D3; i++) { > > > + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; > > > + > > > + if (!ps->flags.valid) > > > + continue; > > > + > > > + if (ps->resources.count > 1) > > > + return 0; > > > + > > > + if (!res_handle) > > > + res_handle = ps->resources.handles[0]; > > > + else if (res_handle != ps->resources.handles[0]) > > > + return 0; > > > + } > > > > I'm not sure what the above checks are needed for. It seems that this function > > will only be called from acpi_bus_add_power_resource() (which needs to be > > modified for this purpose, BTW), so it doesn't need to check all those things > > (those checks have been made already). > > > No. These two APIs are introduced to support the runtime D3_COLD remote > wakeup. And they should be invoked by drivers, either in driver code or > via bus layer code. > > Say, ATA port, who has _PR3 in its ACPI node, knows that it can enter > D3_COLD at run time, and it supports remote wakeup in D3_COLD because it > has _S0W (return value 4). > When remote wakeup event is triggered, there is an ACPI event sent to > the ATA controller/port, which sets the ATA controller/port back to D0 > state. > At this time, what we actually need is to resume the ZPODD, rather than > the ATA controller/port. To follow the runtime PM model (runtime resume > starts from leaf devices), Well, this isn't the case. Parents are always resumed first. > ATA code can use these two APIs to tell ACPI > to runtime resume ZPODD device directly, because ZPODD is powered by > this Power Resource as well. I'm not exactly sure what you're trying to achieve and what you mean by "resume a device directly"? Do you want to run the device's resume callback at the time when another device is being resumed? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 二, 2012-02-14 at 23:36 +0100, Rafael J. Wysocki wrote: > > > > + */ > > > > +struct acpi_powered_device { > > > > + struct list_head node; > > > > + struct device *dev; > > > > +}; > > > > + > > > > /* -------------------------------------------------------------------------- > > > > Power Resource Management > > > > -------------------------------------------------------------------------- */ > > > > @@ -455,6 +470,118 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev) > > > > Device Power Management > > > > -------------------------------------------------------------------------- */ > > > > > > > > +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle) > > > > +{ > > > > + struct acpi_power_resource *resource; > > > > + struct acpi_device *acpi_dev, *res_dev; > > > > + acpi_handle res_handle = NULL; > > > > + struct acpi_powered_device *apd; > > > > + int i; > > > > + int ret; > > > > + > > > > + if (!handle || !dev) > > > > + return -EINVAL; > > > > + > > > > + 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; > > > > + > > > > + for (i = 0; i <= ACPI_STATE_D3; i++) { > > > > + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; > > > > + > > > > + if (!ps->flags.valid) > > > > + continue; > > > > + > > > > + if (ps->resources.count > 1) > > > > + return 0; > > > > + > > > > + if (!res_handle) > > > > + res_handle = ps->resources.handles[0]; > > > > + else if (res_handle != ps->resources.handles[0]) > > > > + return 0; > > > > + } > > > > > > I'm not sure what the above checks are needed for. It seems that this function > > > will only be called from acpi_bus_add_power_resource() (which needs to be > > > modified for this purpose, BTW), so it doesn't need to check all those things > > > (those checks have been made already). > > > > > No. These two APIs are introduced to support the runtime D3_COLD remote > > wakeup. And they should be invoked by drivers, either in driver code or > > via bus layer code. > > > > Say, ATA port, who has _PR3 in its ACPI node, knows that it can enter > > D3_COLD at run time, and it supports remote wakeup in D3_COLD because it > > has _S0W (return value 4). > > When remote wakeup event is triggered, there is an ACPI event sent to > > the ATA controller/port, which sets the ATA controller/port back to D0 > > state. > > At this time, what we actually need is to resume the ZPODD, rather than > > the ATA controller/port. To follow the runtime PM model (runtime resume > > starts from leaf devices), > > Well, this isn't the case. Parents are always resumed first. > Well, I mean runtime resume request issued by leaf devices first. > > ATA code can use these two APIs to tell ACPI > > to runtime resume ZPODD device directly, because ZPODD is powered by > > this Power Resource as well. > > I'm not exactly sure what you're trying to achieve and what you mean by > "resume a device directly"? Do you want to run the device's resume > callback at the time when another device is being resumed? > I mean, wakeup event is sent to ATA port, but our goal is to resume ZPODD after receiving this wakeup event. Ideally, it is ACPI that resumes ATA port. And then, the ATA port runtime resumes ZPODD. But this does not look good to runtime resume a child device in the parent's .runtime_resume callback. So I introduced these two APIs so that an runtime_resume request can be sent to ZPODD directly and the runtime PM core can resume all the parents of ZPODD automatically. thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 16 Feb 2012, Zhang Rui wrote: > > I'm not exactly sure what you're trying to achieve and what you mean by > > "resume a device directly"? Do you want to run the device's resume > > callback at the time when another device is being resumed? > > > I mean, wakeup event is sent to ATA port, but our goal is to resume > ZPODD after receiving this wakeup event. > Ideally, it is ACPI that resumes ATA port. And then, the ATA port > runtime resumes ZPODD. But this does not look good to runtime resume a > child device in the parent's .runtime_resume callback. > So I introduced these two APIs so that an runtime_resume request can be > sent to ZPODD directly and the runtime PM core can resume all the > parents of ZPODD automatically. It's not clear what you're trying to achieve. Do you basically want the ZPODD always to be suspended and resumed along with the ATA port, or should it be possible to suspend the ZPODD while the port remains running? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-02-16 at 10:13 -0500, Alan Stern wrote: > On Thu, 16 Feb 2012, Zhang Rui wrote: > > > > I'm not exactly sure what you're trying to achieve and what you mean by > > > "resume a device directly"? Do you want to run the device's resume > > > callback at the time when another device is being resumed? > > > > > I mean, wakeup event is sent to ATA port, but our goal is to resume > > ZPODD after receiving this wakeup event. > > Ideally, it is ACPI that resumes ATA port. And then, the ATA port > > runtime resumes ZPODD. But this does not look good to runtime resume a > > child device in the parent's .runtime_resume callback. > > So I introduced these two APIs so that an runtime_resume request can be > > sent to ZPODD directly and the runtime PM core can resume all the > > parents of ZPODD automatically. > > It's not clear what you're trying to achieve. Do you basically want > the ZPODD always to be suspended and resumed along with the ATA port, > or should it be possible to suspend the ZPODD while the port remains > running? We want to ZPODD always to be suspended and resumed along with the ATA port. Below is part of the GPE handler for ZPODD device attention event. Scope (\_GPE) { Method (_L13, 0, NotSerialized) { ADBG ("ZPODD DA Event") .... Notify (\_SB.PCI0.SAT0.PRT2, 0x02) .... } } It maybe a bit confused, but actually, \_SB.PCI0.SAT0.PRT2 is bind to the attached device, not the ata port itself. See below commit in linux-next tree. 75d22c(libata: Bind the Linux device tree to the ACPI device tree) And below notify handler(PATCH 6) will resume the attached device(CDROM in ZPODD case). +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); +} + But the code to power on/power off the device is in ata_acpi_set_state, which is called when ata port is resumed/suspended. ata_eh_handle_port_resume/suspend ata_acpi_set_state ata_for_each_dev { acpi_bus_set_power(<the acpi handle of the device>, acpi_state) } Could you take a look at PATCH 6? It's more clear over there. Thanks, Lin Ming > > Alan Stern > -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, alan, > -----Original Message----- > From: Alan Stern [mailto:stern@rowland.harvard.edu] > Sent: Thursday, February 16, 2012 11:14 PM > To: Zhang, Rui > Cc: Rafael J. Wysocki; Lin, Ming M; Jeff Garzik; Tejun Heo; Len Brown; > linux-kernel@vger.kernel.org; linux-ide@vger.kernel.org; linux- > scsi@vger.kernel.org; linux-pm@vger.kernel.org > Subject: Re: [RFC PATCH 2/6] ACPI: Reference devices in ACPI Power > Resource > Importance: High > > On Thu, 16 Feb 2012, Zhang Rui wrote: > > > > I'm not exactly sure what you're trying to achieve and what you > mean by > > > "resume a device directly"? Do you want to run the device's resume > > > callback at the time when another device is being resumed? > > > > > I mean, wakeup event is sent to ATA port, but our goal is to resume > > ZPODD after receiving this wakeup event. > > Ideally, it is ACPI that resumes ATA port. And then, the ATA port > > runtime resumes ZPODD. But this does not look good to runtime resume > a > > child device in the parent's .runtime_resume callback. > > So I introduced these two APIs so that an runtime_resume request can > be > > sent to ZPODD directly and the runtime PM core can resume all the > > parents of ZPODD automatically. > > It's not clear what you're trying to achieve. > Do you basically want > the ZPODD always to be suspended and resumed along with the ATA port, No. ZPODD suspends itself, which put ZPODD to a SCSI low power state (NOT power off/D3_COLD). And then it is the "Runtime PM core" that suspends ATA port after ZPODD being suspended. And the .runtime_suspend callback for ATA port actually turns off the ZPODD power. During resume, ATA port is resumed first because of the ACPI wakeup event. But in fact, this wakeup event should be read as "ZPODD remote wakeup signal", thus runtime resume request is sent to ZPODD, done by Patch 3/6. > or should it be possible to suspend the ZPODD while the port remains > running? > Sure, but the power is still on at this time. Thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 17 Feb 2012, Zhang, Rui wrote: > > Do you basically want > > the ZPODD always to be suspended and resumed along with the ATA port, > > No. ZPODD suspends itself, which put ZPODD to a SCSI low power state (NOT power off/D3_COLD). > And then it is the "Runtime PM core" that suspends ATA port after ZPODD being suspended. > And the .runtime_suspend callback for ATA port actually turns off the ZPODD power. > > During resume, ATA port is resumed first because of the ACPI wakeup event. > But in fact, this wakeup event should be read as "ZPODD remote wakeup signal", thus runtime resume request is sent to ZPODD, done by Patch 3/6. > > > or should it be possible to suspend the ZPODD while the port remains > > running? > > > Sure, but the power is still on at this time. Then maybe you can use pm_runtime_no_callbacks() for the ZPODD device. It's explained in Documentation/power/runtime_pm.txt, and I use it for USB interfaces. The idea is that the ZPODD will never receive any runtime PM callbacks from the PM core. Instead the ATA port callback routines will be responsible for power management of the ZPODD device. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, February 16, 2012, Zhang Rui wrote: > On 二, 2012-02-14 at 23:36 +0100, Rafael J. Wysocki wrote: > > > > > + */ > > > > > +struct acpi_powered_device { > > > > > + struct list_head node; > > > > > + struct device *dev; > > > > > +}; > > > > > + > > > > > /* -------------------------------------------------------------------------- > > > > > Power Resource Management > > > > > -------------------------------------------------------------------------- */ > > > > > @@ -455,6 +470,118 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev) > > > > > Device Power Management > > > > > -------------------------------------------------------------------------- */ > > > > > > > > > > +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle) > > > > > +{ > > > > > + struct acpi_power_resource *resource; > > > > > + struct acpi_device *acpi_dev, *res_dev; > > > > > + acpi_handle res_handle = NULL; > > > > > + struct acpi_powered_device *apd; > > > > > + int i; > > > > > + int ret; > > > > > + > > > > > + if (!handle || !dev) > > > > > + return -EINVAL; > > > > > + > > > > > + 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; > > > > > + > > > > > + for (i = 0; i <= ACPI_STATE_D3; i++) { > > > > > + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; > > > > > + > > > > > + if (!ps->flags.valid) > > > > > + continue; > > > > > + > > > > > + if (ps->resources.count > 1) > > > > > + return 0; > > > > > + > > > > > + if (!res_handle) > > > > > + res_handle = ps->resources.handles[0]; > > > > > + else if (res_handle != ps->resources.handles[0]) > > > > > + return 0; > > > > > + } > > > > > > > > I'm not sure what the above checks are needed for. It seems that this function > > > > will only be called from acpi_bus_add_power_resource() (which needs to be > > > > modified for this purpose, BTW), so it doesn't need to check all those things > > > > (those checks have been made already). > > > > > > > No. These two APIs are introduced to support the runtime D3_COLD remote > > > wakeup. And they should be invoked by drivers, either in driver code or > > > via bus layer code. > > > > > > Say, ATA port, who has _PR3 in its ACPI node, knows that it can enter > > > D3_COLD at run time, and it supports remote wakeup in D3_COLD because it > > > has _S0W (return value 4). > > > When remote wakeup event is triggered, there is an ACPI event sent to > > > the ATA controller/port, which sets the ATA controller/port back to D0 > > > state. > > > At this time, what we actually need is to resume the ZPODD, rather than > > > the ATA controller/port. To follow the runtime PM model (runtime resume > > > starts from leaf devices), > > > > Well, this isn't the case. Parents are always resumed first. > > > Well, I mean runtime resume request issued by leaf devices first. That still causes runtime PM callbacks for parents to be run before runtime PM callbacks for the children. It is impossible to runtime-resume a child device (using the runtime PM framework's functions at least) if the parent of it is not "active". > > > ATA code can use these two APIs to tell ACPI > > > to runtime resume ZPODD device directly, because ZPODD is powered by > > > this Power Resource as well. > > > > I'm not exactly sure what you're trying to achieve and what you mean by > > "resume a device directly"? Do you want to run the device's resume > > callback at the time when another device is being resumed? > > > I mean, wakeup event is sent to ATA port, but our goal is to resume > ZPODD after receiving this wakeup event. > Ideally, it is ACPI that resumes ATA port. And then, the ATA port > runtime resumes ZPODD. But this does not look good to runtime resume a > child device in the parent's .runtime_resume callback. OK, I think I see what the problem is. > So I introduced these two APIs so that an runtime_resume request can be > sent to ZPODD directly and the runtime PM core can resume all the > parents of ZPODD automatically. It seems that you're confusing things. In the Linux driver model a device cannot have more than a single parent. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, February 17, 2012, Lin Ming wrote: > On Thu, 2012-02-16 at 10:13 -0500, Alan Stern wrote: > > On Thu, 16 Feb 2012, Zhang Rui wrote: > > > > > > I'm not exactly sure what you're trying to achieve and what you mean by > > > > "resume a device directly"? Do you want to run the device's resume > > > > callback at the time when another device is being resumed? > > > > > > > I mean, wakeup event is sent to ATA port, but our goal is to resume > > > ZPODD after receiving this wakeup event. > > > Ideally, it is ACPI that resumes ATA port. And then, the ATA port > > > runtime resumes ZPODD. But this does not look good to runtime resume a > > > child device in the parent's .runtime_resume callback. > > > So I introduced these two APIs so that an runtime_resume request can be > > > sent to ZPODD directly and the runtime PM core can resume all the > > > parents of ZPODD automatically. > > > > It's not clear what you're trying to achieve. Do you basically want > > the ZPODD always to be suspended and resumed along with the ATA port, > > or should it be possible to suspend the ZPODD while the port remains > > running? > > We want to ZPODD always to be suspended and resumed along with the ATA > port. > > Below is part of the GPE handler for ZPODD device attention event. > > Scope (\_GPE) > { > Method (_L13, 0, NotSerialized) > { > ADBG ("ZPODD DA Event") > .... > > Notify (\_SB.PCI0.SAT0.PRT2, 0x02) > .... > } > } > > It maybe a bit confused, but actually, \_SB.PCI0.SAT0.PRT2 is bind to > the attached device, not the ata port itself. > > See below commit in linux-next tree. > 75d22c(libata: Bind the Linux device tree to the ACPI device tree) > > And below notify handler(PATCH 6) will resume the attached device(CDROM > in ZPODD case). > > +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); > +} > + > > But the code to power on/power off the device is in ata_acpi_set_state, > which is called when ata port is resumed/suspended. > > ata_eh_handle_port_resume/suspend > ata_acpi_set_state > ata_for_each_dev { > acpi_bus_set_power(<the acpi handle of the device>, acpi_state) > } > > Could you take a look at PATCH 6? > It's more clear over there. It seems that you can use pm_runtime_no_callbacks() to work around this as suggested by Alan. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 五, 2012-02-17 at 23:34 +0100, Rafael J. Wysocki wrote: > > > So I introduced these two APIs so that an runtime_resume request can be > > sent to ZPODD directly and the runtime PM core can resume all the > > parents of ZPODD automatically. > > It seems that you're confusing things. In the Linux driver model a device > cannot have more than a single parent. > sorry, I mean the ancients here, e.g. scsi-host, etc. :) thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 17, 2012 at 11:07 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > On Fri, 17 Feb 2012, Zhang, Rui wrote: > >> > Do you basically want >> > the ZPODD always to be suspended and resumed along with the ATA port, >> >> No. ZPODD suspends itself, which put ZPODD to a SCSI low power state (NOT power off/D3_COLD). >> And then it is the "Runtime PM core" that suspends ATA port after ZPODD being suspended. >> And the .runtime_suspend callback for ATA port actually turns off the ZPODD power. >> >> During resume, ATA port is resumed first because of the ACPI wakeup event. >> But in fact, this wakeup event should be read as "ZPODD remote wakeup signal", thus runtime resume request is sent to ZPODD, done by Patch 3/6. >> >> > or should it be possible to suspend the ZPODD while the port remains >> > running? >> > >> Sure, but the power is still on at this time. > > Then maybe you can use pm_runtime_no_callbacks() for the ZPODD device. > It's explained in Documentation/power/runtime_pm.txt, and I use it for > USB interfaces. If pm_runtime_no_callbacks() is used, runtime PM sysfs attributes won't be created. Then how to disable ZPODD feature in userspace? Currently, I use "control" file of scsi device to enable/disable ZPODD, for example echo auto > /sys/devices/pci0000:00/0000:00:1f.2/ata0/host1/target1:0:0/1:0:0:0/power/control echo on > /sys/devices/pci0000:00/0000:00:1f.2/ata0/host1/target1:0:0/1:0:0:0/power/control > > The idea is that the ZPODD will never receive any runtime PM callbacks > from the PM core. Instead the ATA port callback routines will be > responsible for power management of the ZPODD device. Does the ATA port callback also responsible to resume its child? For example, /sys/devices/pci0000:00/0000:00:1f.2/ata0/host1/target1:0:0/1:0:0:0/ ata0 is resumed. Then who will be responsible to resume host1, target1:0:0 and 1:0:0:0? Or do you mean that we don't need to resume these devices at all? host1 and target1:0:0 are logical devices, but I think 1:0:0:0 is not. Thanks, Lin Ming -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c index 0d681fb..a7e2305 100644 --- a/drivers/acpi/power.c +++ b/drivers/acpi/power.c @@ -84,10 +84,25 @@ struct acpi_power_resource { u32 order; unsigned int ref_count; struct mutex resource_lock; + struct list_head devices; /* list for devices powered by this PR */ }; static struct list_head acpi_power_resource_list; +/* + * When a power resource is turned on, all the devices are put to an uninitialized + * state although their ACPI states are still D0. + * To handle this, we need to keep a list of all devices powered by the power resource, + * and resume all of them. + * Currently, we only support this case: + * 1) multiple devices share the same power resoruce, + * 2) One device uses One Power Resource ONLY. + */ +struct acpi_powered_device { + struct list_head node; + struct device *dev; +}; + /* -------------------------------------------------------------------------- Power Resource Management -------------------------------------------------------------------------- */ @@ -455,6 +470,118 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev) Device Power Management -------------------------------------------------------------------------- */ +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle) +{ + struct acpi_power_resource *resource; + struct acpi_device *acpi_dev, *res_dev; + acpi_handle res_handle = NULL; + struct acpi_powered_device *apd; + int i; + int ret; + + if (!handle || !dev) + return -EINVAL; + + 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; + + for (i = 0; i <= ACPI_STATE_D3; i++) { + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; + + if (!ps->flags.valid) + continue; + + if (ps->resources.count > 1) + return 0; + + if (!res_handle) + res_handle = ps->resources.handles[0]; + else if (res_handle != ps->resources.handles[0]) + return 0; + } + + ret = acpi_bus_get_device(res_handle, &res_dev); + if (ret) + goto no_power_resource; + + resource = res_dev->driver_data; + if (!resource) + goto no_power_resource; + + apd = kzalloc(sizeof(struct acpi_powered_device), GFP_KERNEL); + if (!apd) + return -ENOMEM; + + apd->dev = dev; + mutex_lock(&resource->resource_lock); + list_add_tail(&apd->node, &resource->devices); + mutex_unlock(&resource->resource_lock); + printk(KERN_INFO PREFIX "Device %s uses Power Resource %s\n", dev->kobj.name, acpi_device_name(res_dev)); + + return 0; + +no_power_resource: + printk(KERN_WARNING PREFIX "Invalid Power Resource to register!"); + return -ENODEV; +} + +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle) +{ + struct acpi_power_resource *resource; + struct acpi_device *acpi_dev, *res_dev; + acpi_handle res_handle = NULL; + struct acpi_powered_device *apd, *tmp; + int i; + int ret; + + if (!handle || !dev) + return; + + ret = acpi_bus_get_device(handle, &acpi_dev); + if (ret) + return; + + if (!acpi_dev->power.flags.power_resources) + return; + + for (i = 0; i <= ACPI_STATE_D3; i++) { + struct acpi_device_power_state *ps = &acpi_dev->power.states[i]; + + if (!ps->flags.valid) + continue; + + if (ps->resources.count > 1) + return; + + res_handle = ps->resources.handles[0]; + break; + } + + if (!res_handle) + return; + + ret = acpi_bus_get_device(res_handle, &res_dev); + if (ret) + return; + + resource = res_dev->driver_data; + if (!resource) + return; + + mutex_lock(&resource->resource_lock); + list_for_each_entry_safe(apd, tmp, &resource->devices, node) { + if (apd->dev == dev) { + list_del(&apd->node); + kfree(apd); + } + } + mutex_unlock(&resource->resource_lock); +} + int acpi_power_get_inferred_state(struct acpi_device *device, int *state) { int result = 0; @@ -550,6 +677,7 @@ static int acpi_power_add(struct acpi_device *device) resource->device = device; mutex_init(&resource->resource_lock); + INIT_LIST_HEAD(&resource->devices); strcpy(resource->name, device->pnp.bus_id); strcpy(acpi_device_name(device), ACPI_POWER_DEVICE_NAME); strcpy(acpi_device_class(device), ACPI_POWER_CLASS); diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 6cd5b64..32fb899 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -322,6 +322,8 @@ int acpi_bus_get_status(struct acpi_device *device); 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); +int acpi_power_resource_register_device(struct device *dev, acpi_handle handle); +void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handle); bool acpi_bus_can_wakeup(acpi_handle handle); #ifdef CONFIG_ACPI_PROC_EVENT int acpi_bus_generate_proc_event(struct acpi_device *device, u8 type, int data);