2013-09-11 08:20:25

by Liu, Chuansheng

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


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 <[email protected]>
CC: Li Fei <[email protected]>
Signed-off-by: liu chuansheng <[email protected]>
---
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



2013-09-11 12:25:33

by Rafael J. Wysocki

[permalink] [raw]
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?

Rafael


> CC: lizhuangzhi <[email protected]>
> CC: Li Fei <[email protected]>
> Signed-off-by: liu chuansheng <[email protected]>
> ---
> 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.

2013-09-11 23:56:03

by Liu, Chuansheng

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

Hello Rafael,

> -----Original Message-----
> From: Rafael J. Wysocki [mailto:[email protected]]
> Sent: Wednesday, September 11, 2013 8:37 PM
> To: Liu, Chuansheng
> Cc: [email protected]; [email protected]; [email protected];
> 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 <[email protected]>
> > CC: Li Fei <[email protected]>
> > Signed-off-by: liu chuansheng <[email protected]>
> > ---
> > 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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-12 00:09:30

by Rafael J. Wysocki

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

On Wednesday, September 11, 2013 11:55:53 PM Liu, Chuansheng wrote:
> Hello Rafael,
>
> > -----Original Message-----
> > From: Rafael J. Wysocki [mailto:[email protected]]
> > Sent: Wednesday, September 11, 2013 8:37 PM
> > To: Liu, Chuansheng
> > Cc: [email protected]; [email protected]; [email protected];
> > 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