Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote:
> Rafael J. Wysocki wrote:
> > On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote:
> >> Rafael J. Wysocki wrote:
> >>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote:
> >>>> Rafael J. Wysocki wrote:
> >>>>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote:
> >>>>>> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote:
> >>>>>>>>> No, I do not have CONFIG_ACPI_SLEEP set,
> >>>>>>>>> because I do not have CONFIG_PM_SLEEP set,
> >>>>>>>>> because I do not want SUSPEND and/or HIBERNATION.
> >>>>>>>> Same answer from my side: I do not have CONFIG_ACPI_SLEEP for the same
> >>>>>>>> reason (and this worked fine without them in rc7). I do not think
> >>>>>>>> these settings should have changed between rc7 and rc8.
> >>>>>> Well, we haven't changed much.
> >>>>>>
> >>>>>>> Also, another test I just did: on another computer, rc8 is fine
> >>>>>>> regarding ACPI power off, even if CONFIG_ACPI_SLEEP is not set. I can
> >>>>>>> provide config if needed.
> >>>>>> On the box that fails to power off, can you please test -rc8 with these two
> >>>>>> commits reverted:
> >>>>>>
> >>>>>> commit 5a50fe709d527f31169263e36601dd83446d5744
> >>>>>> ACPI: suspend: consolidate handling of Sx states addendum
> >>>>>>
> >>>>>> commit f216cc3748a3a22c2b99390fddcdafa0583791a2
> >>>>>> ACPI: suspend: consolidate handling of Sx states.
> >>>>>>
> >>>>>> and see if it works?
> >>>>> If it does, please test the patch from this message
> >>>>>
> >>>>> http://marc.info/?l=linux-kernel&m=119052978117735&w=4
> >>>>>
> >>>>> on top of vanilla 2.6.23-rc8.
> >>>> You will need one more patch on top of just mentioned one.
> >>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND?
> >>>
> >>> CONFIG_HIBERNATION needs acpi_target_sleep_state  too.
> >> Agree, attaching updated patch.
> > 
> > Well, please use "ifdef CONFIG_PM_SLEEP" instead of
> > "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)",
> > as you did with the second block.
> I was thinking about that, but it seem to be less clear... 
> We need this variable only for suspend or hibernation, nothing else.
> with pm_sleep it is not visible at all.
> 
> Thoughts?

Well, PM_SLEEP is defined as (SUSPEND || HIBERNATION), please have a look
at kernel/power/Kconfig, and it was introduced exactly for the conditions like
this.

IOW, if we want something to be used for anything else than suspend or
hibernation, it shouldn't be defined under PM_SLEEP.

Greetings,
Rafael