| Message ID | 20180328142020.3275989-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 28, 2018 at 04:19:29PM +0200, Arnd Bergmann wrote: > Two callsites of azx_suspend/azx_resume were removed, leaving these > functions only called from the optional SET_SYSTEM_SLEEP_PM_OPS() > and causing a warning without CONFIG_PM_SLEEP: > > sound/pci/hda/hda_intel.c:1029:12: error: 'azx_resume' defined but not used [-Werror=unused-function] > static int azx_resume(struct device *dev) > ^~~~~~~~~~ > sound/pci/hda/hda_intel.c:994:12: error: 'azx_suspend' defined but not used [-Werror=unused-function] > static int azx_suspend(struct device *dev) > ^~~~~~~~~~~ > > Keeping track of the correct #ifdef checks is hard, so this removes > all the #ifdefs for power management in this file and instead uses > __maybe_unused annotations that let the compiler do the job right > by itself. Ugh, this isn't as hard as it may seem, just replace #if defined(CONFIG_PM_SLEEP) || defined(SUPPORT_VGA_SWITCHEROO) with #if defined(CONFIG_PM_SLEEP) That way it's just a simple one line change which is less intrusive. Care to respin like this? Thanks for the report, Lukas
On Wed, 28 Mar 2018 16:31:50 +0200, Lukas Wunner wrote: > > On Wed, Mar 28, 2018 at 04:19:29PM +0200, Arnd Bergmann wrote: > > Two callsites of azx_suspend/azx_resume were removed, leaving these > > functions only called from the optional SET_SYSTEM_SLEEP_PM_OPS() > > and causing a warning without CONFIG_PM_SLEEP: > > > > sound/pci/hda/hda_intel.c:1029:12: error: 'azx_resume' defined but not used [-Werror=unused-function] > > static int azx_resume(struct device *dev) > > ^~~~~~~~~~ > > sound/pci/hda/hda_intel.c:994:12: error: 'azx_suspend' defined but not used [-Werror=unused-function] > > static int azx_suspend(struct device *dev) > > ^~~~~~~~~~~ > > > > Keeping track of the correct #ifdef checks is hard, so this removes > > all the #ifdefs for power management in this file and instead uses > > __maybe_unused annotations that let the compiler do the job right > > by itself. > > Ugh, this isn't as hard as it may seem, just replace > > #if defined(CONFIG_PM_SLEEP) || defined(SUPPORT_VGA_SWITCHEROO) > > with > > #if defined(CONFIG_PM_SLEEP) > > That way it's just a simple one line change which is less intrusive. > > Care to respin like this? > > Thanks for the report, > > Lukas Also, note that the mentioned vga_switcheroo change is carried via drm-misc tree, so the fix should go to there, too. thanks, Takashi
On Wed, Mar 28, 2018 at 4:31 PM, Lukas Wunner <lukas@wunner.de> wrote: > On Wed, Mar 28, 2018 at 04:19:29PM +0200, Arnd Bergmann wrote: >> Two callsites of azx_suspend/azx_resume were removed, leaving these >> functions only called from the optional SET_SYSTEM_SLEEP_PM_OPS() >> and causing a warning without CONFIG_PM_SLEEP: >> >> sound/pci/hda/hda_intel.c:1029:12: error: 'azx_resume' defined but not used [-Werror=unused-function] >> static int azx_resume(struct device *dev) >> ^~~~~~~~~~ >> sound/pci/hda/hda_intel.c:994:12: error: 'azx_suspend' defined but not used [-Werror=unused-function] >> static int azx_suspend(struct device *dev) >> ^~~~~~~~~~~ >> >> Keeping track of the correct #ifdef checks is hard, so this removes >> all the #ifdefs for power management in this file and instead uses >> __maybe_unused annotations that let the compiler do the job right >> by itself. > > Ugh, this isn't as hard as it may seem, just replace > > #if defined(CONFIG_PM_SLEEP) || defined(SUPPORT_VGA_SWITCHEROO) > > with > > #if defined(CONFIG_PM_SLEEP) > > That way it's just a simple one line change which is less intrusive. > > Care to respin like this? I won't be able to test that properly before the merge window. If you are sure that works, maybe you can send that patch and just mark it as 'Reported-by: Arnd Bergmann <arnd@arndb.de>'? I've mostly stopped trying to figure out what the correct set of #ifdef for power management functions is, since I get those wrong as much as everyone else. ;-). The patch I sent has been through a few days of randconfig testing. Arnd
On Thu, Mar 29, 2018 at 12:14:03AM +0200, Arnd Bergmann wrote: > On Wed, Mar 28, 2018 at 4:31 PM, Lukas Wunner <lukas@wunner.de> wrote: > > On Wed, Mar 28, 2018 at 04:19:29PM +0200, Arnd Bergmann wrote: > >> Two callsites of azx_suspend/azx_resume were removed, leaving these > >> functions only called from the optional SET_SYSTEM_SLEEP_PM_OPS() > >> and causing a warning without CONFIG_PM_SLEEP: > >> > >> sound/pci/hda/hda_intel.c:1029:12: error: 'azx_resume' defined but not used [-Werror=unused-function] > >> static int azx_resume(struct device *dev) > >> ^~~~~~~~~~ > >> sound/pci/hda/hda_intel.c:994:12: error: 'azx_suspend' defined but not used [-Werror=unused-function] > >> static int azx_suspend(struct device *dev) > >> ^~~~~~~~~~~ > >> > >> Keeping track of the correct #ifdef checks is hard, so this removes > >> all the #ifdefs for power management in this file and instead uses > >> __maybe_unused annotations that let the compiler do the job right > >> by itself. > > > > Ugh, this isn't as hard as it may seem, just replace > > > > #if defined(CONFIG_PM_SLEEP) || defined(SUPPORT_VGA_SWITCHEROO) > > > > with > > > > #if defined(CONFIG_PM_SLEEP) > > > > That way it's just a simple one line change which is less intrusive. > > > > Care to respin like this? > > I won't be able to test that properly before the merge window. If you > are sure that works, maybe you can send that patch and just mark > it as 'Reported-by: Arnd Bergmann <arnd@arndb.de>'? Okay, I've just sent out the patch. Could somebody ack this? I'll then be able to push it to drm-misc-next-fixes before the merge window opens. From that point on the issue will be gone in linux-next. The drm pull for 4.17 has already been sent out by Dave Airlie tonight, but he's indicated that he'll send another pull with fixes during the second half of the merge window. Once that lands, the issue will be fixed in Linus' tree as well. I've verified that with this patch, the build compiles cleanly using the drm-misc defconfigs for x86 and arm, and I've also verified that the compiler warning is present without the patch, and gone with the patch. Thanks again for the report. Lukas
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index 087c4d861c6e..01a96972d191 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -198,7 +198,9 @@ static bool power_save_controller = 1; module_param(power_save_controller, bool, 0644); MODULE_PARM_DESC(power_save_controller, "Reset controller in power save mode."); #else -#define power_save 0 +#define power_save 0 +#define power_save_controller 0 +#define pm_blacklist 0 #endif /* CONFIG_PM */ static int align_buffer_size = -1; @@ -941,13 +943,16 @@ static unsigned int azx_get_pos_skl(struct azx *chip, struct azx_dev *azx_dev) return azx_get_pos_posbuf(chip, azx_dev); } -#ifdef CONFIG_PM static DEFINE_MUTEX(card_list_lock); static LIST_HEAD(card_list); static void azx_add_card_list(struct azx *chip) { struct hda_intel *hda = container_of(chip, struct hda_intel, chip); + + if (!IS_ENABLED(CONFIG_PM)) + return; + mutex_lock(&card_list_lock); list_add(&hda->list, &card_list); mutex_unlock(&card_list_lock); @@ -956,13 +961,17 @@ static void azx_add_card_list(struct azx *chip) static void azx_del_card_list(struct azx *chip) { struct hda_intel *hda = container_of(chip, struct hda_intel, chip); + + if (!IS_ENABLED(CONFIG_PM)) + return; + mutex_lock(&card_list_lock); list_del_init(&hda->list); mutex_unlock(&card_list_lock); } /* trigger power-save check at writing parameter */ -static int param_set_xint(const char *val, const struct kernel_param *kp) +static int __maybe_unused param_set_xint(const char *val, const struct kernel_param *kp) { struct hda_intel *hda; struct azx *chip; @@ -982,16 +991,11 @@ static int param_set_xint(const char *val, const struct kernel_param *kp) mutex_unlock(&card_list_lock); return 0; } -#else -#define azx_add_card_list(chip) /* NOP */ -#define azx_del_card_list(chip) /* NOP */ -#endif /* CONFIG_PM */ -#if defined(CONFIG_PM_SLEEP) || defined(SUPPORT_VGA_SWITCHEROO) /* * power management */ -static int azx_suspend(struct device *dev) +static int __maybe_unused azx_suspend(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip; @@ -1026,7 +1030,7 @@ static int azx_suspend(struct device *dev) return 0; } -static int azx_resume(struct device *dev) +static int __maybe_unused azx_resume(struct device *dev) { struct pci_dev *pci = to_pci_dev(dev); struct snd_card *card = dev_get_drvdata(dev); @@ -1068,13 +1072,11 @@ static int azx_resume(struct device *dev) trace_azx_resume(chip); return 0; } -#endif /* CONFIG_PM_SLEEP || SUPPORT_VGA_SWITCHEROO */ -#ifdef CONFIG_PM_SLEEP /* put codec down to D3 at hibernation for Intel SKL+; * otherwise BIOS may still access the codec and screw up the driver */ -static int azx_freeze_noirq(struct device *dev) +static int __maybe_unused azx_freeze_noirq(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip = card->private_data; @@ -1086,7 +1088,7 @@ static int azx_freeze_noirq(struct device *dev) return 0; } -static int azx_thaw_noirq(struct device *dev) +static int __maybe_unused azx_thaw_noirq(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip = card->private_data; @@ -1097,10 +1099,8 @@ static int azx_thaw_noirq(struct device *dev) return 0; } -#endif /* CONFIG_PM_SLEEP */ -#ifdef CONFIG_PM -static int azx_runtime_suspend(struct device *dev) +static int __maybe_unused azx_runtime_suspend(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip; @@ -1132,7 +1132,7 @@ static int azx_runtime_suspend(struct device *dev) return 0; } -static int azx_runtime_resume(struct device *dev) +static int __maybe_unused azx_runtime_resume(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip; @@ -1185,7 +1185,7 @@ static int azx_runtime_resume(struct device *dev) return 0; } -static int azx_runtime_idle(struct device *dev) +static int __maybe_unused azx_runtime_idle(struct device *dev) { struct snd_card *card = dev_get_drvdata(dev); struct azx *chip; @@ -1215,12 +1215,6 @@ static const struct dev_pm_ops azx_pm = { SET_RUNTIME_PM_OPS(azx_runtime_suspend, azx_runtime_resume, azx_runtime_idle) }; -#define AZX_PM_OPS &azx_pm -#else -#define AZX_PM_OPS NULL -#endif /* CONFIG_PM */ - - static int azx_probe_continue(struct azx *chip); #ifdef SUPPORT_VGA_SWITCHEROO @@ -2199,7 +2193,6 @@ static int azx_probe(struct pci_dev *pci, return err; } -#ifdef CONFIG_PM /* On some boards setting power_save to a non 0 value leads to clicking / * popping sounds when ever we enter/leave powersaving mode. Ideally we would * figure out how to avoid these sounds, but that is not always feasible. @@ -2215,7 +2208,6 @@ static struct snd_pci_quirk power_save_blacklist[] = { SND_PCI_QUIRK(0x17aa, 0x2227, "Lenovo X1 Carbon 3rd Gen", 0), {} }; -#endif /* CONFIG_PM */ /* number of codec slots for each chipset: 0 = default slots (i.e. 4) */ static unsigned int azx_max_codecs[AZX_NUM_DRIVERS] = { @@ -2313,8 +2305,7 @@ static int azx_probe_continue(struct azx *chip) azx_add_card_list(chip); val = power_save; -#ifdef CONFIG_PM - if (pm_blacklist) { + if (IS_ENABLED(CONFIG_PM) && pm_blacklist) { const struct snd_pci_quirk *q; q = snd_pci_quirk_lookup(chip->pci, power_save_blacklist); @@ -2324,7 +2315,6 @@ static int azx_probe_continue(struct azx *chip) val = 0; } } -#endif /* CONFIG_PM */ /* * The discrete GPU cannot power down unless the HDA controller runtime @@ -2671,7 +2661,7 @@ static struct pci_driver azx_driver = { .remove = azx_remove, .shutdown = azx_shutdown, .driver = { - .pm = AZX_PM_OPS, + .pm = &azx_pm, }, }; diff --git a/sound/pci/hda/hda_intel_trace.h b/sound/pci/hda/hda_intel_trace.h index 73a7adfa192d..2775fa81a500 100644 --- a/sound/pci/hda/hda_intel_trace.h +++ b/sound/pci/hda/hda_intel_trace.h @@ -34,7 +34,6 @@ DEFINE_EVENT(hda_pm, azx_resume, TP_ARGS(chip) ); -#ifdef CONFIG_PM DEFINE_EVENT(hda_pm, azx_runtime_suspend, TP_PROTO(struct azx *chip), TP_ARGS(chip) @@ -44,7 +43,6 @@ DEFINE_EVENT(hda_pm, azx_runtime_resume, TP_PROTO(struct azx *chip), TP_ARGS(chip) ); -#endif #endif /* _TRACE_HDA_INTEL_H */
Two callsites of azx_suspend/azx_resume were removed, leaving these functions only called from the optional SET_SYSTEM_SLEEP_PM_OPS() and causing a warning without CONFIG_PM_SLEEP: sound/pci/hda/hda_intel.c:1029:12: error: 'azx_resume' defined but not used [-Werror=unused-function] static int azx_resume(struct device *dev) ^~~~~~~~~~ sound/pci/hda/hda_intel.c:994:12: error: 'azx_suspend' defined but not used [-Werror=unused-function] static int azx_suspend(struct device *dev) ^~~~~~~~~~~ Keeping track of the correct #ifdef checks is hard, so this removes all the #ifdefs for power management in this file and instead uses __maybe_unused annotations that let the compiler do the job right by itself. Fixes: 07f4f97d7b4b ("vga_switcheroo: Use device link for HDA controller") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- sound/pci/hda/hda_intel.c | 52 +++++++++++++++++------------------------ sound/pci/hda/hda_intel_trace.h | 2 -- 2 files changed, 21 insertions(+), 33 deletions(-)