Re: [PATCH] Revert "include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume"

[Mario Limonciello <mario.limonciello@amd.com>]

On 4/22/2024 10:18, Rafael J. Wysocki wrote:
> On Mon, Apr 22, 2024 at 5:02 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> On 4/22/2024 09:45, Rafael J. Wysocki wrote:
>>> On Mon, Apr 22, 2024 at 4:33 PM Mario Limonciello
>>> <mario.limonciello@amd.com> wrote:
>>>>
>>>> On 4/22/2024 04:36, xiongxin wrote:
>>>>> This reverts commit cdb8c100d8a4b4e31c829724e40b4fdf32977cce.
>>>>>
>>>>> In the suspend process, pm_pr_dbg() is called before setting
>>>>> pm_suspend_target_state. As a result, this part of the log cannot be
>>>>> output.
>>>>>
>>>>> pm_pr_dbg() also outputs debug logs for hibernate, but
>>>>> pm_suspend_target_state is not set, resulting in hibernate debug logs
>>>>> can only be output through dynamic debug, which is very inconvenient.
>>>>
>>>> As an alternative, how about exporting and renaming the variable
>>>> in_suspend in kernel/power/hibernate.c and considering that to tell if
>>>> the hibernate process is going on?
>>>>
>>>> Then it should work just the same as it does at suspend.
>>>
>>> Well, this is not the only part that stopped working AFAICS.  I'll
>>> queue up the revert.
>>
>> I just tested the revert to see what happens to other drivers but it's
>> going to have more collateral damage.
>>
>> ERROR: modpost: "pm_debug_messages_on"
>> [drivers/platform/x86/amd/pmc/amd-pmc.ko] undefined!
> 
> What about removing the "pm_suspend_target_state != PM_SUSPEND_ON"
> part from pm_debug_messages_should_print()?
> 
> This should be as good as the revert from the POV of restoring the
> previous functionality.

That would probably help this reported issue but it's going to be REALLY 
noisy for the pinctrl-amd driver for anyone that sets 
/sys/power/pm_debug_messages.

There is a message in that driver that is emitted whenever a GPIO is 
active and pm_debug_messages is set.

It's a really useful message for tracking down which GPIO woke the 
system up as the IRQ that is active is the GPIO controller master IRQ 
not an IRQ for the GPIO.

But if that change is made anyone who sets /sys/power/pm_debug_messages 
is going to see their kernel ring buffer flooded with every since 
interrupt associated with an I2C touchpad attention pin (for example).

So if the desire really is to back all this out, I think we need to also 
back out other users of pm_pr_dbg() too.