diff mbox series

[v2,4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for suspend related messages

Message ID 20230522200033.2605-4-mario.limonciello@amd.com
State New
Headers
Series [v2,1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume |

Commit Message

Mario Limonciello May 22, 2023, 8 p.m. UTC 
  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(-)

Comments

Hans de Goede May 23, 2023, 11:07 a.m. UTC | #1 
Hi Mario,

On 5/22/23 22:00, 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>
> ---
>  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);

This does not compile, amd/pmc.c may be build as an amd-pmc.ko module
and currently the pm_debug_messages_on flag used by pm_pr_dbg()
is not exported to modules:

  CC [M]  drivers/platform/x86/amd/pmc.o
  LD [M]  drivers/platform/x86/amd/amd-pmc.o
  MODPOST Module.symvers
ERROR: modpost: "pm_debug_messages_on" [drivers/platform/x86/amd/amd-pmc.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1
make: *** [Makefile:1978: modpost] Error 2

Regards,

Hans
Mario Limonciello May 23, 2023, 4:21 p.m. UTC | #2 
[AMD Official Use Only - General]

> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Tuesday, May 23, 2023 6:08 AM
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; rafael@kernel.org;
> linus.walleij@linaro.org
> Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> gpio@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-
> pm@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>;
> Natikar, Basavaraj <Basavaraj.Natikar@amd.com>
> Subject: Re: [PATCH v2 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for
> suspend related messages
>
> Hi Mario,
>
> On 5/22/23 22:00, 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>
> > ---
> >  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);
>
> This does not compile, amd/pmc.c may be build as an amd-pmc.ko module
> and currently the pm_debug_messages_on flag used by pm_pr_dbg()
> is not exported to modules:
>
>   CC [M]  drivers/platform/x86/amd/pmc.o
>   LD [M]  drivers/platform/x86/amd/amd-pmc.o
>   MODPOST Module.symvers
> ERROR: modpost: "pm_debug_messages_on"
> [drivers/platform/x86/amd/amd-pmc.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1
> make: *** [Makefile:1978: modpost] Error 2
>
> Regards,
>
> Hans
>

My apologies, yes I was compiling in when testing.  Let me ask if this
series makes sense and is "generally" agreeable though.

If it is I'll adjust it so that exports to modules.  If the preference is
to keep /sys/power/pm_debug_messages only for core PM stuff
then I'll just send the one patch improvement for s2idle.c logging.
Andy Shevchenko May 23, 2023, 4:56 p.m. UTC | #3 
Mon, May 22, 2023 at 03:00:33PM -0500, Mario Limonciello kirjoitti:
> 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.

It has the same two regressions as I pointed out in previous reply.
Hans de Goede May 25, 2023, 10:13 a.m. UTC | #4 
Hi Mario,

On 5/23/23 18:21, Limonciello, Mario wrote:
> [AMD Official Use Only - General]
> 
>> -----Original Message-----
>> From: Hans de Goede <hdegoede@redhat.com>
>> Sent: Tuesday, May 23, 2023 6:08 AM
>> To: Limonciello, Mario <Mario.Limonciello@amd.com>; rafael@kernel.org;
>> linus.walleij@linaro.org
>> Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
>> gpio@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-
>> pm@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>;
>> Natikar, Basavaraj <Basavaraj.Natikar@amd.com>
>> Subject: Re: [PATCH v2 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for
>> suspend related messages
>>
>> Hi Mario,
>>
>> On 5/22/23 22:00, 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>
>>> ---
>>>  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);
>>
>> This does not compile, amd/pmc.c may be build as an amd-pmc.ko module
>> and currently the pm_debug_messages_on flag used by pm_pr_dbg()
>> is not exported to modules:
>>
>>   CC [M]  drivers/platform/x86/amd/pmc.o
>>   LD [M]  drivers/platform/x86/amd/amd-pmc.o
>>   MODPOST Module.symvers
>> ERROR: modpost: "pm_debug_messages_on"
>> [drivers/platform/x86/amd/amd-pmc.ko] undefined!
>> make[1]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1
>> make: *** [Makefile:1978: modpost] Error 2
>>
>> Regards,
>>
>> Hans
>>
> 
> My apologies, yes I was compiling in when testing.  Let me ask if this
> series makes sense and is "generally" agreeable though.

I have no objections against this series, otherwise I don't really
have a strong opinion on this series.

If this makes sense and if exporting pm_debug_messages_on is ok
is Rafael's call to make IMHO.

Rafael ?

Regards,

Hans
Rafael J. Wysocki May 25, 2023, 12:06 p.m. UTC | #5 
On Thu, May 25, 2023 at 12:13 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Mario,
>
> On 5/23/23 18:21, Limonciello, Mario wrote:
> > [AMD Official Use Only - General]
> >
> >> -----Original Message-----
> >> From: Hans de Goede <hdegoede@redhat.com>
> >> Sent: Tuesday, May 23, 2023 6:08 AM
> >> To: Limonciello, Mario <Mario.Limonciello@amd.com>; rafael@kernel.org;
> >> linus.walleij@linaro.org
> >> Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> >> gpio@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-
> >> pm@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>;
> >> Natikar, Basavaraj <Basavaraj.Natikar@amd.com>
> >> Subject: Re: [PATCH v2 4/4] platform/x86/amd: pmc: Use pm_pr_dbg() for
> >> suspend related messages
> >>
> >> Hi Mario,
> >>
> >> On 5/22/23 22:00, 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>
> >>> ---
> >>>  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);
> >>
> >> This does not compile, amd/pmc.c may be build as an amd-pmc.ko module
> >> and currently the pm_debug_messages_on flag used by pm_pr_dbg()
> >> is not exported to modules:
> >>
> >>   CC [M]  drivers/platform/x86/amd/pmc.o
> >>   LD [M]  drivers/platform/x86/amd/amd-pmc.o
> >>   MODPOST Module.symvers
> >> ERROR: modpost: "pm_debug_messages_on"
> >> [drivers/platform/x86/amd/amd-pmc.ko] undefined!
> >> make[1]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1
> >> make: *** [Makefile:1978: modpost] Error 2
> >>
> >> Regards,
> >>
> >> Hans
> >>
> >
> > My apologies, yes I was compiling in when testing.  Let me ask if this
> > series makes sense and is "generally" agreeable though.
>
> I have no objections against this series, otherwise I don't really
> have a strong opinion on this series.
>
> If this makes sense and if exporting pm_debug_messages_on is ok
> is Rafael's call to make IMHO.
>
> Rafael ?

I have no strong opinion.

I would do it slightly differently as mentioned in my reply to patch
[1/4] (and then the new function could be used in patch [2/4] I
think).

Otherwise this is fine with me if it helps to debug failures in the field.
diff mbox series

Patch

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;
 }