PNPACPI: Enable Power Support

Subject: Enable PNPACPI _PSx Support

(This is an updated patch of http://lkml.org/lkml/2008/11/23/211)

This patch sets the power of PnP ACPI devices to D0 when they are activated and to D3 when they are disabled. The latter is in correspondence with the ACPI 3.0 specification, whereas the former is added in order to be able to power up a device after it has been previously disabled (or when booting up a system). (As a consequence, the patch makes the PnP ACPI code more ACPI compliant.)

Section 6.2.2 of the ACPI Specification (at least versions 1.0b and 3.0a) states: "Prior to running this control method [_DIS], the OS[PM] will have already put the device in the D3 state." Unfortunately, there is no clear statement as to when to put a device in the D0 state. :-( Therefore, the patch executes the method calls as _PS3/_DIS and _SRS/_PS0. What is clear: "If the device is disabled, _SRS enables the device at the specified resources." (From the ACPI 3.0a Specification.)

The patch fixes a problem with some IBM ThinkPads (at least the 600E and the 600X) where the serial ports have a dedicated power source that needs to be brought up before the serial port can be used. Without this patch, the serial port is enabled but has no power. (In the past, the tpctl utility had to be utilized to turn on the power, but support for this feature stopped with version 5.9 as it did not support the more recent kernel versions.)

No regressions were observed on hardware that does not require this patch.

The patch is applied against 2.6.27.8 (vanilla).

Signed-off-by Witold Szczeponik <[email protected]>

Index: linux/drivers/pnp/pnpacpi/core.c
===================================================================

drivers/pnp/pnpacpi/core.c +5 -2

@@ -98,17 +98,20 @@ static int pnpacpi_set_resources(struct
 	status = acpi_set_current_resources(handle, &buffer);
 	if (ACPI_FAILURE(status))
 		ret = -EINVAL;
+	else
+		acpi_bus_set_power(handle, ACPI_STATE_D0);
 	kfree(buffer.pointer);
 	return ret;
 }
 
 static int pnpacpi_disable_resources(struct pnp_dev *dev)
 {
+	acpi_handle handle = dev->data;
 	acpi_status status;
 
 	/* acpi_unregister_gsi(pnp_irq(dev, 0)); */
-	status = acpi_evaluate_object((acpi_handle) dev->data,
-				      "_DIS", NULL, NULL);
+	acpi_bus_set_power(handle, ACPI_STATE_D3);
+	status = acpi_evaluate_object(handle, "_DIS", NULL, NULL);
 	return ACPI_FAILURE(status) ? -ENODEV : 0;
 }
 

Hi Witold,

All in all your patch looks good, especially in the context of resolving the Thinkpad 600X issue. I have some patches on the way that will add more complete power management support to the PNP stack and hopefully allow for runtime PM of these system devices, but I think this is a good short-term solution.

One quick comment. acpi_bus_set_power() can fail, and pnpacpi_set_resources() needs to handle this gracefully. Please update the patch to include this.

Also, for the follow up to this patch, I'm wondering what sort of ACPI behavior we need to support D1 and D2 states. In PCI, disabling resource decoding (similar to _DIS) is only required for entering D3. The ACPI spec seems to be ambiguous with this issue. Len, any thoughts?

Thanks,
Adam

Quoting Witold Szczeponik <[email protected]>:

Quoted 66 lines

Subject: Enable PNPACPI _PSx Support

(This is an updated patch of http://lkml.org/lkml/2008/11/23/211)

This patch sets the power of PnP ACPI devices to D0 when they are activated and to D3 when they are disabled. The latter is in correspondence with the ACPI 3.0 specification, whereas the former is added in order to be able to power up a device after it has been previously disabled (or when booting up a system). (As a consequence, the patch makes the PnP ACPI code more ACPI compliant.)

Section 6.2.2 of the ACPI Specification (at least versions 1.0b and 3.0a) states: "Prior to running this control method [_DIS], the OS[PM] will have already put the device in the D3 state." Unfortunately, there is no clear statement as to when to put a device in the D0 state. :-( Therefore, the patch executes the method calls as _PS3/_DIS and _SRS/_PS0. What is clear: "If the device is disabled, _SRS enables the device at the specified resources." (From the ACPI 3.0a Specification.)

The patch fixes a problem with some IBM ThinkPads (at least the 600E and the 600X) where the serial ports have a dedicated power source that needs to be brought up before the serial port can be used. Without this patch, the serial port is enabled but has no power. (In the past, the tpctl utility had to be utilized to turn on the power, but support for this feature stopped with version 5.9 as it did not support the more recent kernel versions.)

No regressions were observed on hardware that does not require this patch.

The patch is applied against 2.6.27.8 (vanilla).

Signed-off-by Witold Szczeponik <[email protected]>

Index: linux/drivers/pnp/pnpacpi/core.c
===================================================================

drivers/pnp/pnpacpi/core.c +5 -2

@@ -98,17 +98,20 @@ static int pnpacpi_set_resources(struct
 	status = acpi_set_current_resources(handle, &buffer);
 	if (ACPI_FAILURE(status))
 		ret = -EINVAL;
+	else
+		acpi_bus_set_power(handle, ACPI_STATE_D0);
 	kfree(buffer.pointer);
 	return ret;
 }
 
 static int pnpacpi_disable_resources(struct pnp_dev *dev)
 {
+	acpi_handle handle = dev->data;
 	acpi_status status;
 
 	/* acpi_unregister_gsi(pnp_irq(dev, 0)); */
-	status = acpi_evaluate_object((acpi_handle) dev->data,
-				      "_DIS", NULL, NULL);
+	acpi_bus_set_power(handle, ACPI_STATE_D3);
+	status = acpi_evaluate_object(handle, "_DIS", NULL, NULL);
 	return ACPI_FAILURE(status) ? -ENODEV : 0;
 }

Hi Adam,

Do you have any proposals for how pnpacpi_set_resources() should handle failure? Witold started out with code to call acpi_bus_set_power() only if the device is power-manageable, and he then passed the return code up. I suggested letting acpi_bus_set_power() do the "is-power-manageable" check. But then the caller would have to distinguish the "device isn't power-manageable" error from others, which is kind of ugly.

Bjorn

Hi Witold,

All in all your patch looks good, especially in the context of resolving the Thinkpad 600X issue. I have some patches on the way that will add more complete power management support to the PNP stack and hopefully allow for runtime PM of these system devices, but I think this is a good short-term solution.

One quick comment. acpi_bus_set_power() can fail, and pnpacpi_set_resources() needs to handle this gracefully. Please update the patch to include this.

Also, for the follow up to this patch, I'm wondering what sort of ACPI behavior we need to support D1 and D2 states. In PCI, disabling resource decoding (similar to _DIS) is only required for entering D3. The ACPI spec seems to be ambiguous with this issue. Len, any thoughts?

Thanks,
Adam

Quoting Witold Szczeponik <[email protected]>:

Quoted 68 lines

Subject: Enable PNPACPI _PSx Support

(This is an updated patch of http://lkml.org/lkml/2008/11/23/211)

This patch sets the power of PnP ACPI devices to D0 when they are activated and to D3 when they are disabled. The latter is in correspondence with the ACPI 3.0 specification, whereas the former is added in order to be able to power up a device after it has been previously disabled (or when booting up a system). (As a consequence, the patch makes the PnP ACPI code more ACPI compliant.)

Section 6.2.2 of the ACPI Specification (at least versions 1.0b and 3.0a) states: "Prior to running this control method [_DIS], the OS[PM] will have already put the device in the D3 state." Unfortunately, there is no clear statement as to when to put a device in the D0 state. :-( Therefore, the patch executes the method calls as _PS3/_DIS and _SRS/_PS0. What is clear: "If the device is disabled, _SRS enables the device at the specified resources." (From the ACPI 3.0a Specification.)

The patch fixes a problem with some IBM ThinkPads (at least the 600E and the 600X) where the serial ports have a dedicated power source that needs to be brought up before the serial port can be used. Without this patch, the serial port is enabled but has no power. (In the past, the tpctl utility had to be utilized to turn on the power, but support for this feature stopped with version 5.9 as it did not support the more recent kernel versions.)

No regressions were observed on hardware that does not require this patch.

The patch is applied against 2.6.27.8 (vanilla).

Signed-off-by Witold Szczeponik <[email protected]>

Index: linux/drivers/pnp/pnpacpi/core.c
===================================================================

drivers/pnp/pnpacpi/core.c +5 -2

@@ -98,17 +98,20 @@ static int pnpacpi_set_resources(struct
     status = acpi_set_current_resources(handle, &buffer);
     if (ACPI_FAILURE(status))
         ret = -EINVAL;
+    else
+        acpi_bus_set_power(handle, ACPI_STATE_D0);
     kfree(buffer.pointer);
     return ret;
 }
 
 static int pnpacpi_disable_resources(struct pnp_dev *dev)
 {
+    acpi_handle handle = dev->data;
     acpi_status status;
 
     /* acpi_unregister_gsi(pnp_irq(dev, 0)); */
-    status = acpi_evaluate_object((acpi_handle) dev->data,
-                      "_DIS", NULL, NULL);
+    acpi_bus_set_power(handle, ACPI_STATE_D3);
+    status = acpi_evaluate_object(handle, "_DIS", NULL, NULL);
     return ACPI_FAILURE(status) ? -ENODEV : 0;
 }
 

This patch sets the power of PnP ACPI devices to D0 when they are activated and to D3 when they are disabled. The latter is

Although I do wonder WTF we don't do that on our USB UHCI/EHCI kernel drivers, that ACPI needs to step in to fix it.  If it is not a wake

device, why are we leaving it powered up?  It is bad enough we don't have any sort of proper USB power control, but to leave the entire USB subsystem powered up without reason?!

Vista not only powers down the USB HCIs and ports on STR, it also powers down ports when you "safely remove the USB device", and seems to leave the port powered down until you insert a device (or remove the device already in there, whatever).

Am Mittwoch, 10. Dezember 2008 10:53:15 schrieb Henrique de Moraes Holschuh:

Quoted 2 lines

Although I do wonder WTF we don't do that on our USB UHCI/EHCI kernel drivers, that ACPI needs to step in to fix it.  If it is not a wake

EHCI/UHCI have no way to power down host controllers. It needs external means, namely ACPI.

Quoted 3 lines

device, why are we leaving it powered up?  It is bad enough we don't have any sort of proper USB power control, but to leave the entire USB subsystem powered up without reason?!

We don't. If you unplug all devices, USB will power down as much as is compatible with detecting new devices.

Quoted 4 lines

Vista not only powers down the USB HCIs and ports on STR, it also powers down ports when you "safely remove the USB device", and seems to leave the port powered down until you insert a device (or remove the device already in there, whatever).

If you cut power to a port, you cannot detect a hotplugging. You can suspend it, which Linux does do.

Quoted 2 lines · Witold Szczeponik

This patch sets the power of PnP ACPI devices to D0 when they are activated and to D3 when they are disabled. The latter is

This will likely fix my T43 to power down USB during STR. THANKS!! Now, I just need to check if the ACPI BIOS is smart enough to not do it when they are configured as wake devices [in the BIOS].

Quoted 4 lines · Henrique de Moraes Holschuh

Quoted 2 lines · Witold Szczeponik

This patch sets the power of PnP ACPI devices to D0 when they are activated and to D3 when they are disabled. The latter is

Quoted 4 lines

This will likely fix my T43 to power down USB during STR. THANKS!! Now, I just need to check if the ACPI BIOS is smart enough to not do it when they are configured as wake devices [in the BIOS].

Most likely it won't. :-( My patch takes only care of devices that are properly registered as PnP by ACPI: attached USB devices don't fall under that category.

Quoted 5 lines · Witold Szczeponik

Quoted 4 lines · Henrique de Moraes Holschuh

Quoted 2 lines · Witold Szczeponik

This patch sets the power of PnP ACPI devices to D0 when they are activated and to D3 when they are disabled. The latter is

Quoted 8 lines

Quoted 4 lines

This will likely fix my T43 to power down USB during STR. THANKS!! Now, I just need to check if the ACPI BIOS is smart enough to not do it when they are configured as wake devices [in the BIOS].

Most likely it won't. :-( My patch takes only care of devices that are properly registered as PnP by ACPI: attached USB devices don't fall under that category.

I do mean the UHCI and EHCI bridges, not USB devices hanging off them :)

I will do an in-depth study of the AML in question, I just noticed it is coupled to some EC magic register, which might be doing stuff the USB subsystem just can't do normally.

Bah, never mind. Major issue between the keyboard and the chair, on my end... Got PR# and PS# confused. The ThinkPad ACPI DSDT in question has the standard crap for wake device control, and PR# methods, but no PS# methods.

The PR0..PR2 nodes for the UHCI and EHCI USB ports on the ThinkPad T43 mention a power resource object, and that power resource CAN power on/off the USB ports (and that freaks out the UHCI and EHCI kernel drivers causing over-current reports, but that was to be expected). The AML code tells the EC to do some magic that probably tells a MOSFET somewhere in the mainboard to stop supplying power to the ports.

And Windows XP (this is not a Vista BIOS and I never tried running Vista in it...) is noticing that power resource (even without PS3 or PR3 nodes) and calling that power resource _OFF handler to power down USB ports when it doesn't need these devices for wakeup reasons, before entering STR.

Anyone has any clue on what part of the ACPI spec could result on that behaviour? Because otherwise I will just bloody special-case it into thinkpad-acpi's suspend and resume stuff. It will piss the USB subsystem off if I can't arrange to do it on early resume and late suspend, however.