Re: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()

Re: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()

[Rafael J. Wysocki]

On Thursday, September 12, 2013 01:42:57 AM Chuansheng Liu wrote:
> 
> Currently the acpi_os_sleep() is using the schedule_timeout_interruptible(),
> which can be interrupted by signal, which causes the real sleep time is shorter.
> 
> According to the ACPI spec:
> The Sleep term is used to implement long-term timing requirements.
> Execution is delayed for at least the required number of milliseconds.
> 
> The sleeping time should be at least of the required number msecs, here
> using msleep() to implement it.
> 
> Also if the real time is shorter, we meet the device POWER ON issue.

What exactly is the "power on" issue?

Rafael


> CC: lizhuangzhi <zhuangzhi.li@intel.com>
> CC: Li Fei <fei.li@intel.com>
> Signed-off-by: liu chuansheng <chuansheng.liu@intel.com>
> ---
>  drivers/acpi/osl.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index e5f416c..b1629b5 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -820,7 +820,7 @@ acpi_status acpi_os_remove_interrupt_handler(u32 irq, acpi_osd_handler handler)
>  
>  void acpi_os_sleep(u64 ms)
>  {
> -	schedule_timeout_interruptible(msecs_to_jiffies(ms));
> +	msleep(ms);
>  }
>  
>  void acpi_os_stall(u32 us)
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()

[Chuansheng Liu]

Currently the acpi_os_sleep() is using the schedule_timeout_interruptible(),
which can be interrupted by signal, which causes the real sleep time is shorter.

According to the ACPI spec:
The Sleep term is used to implement long-term timing requirements.
Execution is delayed for at least the required number of milliseconds.

The sleeping time should be at least of the required number msecs, here
using msleep() to implement it.

Also if the real time is shorter, we meet the device POWER ON issue.

CC: lizhuangzhi <zhuangzhi.li@intel.com>
CC: Li Fei <fei.li@intel.com>
Signed-off-by: liu chuansheng <chuansheng.liu@intel.com>
---
 drivers/acpi/osl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index e5f416c..b1629b5 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -820,7 +820,7 @@ acpi_status acpi_os_remove_interrupt_handler(u32 irq, acpi_osd_handler handler)
 
 void acpi_os_sleep(u64 ms)
 {
-	schedule_timeout_interruptible(msecs_to_jiffies(ms));
+	msleep(ms);
 }
 
 void acpi_os_stall(u32 us)
-- 
1.7.0.4

RE: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()

[Liu, Chuansheng]

Hello Rafael,

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
> Sent: Wednesday, September 11, 2013 8:37 PM
> To: Liu, Chuansheng
> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org;
> Li, Zhuangzhi; Li, Fei
> Subject: Re: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()
> 
> On Thursday, September 12, 2013 01:42:57 AM Chuansheng Liu wrote:
> >
> > Currently the acpi_os_sleep() is using the schedule_timeout_interruptible(),
> > which can be interrupted by signal, which causes the real sleep time is
> shorter.
> >
> > According to the ACPI spec:
> > The Sleep term is used to implement long-term timing requirements.
> > Execution is delayed for at least the required number of milliseconds.
> >
> > The sleeping time should be at least of the required number msecs, here
> > using msleep() to implement it.
> >
> > Also if the real time is shorter, we meet the device POWER ON issue.
> 
> What exactly is the "power on" issue?
The case is we have one device _PS0 method in platform.asl like below:
Write the pmcsr REG to power on;
Sleep 10ms;
Read some registers;
...

Here sometimes the actual sleeping time is < 10ms, it causes the following actions failed due this device
need 10ms to power on successfully.

> 
> Rafael
> 
> 
> > CC: lizhuangzhi <zhuangzhi.li@intel.com>
> > CC: Li Fei <fei.li@intel.com>
> > Signed-off-by: liu chuansheng <chuansheng.liu@intel.com>
> > ---
> >  drivers/acpi/osl.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index e5f416c..b1629b5 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -820,7 +820,7 @@ acpi_status acpi_os_remove_interrupt_handler(u32
> irq, acpi_osd_handler handler)
> >
> >  void acpi_os_sleep(u64 ms)
> >  {
> > -	schedule_timeout_interruptible(msecs_to_jiffies(ms));
> > +	msleep(ms);
> >  }
> >
> >  void acpi_os_stall(u32 us)
> >
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()

[Liu, Chuansheng]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2217 bytes --]

Hello Rafael,

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
> Sent: Wednesday, September 11, 2013 8:37 PM
> To: Liu, Chuansheng
> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org;
> Li, Zhuangzhi; Li, Fei
> Subject: Re: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()
> 
> On Thursday, September 12, 2013 01:42:57 AM Chuansheng Liu wrote:
> >
> > Currently the acpi_os_sleep() is using the schedule_timeout_interruptible(),
> > which can be interrupted by signal, which causes the real sleep time is
> shorter.
> >
> > According to the ACPI spec:
> > The Sleep term is used to implement long-term timing requirements.
> > Execution is delayed for at least the required number of milliseconds.
> >
> > The sleeping time should be at least of the required number msecs, here
> > using msleep() to implement it.
> >
> > Also if the real time is shorter, we meet the device POWER ON issue.
> 
> What exactly is the "power on" issue?
The case is we have one device _PS0 method in platform.asl like below:
Write the pmcsr REG to power on;
Sleep 10ms;
Read some registers;
...

Here sometimes the actual sleeping time is < 10ms, it causes the following actions failed due this device
need 10ms to power on successfully.

> 
> Rafael
> 
> 
> > CC: lizhuangzhi <zhuangzhi.li@intel.com>
> > CC: Li Fei <fei.li@intel.com>
> > Signed-off-by: liu chuansheng <chuansheng.liu@intel.com>
> > ---
> >  drivers/acpi/osl.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index e5f416c..b1629b5 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -820,7 +820,7 @@ acpi_status acpi_os_remove_interrupt_handler(u32
> irq, acpi_osd_handler handler)
> >
> >  void acpi_os_sleep(u64 ms)
> >  {
> > -	schedule_timeout_interruptible(msecs_to_jiffies(ms));
> > +	msleep(ms);
> >  }
> >
> >  void acpi_os_stall(u32 us)
> >
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()

[Rafael J. Wysocki]

On Wednesday, September 11, 2013 11:55:53 PM Liu, Chuansheng wrote:
> Hello Rafael,
> 
> > -----Original Message-----
> > From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
> > Sent: Wednesday, September 11, 2013 8:37 PM
> > To: Liu, Chuansheng
> > Cc: lenb@kernel.org; linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org;
> > Li, Zhuangzhi; Li, Fei
> > Subject: Re: [PATCH] ACPI / osl: implement acpi_os_sleep() with msleep()
> > 
> > On Thursday, September 12, 2013 01:42:57 AM Chuansheng Liu wrote:
> > >
> > > Currently the acpi_os_sleep() is using the schedule_timeout_interruptible(),
> > > which can be interrupted by signal, which causes the real sleep time is
> > shorter.
> > >
> > > According to the ACPI spec:
> > > The Sleep term is used to implement long-term timing requirements.
> > > Execution is delayed for at least the required number of milliseconds.
> > >
> > > The sleeping time should be at least of the required number msecs, here
> > > using msleep() to implement it.
> > >
> > > Also if the real time is shorter, we meet the device POWER ON issue.
> > 
> > What exactly is the "power on" issue?
> The case is we have one device _PS0 method in platform.asl like below:
> Write the pmcsr REG to power on;
> Sleep 10ms;
> Read some registers;
> ...
> 
> Here sometimes the actual sleeping time is < 10ms, it causes the following
> actions failed due this device
> need 10ms to power on successfully.

I see.

OK, I'll queue up your patch for 3.13.

Thanks,
Rafael