| Message ID | 1346053126-7646-1-git-send-email-aaron.lu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Monday, August 27, 2012, Aaron Lu wrote: > Currently, when we are trying to get the power state of an acpi device, > we will do the following: > If device is not power manageable, init its power state as its parent or > if it does not have a parent, init as D0; > If device is power manageable, evaluate _PSC and then refine with > acpi_power_get_inferred_state. > > But there exist some devices with _PSx defined, but no _PSC or _PRx. > It is power manageable, but the above method to get power state does > not cover this case and its power state will be UNKNOWN(255). > > So change the check of power manageable to whether _PSC and _PRx > defined. > > Signed-off-by: Aaron Lu <aaron.lu@intel.com> Applied to the linux-next branch of the linux-pm.git tree. I think it should go to -stable too and therefore it would be good to have it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections. Thanks, Rafael > --- > drivers/acpi/bus.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index 9628652..b564e6d 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -207,7 +207,8 @@ static int __acpi_bus_get_power(struct acpi_device *device, int *state) > if (!device || !state) > return -EINVAL; > > - if (!device->flags.power_manageable) { > + if (!device->power.flags.explicit_get && > + !device->power.flags.power_resources) { > /* TBD: Non-recursive algorithm for walking up hierarchy. */ > *state = device->parent ? > device->parent->power.state : ACPI_STATE_D0; > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, September 06, 2012, Rafael J. Wysocki wrote: > On Monday, August 27, 2012, Aaron Lu wrote: > > Currently, when we are trying to get the power state of an acpi device, > > we will do the following: > > If device is not power manageable, init its power state as its parent or > > if it does not have a parent, init as D0; > > If device is power manageable, evaluate _PSC and then refine with > > acpi_power_get_inferred_state. > > > > But there exist some devices with _PSx defined, but no _PSC or _PRx. > > It is power manageable, but the above method to get power state does > > not cover this case and its power state will be UNKNOWN(255). > > > > So change the check of power manageable to whether _PSC and _PRx > > defined. > > > > Signed-off-by: Aaron Lu <aaron.lu@intel.com> > > Applied to the linux-next branch of the linux-pm.git tree. > > I think it should go to -stable too and therefore it would be good to have > it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections. On a second thought, perhaps the initial power state of those devices _should_ be "unknown", hmm? After all, we don't know what power state the device is in. Is there any practical user-visible problem this causes to happen? Rafael > > --- > > drivers/acpi/bus.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > > index 9628652..b564e6d 100644 > > --- a/drivers/acpi/bus.c > > +++ b/drivers/acpi/bus.c > > @@ -207,7 +207,8 @@ static int __acpi_bus_get_power(struct acpi_device *device, int *state) > > if (!device || !state) > > return -EINVAL; > > > > - if (!device->flags.power_manageable) { > > + if (!device->power.flags.explicit_get && > > + !device->power.flags.power_resources) { > > /* TBD: Non-recursive algorithm for walking up hierarchy. */ > > *state = device->parent ? > > device->parent->power.state : ACPI_STATE_D0; > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 06, 2012 at 11:53:34PM +0200, Rafael J. Wysocki wrote: > On Thursday, September 06, 2012, Rafael J. Wysocki wrote: > > On Monday, August 27, 2012, Aaron Lu wrote: > > > Currently, when we are trying to get the power state of an acpi device, > > > we will do the following: > > > If device is not power manageable, init its power state as its parent or > > > if it does not have a parent, init as D0; > > > If device is power manageable, evaluate _PSC and then refine with > > > acpi_power_get_inferred_state. > > > > > > But there exist some devices with _PSx defined, but no _PSC or _PRx. > > > It is power manageable, but the above method to get power state does > > > not cover this case and its power state will be UNKNOWN(255). > > > > > > So change the check of power manageable to whether _PSC and _PRx > > > defined. > > > > > > Signed-off-by: Aaron Lu <aaron.lu@intel.com> > > > > Applied to the linux-next branch of the linux-pm.git tree. > > > > I think it should go to -stable too and therefore it would be good to have > > it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections. > > On a second thought, perhaps the initial power state of those devices > _should_ be "unknown", hmm? > > After all, we don't know what power state the device is in. Agree here but I think this is a safe assumption that on initial system boot, all devices should be at D0. We have already assumed this fact if the device is not power manageable and has no parent. And this patch just changed the condition to not judging if power manageable but if it has _PSC or _PRx, since we will use that to know the device's power state, so I think using _PSC or _PRx as the condition check is more precise. > > Is there any practical user-visible problem this causes to happen? Yes, on a test system, when I try to put a device into D3 cold and ACPI will complain that I can't due to its parent is in a even lower power state UNKNOWN(255), this parent device is power manageable but has no _PSC and _PRx defined. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, September 07, 2012, Aaron Lu wrote: > On Thu, Sep 06, 2012 at 11:53:34PM +0200, Rafael J. Wysocki wrote: > > On Thursday, September 06, 2012, Rafael J. Wysocki wrote: > > > On Monday, August 27, 2012, Aaron Lu wrote: > > > > Currently, when we are trying to get the power state of an acpi device, > > > > we will do the following: > > > > If device is not power manageable, init its power state as its parent or > > > > if it does not have a parent, init as D0; > > > > If device is power manageable, evaluate _PSC and then refine with > > > > acpi_power_get_inferred_state. > > > > > > > > But there exist some devices with _PSx defined, but no _PSC or _PRx. > > > > It is power manageable, but the above method to get power state does > > > > not cover this case and its power state will be UNKNOWN(255). > > > > > > > > So change the check of power manageable to whether _PSC and _PRx > > > > defined. > > > > > > > > Signed-off-by: Aaron Lu <aaron.lu@intel.com> > > > > > > Applied to the linux-next branch of the linux-pm.git tree. > > > > > > I think it should go to -stable too and therefore it would be good to have > > > it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections. > > > > On a second thought, perhaps the initial power state of those devices > > _should_ be "unknown", hmm? > > > > After all, we don't know what power state the device is in. > > Agree here but I think this is a safe assumption that on initial system > boot, all devices should be at D0. We have already assumed this fact if > the device is not power manageable and has no parent. That is mandated by the spec, though. > And this patch just changed the condition to not judging if power > manageable but if it has _PSC or _PRx, since we will use that to know > the device's power state, so I think using _PSC or _PRx as the > condition check is more precise. It is not clear if we can assume anything about the initial power states of devices having _PSx defined if they cannot be determined through _PSC or power resources. > > Is there any practical user-visible problem this causes to happen? > > Yes, on a test system, when I try to put a device into D3 cold and ACPI > will complain that I can't due to its parent is in a even lower power > state UNKNOWN(255), this parent device is power manageable but has no > _PSC and _PRx defined. Perhaps we can force _PS0 for such devices to start with, so that we know for sure that the initial state is D0? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/07/2012 07:46 PM, Rafael J. Wysocki wrote: >> Yes, on a test system, when I try to put a device into D3 cold and ACPI >> will complain that I can't due to its parent is in a even lower power >> state UNKNOWN(255), this parent device is power manageable but has no >> _PSC and _PRx defined. > > Perhaps we can force _PS0 for such devices to start with, so that we know > for sure that the initial state is D0? Sounds good, I'll update the patch, thanks for the advice. -Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" 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/bus.c b/drivers/acpi/bus.c index 9628652..b564e6d 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -207,7 +207,8 @@ static int __acpi_bus_get_power(struct acpi_device *device, int *state) if (!device || !state) return -EINVAL; - if (!device->flags.power_manageable) { + if (!device->power.flags.explicit_get && + !device->power.flags.power_resources) { /* TBD: Non-recursive algorithm for walking up hierarchy. */ *state = device->parent ? device->parent->power.state : ACPI_STATE_D0;
Currently, when we are trying to get the power state of an acpi device, we will do the following: If device is not power manageable, init its power state as its parent or if it does not have a parent, init as D0; If device is power manageable, evaluate _PSC and then refine with acpi_power_get_inferred_state. But there exist some devices with _PSx defined, but no _PSC or _PRx. It is power manageable, but the above method to get power state does not cover this case and its power state will be UNKNOWN(255). So change the check of power manageable to whether _PSC and _PRx defined. Signed-off-by: Aaron Lu <aaron.lu@intel.com> --- drivers/acpi/bus.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)