Re: [PATCH] PNPACPI: Enable Power Support

[Witold Szczeponik <Witold.Szczeponik@gmx.net>]

Adam M Belay wrote:

> 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.

Hi Adam,

please take a look at my patch at http://lkml.org/lkml/2008/11/23/211,
a "precursor" to the current patch, which contains the required tests
(it, does, however, also remove a comment that Bjorn asked me to keep).
Would that be the type of tests you're thinking of?

--- Witold

> 
> 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 <Witold.Szczeponik@gmx.net>:
> 
>> 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 <Witold.Szczeponik@gmx.net>
>>
>>
>> Index: linux/drivers/pnp/pnpacpi/core.c
>> ===================================================================
>> --- linux.orig/drivers/pnp/pnpacpi/core.c
>> +++ linux/drivers/pnp/pnpacpi/core.c
>> @@ -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;
>>  }
>>
>>
> 
>