Re: [PATCH] PM: sleep: Optimize the pm_debug_messages_should_print() function

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

>>> Can I change pm_pr_dbg() in amd_pmc_idlemask_read() to pr_debug() 
>>> based on
>>>
>>> pm_debug_messages_on condition?
>>>
>>> I suggest not adding a new variable to this.
>>>
>>
>> I don't understand the opposition to the new variable.
>>
>> The whole point of /sys/power/pm_debug_messages is so that it's a one 
>> stop shop to turn on power management related debugging at power state 
>> but nothing more.
>>
>> You turn that on and you can get messages from the core and also any 
>> drivers that want to emit messages during that time.
>>
>> If changing drivers back to pr_debug that means that users and 
>> software need to manually turn on BOTH /sys/power/pm_debug_messages as 
>> well as dynamic debug for any power management related messages.
>>
>> Whereas if just adding another variable for a condition then just turn 
>> on the sysfs file for any hibernate or suspend debugging.
> 
> Your patch makes the output of pm_pr_dbg() based on the values of 
> pm_debug_messages_on and pm_suspend_target_state; However, 
> pm_suspend_target_state's impact domain does not include enter_state() 
> and hibernate processes;
> 
> The patch affects the output of the sleep mainline debug log, which is 
> very unfriendly to others developers, and it is even more troublesome
> to add a new variable based on your suggestion.

Why is adding a new variable more troublesome?  We're talking about a 
one line change and then it can run in more power management situations.

> 
> The kernel already has a log output solution based on the value of 
> pm_suspend_target_state. I will issue a repair patch as follows in
> amd_pmc_idlemask_read():
> 
> if (dev && pm_suspend_target_state != PM_SUSPEND_ON)
>      pr_info("SMU idlemask s0i3: 0x%x\n", val);

But then this is going to be really noisy still for the general purpose 
users.

The point of pm_pr_dbg() is that it only outputs the debugging message 
when /sys/power/pm_debug_messages is set.

99% of people don't need this message, but when someone comes to say "it 
doesn't work!" changing one sysfs file gets me a lot more data about 
/why/ it doesn't work without compromising everyone else's logs.