Re: 2.6.23-rc6: S4 and S5 no longer listed as supported on ToshibaSatellite A40

[Alexey Starikovskiy]

Rafael J. Wysocki wrote:
> On Thursday, 20 September 2007 22:32, Frans Pop wrote:
>> On Thursday 20 September 2007, Rafael J. Wysocki wrote:
>>> On Thursday, 20 September 2007 20:33, Alexey Starikovskiy wrote:
>>>> Frans Pop wrote:
>>>>> On Thursday 20 September 2007, you wrote:
>>>>>> Please try this patch.
>>>>> Works. All states are now listed again.
>>>>> I've not tested suspend to disk, but suspend to ram and power off
>>>>> work fine.
>>>>>
>>>>>> +printk(KERN_INFO PREFIX "(supports");
>> Note that this printk should be indented.
>>
>>>>>>  #ifdef CONFIG_SUSPEND
>>>>>> -       printk(KERN_INFO PREFIX "(supports");
>>>>>>         for (i = ACPI_STATE_S0; i < ACPI_STATE_S4; i++) {
>>>>> Isn't there a risk now that we now end up printing
>>>>>    ACPI: (supports)
>>>>> if CONFIG_SUSPEND is not enabled and >S4 is not supported?
>>>>>
>>>>> Or, more probably, it would print
>>>>>    ACPI: (supports S5)
>>>> Don't know what does it mean to support S0 exactly... :)
>> Agreed, though arguably the same goes for S5. I guess you could say they are 
>> all states that can be switched to.
>>
>>>>> as it is unlikely that "off" is not supported :-)
>>>>>
>>>>> Maybe S0 should be taken outside the #ifdef and the loop as that
>>>>> state is also basically always there?
>>>> Don't think it is worth the trouble. We already have this loop almost
>>>> completely unrolled, let's not make it complete mess...
>>> Well, you could use "(supports S0" instead of just "(supports". ;-)
>> After thinking about this a bit more, I think this does make sense for three 
>> (admittedly minor) reasons:
>> - consistency between messages with and without CONFIG_SUSPEND
>> - consistency with /proc/acpi/sleep
>> - avoiding unnecessary change from previous versions.
>>
>> Please consider the attached patch which applies on top of Alexey's. Feel 
>> free to integrate it in his patch.
>>
>> Signed-off-by: Frans Pop <elendil@xxxxxxxxx>
> 
> Alexey, do you agree?
Yes, was thinking to do it myself, but my ISP died this morning....

Regards,
Alex.
> 
> (patch reproduced below for convenience).
> 
> diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
> index 638172f..85633c5 100644
> --- a/drivers/acpi/sleep/main.c
> +++ b/drivers/acpi/sleep/main.c
> @@ -401,9 +401,11 @@ int __init acpi_sleep_init(void)
>         if (acpi_disabled)
>                 return 0;
>  
> -printk(KERN_INFO PREFIX "(supports");
> +       sleep_states[ACPI_STATE_S0] = 1;
> +       printk(KERN_INFO PREFIX "(supports S0");
> +
>  #ifdef CONFIG_SUSPEND
> -       for (i = ACPI_STATE_S0; i < ACPI_STATE_S4; i++) {
> +       for (i = ACPI_STATE_S1; i < ACPI_STATE_S4; i++) {
>                 status = acpi_get_sleep_type_data(i, &type_a, &type_b);
>                 if (ACPI_SUCCESS(status)) {
>                         sleep_states[i] = 1;
> 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/