| Message ID | 20230522200033.2605-4-mario.limonciello@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/4] include/linux/suspend.h: Only show pm_pr_dbg messages at suspend/resume | expand |
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
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 --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; }
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(-)