linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
@ 2023-06-02  7:30 Mario Limonciello
  2023-06-02  7:30 ` [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking Mario Limonciello
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-06-02  7:30 UTC (permalink / raw)
  To: Rafael Wysocki, hdegoede, linus.walleij
  Cc: linux-acpi, linux-kernel, platform-driver-x86, linux-gpio,
	linux-pm, Basavaraj.Natikar, Shyam-sundar.S-k, Mario Limonciello

All uses in the kernel are currently already oriented around
suspend/resume. As some other parts of the kernel may also use these
messages in functions that could also be used outside of
suspend/resume, only enable in suspend/resume path.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v3->v4:
 * add back do/while as it wasn't pointless.  It fixes a warning.
---
 include/linux/suspend.h | 8 +++++---
 kernel/power/main.c     | 6 ++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 1a0426e6761c..74f406c53ac0 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
 #ifdef CONFIG_PM_SLEEP_DEBUG
 extern bool pm_print_times_enabled;
 extern bool pm_debug_messages_on;
+extern bool pm_debug_messages_should_print(void);
 static inline int pm_dyn_debug_messages_on(void)
 {
 #ifdef CONFIG_DYNAMIC_DEBUG
@@ -568,14 +569,14 @@ static inline int pm_dyn_debug_messages_on(void)
 #endif
 #define __pm_pr_dbg(fmt, ...)					\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 		else if (pm_dyn_debug_messages_on())		\
 			pr_debug(fmt, ##__VA_ARGS__);	\
 	} while (0)
 #define __pm_deferred_pr_dbg(fmt, ...)				\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 	} while (0)
 #else
@@ -593,7 +594,8 @@ static inline int pm_dyn_debug_messages_on(void)
 /**
  * pm_pr_dbg - print pm sleep debug messages
  *
- * If pm_debug_messages_on is enabled, print message.
+ * If pm_debug_messages_on is enabled and the system is entering/leaving
+ *      suspend, print message.
  * If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
  *	print message only from instances explicitly enabled on dynamic debug's
  *	control.
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3113ec2f1db4..daa535012e51 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
 
 bool pm_debug_messages_on __read_mostly;
 
+bool pm_debug_messages_should_print(void)
+{
+	return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;
+}
+EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
+
 static ssize_t pm_debug_messages_show(struct kobject *kobj,
 				      struct kobj_attribute *attr, char *buf)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking
  2023-06-02  7:30 [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume Mario Limonciello
@ 2023-06-02  7:30 ` Mario Limonciello
  2023-06-04 16:04   ` Rafael J. Wysocki
  2023-06-02  7:30 ` [PATCH v4 3/4] pinctrl: amd: Use pm_pr_dbg to show debugging messages Mario Limonciello
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Mario Limonciello @ 2023-06-02  7:30 UTC (permalink / raw)
  To: Rafael Wysocki, hdegoede, linus.walleij
  Cc: linux-acpi, linux-kernel, platform-driver-x86, linux-gpio,
	linux-pm, Basavaraj.Natikar, Shyam-sundar.S-k, Mario Limonciello

Enabling debugging messages for the state requires turning on dynamic
debugging for the file. To make it more accessible, use
`pm_debug_messages` and clearer strings for what is happening.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/x86/s2idle.c | 52 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index e499c60c4579..7681f6ecab67 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -59,6 +59,7 @@ static int lps0_dsm_func_mask;
 
 static guid_t lps0_dsm_guid_microsoft;
 static int lps0_dsm_func_mask_microsoft;
+static int lps0_dsm_state;
 
 /* Device constraint entry structure */
 struct lpi_device_info {
@@ -320,6 +321,44 @@ static void lpi_check_constraints(void)
 	}
 }
 
+static bool acpi_s2idle_vendor_amd(void)
+{
+	return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
+}
+
+static const char *acpi_sleep_dsm_state_to_str(unsigned int state)
+{
+	if (lps0_dsm_func_mask_microsoft || !acpi_s2idle_vendor_amd()) {
+		switch (state) {
+		case ACPI_LPS0_SCREEN_OFF:
+			return "screen off";
+		case ACPI_LPS0_SCREEN_ON:
+			return "screen on";
+		case ACPI_LPS0_ENTRY:
+			return "lps0 entry";
+		case ACPI_LPS0_EXIT:
+			return "lps0 exit";
+		case ACPI_LPS0_MS_ENTRY:
+			return "lps0 ms entry";
+		case ACPI_LPS0_MS_EXIT:
+			return "lps0 ms exit";
+		}
+	} else {
+		switch (state) {
+		case ACPI_LPS0_SCREEN_ON_AMD:
+			return "screen on";
+		case ACPI_LPS0_SCREEN_OFF_AMD:
+			return "screen off";
+		case ACPI_LPS0_ENTRY_AMD:
+			return "lps0 entry";
+		case ACPI_LPS0_EXIT_AMD:
+			return "lps0 exit";
+		}
+	}
+
+	return "unknown";
+}
+
 static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, guid_t dsm_guid)
 {
 	union acpi_object *out_obj;
@@ -331,14 +370,15 @@ static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, g
 					rev_id, func, NULL);
 	ACPI_FREE(out_obj);
 
-	acpi_handle_debug(lps0_device_handle, "_DSM function %u evaluation %s\n",
-			  func, out_obj ? "successful" : "failed");
+	lps0_dsm_state = func;
+	if (pm_debug_messages_on) {
+		acpi_handle_info(lps0_device_handle,
+				"%s transitioned to state %s\n",
+				 out_obj ? "Successfully" : "Failed to",
+				 acpi_sleep_dsm_state_to_str(lps0_dsm_state));
+	}
 }
 
-static bool acpi_s2idle_vendor_amd(void)
-{
-	return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
-}
 
 static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *dsm_guid)
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/4] pinctrl: amd: Use pm_pr_dbg to show debugging messages
  2023-06-02  7:30 [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume Mario Limonciello
  2023-06-02  7:30 ` [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking Mario Limonciello
@ 2023-06-02  7:30 ` Mario Limonciello
  2023-06-09  7:20   ` Linus Walleij
  2023-06-02  7:30 ` [PATCH v4 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for suspend related messages Mario Limonciello
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Mario Limonciello @ 2023-06-02  7:30 UTC (permalink / raw)
  To: Rafael Wysocki, hdegoede, linus.walleij
  Cc: linux-acpi, linux-kernel, platform-driver-x86, linux-gpio,
	linux-pm, Basavaraj.Natikar, Shyam-sundar.S-k, Mario Limonciello

To make the GPIO tracking around suspend easier for end users to
use, link it with pm_debug_messages.  This will make discovering
sources of spurious GPIOs around suspend easier.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/pinctrl/pinctrl-amd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index f279b360c20d..43d3530bab48 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -30,6 +30,7 @@
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinconf-generic.h>
 #include <linux/pinctrl/pinmux.h>
+#include <linux/suspend.h>
 
 #include "core.h"
 #include "pinctrl-utils.h"
@@ -636,9 +637,8 @@ static bool do_amd_gpio_irq_handler(int irq, void *dev_id)
 			regval = readl(regs + i);
 
 			if (regval & PIN_IRQ_PENDING)
-				dev_dbg(&gpio_dev->pdev->dev,
-					"GPIO %d is active: 0x%x",
-					irqnr + i, regval);
+				pm_pr_dbg("GPIO %d is active: 0x%x",
+					  irqnr + i, regval);
 
 			/* caused wake on resume context for shared IRQ */
 			if (irq < 0 && (regval & BIT(WAKE_STS_OFF)))
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for suspend related messages
  2023-06-02  7:30 [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume Mario Limonciello
  2023-06-02  7:30 ` [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking Mario Limonciello
  2023-06-02  7:30 ` [PATCH v4 3/4] pinctrl: amd: Use pm_pr_dbg to show debugging messages Mario Limonciello
@ 2023-06-02  7:30 ` Mario Limonciello
  2023-06-04 17:08   ` Hans de Goede
  2024-04-07 12:39 ` [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume xiongxin
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Mario Limonciello @ 2023-06-02  7:30 UTC (permalink / raw)
  To: Rafael Wysocki, hdegoede, linus.walleij
  Cc: linux-acpi, linux-kernel, platform-driver-x86, linux-gpio,
	linux-pm, Basavaraj.Natikar, Shyam-sundar.S-k, Mario Limonciello

Using pm_pr_dbg() allows users to toggle `/sys/power/pm_debug_messages`
as a single knob to turn on messages that amd-pmc can emit to aid in
any s2idle debugging.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 427905714f79..1304cd6f13f6 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -543,7 +543,7 @@ static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
 	}
 
 	if (dev)
-		dev_dbg(pdev->dev, "SMU idlemask s0i3: 0x%x\n", val);
+		pm_pr_dbg("SMU idlemask s0i3: 0x%x\n", val);
 
 	if (s)
 		seq_printf(s, "SMU idlemask : 0x%x\n", val);
@@ -769,7 +769,7 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
 
 	*arg |= (duration << 16);
 	rc = rtc_alarm_irq_enable(rtc_device, 0);
-	dev_dbg(pdev->dev, "wakeup timer programmed for %lld seconds\n", duration);
+	pm_pr_dbg("wakeup timer programmed for %lld seconds\n", duration);
 
 	return rc;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking
  2023-06-02  7:30 ` [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking Mario Limonciello
@ 2023-06-04 16:04   ` Rafael J. Wysocki
  2023-06-12 17:58     ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2023-06-04 16:04 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Rafael Wysocki, hdegoede, linus.walleij, linux-acpi,
	linux-kernel, platform-driver-x86, linux-gpio, linux-pm,
	Basavaraj.Natikar, Shyam-sundar.S-k

On Fri, Jun 2, 2023 at 9:32 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> Enabling debugging messages for the state requires turning on dynamic
> debugging for the file. To make it more accessible, use
> `pm_debug_messages` and clearer strings for what is happening.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

I'm inclined to apply this one and the [1/4] at this point.

I can also apply the 2 remaining patches in this series if I get ACKs
for them from the respective subsystem maintainers.

> ---
>  drivers/acpi/x86/s2idle.c | 52 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index e499c60c4579..7681f6ecab67 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -59,6 +59,7 @@ static int lps0_dsm_func_mask;
>
>  static guid_t lps0_dsm_guid_microsoft;
>  static int lps0_dsm_func_mask_microsoft;
> +static int lps0_dsm_state;
>
>  /* Device constraint entry structure */
>  struct lpi_device_info {
> @@ -320,6 +321,44 @@ static void lpi_check_constraints(void)
>         }
>  }
>
> +static bool acpi_s2idle_vendor_amd(void)
> +{
> +       return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
> +}
> +
> +static const char *acpi_sleep_dsm_state_to_str(unsigned int state)
> +{
> +       if (lps0_dsm_func_mask_microsoft || !acpi_s2idle_vendor_amd()) {
> +               switch (state) {
> +               case ACPI_LPS0_SCREEN_OFF:
> +                       return "screen off";
> +               case ACPI_LPS0_SCREEN_ON:
> +                       return "screen on";
> +               case ACPI_LPS0_ENTRY:
> +                       return "lps0 entry";
> +               case ACPI_LPS0_EXIT:
> +                       return "lps0 exit";
> +               case ACPI_LPS0_MS_ENTRY:
> +                       return "lps0 ms entry";
> +               case ACPI_LPS0_MS_EXIT:
> +                       return "lps0 ms exit";
> +               }
> +       } else {
> +               switch (state) {
> +               case ACPI_LPS0_SCREEN_ON_AMD:
> +                       return "screen on";
> +               case ACPI_LPS0_SCREEN_OFF_AMD:
> +                       return "screen off";
> +               case ACPI_LPS0_ENTRY_AMD:
> +                       return "lps0 entry";
> +               case ACPI_LPS0_EXIT_AMD:
> +                       return "lps0 exit";
> +               }
> +       }
> +
> +       return "unknown";
> +}
> +
>  static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, guid_t dsm_guid)
>  {
>         union acpi_object *out_obj;
> @@ -331,14 +370,15 @@ static void acpi_sleep_run_lps0_dsm(unsigned int func, unsigned int func_mask, g
>                                         rev_id, func, NULL);
>         ACPI_FREE(out_obj);
>
> -       acpi_handle_debug(lps0_device_handle, "_DSM function %u evaluation %s\n",
> -                         func, out_obj ? "successful" : "failed");
> +       lps0_dsm_state = func;
> +       if (pm_debug_messages_on) {
> +               acpi_handle_info(lps0_device_handle,
> +                               "%s transitioned to state %s\n",
> +                                out_obj ? "Successfully" : "Failed to",
> +                                acpi_sleep_dsm_state_to_str(lps0_dsm_state));
> +       }
>  }
>
> -static bool acpi_s2idle_vendor_amd(void)
> -{
> -       return boot_cpu_data.x86_vendor == X86_VENDOR_AMD;
> -}
>
>  static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *dsm_guid)
>  {
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for suspend related messages
  2023-06-02  7:30 ` [PATCH v4 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for suspend related messages Mario Limonciello
@ 2023-06-04 17:08   ` Hans de Goede
  0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2023-06-04 17:08 UTC (permalink / raw)
  To: Mario Limonciello, Rafael Wysocki, linus.walleij
  Cc: linux-acpi, linux-kernel, platform-driver-x86, linux-gpio,
	linux-pm, Basavaraj.Natikar, Shyam-sundar.S-k

Hi,

On 6/2/23 09:30, Mario Limonciello wrote:
> Using pm_pr_dbg() allows users to toggle `/sys/power/pm_debug_messages`
> as a single knob to turn on messages that amd-pmc can emit to aid in
> any s2idle debugging.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Thanks, patch looks good to me.

Here is my ack for merging this through the linux-pm tree:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans




> ---
>  drivers/platform/x86/amd/pmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
> index 427905714f79..1304cd6f13f6 100644
> --- a/drivers/platform/x86/amd/pmc.c
> +++ b/drivers/platform/x86/amd/pmc.c
> @@ -543,7 +543,7 @@ static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
>  	}
>  
>  	if (dev)
> -		dev_dbg(pdev->dev, "SMU idlemask s0i3: 0x%x\n", val);
> +		pm_pr_dbg("SMU idlemask s0i3: 0x%x\n", val);
>  
>  	if (s)
>  		seq_printf(s, "SMU idlemask : 0x%x\n", val);
> @@ -769,7 +769,7 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
>  
>  	*arg |= (duration << 16);
>  	rc = rtc_alarm_irq_enable(rtc_device, 0);
> -	dev_dbg(pdev->dev, "wakeup timer programmed for %lld seconds\n", duration);
> +	pm_pr_dbg("wakeup timer programmed for %lld seconds\n", duration);
>  
>  	return rc;
>  }


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/4] pinctrl: amd: Use pm_pr_dbg to show debugging messages
  2023-06-02  7:30 ` [PATCH v4 3/4] pinctrl: amd: Use pm_pr_dbg to show debugging messages Mario Limonciello
@ 2023-06-09  7:20   ` Linus Walleij
  0 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2023-06-09  7:20 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Rafael Wysocki, hdegoede, linux-acpi, linux-kernel,
	platform-driver-x86, linux-gpio, linux-pm, Basavaraj.Natikar,
	Shyam-sundar.S-k

On Fri, Jun 2, 2023 at 9:32 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:

> To make the GPIO tracking around suspend easier for end users to
> use, link it with pm_debug_messages.  This will make discovering
> sources of spurious GPIOs around suspend easier.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

If the PM people merge the other patches they can take this too because
of the dependency.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking
  2023-06-04 16:04   ` Rafael J. Wysocki
@ 2023-06-12 17:58     ` Rafael J. Wysocki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2023-06-12 17:58 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Mario Limonciello, hdegoede, linus.walleij, linux-acpi,
	linux-kernel, platform-driver-x86, linux-gpio, linux-pm,
	Basavaraj.Natikar, Shyam-sundar.S-k

On Sun, Jun 4, 2023 at 6:04 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Jun 2, 2023 at 9:32 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> >
> > Enabling debugging messages for the state requires turning on dynamic
> > debugging for the file. To make it more accessible, use
> > `pm_debug_messages` and clearer strings for what is happening.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>
> I'm inclined to apply this one and the [1/4] at this point.
>
> I can also apply the 2 remaining patches in this series if I get ACKs
> for them from the respective subsystem maintainers.

The ACKs were provided, so the entire series has been applied as 6.5
material, thanks!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
  2023-06-02  7:30 [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume Mario Limonciello
                   ` (2 preceding siblings ...)
  2023-06-02  7:30 ` [PATCH v4 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for suspend related messages Mario Limonciello
@ 2024-04-07 12:39 ` xiongxin
  2024-04-07 12:49 ` xiongxin
  2024-04-07 15:32 ` Randy Dunlap
  5 siblings, 0 replies; 11+ messages in thread
From: xiongxin @ 2024-04-07 12:39 UTC (permalink / raw)
  To: xiongxin, Rafael Wysocki, hdegoede, linus.walleij
  Cc: Mario Limonciello, linux-acpi, linux-kernel, platform-driver-x86,
	linux-gpio, linux-pm, Basavaraj.Natikar, Shyam-sundar.S-k

From: Mario Limonciello <mario.limonciello@amd.com>

All uses in the kernel are currently already oriented around
suspend/resume. As some other parts of the kernel may also use these
messages in functions that could also be used outside of
suspend/resume, only enable in suspend/resume path.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v3->v4:
 * add back do/while as it wasn't pointless.  It fixes a warning.
---
 include/linux/suspend.h | 8 +++++---
 kernel/power/main.c     | 6 ++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 1a0426e6761c..74f406c53ac0 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
 #ifdef CONFIG_PM_SLEEP_DEBUG
 extern bool pm_print_times_enabled;
 extern bool pm_debug_messages_on;
+extern bool pm_debug_messages_should_print(void);
 static inline int pm_dyn_debug_messages_on(void)
 {
 #ifdef CONFIG_DYNAMIC_DEBUG
@@ -568,14 +569,14 @@ static inline int pm_dyn_debug_messages_on(void)
 #endif
 #define __pm_pr_dbg(fmt, ...)					\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 		else if (pm_dyn_debug_messages_on())		\
 			pr_debug(fmt, ##__VA_ARGS__);	\
 	} while (0)
 #define __pm_deferred_pr_dbg(fmt, ...)				\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 	} while (0)
 #else
@@ -593,7 +594,8 @@ static inline int pm_dyn_debug_messages_on(void)
 /**
  * pm_pr_dbg - print pm sleep debug messages
  *
- * If pm_debug_messages_on is enabled, print message.
+ * If pm_debug_messages_on is enabled and the system is entering/leaving
+ *      suspend, print message.
  * If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
  *	print message only from instances explicitly enabled on dynamic debug's
  *	control.
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3113ec2f1db4..daa535012e51 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
 
 bool pm_debug_messages_on __read_mostly;
 
+bool pm_debug_messages_should_print(void)
+{
+	return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;

> hibernate processes also mostly use the pm_pr_dbg() function, which
> results in hibernate processes only being able to output such logs
> through dynamic debug, which is unfriendly to kernels without
> CONFIG_DYNAMIC_DEBUG configuration.

+}
+EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
+
 static ssize_t pm_debug_messages_show(struct kobject *kobj,
 				      struct kobj_attribute *attr, char *buf)
 {

-- 
2.34.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
  2023-06-02  7:30 [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume Mario Limonciello
                   ` (3 preceding siblings ...)
  2024-04-07 12:39 ` [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume xiongxin
@ 2024-04-07 12:49 ` xiongxin
  2024-04-07 15:32 ` Randy Dunlap
  5 siblings, 0 replies; 11+ messages in thread
From: xiongxin @ 2024-04-07 12:49 UTC (permalink / raw)
  To: mario.limonciello, Rafael Wysocki, hdegoede, linus.walleij
  Cc: Basavaraj.Natikar, Shyam-sundar.S-k, linux-pm, linux-acpi,
	linux-kernel, platform-driver-x86, linux-gpio

From: Mario Limonciello <mario.limonciello@amd.com>

All uses in the kernel are currently already oriented around
suspend/resume. As some other parts of the kernel may also use these
messages in functions that could also be used outside of
suspend/resume, only enable in suspend/resume path.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v3->v4:
 * add back do/while as it wasn't pointless.  It fixes a warning.
---
 include/linux/suspend.h | 8 +++++---
 kernel/power/main.c     | 6 ++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 1a0426e6761c..74f406c53ac0 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
 #ifdef CONFIG_PM_SLEEP_DEBUG
 extern bool pm_print_times_enabled;
 extern bool pm_debug_messages_on;
+extern bool pm_debug_messages_should_print(void);
 static inline int pm_dyn_debug_messages_on(void)
 {
 #ifdef CONFIG_DYNAMIC_DEBUG
@@ -568,14 +569,14 @@ static inline int pm_dyn_debug_messages_on(void)
 #endif
 #define __pm_pr_dbg(fmt, ...)					\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 		else if (pm_dyn_debug_messages_on())		\
 			pr_debug(fmt, ##__VA_ARGS__);	\
 	} while (0)
 #define __pm_deferred_pr_dbg(fmt, ...)				\
 	do {							\
-		if (pm_debug_messages_on)			\
+		if (pm_debug_messages_should_print())		\
 			printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 	} while (0)
 #else
@@ -593,7 +594,8 @@ static inline int pm_dyn_debug_messages_on(void)
 /**
  * pm_pr_dbg - print pm sleep debug messages
  *
- * If pm_debug_messages_on is enabled, print message.
+ * If pm_debug_messages_on is enabled and the system is entering/leaving
+ *      suspend, print message.
  * If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
  *	print message only from instances explicitly enabled on dynamic debug's
  *	control.
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3113ec2f1db4..daa535012e51 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
 
 bool pm_debug_messages_on __read_mostly;
 
+bool pm_debug_messages_should_print(void)
+{
+	return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;

> hibernate processes also mostly use the pm_pr_dbg() function, which
> results in hibernate processes only being able to output such logs
> through dynamic debug, which is unfriendly to kernels without
> CONFIG_DYNAMIC_DEBUG configuration.

+}
+EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
+
 static ssize_t pm_debug_messages_show(struct kobject *kobj,
 				      struct kobj_attribute *attr, char *buf)
 {

-- 
2.34.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume
  2023-06-02  7:30 [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume Mario Limonciello
                   ` (4 preceding siblings ...)
  2024-04-07 12:49 ` xiongxin
@ 2024-04-07 15:32 ` Randy Dunlap
  5 siblings, 0 replies; 11+ messages in thread
From: Randy Dunlap @ 2024-04-07 15:32 UTC (permalink / raw)
  To: xiongxin, mario.limonciello, Rafael Wysocki, hdegoede, linus.walleij
  Cc: Basavaraj.Natikar, Shyam-sundar.S-k, linux-pm, linux-acpi,
	linux-kernel, platform-driver-x86, linux-gpio



On 4/7/24 5:49 AM, xiongxin wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> All uses in the kernel are currently already oriented around
> suspend/resume. As some other parts of the kernel may also use these
> messages in functions that could also be used outside of
> suspend/resume, only enable in suspend/resume path.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v3->v4:
>  * add back do/while as it wasn't pointless.  It fixes a warning.
> ---
>  include/linux/suspend.h | 8 +++++---
>  kernel/power/main.c     | 6 ++++++
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 1a0426e6761c..74f406c53ac0 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -555,6 +555,7 @@ static inline void unlock_system_sleep(unsigned int flags) {}
>  #ifdef CONFIG_PM_SLEEP_DEBUG
>  extern bool pm_print_times_enabled;
>  extern bool pm_debug_messages_on;
> +extern bool pm_debug_messages_should_print(void);
>  static inline int pm_dyn_debug_messages_on(void)
>  {
>  #ifdef CONFIG_DYNAMIC_DEBUG
> @@ -568,14 +569,14 @@ static inline int pm_dyn_debug_messages_on(void)
>  #endif
>  #define __pm_pr_dbg(fmt, ...)					\
>  	do {							\
> -		if (pm_debug_messages_on)			\
> +		if (pm_debug_messages_should_print())		\
>  			printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
>  		else if (pm_dyn_debug_messages_on())		\
>  			pr_debug(fmt, ##__VA_ARGS__);	\
>  	} while (0)
>  #define __pm_deferred_pr_dbg(fmt, ...)				\
>  	do {							\
> -		if (pm_debug_messages_on)			\
> +		if (pm_debug_messages_should_print())		\
>  			printk_deferred(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
>  	} while (0)
>  #else
> @@ -593,7 +594,8 @@ static inline int pm_dyn_debug_messages_on(void)
>  /**
>   * pm_pr_dbg - print pm sleep debug messages
>   *
> - * If pm_debug_messages_on is enabled, print message.
> + * If pm_debug_messages_on is enabled and the system is entering/leaving
> + *      suspend, print message.
>   * If pm_debug_messages_on is disabled and CONFIG_DYNAMIC_DEBUG is enabled,
>   *	print message only from instances explicitly enabled on dynamic debug's
>   *	control.
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 3113ec2f1db4..daa535012e51 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -556,6 +556,12 @@ power_attr_ro(pm_wakeup_irq);
>  
>  bool pm_debug_messages_on __read_mostly;
>  
> +bool pm_debug_messages_should_print(void)
> +{
> +	return pm_debug_messages_on && pm_suspend_target_state != PM_SUSPEND_ON;
> 
>> hibernate processes also mostly use the pm_pr_dbg() function, which
>> results in hibernate processes only being able to output such logs
>> through dynamic debug, which is unfriendly to kernels without
>> CONFIG_DYNAMIC_DEBUG configuration.

This part of the patch doesn't look so good. ^^^^^^^^^^^^^^^^^^^^

> 
> +}
> +EXPORT_SYMBOL_GPL(pm_debug_messages_should_print);
> +
>  static ssize_t pm_debug_messages_show(struct kobject *kobj,
>  				      struct kobj_attribute *attr, char *buf)
>  {
> 

-- 
#Randy

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-04-07 15:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-02  7:30 [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume Mario Limonciello
2023-06-02  7:30 ` [PATCH v4 2/4] ACPI: x86: Add pm_debug_messages for LPS0 _DSM state tracking Mario Limonciello
2023-06-04 16:04   ` Rafael J. Wysocki
2023-06-12 17:58     ` Rafael J. Wysocki
2023-06-02  7:30 ` [PATCH v4 3/4] pinctrl: amd: Use pm_pr_dbg to show debugging messages Mario Limonciello
2023-06-09  7:20   ` Linus Walleij
2023-06-02  7:30 ` [PATCH v4 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for suspend related messages Mario Limonciello
2023-06-04 17:08   ` Hans de Goede
2024-04-07 12:39 ` [PATCH v4 1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume xiongxin
2024-04-07 12:49 ` xiongxin
2024-04-07 15:32 ` Randy Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).