| Message ID | 20241101101441.3518612-2-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: sdhci-acpi: A few cleanups | expand |
On 1/11/24 12:11, Andy Shevchenko wrote: > Switch from ugly ifdeffery to using EXPORT_PM_FN_NS_GPL() > for exporting PM functions. This helps cleaning up the other > SDHCI drivers in the future. It seems sdhci is the first code in the kernel to use EXPORT_PM_FN_NS_GPL() but it was not asked for ;-) As such, can you fill in a little background. I am not sure what it achieves. Why have CONFIG_PM if not to #ifdef dependent code behind it? > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/mmc/host/sdhci.c | 14 ++++---------- > drivers/mmc/host/sdhci.h | 2 -- > 2 files changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index f4a7733a8ad2..2214280ca5fb 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -3733,8 +3733,6 @@ EXPORT_SYMBOL_GPL(sdhci_thread_irq); > * * > \*****************************************************************************/ > > -#ifdef CONFIG_PM > - > static bool sdhci_cd_irq_can_wakeup(struct sdhci_host *host) > { > return mmc_card_is_removable(host->mmc) && > @@ -3814,8 +3812,7 @@ int sdhci_suspend_host(struct sdhci_host *host) > > return 0; > } > - > -EXPORT_SYMBOL_GPL(sdhci_suspend_host); > +EXPORT_PM_FN_GPL(sdhci_suspend_host); > > int sdhci_resume_host(struct sdhci_host *host) > { > @@ -3853,8 +3850,7 @@ int sdhci_resume_host(struct sdhci_host *host) > > return ret; > } > - > -EXPORT_SYMBOL_GPL(sdhci_resume_host); > +EXPORT_PM_FN_GPL(sdhci_resume_host); > > int sdhci_runtime_suspend_host(struct sdhci_host *host) > { > @@ -3876,7 +3872,7 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host) > > return 0; > } > -EXPORT_SYMBOL_GPL(sdhci_runtime_suspend_host); > +EXPORT_PM_FN_GPL(sdhci_runtime_suspend_host); > > int sdhci_runtime_resume_host(struct sdhci_host *host, int soft_reset) > { > @@ -3927,9 +3923,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host, int soft_reset) > > return 0; > } > -EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host); > - > -#endif /* CONFIG_PM */ > +EXPORT_PM_FN_GPL(sdhci_runtime_resume_host); > > /*****************************************************************************\ > * * > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index cd0e35a80542..4ee2695b0202 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -874,12 +874,10 @@ irqreturn_t sdhci_thread_irq(int irq, void *dev_id); > void sdhci_adma_write_desc(struct sdhci_host *host, void **desc, > dma_addr_t addr, int len, unsigned int cmd); > > -#ifdef CONFIG_PM > int sdhci_suspend_host(struct sdhci_host *host); > int sdhci_resume_host(struct sdhci_host *host); > int sdhci_runtime_suspend_host(struct sdhci_host *host); > int sdhci_runtime_resume_host(struct sdhci_host *host, int soft_reset); > -#endif > > void sdhci_cqe_enable(struct mmc_host *mmc); > void sdhci_cqe_disable(struct mmc_host *mmc, bool recovery);
On Mon, Dec 09, 2024 at 12:38:59PM +0200, Adrian Hunter wrote: > On 1/11/24 12:11, Andy Shevchenko wrote: > > Switch from ugly ifdeffery to using EXPORT_PM_FN_NS_GPL() > > for exporting PM functions. This helps cleaning up the other > > SDHCI drivers in the future. > > It seems sdhci is the first code in the kernel to use > EXPORT_PM_FN_NS_GPL() but it was not asked for ;-) > > As such, can you fill in a little background. I am not > sure what it achieves. Why have CONFIG_PM if not to > #ifdef dependent code behind it? It makes sure that the code elimination happens at compile time and at the same time gives developer less uglified (by ifdeffery) code. It means there is less risk to miss anything of that which make become a compile-time warning of unused function, or even issues during linking with modules, etc. Should I update a commit message with that?
On 9/12/24 18:36, Andy Shevchenko wrote: > On Mon, Dec 09, 2024 at 12:38:59PM +0200, Adrian Hunter wrote: >> On 1/11/24 12:11, Andy Shevchenko wrote: >>> Switch from ugly ifdeffery to using EXPORT_PM_FN_NS_GPL() >>> for exporting PM functions. This helps cleaning up the other >>> SDHCI drivers in the future. >> >> It seems sdhci is the first code in the kernel to use >> EXPORT_PM_FN_NS_GPL() but it was not asked for ;-) >> >> As such, can you fill in a little background. I am not >> sure what it achieves. Why have CONFIG_PM if not to >> #ifdef dependent code behind it? > > It makes sure that the code elimination happens at compile time and Does it eliminate the code? Maybe I am missing something, but it looks like it is still there: $ grep CONFIG_PM .config # CONFIG_PM is not set # CONFIG_PMIC_OPREGION is not set # CONFIG_PMBUS is not set # CONFIG_PMIC_ADP5520 is not set # CONFIG_PMIC_DA903X is not set CONFIG_PM_DEVFREQ=y # CONFIG_PM_DEVFREQ_EVENT is not set CONFIG_PM_OPP=y $ objdump -d drivers/mmc/host/sdhci.ko | grep sdhci_suspend_host 00000000000089f0 <__pfx_sdhci_suspend_host>: 0000000000008a00 <sdhci_suspend_host>: 8a16: e8 00 00 00 00 call 8a1b <sdhci_suspend_host+0x1b> 8a29: 74 0c je 8a37 <sdhci_suspend_host+0x37> 8a35: 75 54 jne 8a8b <sdhci_suspend_host+0x8b> 8a4c: 0f 85 06 01 00 00 jne 8b58 <sdhci_suspend_host+0x158> 8a66: 0f 85 00 01 00 00 jne 8b6c <sdhci_suspend_host+0x16c> 8a7b: e8 00 00 00 00 call 8a80 <sdhci_suspend_host+0x80> 8a86: e9 00 00 00 00 jmp 8a8b <sdhci_suspend_host+0x8b> 8a92: 75 0a jne 8a9e <sdhci_suspend_host+0x9e> 8a98: 0f 84 87 00 00 00 je 8b25 <sdhci_suspend_host+0x125> 8aa5: 74 90 je 8a37 <sdhci_suspend_host+0x37> 8ab8: 0f 85 f5 00 00 00 jne 8bb3 <sdhci_suspend_host+0x1b3> 8ad8: 0f 85 c0 00 00 00 jne 8b9e <sdhci_suspend_host+0x19e> 8af0: 0f 85 93 00 00 00 jne 8b89 <sdhci_suspend_host+0x189> 8b06: e8 00 00 00 00 call 8b0b <sdhci_suspend_host+0x10b> 8b14: 0f 85 1d ff ff ff jne 8a37 <sdhci_suspend_host+0x37> 8b20: e9 00 00 00 00 jmp 8b25 <sdhci_suspend_host+0x125> 8b25: e8 00 00 00 00 call 8b2a <sdhci_suspend_host+0x12a> 8b2c: 75 52 jne 8b80 <sdhci_suspend_host+0x180> 8b53: e9 55 ff ff ff jmp 8aad <sdhci_suspend_host+0xad> 8b62: e8 00 00 00 00 call 8b67 <sdhci_suspend_host+0x167> 8b67: e9 ef fe ff ff jmp 8a5b <sdhci_suspend_host+0x5b> 8b76: e8 00 00 00 00 call 8b7b <sdhci_suspend_host+0x17b> 8b7b: e9 f5 fe ff ff jmp 8a75 <sdhci_suspend_host+0x75> 8b84: e9 15 ff ff ff jmp 8a9e <sdhci_suspend_host+0x9e> 8b94: e8 00 00 00 00 call 8b99 <sdhci_suspend_host+0x199> 8b99: e9 60 ff ff ff jmp 8afe <sdhci_suspend_host+0xfe> 8ba9: e8 00 00 00 00 call 8bae <sdhci_suspend_host+0x1ae> 8bae: e9 32 ff ff ff jmp 8ae5 <sdhci_suspend_host+0xe5> 8bbb: e8 00 00 00 00 call 8bc0 <sdhci_suspend_host+0x1c0> 8bc0: e9 03 ff ff ff jmp 8ac8 <sdhci_suspend_host+0xc8> $ > at the same time gives developer less uglified (by ifdeffery) code. > It means there is less risk to miss anything of that which make become > a compile-time warning of unused function, or even issues during linking > with modules, etc. > > Should I update a commit message with that?
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index f4a7733a8ad2..2214280ca5fb 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -3733,8 +3733,6 @@ EXPORT_SYMBOL_GPL(sdhci_thread_irq); * * \*****************************************************************************/ -#ifdef CONFIG_PM - static bool sdhci_cd_irq_can_wakeup(struct sdhci_host *host) { return mmc_card_is_removable(host->mmc) && @@ -3814,8 +3812,7 @@ int sdhci_suspend_host(struct sdhci_host *host) return 0; } - -EXPORT_SYMBOL_GPL(sdhci_suspend_host); +EXPORT_PM_FN_GPL(sdhci_suspend_host); int sdhci_resume_host(struct sdhci_host *host) { @@ -3853,8 +3850,7 @@ int sdhci_resume_host(struct sdhci_host *host) return ret; } - -EXPORT_SYMBOL_GPL(sdhci_resume_host); +EXPORT_PM_FN_GPL(sdhci_resume_host); int sdhci_runtime_suspend_host(struct sdhci_host *host) { @@ -3876,7 +3872,7 @@ int sdhci_runtime_suspend_host(struct sdhci_host *host) return 0; } -EXPORT_SYMBOL_GPL(sdhci_runtime_suspend_host); +EXPORT_PM_FN_GPL(sdhci_runtime_suspend_host); int sdhci_runtime_resume_host(struct sdhci_host *host, int soft_reset) { @@ -3927,9 +3923,7 @@ int sdhci_runtime_resume_host(struct sdhci_host *host, int soft_reset) return 0; } -EXPORT_SYMBOL_GPL(sdhci_runtime_resume_host); - -#endif /* CONFIG_PM */ +EXPORT_PM_FN_GPL(sdhci_runtime_resume_host); /*****************************************************************************\ * * diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index cd0e35a80542..4ee2695b0202 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -874,12 +874,10 @@ irqreturn_t sdhci_thread_irq(int irq, void *dev_id); void sdhci_adma_write_desc(struct sdhci_host *host, void **desc, dma_addr_t addr, int len, unsigned int cmd); -#ifdef CONFIG_PM int sdhci_suspend_host(struct sdhci_host *host); int sdhci_resume_host(struct sdhci_host *host); int sdhci_runtime_suspend_host(struct sdhci_host *host); int sdhci_runtime_resume_host(struct sdhci_host *host, int soft_reset); -#endif void sdhci_cqe_enable(struct mmc_host *mmc); void sdhci_cqe_disable(struct mmc_host *mmc, bool recovery);
Switch from ugly ifdeffery to using EXPORT_PM_FN_NS_GPL() for exporting PM functions. This helps cleaning up the other SDHCI drivers in the future. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/mmc/host/sdhci.c | 14 ++++---------- drivers/mmc/host/sdhci.h | 2 -- 2 files changed, 4 insertions(+), 12 deletions(-)