On Tuesday, 19 June 2007 04:33, Shaohua Li wrote:
> Based on David's patch
> http://marc.info/?l=linux-acpi&m=117873972806360&w=2
> I slightly changed it.
>
> Add a helper routine, which gets the sleep state of a ACPI device.
Is it going to work with the recent code ordering changes? I mean,
acpi_pm_prepare() is now called after device_suspend() (and analogously for
the hibernation), so the target ACPI state is not known when the drivers'
.suspend() routines are being called.
> Index: 2.6.22-rc2/drivers/acpi/sleep/main.c
> ===================================================================
> --- 2.6.22-rc2.orig/drivers/acpi/sleep/main.c 2007-05-23 09:15:14.000000000
> +0800
> +++ 2.6.22-rc2/drivers/acpi/sleep/main.c 2007-06-19 09:19:09.000000000
> +0800
> @@ -34,6 +34,8 @@ static u32 acpi_suspend_states[] = {
>
> static int init_8259A_after_S1;
>
> +static int acpi_target_sleep_state = ACPI_STATE_S0;
> +
> /**
> * acpi_pm_prepare - Do preliminary suspend work.
> * @pm_state: suspend state we're entering.
> @@ -54,6 +56,7 @@ static int acpi_pm_prepare(suspend_state
> printk("acpi_pm_prepare does not support %d \n", pm_state);
> return -EPERM;
> }
> + acpi_target_sleep_state = acpi_state;
> return acpi_sleep_prepare(acpi_state);
> }
>
> @@ -140,6 +143,7 @@ static int acpi_pm_finish(suspend_state_
> printk("Broken toshiba laptop -> kicking interrupts\n");
> init_8259A(0);
> }
> + acpi_target_sleep_state = ACPI_STATE_S0;
> return 0;
> }
>
> @@ -184,6 +188,7 @@ static struct pm_ops acpi_pm_ops = {
> #ifdef CONFIG_SOFTWARE_SUSPEND
> static int acpi_hibernation_prepare(void)
> {
> + acpi_target_sleep_state = ACPI_STATE_S4;
> return acpi_sleep_prepare(ACPI_STATE_S4);
> }
>
> @@ -215,6 +220,7 @@ static void acpi_hibernation_finish(void
> printk("Broken toshiba laptop -> kicking interrupts\n");
> init_8259A(0);
> }
> + acpi_target_sleep_state = ACPI_STATE_S0;
This will clash with the recent Pavel's patch that removes the Toshiba
quirk from the hibernation code path
http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc5/patches/35-ACPI-remove-S1-workaround-from-hibernation-code-path.patch
> }
>
> static struct hibernation_ops acpi_hibernation_ops = {
> @@ -224,6 +230,49 @@ static struct hibernation_ops acpi_hiber
> };
> #endif /* CONFIG_SOFTWARE_SUSPEND */
>
> +int acpi_pm_choose_sleep_state(acpi_handle handle, pm_message_t state)
> +{
The second argument doesn't seem to be used. Is that intentional and if so,
then why?
> + char sxd[] = "_SxD";
> + unsigned long d_min, d_max;
> + struct acpi_device *dev;
> +
> + /*
> + * If the sleep state is S0, we will return D3, but if the device has
> + * _S0W, we will use the value from _S0W
> + */
Hmm, is the comment right? Why should we return D3 for S0?
> + d_min = ACPI_STATE_D3;
> + d_max = ACPI_STATE_D3;
> +
> + /* If present, _SxD methods give the minimum D-state we may use
> + * for each S-state ... with lowest latency state switching.
> + *
> + * We rely on acpi_evaluate_integer() not clobbering the integer
> + * provided -- that's our fault recovery, we ignore retval.
> + */
> + sxd[2] = '0' + acpi_target_sleep_state;
> + if (acpi_target_sleep_state > ACPI_STATE_S0)
> + acpi_evaluate_integer(handle, sxd, NULL, &d_min);
> +
> + /*
> + * If _PRW says we can wake from the upcoming system state, the _SxD
> + * value can wake ... and we'll assume a wakeup-aware driver. If _SxW
> + * methods exist (ACPI 3.x), they give the lowest power D-state that
> + * can also wake the system. _S0W can be valid.
> + */
> + if (acpi_bus_get_device(handle, &dev)) {
> + printk(KERN_ERR"ACPI handle hasn't context\n");
> + return d_max;
> + }
> + if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> + (dev->wakeup.state.enabled &&
> + dev->wakeup.sleep_state <= acpi_target_sleep_state)) {
> + sxd[3] = 'W';
Looks ugly. Why don't we call the array 'method_name' or something like this?
> + acpi_evaluate_integer(handle, sxd, NULL, &d_max);
> + }
> + /* choose a state within d_min to d_max, we just choose the max */
> + return d_max;
> +}
> +
> /*
> * Toshiba fails to preserve interrupts over S1, reinitialization
> * of 8259 is needed after S1 resume.
> Index: 2.6.22-rc2/include/acpi/acpi_bus.h
> ===================================================================
> --- 2.6.22-rc2.orig/include/acpi/acpi_bus.h 2007-05-23 09:15:14.000000000
> +0800
> +++ 2.6.22-rc2/include/acpi/acpi_bus.h 2007-06-19 09:23:54.000000000
> +0800
> @@ -364,6 +364,8 @@ acpi_handle acpi_get_child(acpi_handle,
> acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
> #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
>
> +int acpi_pm_choose_sleep_state(acpi_handle handle, pm_message_t state);
> +
> #endif /* CONFIG_ACPI */
>
> #endif /*__ACPI_BUS_H__*/
> -
Greetings,
Rafael
--
"Premature optimization is the root of all evil." - Donald Knuth
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
