| Message ID | 20250410140921.849213-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: esdhc-imx: convert to modern PM_OPS | expand |
> -----Original Message----- > From: Arnd Bergmann <arnd@kernel.org> > Sent: 2025年4月10日 22:09 > To: Adrian Hunter <adrian.hunter@intel.com>; Bough Chen > <haibo.chen@nxp.com>; Ulf Hansson <ulf.hansson@linaro.org>; Shawn Guo > <shawnguo@kernel.org>; Sascha Hauer <s.hauer@pengutronix.de>; Luke > Wang <ziniu.wang_1@nxp.com> > Cc: Arnd Bergmann <arnd@arndb.de>; Pengutronix Kernel Team > <kernel@pengutronix.de>; Fabio Estevam <festevam@gmail.com>; Ciprian > Marian Costea (OSS) <ciprianmarian.costea@oss.nxp.com>; Josua Mayer > <josua@solid-run.com>; imx@lists.linux.dev; linux-mmc@vger.kernel.org; > dl-S32 <S32@nxp.com>; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org > Subject: [PATCH] mmc: esdhc-imx: convert to modern PM_OPS > > From: Arnd Bergmann <arnd@arndb.de> > > Two newly added functions are unused in configurations without power > management support: > > drivers/mmc/host/sdhci-esdhc-imx.c:1586:13: error: unused function > 'sdhc_esdhc_tuning_save' [-Werror,-Wunused-function] > 1586 | static void sdhc_esdhc_tuning_save(struct sdhci_host *host) > | ^~~~~~~~~~~~~~~~~~~~~~ > drivers/mmc/host/sdhci-esdhc-imx.c:1608:13: error: unused function > 'sdhc_esdhc_tuning_restore' [-Werror,-Wunused-function] > 1608 | static void sdhc_esdhc_tuning_restore(struct sdhci_host *host) > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > > Remove the #ifdef checks and instead use the better macros that silently drop > the unused functions when PM is disabled. Hi Arnd, Thanks for this catching. How about add __maybe_unused to define this tuning_save/retore function? Regards Haibo Chen > > Fixes: 3d1eea493894 ("mmc: sdhci-esdhc-imx: Save tuning value when card > stays powered in suspend") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > b/drivers/mmc/host/sdhci-esdhc-imx.c > index 7e8addaed697..8742622da55a 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -1942,7 +1942,6 @@ static void sdhci_esdhc_imx_remove(struct > platform_device *pdev) > sdhci_pltfm_free(pdev); > } > > -#ifdef CONFIG_PM_SLEEP > static int sdhci_esdhc_suspend(struct device *dev) { > struct sdhci_host *host = dev_get_drvdata(dev); @@ -2021,9 +2020,7 @@ > static int sdhci_esdhc_resume(struct device *dev) > > return ret; > } > -#endif > > -#ifdef CONFIG_PM > static int sdhci_esdhc_runtime_suspend(struct device *dev) { > struct sdhci_host *host = dev_get_drvdata(dev); @@ -2103,11 +2100,10 > @@ static int sdhci_esdhc_runtime_resume(struct device *dev) > cpu_latency_qos_remove_request(&imx_data->pm_qos_req); > return err; > } > -#endif > > static const struct dev_pm_ops sdhci_esdhc_pmops = { > - SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume) > - SET_RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend, > + SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume) > + RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend, > sdhci_esdhc_runtime_resume, NULL) > }; > > -- > 2.39.5
On Fri, Apr 11, 2025, at 04:45, Bough Chen wrote: >> From: Arnd Bergmann <arnd@kernel.org> >> drivers/mmc/host/sdhci-esdhc-imx.c:1586:13: error: unused function >> 'sdhc_esdhc_tuning_save' [-Werror,-Wunused-function] >> 1586 | static void sdhc_esdhc_tuning_save(struct sdhci_host *host) >> | ^~~~~~~~~~~~~~~~~~~~~~ >> drivers/mmc/host/sdhci-esdhc-imx.c:1608:13: error: unused function >> 'sdhc_esdhc_tuning_restore' [-Werror,-Wunused-function] >> 1608 | static void sdhc_esdhc_tuning_restore(struct sdhci_host *host) >> | ^~~~~~~~~~~~~~~~~~~~~~~~~ >> >> Remove the #ifdef checks and instead use the better macros that silently drop >> the unused functions when PM is disabled. > > Thanks for this catching. > > How about add __maybe_unused to define this tuning_save/retore function? That sounds counterproductive. The new macros were introduced to allow removing all the pointless __maybe_unused annotations, I see no reason to stick with the legacy helpers here. I did make a small mistake and ran into another warning later, but this fixup should address that: --- a/drivers/mmc/host/sdhci-esdhc-imx.c +++ b/drivers/mmc/host/sdhci-esdhc-imx.c @@ -2112,7 +2112,7 @@ static struct platform_driver sdhci_esdhc_imx_driver = { .name = "sdhci-esdhc-imx", .probe_type = PROBE_PREFER_ASYNCHRONOUS, .of_match_table = imx_esdhc_dt_ids, - .pm = &sdhci_esdhc_pmops, + .pm = pm_ptr(&sdhci_esdhc_pmops), }, .probe = sdhci_esdhc_imx_probe, .remove = sdhci_esdhc_imx_remove, Arnd
On 11/04/25 08:45, Arnd Bergmann wrote: > On Fri, Apr 11, 2025, at 04:45, Bough Chen wrote: >>> From: Arnd Bergmann <arnd@kernel.org> >>> drivers/mmc/host/sdhci-esdhc-imx.c:1586:13: error: unused function >>> 'sdhc_esdhc_tuning_save' [-Werror,-Wunused-function] >>> 1586 | static void sdhc_esdhc_tuning_save(struct sdhci_host *host) >>> | ^~~~~~~~~~~~~~~~~~~~~~ >>> drivers/mmc/host/sdhci-esdhc-imx.c:1608:13: error: unused function >>> 'sdhc_esdhc_tuning_restore' [-Werror,-Wunused-function] >>> 1608 | static void sdhc_esdhc_tuning_restore(struct sdhci_host *host) >>> | ^~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> Remove the #ifdef checks and instead use the better macros that silently drop >>> the unused functions when PM is disabled. >> >> Thanks for this catching. >> >> How about add __maybe_unused to define this tuning_save/retore function? > > That sounds counterproductive. > > The new macros were introduced to allow removing all the pointless > __maybe_unused annotations, I see no reason to stick with the > legacy helpers here. > > I did make a small mistake and ran into another warning later, > but this fixup should address that: > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -2112,7 +2112,7 @@ static struct platform_driver sdhci_esdhc_imx_driver = { > .name = "sdhci-esdhc-imx", > .probe_type = PROBE_PREFER_ASYNCHRONOUS, > .of_match_table = imx_esdhc_dt_ids, > - .pm = &sdhci_esdhc_pmops, > + .pm = pm_ptr(&sdhci_esdhc_pmops), > }, > .probe = sdhci_esdhc_imx_probe, > .remove = sdhci_esdhc_imx_remove, > Can someone provide a Tested-by?
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c index 7e8addaed697..8742622da55a 100644 --- a/drivers/mmc/host/sdhci-esdhc-imx.c +++ b/drivers/mmc/host/sdhci-esdhc-imx.c @@ -1942,7 +1942,6 @@ static void sdhci_esdhc_imx_remove(struct platform_device *pdev) sdhci_pltfm_free(pdev); } -#ifdef CONFIG_PM_SLEEP static int sdhci_esdhc_suspend(struct device *dev) { struct sdhci_host *host = dev_get_drvdata(dev); @@ -2021,9 +2020,7 @@ static int sdhci_esdhc_resume(struct device *dev) return ret; } -#endif -#ifdef CONFIG_PM static int sdhci_esdhc_runtime_suspend(struct device *dev) { struct sdhci_host *host = dev_get_drvdata(dev); @@ -2103,11 +2100,10 @@ static int sdhci_esdhc_runtime_resume(struct device *dev) cpu_latency_qos_remove_request(&imx_data->pm_qos_req); return err; } -#endif static const struct dev_pm_ops sdhci_esdhc_pmops = { - SET_SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume) - SET_RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend, + SYSTEM_SLEEP_PM_OPS(sdhci_esdhc_suspend, sdhci_esdhc_resume) + RUNTIME_PM_OPS(sdhci_esdhc_runtime_suspend, sdhci_esdhc_runtime_resume, NULL) };