Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1428267 > unrolled thread

Re: [PATCH] ACPI: Execute the _PTS method when system reboot

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] ACPI: Execute the _PTS method when system reboot "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-06-22 02:00 +0200
    RE: [PATCH] ACPI: Execute the _PTS method when system reboot Ocean HY1 He <hehy1@lenovo.com> - 2016-06-23 15:00 +0200
      Re: [PATCH] ACPI: Execute the _PTS method when system reboot "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-23 15:20 +0200
        RE: [PATCH] ACPI: Execute the _PTS method when system reboot Ocean HY1 He <hehy1@lenovo.com> - 2016-06-24 05:10 +0200
          Re: [PATCH] ACPI: Execute the _PTS method when system reboot "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-06-24 14:40 +0200
      RE: [PATCH] ACPI: Execute the _PTS method when system reboot "Zheng, Lv" <lv.zheng@intel.com> - 2016-06-24 02:40 +0200
        RE: [PATCH] ACPI: Execute the _PTS method when system reboot Ocean HY1 He <hehy1@lenovo.com> - 2016-06-24 04:10 +0200

#1428267 — Re: [PATCH] ACPI: Execute the _PTS method when system reboot

On Monday, May 09, 2016 05:50:11 AM Ocean HY1 He wrote:
> The _PTS control method is defined in the section 7.4.1 of acpi 6.0
> spec. The _PTS control method is executed by the OS during the sleep
> transition process for S1, S2, S3, S4, and for orderly S5 shutdown.
> The sleeping state value (For example, 1, 2, 3, 4 or 5 for the S5
> soft-off state) is passed to the _PTS control method. This method
> is called after OSPM has notified native device drivers of the sleep
> state transition and before the OSPM has had a chance to fully
> prepare the system for a sleep state transition.
> 
> The _PTS control method provides the BIOS a mechanism for performing
> some housekeeping, such as writing the sleep type value to the embedded
> controller, before entering the system sleeping state.
> 
> According to section 7.5 of acpi 6.0 spec, _PTS should run after _TTS.
> 
> Thus, a _PTS block notifier is added to the reboot notifier list so that
> the _PTS object will also be evaluated when the system reboot.

So I understand why it may be necessary to evaluate _PTS before entering S5,
but I'm totally unsure about reboot.

What does reboot have to do with S5?

> Signed-off-by: Ocean He <hehy1@lenovo.com>
> Signed-off-by: Nagananda Chumbalkar <nchumbalkar@lenovo.com>
> ---
>  drivers/acpi/sleep.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index 2a8b596..8b290fb 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -55,6 +55,26 @@ static struct notifier_block tts_notifier = {
>  	.priority	= 0,
>  };
>  
> +static int pts_notify_reboot(struct notifier_block *this,
> +			unsigned long code, void *x)
> +{
> +	acpi_status status;
> +
> +	status = acpi_execute_simple_method(NULL, "\\_PTS", ACPI_STATE_S5);
> +	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> +		/* It won't break anything. */
> +		printk(KERN_NOTICE "Failure in evaluating _PTS object\n");
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block pts_notifier = {
> +	.notifier_call	= pts_notify_reboot,
> +	.next		= NULL,
> +	.priority	= 0,
> +};
> +
>  static int acpi_sleep_prepare(u32 acpi_state)
>  {
>  #ifdef CONFIG_ACPI_SLEEP
> @@ -896,5 +916,12 @@ int __init acpi_sleep_init(void)
>  	 * object can also be evaluated when the system enters S5.
>  	 */
>  	register_reboot_notifier(&tts_notifier);
> +
> +	/*
> +	 * According to section 7.5 of acpi 6.0 spec, _PTS should run after
> +	 * _TTS when the system enters S5.
> +	 */
> +	register_reboot_notifier(&pts_notifier);

Why do you have to add a second notifier?

Why can't _TTS and _PTS be evaluated from one notifier?

> +
>  	return 0;
>  }
> 

Thanks,
Rafael

[toc] | [next] | [standalone]

#1429787

Hi Rafael,
Please see my reply in below.

Regards,
Ocean He
SW Development Dept. 
Beijing Design Center
Enterprise Product Group
Mobile: 18911778926
E-mail: hehy1@lenovo.com
No.6 Chuang Ye Road, Haidian District, Beijing, China 100085

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Wednesday, June 22, 2016 7:56 AM
> To: Ocean HY1 He
> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> Subject: Re: [PATCH] ACPI: Execute the _PTS method when system reboot
> 
> On Monday, May 09, 2016 05:50:11 AM Ocean HY1 He wrote:
> > The _PTS control method is defined in the section 7.4.1 of acpi 6.0
> > spec. The _PTS control method is executed by the OS during the sleep
> > transition process for S1, S2, S3, S4, and for orderly S5 shutdown.
> > The sleeping state value (For example, 1, 2, 3, 4 or 5 for the S5
> > soft-off state) is passed to the _PTS control method. This method
> > is called after OSPM has notified native device drivers of the sleep
> > state transition and before the OSPM has had a chance to fully
> > prepare the system for a sleep state transition.
> >
> > The _PTS control method provides the BIOS a mechanism for performing
> > some housekeeping, such as writing the sleep type value to the
> embedded
> > controller, before entering the system sleeping state.
> >
> > According to section 7.5 of acpi 6.0 spec, _PTS should run after _TTS.
> >
> > Thus, a _PTS block notifier is added to the reboot notifier list so that
> > the _PTS object will also be evaluated when the system reboot.
> 
> So I understand why it may be necessary to evaluate _PTS before entering
> S5,
> but I'm totally unsure about reboot.
> 
> What does reboot have to do with S5?
> 
In ACPI spec, there is no explicit words saying _PTS should be 
executed when reboot. But reboot could be equal to the 
process S0->S5->S0. Thus _PTS should be executed when reboot.

I am thinking this is the same as _TTS. In ACPI spec, there is also
no explicit words saying _TTS should be executed when reboot.
But kernel executes _TTS when reboot indeed.

> > Signed-off-by: Ocean He <hehy1@lenovo.com>
> > Signed-off-by: Nagananda Chumbalkar <nchumbalkar@lenovo.com>
> > ---
> >  drivers/acpi/sleep.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > index 2a8b596..8b290fb 100644
> > --- a/drivers/acpi/sleep.c
> > +++ b/drivers/acpi/sleep.c
> > @@ -55,6 +55,26 @@ static struct notifier_block tts_notifier = {
> >  	.priority	= 0,
> >  };
> >
> > +static int pts_notify_reboot(struct notifier_block *this,
> > +			unsigned long code, void *x)
> > +{
> > +	acpi_status status;
> > +
> > +	status = acpi_execute_simple_method(NULL, "\\_PTS",
> ACPI_STATE_S5);
> > +	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > +		/* It won't break anything. */
> > +		printk(KERN_NOTICE "Failure in evaluating _PTS object\n");
> > +	}
> > +
> > +	return NOTIFY_DONE;
> > +}
> > +
> > +static struct notifier_block pts_notifier = {
> > +	.notifier_call	= pts_notify_reboot,
> > +	.next		= NULL,
> > +	.priority	= 0,
> > +};
> > +
> >  static int acpi_sleep_prepare(u32 acpi_state)
> >  {
> >  #ifdef CONFIG_ACPI_SLEEP
> > @@ -896,5 +916,12 @@ int __init acpi_sleep_init(void)
> >  	 * object can also be evaluated when the system enters S5.
> >  	 */
> >  	register_reboot_notifier(&tts_notifier);
> > +
> > +	/*
> > +	 * According to section 7.5 of acpi 6.0 spec, _PTS should run after
> > +	 * _TTS when the system enters S5.
> > +	 */
> > +	register_reboot_notifier(&pts_notifier);
> 
> Why do you have to add a second notifier?
> 
> Why can't _TTS and _PTS be evaluated from one notifier?
> 
If execute _PTS method in tts_notify_reboot(), then it would break
definition of tts_notify_reboot(). My intention is to keep new codes 
has limited impact on existed codes.
Of course, it's possible to merge _TTS and _PTS into one unified notifier.
The advantage is more actions could be added into the unified notifier in future.
Which way you prefer?
> > +
> >  	return 0;
> >  }
> >
> 
> Thanks,
> Rafael

[toc] | [prev] | [next] | [standalone]

#1429809

On Thu, Jun 23, 2016 at 2:55 PM, Ocean HY1 He <hehy1@lenovo.com> wrote:
> Hi Rafael,
> Please see my reply in below.
>
> Regards,
> Ocean He
> SW Development Dept.
> Beijing Design Center
> Enterprise Product Group
> Mobile: 18911778926
> E-mail: hehy1@lenovo.com
> No.6 Chuang Ye Road, Haidian District, Beijing, China 100085
>
>> -----Original Message-----
>> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
>> Sent: Wednesday, June 22, 2016 7:56 AM
>> To: Ocean HY1 He
>> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
>> linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
>> Subject: Re: [PATCH] ACPI: Execute the _PTS method when system reboot
>>
>> On Monday, May 09, 2016 05:50:11 AM Ocean HY1 He wrote:
>> > The _PTS control method is defined in the section 7.4.1 of acpi 6.0
>> > spec. The _PTS control method is executed by the OS during the sleep
>> > transition process for S1, S2, S3, S4, and for orderly S5 shutdown.
>> > The sleeping state value (For example, 1, 2, 3, 4 or 5 for the S5
>> > soft-off state) is passed to the _PTS control method. This method
>> > is called after OSPM has notified native device drivers of the sleep
>> > state transition and before the OSPM has had a chance to fully
>> > prepare the system for a sleep state transition.
>> >
>> > The _PTS control method provides the BIOS a mechanism for performing
>> > some housekeeping, such as writing the sleep type value to the
>> embedded
>> > controller, before entering the system sleeping state.
>> >
>> > According to section 7.5 of acpi 6.0 spec, _PTS should run after _TTS.
>> >
>> > Thus, a _PTS block notifier is added to the reboot notifier list so that
>> > the _PTS object will also be evaluated when the system reboot.
>>
>> So I understand why it may be necessary to evaluate _PTS before entering
>> S5,
>> but I'm totally unsure about reboot.
>>
>> What does reboot have to do with S5?
>>
> In ACPI spec, there is no explicit words saying _PTS should be
> executed when reboot. But reboot could be equal to the
> process S0->S5->S0.

Not in general.

In particular, wakeup devices that would be set up for S5 need not be
set up for that.  Also the mechanism by which transitions to S5 are
entered is different from the reboot one, at least from the OS
perspective.

> Thus _PTS should be executed when reboot.

No, it doesn't follow.

> I am thinking this is the same as _TTS. In ACPI spec, there is also
> no explicit words saying _TTS should be executed when reboot.
> But kernel executes _TTS when reboot indeed.

Yes, it does.  Maybe it shouldn't?

It may not hurt to call _PTS before reboot too, but is it guaranteed
to work across the board on all systems everywhere?

>> > Signed-off-by: Ocean He <hehy1@lenovo.com>
>> > Signed-off-by: Nagananda Chumbalkar <nchumbalkar@lenovo.com>
>> > ---
>> >  drivers/acpi/sleep.c | 27 +++++++++++++++++++++++++++
>> >  1 file changed, 27 insertions(+)
>> >
>> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
>> > index 2a8b596..8b290fb 100644
>> > --- a/drivers/acpi/sleep.c
>> > +++ b/drivers/acpi/sleep.c
>> > @@ -55,6 +55,26 @@ static struct notifier_block tts_notifier = {
>> >     .priority       = 0,
>> >  };
>> >
>> > +static int pts_notify_reboot(struct notifier_block *this,
>> > +                   unsigned long code, void *x)
>> > +{
>> > +   acpi_status status;
>> > +
>> > +   status = acpi_execute_simple_method(NULL, "\\_PTS",
>> ACPI_STATE_S5);
>> > +   if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
>> > +           /* It won't break anything. */
>> > +           printk(KERN_NOTICE "Failure in evaluating _PTS object\n");
>> > +   }
>> > +
>> > +   return NOTIFY_DONE;
>> > +}
>> > +
>> > +static struct notifier_block pts_notifier = {
>> > +   .notifier_call  = pts_notify_reboot,
>> > +   .next           = NULL,
>> > +   .priority       = 0,
>> > +};
>> > +
>> >  static int acpi_sleep_prepare(u32 acpi_state)
>> >  {
>> >  #ifdef CONFIG_ACPI_SLEEP
>> > @@ -896,5 +916,12 @@ int __init acpi_sleep_init(void)
>> >      * object can also be evaluated when the system enters S5.
>> >      */
>> >     register_reboot_notifier(&tts_notifier);
>> > +
>> > +   /*
>> > +    * According to section 7.5 of acpi 6.0 spec, _PTS should run after
>> > +    * _TTS when the system enters S5.
>> > +    */
>> > +   register_reboot_notifier(&pts_notifier);
>>
>> Why do you have to add a second notifier?
>>
>> Why can't _TTS and _PTS be evaluated from one notifier?
>>
> If execute _PTS method in tts_notify_reboot(), then it would break
> definition of tts_notify_reboot().

What exactly would it break?

> My intention is to keep new codes
> has limited impact on existed codes.

Even if that makes a little sense?

> Of course, it's possible to merge _TTS and _PTS into one unified notifier.
> The advantage is more actions could be added into the unified notifier in future.
> Which way you prefer?

I would just use one notifier.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1430326

Regards,
Ocean He
SW Development Dept. 
Beijing Design Center
Enterprise Product Group
Mobile: 18911778926
E-mail: hehy1@lenovo.com
No.6 Chuang Ye Road, Haidian District, Beijing, China 100085


> -----Original Message-----
> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of
> Rafael J. Wysocki
> Sent: Thursday, June 23, 2016 9:13 PM
> To: Ocean HY1 He
> Cc: Rafael J. Wysocki; lenb@kernel.org; linux-acpi@vger.kernel.org;
> linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> Subject: Re: [PATCH] ACPI: Execute the _PTS method when system reboot
> 
> On Thu, Jun 23, 2016 at 2:55 PM, Ocean HY1 He <hehy1@lenovo.com>
> wrote:
> > Hi Rafael,
> > Please see my reply in below.
> >
> > Regards,
> > Ocean He
> > SW Development Dept.
> > Beijing Design Center
> > Enterprise Product Group
> > Mobile: 18911778926
> > E-mail: hehy1@lenovo.com
> > No.6 Chuang Ye Road, Haidian District, Beijing, China 100085
> >
> >> -----Original Message-----
> >> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> >> Sent: Wednesday, June 22, 2016 7:56 AM
> >> To: Ocean HY1 He
> >> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> >> Subject: Re: [PATCH] ACPI: Execute the _PTS method when system
> reboot
> >>
> >> On Monday, May 09, 2016 05:50:11 AM Ocean HY1 He wrote:
> >> > The _PTS control method is defined in the section 7.4.1 of acpi 6.0
> >> > spec. The _PTS control method is executed by the OS during the sleep
> >> > transition process for S1, S2, S3, S4, and for orderly S5 shutdown.
> >> > The sleeping state value (For example, 1, 2, 3, 4 or 5 for the S5
> >> > soft-off state) is passed to the _PTS control method. This method
> >> > is called after OSPM has notified native device drivers of the sleep
> >> > state transition and before the OSPM has had a chance to fully
> >> > prepare the system for a sleep state transition.
> >> >
> >> > The _PTS control method provides the BIOS a mechanism for
> performing
> >> > some housekeeping, such as writing the sleep type value to the
> >> embedded
> >> > controller, before entering the system sleeping state.
> >> >
> >> > According to section 7.5 of acpi 6.0 spec, _PTS should run after _TTS.
> >> >
> >> > Thus, a _PTS block notifier is added to the reboot notifier list so that
> >> > the _PTS object will also be evaluated when the system reboot.
> >>
> >> So I understand why it may be necessary to evaluate _PTS before
> entering
> >> S5,
> >> but I'm totally unsure about reboot.
> >>
> >> What does reboot have to do with S5?
> >>
> > In ACPI spec, there is no explicit words saying _PTS should be
> > executed when reboot. But reboot could be equal to the
> > process S0->S5->S0.
> 
> Not in general.
> 
> In particular, wakeup devices that would be set up for S5 need not be
> set up for that.  Also the mechanism by which transitions to S5 are
> entered is different from the reboot one, at least from the OS
> perspective.
> 
> > Thus _PTS should be executed when reboot.
> 
> No, it doesn't follow.
> 
> > I am thinking this is the same as _TTS. In ACPI spec, there is also
> > no explicit words saying _TTS should be executed when reboot.
> > But kernel executes _TTS when reboot indeed.
> 
> Yes, it does.  Maybe it shouldn't?
> 
> It may not hurt to call _PTS before reboot too, but is it guaranteed
> to work across the board on all systems everywhere?
> 
I try to clarify the key point of this case: does devices should go to S5(shutdown) when reboot?

I think the answer is yes. 
And It has no hurt to let devices go to S5 before reboot is invoked, here is the reasons:
#1 The new _PTS codes block nothing thus reboot can be guaranteed to be invoked eventually.
#2. Devices are mandatory to support S5 state, this means go to S5 could be a safe trip.
#3 Reboot would cause devices re-initialization from the scratch.

What's your decision then? ;-)

Regards,
Ocean.

> >> > Signed-off-by: Ocean He <hehy1@lenovo.com>
> >> > Signed-off-by: Nagananda Chumbalkar <nchumbalkar@lenovo.com>
> >> > ---
> >> >  drivers/acpi/sleep.c | 27 +++++++++++++++++++++++++++
> >> >  1 file changed, 27 insertions(+)
> >> >
> >> > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> >> > index 2a8b596..8b290fb 100644
> >> > --- a/drivers/acpi/sleep.c
> >> > +++ b/drivers/acpi/sleep.c
> >> > @@ -55,6 +55,26 @@ static struct notifier_block tts_notifier = {
> >> >     .priority       = 0,
> >> >  };
> >> >
> >> > +static int pts_notify_reboot(struct notifier_block *this,
> >> > +                   unsigned long code, void *x)
> >> > +{
> >> > +   acpi_status status;
> >> > +
> >> > +   status = acpi_execute_simple_method(NULL, "\\_PTS",
> >> ACPI_STATE_S5);
> >> > +   if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> >> > +           /* It won't break anything. */
> >> > +           printk(KERN_NOTICE "Failure in evaluating _PTS
> object\n");
> >> > +   }
> >> > +
> >> > +   return NOTIFY_DONE;
> >> > +}
> >> > +
> >> > +static struct notifier_block pts_notifier = {
> >> > +   .notifier_call  = pts_notify_reboot,
> >> > +   .next           = NULL,
> >> > +   .priority       = 0,
> >> > +};
> >> > +
> >> >  static int acpi_sleep_prepare(u32 acpi_state)
> >> >  {
> >> >  #ifdef CONFIG_ACPI_SLEEP
> >> > @@ -896,5 +916,12 @@ int __init acpi_sleep_init(void)
> >> >      * object can also be evaluated when the system enters S5.
> >> >      */
> >> >     register_reboot_notifier(&tts_notifier);
> >> > +
> >> > +   /*
> >> > +    * According to section 7.5 of acpi 6.0 spec, _PTS should run
> after
> >> > +    * _TTS when the system enters S5.
> >> > +    */
> >> > +   register_reboot_notifier(&pts_notifier);
> >>
> >> Why do you have to add a second notifier?
> >>
> >> Why can't _TTS and _PTS be evaluated from one notifier?
> >>
> > If execute _PTS method in tts_notify_reboot(), then it would break
> > definition of tts_notify_reboot().
> 
> What exactly would it break?
> 
> > My intention is to keep new codes
> > has limited impact on existed codes.
> 
> Even if that makes a little sense?
> 
> > Of course, it's possible to merge _TTS and _PTS into one unified notifier.
> > The advantage is more actions could be added into the unified notifier
> in future.
> > Which way you prefer?
> 
> I would just use one notifier.
I follow you.
> 
> Thanks,
> Rafael

[toc] | [prev] | [next] | [standalone]

#1430633

On Friday, June 24, 2016 02:59:24 AM Ocean HY1 He wrote:
> 
> Regards,
> Ocean He
> SW Development Dept. 
> Beijing Design Center
> Enterprise Product Group
> Mobile: 18911778926
> E-mail: hehy1@lenovo.com
> No.6 Chuang Ye Road, Haidian District, Beijing, China 100085
> 
> 
> > -----Original Message-----
> > From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of
> > Rafael J. Wysocki
> > Sent: Thursday, June 23, 2016 9:13 PM
> > To: Ocean HY1 He
> > Cc: Rafael J. Wysocki; lenb@kernel.org; linux-acpi@vger.kernel.org;
> > linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> > Subject: Re: [PATCH] ACPI: Execute the _PTS method when system reboot
> > 
> > On Thu, Jun 23, 2016 at 2:55 PM, Ocean HY1 He <hehy1@lenovo.com>
> > wrote:
> > > Hi Rafael,
> > > Please see my reply in below.
> > >
> > > Regards,
> > > Ocean He
> > > SW Development Dept.
> > > Beijing Design Center
> > > Enterprise Product Group
> > > Mobile: 18911778926
> > > E-mail: hehy1@lenovo.com
> > > No.6 Chuang Ye Road, Haidian District, Beijing, China 100085
> > >
> > >> -----Original Message-----
> > >> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > >> Sent: Wednesday, June 22, 2016 7:56 AM
> > >> To: Ocean HY1 He
> > >> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> > >> linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> > >> Subject: Re: [PATCH] ACPI: Execute the _PTS method when system
> > reboot
> > >>
> > >> On Monday, May 09, 2016 05:50:11 AM Ocean HY1 He wrote:
> > >> > The _PTS control method is defined in the section 7.4.1 of acpi 6.0
> > >> > spec. The _PTS control method is executed by the OS during the sleep
> > >> > transition process for S1, S2, S3, S4, and for orderly S5 shutdown.
> > >> > The sleeping state value (For example, 1, 2, 3, 4 or 5 for the S5
> > >> > soft-off state) is passed to the _PTS control method. This method
> > >> > is called after OSPM has notified native device drivers of the sleep
> > >> > state transition and before the OSPM has had a chance to fully
> > >> > prepare the system for a sleep state transition.
> > >> >
> > >> > The _PTS control method provides the BIOS a mechanism for
> > performing
> > >> > some housekeeping, such as writing the sleep type value to the
> > >> embedded
> > >> > controller, before entering the system sleeping state.
> > >> >
> > >> > According to section 7.5 of acpi 6.0 spec, _PTS should run after _TTS.
> > >> >
> > >> > Thus, a _PTS block notifier is added to the reboot notifier list so that
> > >> > the _PTS object will also be evaluated when the system reboot.
> > >>
> > >> So I understand why it may be necessary to evaluate _PTS before
> > entering
> > >> S5,
> > >> but I'm totally unsure about reboot.
> > >>
> > >> What does reboot have to do with S5?
> > >>
> > > In ACPI spec, there is no explicit words saying _PTS should be
> > > executed when reboot. But reboot could be equal to the
> > > process S0->S5->S0.
> > 
> > Not in general.
> > 
> > In particular, wakeup devices that would be set up for S5 need not be
> > set up for that.  Also the mechanism by which transitions to S5 are
> > entered is different from the reboot one, at least from the OS
> > perspective.
> > 
> > > Thus _PTS should be executed when reboot.
> > 
> > No, it doesn't follow.
> > 
> > > I am thinking this is the same as _TTS. In ACPI spec, there is also
> > > no explicit words saying _TTS should be executed when reboot.
> > > But kernel executes _TTS when reboot indeed.
> > 
> > Yes, it does.  Maybe it shouldn't?
> > 
> > It may not hurt to call _PTS before reboot too, but is it guaranteed
> > to work across the board on all systems everywhere?
> > 
> I try to clarify the key point of this case: does devices should go to S5(shutdown) when reboot?
> 
> I think the answer is yes. 
> And It has no hurt to let devices go to S5 before reboot is invoked, here is the reasons:

S5 is a platform (system) state, not a device state.

You seem to be confusing platforms with devices.

> #1 The new _PTS codes block nothing thus reboot can be guaranteed to be invoked eventually.
> #2. Devices are mandatory to support S5 state, this means go to S5 could be a safe trip.
> #3 Reboot would cause devices re-initialization from the scratch.
> 
> What's your decision then? ;-)

I guess you can argue that since _TTS is already executed in the reboot
path, it might be good to execute _PTS in that path too and that might help
to avoid issues in some cases.

That is a reasonable argument, but then there still is the question if that
is not going to break anything.

I guess that cannot be answered without trying, so the simplest patch would
be to add a _PTS evaulation to the existing notifier (and maybe change the
notifiers name to reflect the fact that it's not for _TTS ony any more).

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1430265

Hi,

> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Ocean HY1 He
> Subject: RE: [PATCH] ACPI: Execute the _PTS method when system reboot
> 
> Hi Rafael,
> Please see my reply in below.
> 
> Regards,
> Ocean He
> SW Development Dept.
> Beijing Design Center
> Enterprise Product Group
> Mobile: 18911778926
> E-mail: hehy1@lenovo.com
> No.6 Chuang Ye Road, Haidian District, Beijing, China 100085
> 
> > -----Original Message-----
> > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > Sent: Wednesday, June 22, 2016 7:56 AM
> > To: Ocean HY1 He
> > Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> > linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> > Subject: Re: [PATCH] ACPI: Execute the _PTS method when system
> reboot
> >
> > On Monday, May 09, 2016 05:50:11 AM Ocean HY1 He wrote:
> > > The _PTS control method is defined in the section 7.4.1 of acpi 6.0
> > > spec. The _PTS control method is executed by the OS during the sleep
> > > transition process for S1, S2, S3, S4, and for orderly S5 shutdown.
> > > The sleeping state value (For example, 1, 2, 3, 4 or 5 for the S5
> > > soft-off state) is passed to the _PTS control method. This method
> > > is called after OSPM has notified native device drivers of the sleep
> > > state transition and before the OSPM has had a chance to fully
> > > prepare the system for a sleep state transition.
> > >
> > > The _PTS control method provides the BIOS a mechanism for
> performing
> > > some housekeeping, such as writing the sleep type value to the
> > embedded
> > > controller, before entering the system sleeping state.
> > >
> > > According to section 7.5 of acpi 6.0 spec, _PTS should run after _TTS.
> > >
> > > Thus, a _PTS block notifier is added to the reboot notifier list so that
> > > the _PTS object will also be evaluated when the system reboot.
> >
> > So I understand why it may be necessary to evaluate _PTS before
> entering
> > S5,
> > but I'm totally unsure about reboot.
> >
> > What does reboot have to do with S5?
> >
> In ACPI spec, there is no explicit words saying _PTS should be
> executed when reboot. But reboot could be equal to the
> process S0->S5->S0. Thus _PTS should be executed when reboot.
> 
> I am thinking this is the same as _TTS. In ACPI spec, there is also
> no explicit words saying _TTS should be executed when reboot.
> But kernel executes _TTS when reboot indeed.
[Lv Zheng] 
What's the de-facto standard behavior here?
Lenovo should be able to modify BIOS, should you try to boot different Windows clones to confirm the necessity of doing this?

Thanks and best regards
-Lv

> 
> > > Signed-off-by: Ocean He <hehy1@lenovo.com>
> > > Signed-off-by: Nagananda Chumbalkar <nchumbalkar@lenovo.com>
> > > ---
> > >  drivers/acpi/sleep.c | 27 +++++++++++++++++++++++++++
> > >  1 file changed, 27 insertions(+)
> > >
> > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > > index 2a8b596..8b290fb 100644
> > > --- a/drivers/acpi/sleep.c
> > > +++ b/drivers/acpi/sleep.c
> > > @@ -55,6 +55,26 @@ static struct notifier_block tts_notifier = {
> > >  	.priority	= 0,
> > >  };
> > >
> > > +static int pts_notify_reboot(struct notifier_block *this,
> > > +			unsigned long code, void *x)
> > > +{
> > > +	acpi_status status;
> > > +
> > > +	status = acpi_execute_simple_method(NULL, "\\_PTS",
> > ACPI_STATE_S5);
> > > +	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > > +		/* It won't break anything. */
> > > +		printk(KERN_NOTICE "Failure in evaluating _PTS object\n");
> > > +	}
> > > +
> > > +	return NOTIFY_DONE;
> > > +}
> > > +
> > > +static struct notifier_block pts_notifier = {
> > > +	.notifier_call	= pts_notify_reboot,
> > > +	.next		= NULL,
> > > +	.priority	= 0,
> > > +};
> > > +
> > >  static int acpi_sleep_prepare(u32 acpi_state)
> > >  {
> > >  #ifdef CONFIG_ACPI_SLEEP
> > > @@ -896,5 +916,12 @@ int __init acpi_sleep_init(void)
> > >  	 * object can also be evaluated when the system enters S5.
> > >  	 */
> > >  	register_reboot_notifier(&tts_notifier);
> > > +
> > > +	/*
> > > +	 * According to section 7.5 of acpi 6.0 spec, _PTS should run after
> > > +	 * _TTS when the system enters S5.
> > > +	 */
> > > +	register_reboot_notifier(&pts_notifier);
> >
> > Why do you have to add a second notifier?
> >
> > Why can't _TTS and _PTS be evaluated from one notifier?
> >
> If execute _PTS method in tts_notify_reboot(), then it would break
> definition of tts_notify_reboot(). My intention is to keep new codes
> has limited impact on existed codes.
> Of course, it's possible to merge _TTS and _PTS into one unified notifier.
> The advantage is more actions could be added into the unified notifier in
> future.
> Which way you prefer?
> > > +
> > >  	return 0;
> > >  }
> > >
> >
> > Thanks,
> > Rafael
> 
> ��칻�&�~�&���+-
> ��ݶ��w��˛���m�b��Zr����^n�r���z���h����&���G���h�(�階
> �ݢj"���m�����z�ޖ���f���h���~�m�

[toc] | [prev] | [next] | [standalone]

#1430287

Regards,
Ocean He
SW Development Dept. 
Beijing Design Center
Enterprise Product Group
Mobile: 18911778926
E-mail: hehy1@lenovo.com
No.6 Chuang Ye Road, Haidian District, Beijing, China 100085

> -----Original Message-----
> From: Zheng, Lv [mailto:lv.zheng@intel.com]
> Sent: Friday, June 24, 2016 8:30 AM
> To: Ocean HY1 He; Rafael J. Wysocki
> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> Subject: RE: [PATCH] ACPI: Execute the _PTS method when system reboot
> 
> Hi,
> 
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> > owner@vger.kernel.org] On Behalf Of Ocean HY1 He
> > Subject: RE: [PATCH] ACPI: Execute the _PTS method when system
> reboot
> >
> > Hi Rafael,
> > Please see my reply in below.
> >
> > Regards,
> > Ocean He
> > SW Development Dept.
> > Beijing Design Center
> > Enterprise Product Group
> > Mobile: 18911778926
> > E-mail: hehy1@lenovo.com
> > No.6 Chuang Ye Road, Haidian District, Beijing, China 100085
> >
> > > -----Original Message-----
> > > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > > Sent: Wednesday, June 22, 2016 7:56 AM
> > > To: Ocean HY1 He
> > > Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; David Tanaka; Nagananda Chumbalkar
> > > Subject: Re: [PATCH] ACPI: Execute the _PTS method when system
> > reboot
> > >
> > > On Monday, May 09, 2016 05:50:11 AM Ocean HY1 He wrote:
> > > > The _PTS control method is defined in the section 7.4.1 of acpi 6.0
> > > > spec. The _PTS control method is executed by the OS during the
> sleep
> > > > transition process for S1, S2, S3, S4, and for orderly S5 shutdown.
> > > > The sleeping state value (For example, 1, 2, 3, 4 or 5 for the S5
> > > > soft-off state) is passed to the _PTS control method. This method
> > > > is called after OSPM has notified native device drivers of the sleep
> > > > state transition and before the OSPM has had a chance to fully
> > > > prepare the system for a sleep state transition.
> > > >
> > > > The _PTS control method provides the BIOS a mechanism for
> > performing
> > > > some housekeeping, such as writing the sleep type value to the
> > > embedded
> > > > controller, before entering the system sleeping state.
> > > >
> > > > According to section 7.5 of acpi 6.0 spec, _PTS should run after _TTS.
> > > >
> > > > Thus, a _PTS block notifier is added to the reboot notifier list so that
> > > > the _PTS object will also be evaluated when the system reboot.
> > >
> > > So I understand why it may be necessary to evaluate _PTS before
> > entering
> > > S5,
> > > but I'm totally unsure about reboot.
> > >
> > > What does reboot have to do with S5?
> > >
> > In ACPI spec, there is no explicit words saying _PTS should be
> > executed when reboot. But reboot could be equal to the
> > process S0->S5->S0. Thus _PTS should be executed when reboot.
> >
> > I am thinking this is the same as _TTS. In ACPI spec, there is also
> > no explicit words saying _TTS should be executed when reboot.
> > But kernel executes _TTS when reboot indeed.
> [Lv Zheng]
> What's the de-facto standard behavior here?
> Lenovo should be able to modify BIOS, should you try to boot different
> Windows clones to confirm the necessity of doing this?
> 
> Thanks and best regards
> -Lv
> 
I have consulted Lenovo UEFI/BIOS archtect, David Tanaka. He says
the de-facto standard behavior is that _PTS should be executed when reboot,
at least for Windows. Windows does not differentiate OS reboot and shutdown.

And, our test on Windows shows _PTS is executed when reboot actually.

I try to find the difference between OS reboot and shutdown(S5) in ACPI spec,
but I cann't find the answer. So, in my mind I consider reboot equals S0->S5->S0.
Maybe you have evidence shows they are different, please let me know. It would
help me a lot. Thanks.

Ocean.
> >
> > > > Signed-off-by: Ocean He <hehy1@lenovo.com>
> > > > Signed-off-by: Nagananda Chumbalkar <nchumbalkar@lenovo.com>
> > > > ---
> > > >  drivers/acpi/sleep.c | 27 +++++++++++++++++++++++++++
> > > >  1 file changed, 27 insertions(+)
> > > >
> > > > diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> > > > index 2a8b596..8b290fb 100644
> > > > --- a/drivers/acpi/sleep.c
> > > > +++ b/drivers/acpi/sleep.c
> > > > @@ -55,6 +55,26 @@ static struct notifier_block tts_notifier = {
> > > >  	.priority	= 0,
> > > >  };
> > > >
> > > > +static int pts_notify_reboot(struct notifier_block *this,
> > > > +			unsigned long code, void *x)
> > > > +{
> > > > +	acpi_status status;
> > > > +
> > > > +	status = acpi_execute_simple_method(NULL, "\\_PTS",
> > > ACPI_STATE_S5);
> > > > +	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> > > > +		/* It won't break anything. */
> > > > +		printk(KERN_NOTICE "Failure in evaluating _PTS object\n");
> > > > +	}
> > > > +
> > > > +	return NOTIFY_DONE;
> > > > +}
> > > > +
> > > > +static struct notifier_block pts_notifier = {
> > > > +	.notifier_call	= pts_notify_reboot,
> > > > +	.next		= NULL,
> > > > +	.priority	= 0,
> > > > +};
> > > > +
> > > >  static int acpi_sleep_prepare(u32 acpi_state)
> > > >  {
> > > >  #ifdef CONFIG_ACPI_SLEEP
> > > > @@ -896,5 +916,12 @@ int __init acpi_sleep_init(void)
> > > >  	 * object can also be evaluated when the system enters S5.
> > > >  	 */
> > > >  	register_reboot_notifier(&tts_notifier);
> > > > +
> > > > +	/*
> > > > +	 * According to section 7.5 of acpi 6.0 spec, _PTS should run after
> > > > +	 * _TTS when the system enters S5.
> > > > +	 */
> > > > +	register_reboot_notifier(&pts_notifier);
> > >
> > > Why do you have to add a second notifier?
> > >
> > > Why can't _TTS and _PTS be evaluated from one notifier?
> > >
> > If execute _PTS method in tts_notify_reboot(), then it would break
> > definition of tts_notify_reboot(). My intention is to keep new codes
> > has limited impact on existed codes.
> > Of course, it's possible to merge _TTS and _PTS into one unified notifier.
> > The advantage is more actions could be added into the unified notifier
> in
> > future.
> > Which way you prefer?
> > > > +
> > > >  	return 0;
> > > >  }
> > > >
> > >
> > > Thanks,
> > > Rafael
> >
> > ��칻�&�~�&���+-
> >
> ��ݶ��w��˛���m�b��Zr����^n�r���z���h����&���G���h�(�
> 階
> > �ݢj"���m�����z�ޖ���f���h���~�m�

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web