[PATCH 1/4] acpi: exsystem: Add units to time variable names

[PATCH 1/4] acpi: exsystem: Add units to time variable names

[Paul Menzel]

`how_long` uses different units in both functions, so make it more
clear, what unit they expect.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/acpi/acpica/exsystem.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
index 1281c07112de..6bc5b46e6927 100644
--- a/drivers/acpi/acpica/exsystem.c
+++ b/drivers/acpi/acpica/exsystem.c
@@ -107,7 +107,7 @@ acpi_status acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
  *
  * FUNCTION:    acpi_ex_system_do_stall
  *
- * PARAMETERS:  how_long        - The amount of time to stall,
+ * PARAMETERS:  how_long_us     - The amount of time to stall,
  *                                in microseconds
  *
  * RETURN:      Status
@@ -120,13 +120,13 @@ acpi_status acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
  *
  ******************************************************************************/
 
-acpi_status acpi_ex_system_do_stall(u32 how_long)
+acpi_status acpi_ex_system_do_stall(u32 how_long_us)
 {
 	acpi_status status = AE_OK;
 
 	ACPI_FUNCTION_ENTRY();
 
-	if (how_long > 255) {	/* 255 microseconds */
+	if (how_long_us > 255) {	/* 255 microseconds */
 		/*
 		 * Longer than 255 usec, this is an error
 		 *
@@ -134,10 +134,10 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
 		 * order to support existing BIOSs)
 		 */
 		ACPI_ERROR((AE_INFO,
-			    "Time parameter is too large (%u)", how_long));
+			    "Time parameter is too large (%u)", how_long_us));
 		status = AE_AML_OPERAND_VALUE;
 	} else {
-		acpi_os_stall(how_long);
+		acpi_os_stall(how_long_us);
 	}
 
 	return (status);
@@ -147,7 +147,7 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
  *
  * FUNCTION:    acpi_ex_system_do_sleep
  *
- * PARAMETERS:  how_long        - The amount of time to sleep,
+ * PARAMETERS:  how_long_ms     - The amount of time to sleep,
  *                                in milliseconds
  *
  * RETURN:      None
@@ -156,7 +156,7 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
  *
  ******************************************************************************/
 
-acpi_status acpi_ex_system_do_sleep(u64 how_long)
+acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
 {
 	ACPI_FUNCTION_ENTRY();
 
@@ -168,11 +168,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long)
 	 * For compatibility with other ACPI implementations and to prevent
 	 * accidental deep sleeps, limit the sleep time to something reasonable.
 	 */
-	if (how_long > ACPI_MAX_SLEEP) {
-		how_long = ACPI_MAX_SLEEP;
+	if (how_long_ms > ACPI_MAX_SLEEP) {
+		how_long_ms = ACPI_MAX_SLEEP;
 	}
 
-	acpi_os_sleep(how_long);
+	acpi_os_sleep(how_long_ms);
 
 	/* And now we must get the interpreter again */
 
-- 
2.35.1

[PATCH 2/4] acpi: exsystem: Inform users about ACPI spec violation

[Paul Menzel]

Inform users if firmware violates the ACPI specification.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/acpi/acpica/exsystem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
index 6bc5b46e6927..00f66af31ffa 100644
--- a/drivers/acpi/acpica/exsystem.c
+++ b/drivers/acpi/acpica/exsystem.c
@@ -137,6 +137,9 @@ acpi_status acpi_ex_system_do_stall(u32 how_long_us)
 			    "Time parameter is too large (%u)", how_long_us));
 		status = AE_AML_OPERAND_VALUE;
 	} else {
+		if (how_long_us > 100)	/* 100 microseconds */
+			ACPI_WARNING((AE_INFO,
+				      "Time parameter %u us > 100 us violating ACPI spec, please fix the firmware.", how_long_us));
 		acpi_os_stall(how_long_us);
 	}
 
-- 
2.35.1

[PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

[Paul Menzel]

Quick boottime is important, so warn about sleeps greater than 50 ms in
ACPI.

50 ms is still long compared to distribution Linux kernels reaching initrd
in 350 ms, so should probably changed to 10 ms, so people are aware
about this.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/acpi/acpica/exsystem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
index 00f66af31ffa..bdffb8aac05c 100644
--- a/drivers/acpi/acpica/exsystem.c
+++ b/drivers/acpi/acpica/exsystem.c
@@ -167,6 +167,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
 
 	acpi_ex_exit_interpreter();
 
+	if (how_long_ms > 50) {
+		ACPI_WARNING((AE_INFO,
+		    "Time parameter %llu > 50 ms. Please contact firmware vendor for more responsive system.", how_long_ms));
+	}
+
 	/*
 	 * For compatibility with other ACPI implementations and to prevent
 	 * accidental deep sleeps, limit the sleep time to something reasonable.
-- 
2.35.1

[PATCH 4/4] acpi: exsystem: Decrease sleep warning threshold to 10 ms

[Paul Menzel]

50 ms is quite long for todays systems and requirements. 10 ms seems
like a more appropriate limit, and everything else needing more should
be rewritten to use polling.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/acpi/acpica/exsystem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
index bdffb8aac05c..676459ccc8d7 100644
--- a/drivers/acpi/acpica/exsystem.c
+++ b/drivers/acpi/acpica/exsystem.c
@@ -167,9 +167,9 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
 
 	acpi_ex_exit_interpreter();
 
-	if (how_long_ms > 50) {
+	if (how_long_ms > 10) {
 		ACPI_WARNING((AE_INFO,
-		    "Time parameter %llu > 50 ms. Please contact firmware vendor for more responsive system.", how_long_ms));
+		    "Time parameter %llu > 10 ms. Please contact firmware vendor for more responsive system.", how_long_ms));
 	}
 
 	/*
-- 
2.35.1

RE: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

[Moore, Robert]

So, this is the current implementation:

    /*
     * For compatibility with other ACPI implementations and to prevent
     * accidental deep sleeps, limit the sleep time to something reasonable.
     */
    if (HowLong > ACPI_MAX_SLEEP)
    {
        HowLong = ACPI_MAX_SLEEP;
    }

    AcpiOsSleep (HowLong);

Where ACPI_MAX_SLEEP is: 

#define ACPI_MAX_SLEEP                  2000    /* 2000 millisec == two seconds */

-----Original Message-----
From: Paul Menzel <pmenzel@molgen.mpg.de> 
Sent: Thursday, February 24, 2022 3:38 AM
To: Moore, Robert <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Len Brown <lenb@kernel.org>
Cc: Paul Menzel <pmenzel@molgen.mpg.de>; linux-acpi@vger.kernel.org; devel@acpica.org; linux-kernel@vger.kernel.org
Subject: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

Quick boottime is important, so warn about sleeps greater than 50 ms in ACPI.

50 ms is still long compared to distribution Linux kernels reaching initrd in 350 ms, so should probably changed to 10 ms, so people are aware about this.

Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---
 drivers/acpi/acpica/exsystem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c index 00f66af31ffa..bdffb8aac05c 100644
--- a/drivers/acpi/acpica/exsystem.c
+++ b/drivers/acpi/acpica/exsystem.c
@@ -167,6 +167,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
 
 	acpi_ex_exit_interpreter();
 
+	if (how_long_ms > 50) {
+		ACPI_WARNING((AE_INFO,
+		    "Time parameter %llu > 50 ms. Please contact firmware vendor for more responsive system.", how_long_ms));
+	}
+
 	/*
 	 * For compatibility with other ACPI implementations and to prevent
 	 * accidental deep sleeps, limit the sleep time to something reasonable.
--
2.35.1

[Devel] Re: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

[Moore, Robert]

[-- Attachment #1: Type: text/plain, Size: 1912 bytes --]

So, this is the current implementation:

    /*
     * For compatibility with other ACPI implementations and to prevent
     * accidental deep sleeps, limit the sleep time to something reasonable.
     */
    if (HowLong > ACPI_MAX_SLEEP)
    {
        HowLong = ACPI_MAX_SLEEP;
    }

    AcpiOsSleep (HowLong);

Where ACPI_MAX_SLEEP is: 

#define ACPI_MAX_SLEEP                  2000    /* 2000 millisec == two seconds */

-----Original Message-----
From: Paul Menzel <pmenzel(a)molgen.mpg.de> 
Sent: Thursday, February 24, 2022 3:38 AM
To: Moore, Robert <robert.moore(a)intel.com>; Wysocki, Rafael J <rafael.j.wysocki(a)intel.com>; Len Brown <lenb(a)kernel.org>
Cc: Paul Menzel <pmenzel(a)molgen.mpg.de>; linux-acpi(a)vger.kernel.org; devel(a)acpica.org; linux-kernel(a)vger.kernel.org
Subject: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

Quick boottime is important, so warn about sleeps greater than 50 ms in ACPI.

50 ms is still long compared to distribution Linux kernels reaching initrd in 350 ms, so should probably changed to 10 ms, so people are aware about this.

Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de>
---
 drivers/acpi/acpica/exsystem.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c index 00f66af31ffa..bdffb8aac05c 100644
--- a/drivers/acpi/acpica/exsystem.c
+++ b/drivers/acpi/acpica/exsystem.c
@@ -167,6 +167,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
 
 	acpi_ex_exit_interpreter();
 
+	if (how_long_ms > 50) {
+		ACPI_WARNING((AE_INFO,
+		    "Time parameter %llu > 50 ms. Please contact firmware vendor for more responsive system.", how_long_ms));
+	}
+
 	/*
 	 * For compatibility with other ACPI implementations and to prevent
 	 * accidental deep sleeps, limit the sleep time to something reasonable.
--
2.35.1

Re: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

[Paul Menzel]

Dear Robert,


Am 24.02.22 um 17:19 schrieb Moore, Robert:
> So, this is the current implementation:
> 
>      /*
>       * For compatibility with other ACPI implementations and to prevent
>       * accidental deep sleeps, limit the sleep time to something reasonable.
>       */
>      if (HowLong > ACPI_MAX_SLEEP)
>      {
>          HowLong = ACPI_MAX_SLEEP;
>      }
> 
>      AcpiOsSleep (HowLong);
> 
> Where ACPI_MAX_SLEEP is:
> 
> #define ACPI_MAX_SLEEP                  2000    /* 2000 millisec == two seconds */

Exactly. I am not changing it though, but just add a warning.


Kind regards,

Paul


> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Thursday, February 24, 2022 3:38 AM
> To: Moore, Robert <robert.moore@intel.com>; Wysocki, Rafael J <rafael.j.wysocki@intel.com>; Len Brown <lenb@kernel.org>
> Cc: Paul Menzel <pmenzel@molgen.mpg.de>; linux-acpi@vger.kernel.org; devel@acpica.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms
> 
> Quick boottime is important, so warn about sleeps greater than 50 ms in ACPI.
> 
> 50 ms is still long compared to distribution Linux kernels reaching initrd in 350 ms, so should probably changed to 10 ms, so people are aware about this.
> 
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>   drivers/acpi/acpica/exsystem.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c index 00f66af31ffa..bdffb8aac05c 100644
> --- a/drivers/acpi/acpica/exsystem.c
> +++ b/drivers/acpi/acpica/exsystem.c
> @@ -167,6 +167,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
>   
>   	acpi_ex_exit_interpreter();
>   
> +	if (how_long_ms > 50) {
> +		ACPI_WARNING((AE_INFO,
> +		    "Time parameter %llu > 50 ms. Please contact firmware vendor for more responsive system.", how_long_ms));
> +	}
> +
>   	/*
>   	 * For compatibility with other ACPI implementations and to prevent
>   	 * accidental deep sleeps, limit the sleep time to something reasonable.
> --
> 2.35.1

Re: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

[Rafael J. Wysocki]

On Thu, Feb 24, 2022 at 12:38 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Quick boottime is important, so warn about sleeps greater than 50 ms in
> ACPI.
>
> 50 ms is still long compared to distribution Linux kernels reaching initrd
> in 350 ms, so should probably changed to 10 ms, so people are aware
> about this.
>
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>

First off, as ACPICA material, this should be submitted to the
upstream project via https://github.com/acpica/acpica/.

> ---
>  drivers/acpi/acpica/exsystem.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
> index 00f66af31ffa..bdffb8aac05c 100644
> --- a/drivers/acpi/acpica/exsystem.c
> +++ b/drivers/acpi/acpica/exsystem.c
> @@ -167,6 +167,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
>
>         acpi_ex_exit_interpreter();
>
> +       if (how_long_ms > 50) {
> +               ACPI_WARNING((AE_INFO,

Second, the log level is somewhat high for something like this.

> +                   "Time parameter %llu > 50 ms. Please contact firmware vendor for more responsive system.", how_long_ms));

Also, I would rephrase the warning message to something like "Firmware
issue: Excessive delay (%llu ms) in ACPI Control Method".

> +       }
> +
>         /*
>          * For compatibility with other ACPI implementations and to prevent
>          * accidental deep sleeps, limit the sleep time to something reasonable.
> --
> 2.35.1
>

[Devel] Re: [PATCH 3/4] acpi: exsystem: Warn about sleeps greater than 50 ms

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]

On Thu, Feb 24, 2022 at 12:38 PM Paul Menzel <pmenzel(a)molgen.mpg.de> wrote:
>
> Quick boottime is important, so warn about sleeps greater than 50 ms in
> ACPI.
>
> 50 ms is still long compared to distribution Linux kernels reaching initrd
> in 350 ms, so should probably changed to 10 ms, so people are aware
> about this.
>
> Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de>

First off, as ACPICA material, this should be submitted to the
upstream project via https://github.com/acpica/acpica/.

> ---
>  drivers/acpi/acpica/exsystem.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
> index 00f66af31ffa..bdffb8aac05c 100644
> --- a/drivers/acpi/acpica/exsystem.c
> +++ b/drivers/acpi/acpica/exsystem.c
> @@ -167,6 +167,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
>
>         acpi_ex_exit_interpreter();
>
> +       if (how_long_ms > 50) {
> +               ACPI_WARNING((AE_INFO,

Second, the log level is somewhat high for something like this.

> +                   "Time parameter %llu > 50 ms. Please contact firmware vendor for more responsive system.", how_long_ms));

Also, I would rephrase the warning message to something like "Firmware
issue: Excessive delay (%llu ms) in ACPI Control Method".

> +       }
> +
>         /*
>          * For compatibility with other ACPI implementations and to prevent
>          * accidental deep sleeps, limit the sleep time to something reasonable.
> --
> 2.35.1
>

Re: [PATCH 1/4] acpi: exsystem: Add units to time variable names

[Rafael J. Wysocki]

On Thu, Feb 24, 2022 at 12:38 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> `how_long` uses different units in both functions, so make it more
> clear, what unit they expect.
>
> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
> ---
>  drivers/acpi/acpica/exsystem.c | 20 ++++++++++----------

As ACPICA material, this should be submitted to the upstream project
via https://github.com/acpica/acpica/.

This applies to the other patches in the series too.

>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
> index 1281c07112de..6bc5b46e6927 100644
> --- a/drivers/acpi/acpica/exsystem.c
> +++ b/drivers/acpi/acpica/exsystem.c
> @@ -107,7 +107,7 @@ acpi_status acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
>   *
>   * FUNCTION:    acpi_ex_system_do_stall
>   *
> - * PARAMETERS:  how_long        - The amount of time to stall,
> + * PARAMETERS:  how_long_us     - The amount of time to stall,
>   *                                in microseconds
>   *
>   * RETURN:      Status
> @@ -120,13 +120,13 @@ acpi_status acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
>   *
>   ******************************************************************************/
>
> -acpi_status acpi_ex_system_do_stall(u32 how_long)
> +acpi_status acpi_ex_system_do_stall(u32 how_long_us)
>  {
>         acpi_status status = AE_OK;
>
>         ACPI_FUNCTION_ENTRY();
>
> -       if (how_long > 255) {   /* 255 microseconds */
> +       if (how_long_us > 255) {        /* 255 microseconds */
>                 /*
>                  * Longer than 255 usec, this is an error
>                  *
> @@ -134,10 +134,10 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
>                  * order to support existing BIOSs)
>                  */
>                 ACPI_ERROR((AE_INFO,
> -                           "Time parameter is too large (%u)", how_long));
> +                           "Time parameter is too large (%u)", how_long_us));
>                 status = AE_AML_OPERAND_VALUE;
>         } else {
> -               acpi_os_stall(how_long);
> +               acpi_os_stall(how_long_us);
>         }
>
>         return (status);
> @@ -147,7 +147,7 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
>   *
>   * FUNCTION:    acpi_ex_system_do_sleep
>   *
> - * PARAMETERS:  how_long        - The amount of time to sleep,
> + * PARAMETERS:  how_long_ms     - The amount of time to sleep,
>   *                                in milliseconds
>   *
>   * RETURN:      None
> @@ -156,7 +156,7 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
>   *
>   ******************************************************************************/
>
> -acpi_status acpi_ex_system_do_sleep(u64 how_long)
> +acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
>  {
>         ACPI_FUNCTION_ENTRY();
>
> @@ -168,11 +168,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long)
>          * For compatibility with other ACPI implementations and to prevent
>          * accidental deep sleeps, limit the sleep time to something reasonable.
>          */
> -       if (how_long > ACPI_MAX_SLEEP) {
> -               how_long = ACPI_MAX_SLEEP;
> +       if (how_long_ms > ACPI_MAX_SLEEP) {
> +               how_long_ms = ACPI_MAX_SLEEP;
>         }
>
> -       acpi_os_sleep(how_long);
> +       acpi_os_sleep(how_long_ms);
>
>         /* And now we must get the interpreter again */
>
> --
> 2.35.1
>

[Devel] Re: [PATCH 1/4] acpi: exsystem: Add units to time variable names

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 3572 bytes --]

On Thu, Feb 24, 2022 at 12:38 PM Paul Menzel <pmenzel(a)molgen.mpg.de> wrote:
>
> `how_long` uses different units in both functions, so make it more
> clear, what unit they expect.
>
> Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de>
> ---
>  drivers/acpi/acpica/exsystem.c | 20 ++++++++++----------

As ACPICA material, this should be submitted to the upstream project
via https://github.com/acpica/acpica/.

This applies to the other patches in the series too.

>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/acpica/exsystem.c b/drivers/acpi/acpica/exsystem.c
> index 1281c07112de..6bc5b46e6927 100644
> --- a/drivers/acpi/acpica/exsystem.c
> +++ b/drivers/acpi/acpica/exsystem.c
> @@ -107,7 +107,7 @@ acpi_status acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
>   *
>   * FUNCTION:    acpi_ex_system_do_stall
>   *
> - * PARAMETERS:  how_long        - The amount of time to stall,
> + * PARAMETERS:  how_long_us     - The amount of time to stall,
>   *                                in microseconds
>   *
>   * RETURN:      Status
> @@ -120,13 +120,13 @@ acpi_status acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
>   *
>   ******************************************************************************/
>
> -acpi_status acpi_ex_system_do_stall(u32 how_long)
> +acpi_status acpi_ex_system_do_stall(u32 how_long_us)
>  {
>         acpi_status status = AE_OK;
>
>         ACPI_FUNCTION_ENTRY();
>
> -       if (how_long > 255) {   /* 255 microseconds */
> +       if (how_long_us > 255) {        /* 255 microseconds */
>                 /*
>                  * Longer than 255 usec, this is an error
>                  *
> @@ -134,10 +134,10 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
>                  * order to support existing BIOSs)
>                  */
>                 ACPI_ERROR((AE_INFO,
> -                           "Time parameter is too large (%u)", how_long));
> +                           "Time parameter is too large (%u)", how_long_us));
>                 status = AE_AML_OPERAND_VALUE;
>         } else {
> -               acpi_os_stall(how_long);
> +               acpi_os_stall(how_long_us);
>         }
>
>         return (status);
> @@ -147,7 +147,7 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
>   *
>   * FUNCTION:    acpi_ex_system_do_sleep
>   *
> - * PARAMETERS:  how_long        - The amount of time to sleep,
> + * PARAMETERS:  how_long_ms     - The amount of time to sleep,
>   *                                in milliseconds
>   *
>   * RETURN:      None
> @@ -156,7 +156,7 @@ acpi_status acpi_ex_system_do_stall(u32 how_long)
>   *
>   ******************************************************************************/
>
> -acpi_status acpi_ex_system_do_sleep(u64 how_long)
> +acpi_status acpi_ex_system_do_sleep(u64 how_long_ms)
>  {
>         ACPI_FUNCTION_ENTRY();
>
> @@ -168,11 +168,11 @@ acpi_status acpi_ex_system_do_sleep(u64 how_long)
>          * For compatibility with other ACPI implementations and to prevent
>          * accidental deep sleeps, limit the sleep time to something reasonable.
>          */
> -       if (how_long > ACPI_MAX_SLEEP) {
> -               how_long = ACPI_MAX_SLEEP;
> +       if (how_long_ms > ACPI_MAX_SLEEP) {
> +               how_long_ms = ACPI_MAX_SLEEP;
>         }
>
> -       acpi_os_sleep(how_long);
> +       acpi_os_sleep(how_long_ms);
>
>         /* And now we must get the interpreter again */
>
> --
> 2.35.1
>

Re: [PATCH 1/4] acpi: exsystem: Add units to time variable names

[Paul Menzel]

Dear Rafael,


Am 01.03.22 um 20:40 schrieb Rafael J. Wysocki:
> On Thu, Feb 24, 2022 at 12:38 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>>
>> `how_long` uses different units in both functions, so make it more
>> clear, what unit they expect.
>>
>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> ---
>>   drivers/acpi/acpica/exsystem.c | 20 ++++++++++----------
> 
> As ACPICA material, this should be submitted to the upstream project
> via https://github.com/acpica/acpica/.
> 
> This applies to the other patches in the series too.

Thank you for the hint. Done [1].

[…]


Kind regards,

Paul


PS: I should have read [2] before manually porting my patches from Linux 
to ACPICA.


[1]: https://github.com/acpica/acpica/pull/754
[2]: Documentation/driver-api/acpi/linuxized-acpica.rst