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