* [PATCH 0/1] platform/x86: asus-wmi: fix build without CONFIG_SUSPEND @ 2025-05-23 13:14 Luke Jones 2025-05-23 13:14 ` [PATCH 1/1] " Luke Jones 0 siblings, 1 reply; 5+ messages in thread From: Luke Jones @ 2025-05-23 13:14 UTC (permalink / raw) To: linux-kernel Cc: hdegoede, ilpo.jarvinen, platform-driver-x86, mario.limonciello, andriy.shevchenko, Luke Jones Fixes "platform/x86: asus-wmi: Refactor Ally suspend/resume". Verified using the following details provided by test robot: tree: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git review-ilpo-next head: 83579675331059689e2869bf752ca9e17fadbd82 commit: feea7bd6b02d43a794e3f065650d89cf8d8e8e59 [74/89] platform/x86: asus-wmi: Refactor Ally suspend/resume config: x86_64-buildonly-randconfig-004-20250509 (https://download.01.org/0day-ci/archive/20250509/202505090418.DaeaXe4i-lkp@intel.com/config) compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250509/202505090418.DaeaXe4i-lkp@intel.com/reproduce) I've not done anything fancy here and left asus_hotk_prepare() as is, as it is *incredibly* unlikely anyone would ever run a ROG Ally without suspend enabled. Luke Jones (1): platform/x86: asus-wmi: fix build without CONFIG_SUSPEND drivers/platform/x86/asus-wmi.c | 9 +++++++++ 1 file changed, 9 insertions(+) -- 2.49.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] platform/x86: asus-wmi: fix build without CONFIG_SUSPEND 2025-05-23 13:14 [PATCH 0/1] platform/x86: asus-wmi: fix build without CONFIG_SUSPEND Luke Jones @ 2025-05-23 13:14 ` Luke Jones 2025-05-23 13:25 ` Ilpo Järvinen 0 siblings, 1 reply; 5+ messages in thread From: Luke Jones @ 2025-05-23 13:14 UTC (permalink / raw) To: linux-kernel Cc: hdegoede, ilpo.jarvinen, platform-driver-x86, mario.limonciello, andriy.shevchenko, Luke Jones, kernel test robot The patch "Refactor Ally suspend/resume" introduced an acpi_s2idle_dev_ops for use with ROG Ally which caused a build error if CONFIG_SUSPEND was not defined. Signed-off-by: Luke Jones <luke@ljones.dev> Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202505090418.DaeaXe4i-lkp@intel.com/ Fixes: feea7bd6b02d ("platform/x86: asus-wmi: Refactor Ally suspend/resume") --- drivers/platform/x86/asus-wmi.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 27f11643a00d..087318e0d595 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -5005,6 +5005,7 @@ static int asus_hotk_restore(struct device *device) return 0; } +#if defined(CONFIG_SUSPEND) static void asus_ally_s2idle_restore(void) { if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_ENABLED) { @@ -5013,6 +5014,7 @@ static void asus_ally_s2idle_restore(void) msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); } } +#endif /* CONFIG_SUSPEND */ static int asus_hotk_prepare(struct device *device) { @@ -5025,9 +5027,11 @@ static int asus_hotk_prepare(struct device *device) } /* Use only for Ally devices due to the wake_on_ac */ +#if defined(CONFIG_SUSPEND) static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = { .restore = asus_ally_s2idle_restore, }; +#endif /* CONFIG_SUSPEND */ static const struct dev_pm_ops asus_pm_ops = { .thaw = asus_hotk_thaw, @@ -5060,9 +5064,11 @@ static int asus_wmi_probe(struct platform_device *pdev) return ret; } + #if defined(CONFIG_SUSPEND) ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops); if (ret) pr_warn("failed to register LPS0 sleep handler in asus-wmi\n"); + #endif /* CONFIG_SUSPEND */ return asus_wmi_add(pdev); } @@ -5096,7 +5102,10 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver); void asus_wmi_unregister_driver(struct asus_wmi_driver *driver) { + #if defined(CONFIG_SUSPEND) acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops); + #endif /* CONFIG_SUSPEND */ + platform_device_unregister(driver->platform_device); platform_driver_unregister(&driver->platform_driver); used = false; -- 2.49.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] platform/x86: asus-wmi: fix build without CONFIG_SUSPEND 2025-05-23 13:14 ` [PATCH 1/1] " Luke Jones @ 2025-05-23 13:25 ` Ilpo Järvinen 2025-05-23 18:44 ` Luke Jones 0 siblings, 1 reply; 5+ messages in thread From: Ilpo Järvinen @ 2025-05-23 13:25 UTC (permalink / raw) To: Luke Jones Cc: LKML, Hans de Goede, platform-driver-x86, mario.limonciello, Andy Shevchenko, kernel test robot On Fri, 23 May 2025, Luke Jones wrote: > The patch "Refactor Ally suspend/resume" introduced an The commit feea7bd6b02d ("...") > acpi_s2idle_dev_ops for use with ROG Ally which caused a build error > if CONFIG_SUSPEND was not defined. > > Signed-off-by: Luke Jones <luke@ljones.dev> > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202505090418.DaeaXe4i-lkp@intel.com/ > Fixes: feea7bd6b02d ("platform/x86: asus-wmi: Refactor Ally suspend/resume") > --- > drivers/platform/x86/asus-wmi.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > index 27f11643a00d..087318e0d595 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -5005,6 +5005,7 @@ static int asus_hotk_restore(struct device *device) > return 0; > } > > +#if defined(CONFIG_SUSPEND) > static void asus_ally_s2idle_restore(void) > { > if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_ENABLED) { > @@ -5013,6 +5014,7 @@ static void asus_ally_s2idle_restore(void) > msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); > } > } > +#endif /* CONFIG_SUSPEND */ Move this function below asus_hotk_prepare() next to ops, so that only one #if block is needed for them. > > static int asus_hotk_prepare(struct device *device) > { > @@ -5025,9 +5027,11 @@ static int asus_hotk_prepare(struct device *device) > } > > /* Use only for Ally devices due to the wake_on_ac */ > +#if defined(CONFIG_SUSPEND) > static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = { > .restore = asus_ally_s2idle_restore, > }; > +#endif /* CONFIG_SUSPEND */ > > static const struct dev_pm_ops asus_pm_ops = { > .thaw = asus_hotk_thaw, > @@ -5060,9 +5064,11 @@ static int asus_wmi_probe(struct platform_device *pdev) > return ret; > } > > + #if defined(CONFIG_SUSPEND) > ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops); > if (ret) > pr_warn("failed to register LPS0 sleep handler in asus-wmi\n"); > + #endif /* CONFIG_SUSPEND */ > > return asus_wmi_add(pdev); > } > @@ -5096,7 +5102,10 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver); > > void asus_wmi_unregister_driver(struct asus_wmi_driver *driver) > { > + #if defined(CONFIG_SUSPEND) > acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops); > + #endif /* CONFIG_SUSPEND */ I'd have preferred these reg/unreg be solved with wrappers (see pmc_atom.c). -- i. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] platform/x86: asus-wmi: fix build without CONFIG_SUSPEND 2025-05-23 13:25 ` Ilpo Järvinen @ 2025-05-23 18:44 ` Luke Jones 2025-05-25 20:52 ` Ilpo Järvinen 0 siblings, 1 reply; 5+ messages in thread From: Luke Jones @ 2025-05-23 18:44 UTC (permalink / raw) To: Ilpo Järvinen Cc: LKML, Hans de Goede, platform-driver-x86, Mario Limonciello, Andy Shevchenko, kernel test robot On Fri, 23 May 2025, at 3:25 PM, Ilpo Järvinen wrote: > On Fri, 23 May 2025, Luke Jones wrote: > >> The patch "Refactor Ally suspend/resume" introduced an > > The commit feea7bd6b02d ("...") > >> acpi_s2idle_dev_ops for use with ROG Ally which caused a build error >> if CONFIG_SUSPEND was not defined. >> >> Signed-off-by: Luke Jones <luke@ljones.dev> >> Reported-by: kernel test robot <lkp@intel.com> >> Closes: https://lore.kernel.org/oe-kbuild-all/202505090418.DaeaXe4i-lkp@intel.com/ >> Fixes: feea7bd6b02d ("platform/x86: asus-wmi: Refactor Ally suspend/resume") >> --- >> drivers/platform/x86/asus-wmi.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c >> index 27f11643a00d..087318e0d595 100644 >> --- a/drivers/platform/x86/asus-wmi.c >> +++ b/drivers/platform/x86/asus-wmi.c >> @@ -5005,6 +5005,7 @@ static int asus_hotk_restore(struct device *device) >> return 0; >> } >> >> +#if defined(CONFIG_SUSPEND) >> static void asus_ally_s2idle_restore(void) >> { >> if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_ENABLED) { >> @@ -5013,6 +5014,7 @@ static void asus_ally_s2idle_restore(void) >> msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); >> } >> } >> +#endif /* CONFIG_SUSPEND */ > > Move this function below asus_hotk_prepare() next to ops, so that only one > #if block is needed for them. > Done >> >> static int asus_hotk_prepare(struct device *device) >> { >> @@ -5025,9 +5027,11 @@ static int asus_hotk_prepare(struct device *device) >> } >> >> /* Use only for Ally devices due to the wake_on_ac */ >> +#if defined(CONFIG_SUSPEND) >> static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = { >> .restore = asus_ally_s2idle_restore, >> }; >> +#endif /* CONFIG_SUSPEND */ >> >> static const struct dev_pm_ops asus_pm_ops = { >> .thaw = asus_hotk_thaw, >> @@ -5060,9 +5064,11 @@ static int asus_wmi_probe(struct platform_device *pdev) >> return ret; >> } >> >> + #if defined(CONFIG_SUSPEND) >> ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops); >> if (ret) >> pr_warn("failed to register LPS0 sleep handler in asus-wmi\n"); >> + #endif /* CONFIG_SUSPEND */ >> >> return asus_wmi_add(pdev); >> } >> @@ -5096,7 +5102,10 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver); >> >> void asus_wmi_unregister_driver(struct asus_wmi_driver *driver) >> { >> + #if defined(CONFIG_SUSPEND) >> acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops); >> + #endif /* CONFIG_SUSPEND */ > > I'd have preferred these reg/unreg be solved with wrappers (see > pmc_atom.c). Can do, but I don't really understand the difference it makes in this context. Should I log that CONFIG_SUSPEND was not enabled? Otherwise it's just as good as not there, right? > > > -- > i. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] platform/x86: asus-wmi: fix build without CONFIG_SUSPEND 2025-05-23 18:44 ` Luke Jones @ 2025-05-25 20:52 ` Ilpo Järvinen 0 siblings, 0 replies; 5+ messages in thread From: Ilpo Järvinen @ 2025-05-25 20:52 UTC (permalink / raw) To: Luke Jones Cc: LKML, Hans de Goede, platform-driver-x86, Mario Limonciello, Andy Shevchenko, kernel test robot [-- Attachment #1: Type: text/plain, Size: 3322 bytes --] On Fri, 23 May 2025, Luke Jones wrote: > On Fri, 23 May 2025, at 3:25 PM, Ilpo Järvinen wrote: > > On Fri, 23 May 2025, Luke Jones wrote: > > > >> The patch "Refactor Ally suspend/resume" introduced an > > > > The commit feea7bd6b02d ("...") > > > >> acpi_s2idle_dev_ops for use with ROG Ally which caused a build error > >> if CONFIG_SUSPEND was not defined. > >> > >> Signed-off-by: Luke Jones <luke@ljones.dev> > >> Reported-by: kernel test robot <lkp@intel.com> > >> Closes: https://lore.kernel.org/oe-kbuild-all/202505090418.DaeaXe4i-lkp@intel.com/ > >> Fixes: feea7bd6b02d ("platform/x86: asus-wmi: Refactor Ally suspend/resume") > >> --- > >> drivers/platform/x86/asus-wmi.c | 9 +++++++++ > >> 1 file changed, 9 insertions(+) > >> > >> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > >> index 27f11643a00d..087318e0d595 100644 > >> --- a/drivers/platform/x86/asus-wmi.c > >> +++ b/drivers/platform/x86/asus-wmi.c > >> @@ -5005,6 +5005,7 @@ static int asus_hotk_restore(struct device *device) > >> return 0; > >> } > >> > >> +#if defined(CONFIG_SUSPEND) > >> static void asus_ally_s2idle_restore(void) > >> { > >> if (use_ally_mcu_hack == ASUS_WMI_ALLY_MCU_HACK_ENABLED) { > >> @@ -5013,6 +5014,7 @@ static void asus_ally_s2idle_restore(void) > >> msleep(ASUS_USB0_PWR_EC0_CSEE_WAIT); > >> } > >> } > >> +#endif /* CONFIG_SUSPEND */ > > > > Move this function below asus_hotk_prepare() next to ops, so that only one > > #if block is needed for them. > > > > Done > > >> > >> static int asus_hotk_prepare(struct device *device) > >> { > >> @@ -5025,9 +5027,11 @@ static int asus_hotk_prepare(struct device *device) > >> } > >> > >> /* Use only for Ally devices due to the wake_on_ac */ > >> +#if defined(CONFIG_SUSPEND) > >> static struct acpi_s2idle_dev_ops asus_ally_s2idle_dev_ops = { > >> .restore = asus_ally_s2idle_restore, > >> }; > >> +#endif /* CONFIG_SUSPEND */ > >> > >> static const struct dev_pm_ops asus_pm_ops = { > >> .thaw = asus_hotk_thaw, > >> @@ -5060,9 +5064,11 @@ static int asus_wmi_probe(struct platform_device *pdev) > >> return ret; > >> } > >> > >> + #if defined(CONFIG_SUSPEND) > >> ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops); > >> if (ret) > >> pr_warn("failed to register LPS0 sleep handler in asus-wmi\n"); > >> + #endif /* CONFIG_SUSPEND */ > >> > >> return asus_wmi_add(pdev); > >> } > >> @@ -5096,7 +5102,10 @@ EXPORT_SYMBOL_GPL(asus_wmi_register_driver); > >> > >> void asus_wmi_unregister_driver(struct asus_wmi_driver *driver) > >> { > >> + #if defined(CONFIG_SUSPEND) > >> acpi_unregister_lps0_dev(&asus_ally_s2idle_dev_ops); > >> + #endif /* CONFIG_SUSPEND */ > > > > I'd have preferred these reg/unreg be solved with wrappers (see > > pmc_atom.c). > > Can do, but I don't really understand the difference it makes in this context. Should I log that CONFIG_SUSPEND was not enabled? Otherwise it's just as good as not there, right? I just want to limit the use of ifdeffery to minimum and prefer using #else + a stub over sprinkling ifdefs all over the place. And it's orthogonal whether the CONFIG_SUSPEND=n should print a warning or not. I'll take the v3. -- i. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-25 20:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-23 13:14 [PATCH 0/1] platform/x86: asus-wmi: fix build without CONFIG_SUSPEND Luke Jones 2025-05-23 13:14 ` [PATCH 1/1] " Luke Jones 2025-05-23 13:25 ` Ilpo Järvinen 2025-05-23 18:44 ` Luke Jones 2025-05-25 20:52 ` Ilpo Järvinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox