| Message ID | 20220704025632.235968-1-renzhijie2@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] scsi: ufs: ufs-mediatek: Fix build error | expand |
Hi, Just friendly ping... 在 2022/7/4 10:56, Ren Zhijie 写道: > If CONFIG_PM is not set, > make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-, will be failed, like this: > drivers/ufs/host/ufs-mediatek.c: In function ‘ufs_mtk_runtime_suspend’: > drivers/ufs/host/ufs-mediatek.c:1435:8: error: implicit declaration of function ‘ufshcd_runtime_suspend’; did you mean ‘ufs_mtk_runtime_suspend’? [-Werror=implicit-function-declaration] > ret = ufshcd_runtime_suspend(dev); > ^~~~~~~~~~~~~~~~~~~~~~ > ufs_mtk_runtime_suspend > drivers/ufs/host/ufs-mediatek.c: In function ‘ufs_mtk_runtime_resume’: > drivers/ufs/host/ufs-mediatek.c:1450:9: error: implicit declaration of function ‘ufshcd_runtime_resume’; did you mean ‘ufs_mtk_runtime_resume’? [-Werror=implicit-function-declaration] > return ufshcd_runtime_resume(dev); > ^~~~~~~~~~~~~~~~~~~~~ > ufs_mtk_runtime_resume > At top level: > drivers/ufs/host/ufs-mediatek.c:1444:12: error: ‘ufs_mtk_runtime_resume’ defined but not used [-Werror=unused-function] > static int ufs_mtk_runtime_resume(struct device *dev) > ^~~~~~~~~~~~~~~~~~~~~~ > drivers/ufs/host/ufs-mediatek.c:1430:12: error: ‘ufs_mtk_runtime_suspend’ defined but not used [-Werror=unused-function] > static int ufs_mtk_runtime_suspend(struct device *dev) > ^~~~~~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > > The declarations of func "ufshcd_runtime_suspend()" and "ufshcd_runtime_resume()" depended on CONFIG_PM, so the function wrappers "ufs_mtk_runtime_suspend()" and "ufs_mtk_runtime_resume()" both should warpped by CONFIG_PM too. > > Reported-by: Hulk Robot <hulkci@huawei.com> > Fixes: e7bf1d50063c ("scsi: ufs: ufs-mediatek: Fix build warnings") > Signed-off-by: Ren Zhijie <renzhijie2@huawei.com> > --- > drivers/ufs/host/ufs-mediatek.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c > index c958279bdd8f..e006c2528a3a 100644 > --- a/drivers/ufs/host/ufs-mediatek.c > +++ b/drivers/ufs/host/ufs-mediatek.c > @@ -1427,6 +1427,7 @@ static int ufs_mtk_system_resume(struct device *dev) > } > #endif > > +#ifdef CONFIG_PM > static int ufs_mtk_runtime_suspend(struct device *dev) > { > struct ufs_hba *hba = dev_get_drvdata(dev); > @@ -1449,6 +1450,7 @@ static int ufs_mtk_runtime_resume(struct device *dev) > > return ufshcd_runtime_resume(dev); > } > +#endif > > static const struct dev_pm_ops ufs_mtk_pm_ops = { > SET_SYSTEM_SLEEP_PM_OPS(ufs_mtk_system_suspend,
On 7/3/22 19:56, Ren Zhijie wrote: > Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Hi Ren, On Mon, 2022-07-11 at 09:23 +0800, Ren Zhijie wrote: > Hi, > Thanks for this fix. Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
On Mon, Jul 4, 2022 at 4:56 AM Ren Zhijie <renzhijie2@huawei.com> wrote: > diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c > index c958279bdd8f..e006c2528a3a 100644 > --- a/drivers/ufs/host/ufs-mediatek.c > +++ b/drivers/ufs/host/ufs-mediatek.c > @@ -1427,6 +1427,7 @@ static int ufs_mtk_system_resume(struct device *dev) > } > #endif > > +#ifdef CONFIG_PM > static int ufs_mtk_runtime_suspend(struct device *dev) > { > struct ufs_hba *hba = dev_get_drvdata(dev); > @@ -1449,6 +1450,7 @@ static int ufs_mtk_runtime_resume(struct device *dev) > > return ufshcd_runtime_resume(dev); > } > +#endif > > static const struct dev_pm_ops ufs_mtk_pm_ops = { > SET_SYSTEM_SLEEP_PM_OPS(ufs_mtk_system_suspend, This change works, but it's not great. It's better to change the SET_SYSTEM_SLEEP_PM_OPS() to the new SYSTEM_SLEEP_PM_OPS() that works without the #ifdef. Arnd
在 2022/7/12 16:27, Arnd Bergmann 写道: > On Mon, Jul 4, 2022 at 4:56 AM Ren Zhijie <renzhijie2@huawei.com> wrote: >> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c >> index c958279bdd8f..e006c2528a3a 100644 >> --- a/drivers/ufs/host/ufs-mediatek.c >> +++ b/drivers/ufs/host/ufs-mediatek.c >> @@ -1427,6 +1427,7 @@ static int ufs_mtk_system_resume(struct device *dev) >> } >> #endif >> >> +#ifdef CONFIG_PM >> static int ufs_mtk_runtime_suspend(struct device *dev) >> { >> struct ufs_hba *hba = dev_get_drvdata(dev); >> @@ -1449,6 +1450,7 @@ static int ufs_mtk_runtime_resume(struct device *dev) >> >> return ufshcd_runtime_resume(dev); >> } >> +#endif >> >> static const struct dev_pm_ops ufs_mtk_pm_ops = { >> SET_SYSTEM_SLEEP_PM_OPS(ufs_mtk_system_suspend, > This change works, but it's not great. It's better to change the > SET_SYSTEM_SLEEP_PM_OPS() to the new SYSTEM_SLEEP_PM_OPS() > that works without the #ifdef. > > Arnd Hi Arnd, Thanks for your suggestion. How does it to fix the implicit-function-declaration error? Thanks, Ren Zhijie > .
On Wed, Jul 13, 2022 at 3:44 AM Ren Zhijie <renzhijie2@huawei.com> wrote: > 在 2022/7/12 16:27, Arnd Bergmann 写道: > > Thanks for your suggestion. > > How does it to fix the implicit-function-declaration error? > I missed that part at first. I would say the #ifdef around the declarations in the header should be removed here, it serves no purpose, and it is safe to rely on the compiler to perform dead code elimination so this does not lead to a link error even if a dead function references another function that is not reachable. Arnd
在 2022/7/13 16:48, Arnd Bergmann 写道: > On Wed, Jul 13, 2022 at 3:44 AM Ren Zhijie <renzhijie2@huawei.com> wrote: >> 在 2022/7/12 16:27, Arnd Bergmann 写道: >> >> Thanks for your suggestion. >> >> How does it to fix the implicit-function-declaration error? >> > I missed that part at first. I would say the #ifdef around the > declarations in the > header should be removed here, it serves no purpose, and it is safe to rely > on the compiler to perform dead code elimination so this does not lead to > a link error even if a dead function references another function that is not > reachable. > > Arnd Hi Arnd, I try to use the new marcos SYSTEM_SLEEP_PM_OPS and RUNTIME_PM_OPS to replace the old ones, and remove #ifdef around the declarations in the header, my local changes attach below. But it seems that doesn't work, which has ld errors: aarch64-linux-gnu-ld: Unexpected GOT/PLT entries detected! aarch64-linux-gnu-ld: Unexpected run-time procedure linkages detected! drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_resume': ufs-mediatek.c:(.text+0x1d0c): undefined reference to `ufshcd_runtime_resume' drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_suspend': ufs-mediatek.c:(.text+0x1d64): undefined reference to `ufshcd_runtime_suspend' Makefile:1255: recipe for target 'vmlinux' failed make: *** [vmlinux] Error 1 (CONFIG_PM and CONFIG_PM_SLEEP are both not set, and run cmd: make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-) Thanks, Ren Zhijie -- diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c index c958279bdd8f..8c0c6f04eed1 100644 --- a/drivers/ufs/host/ufs-mediatek.c +++ b/drivers/ufs/host/ufs-mediatek.c @@ -1402,7 +1402,6 @@ static int ufs_mtk_remove(struct platform_device *pdev) return 0; } -#ifdef CONFIG_PM_SLEEP static int ufs_mtk_system_suspend(struct device *dev) { struct ufs_hba *hba = dev_get_drvdata(dev); @@ -1425,7 +1424,6 @@ static int ufs_mtk_system_resume(struct device *dev) return ufshcd_system_resume(dev); } -#endif static int ufs_mtk_runtime_suspend(struct device *dev) { @@ -1451,9 +1449,9 @@ static int ufs_mtk_runtime_resume(struct device *dev) } static const struct dev_pm_ops ufs_mtk_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(ufs_mtk_system_suspend, + SYSTEM_SLEEP_PM_OPS(ufs_mtk_system_suspend, ufs_mtk_system_resume) - SET_RUNTIME_PM_OPS(ufs_mtk_runtime_suspend, + RUNTIME_PM_OPS(ufs_mtk_runtime_suspend, ufs_mtk_runtime_resume, NULL) .prepare = ufshcd_suspend_prepare, .complete = ufshcd_resume_complete, diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index 7fe1a926cd99..17ec18d55470 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -1081,14 +1081,10 @@ static inline void *ufshcd_get_variant(struct ufs_hba *hba) return hba->priv; } -#ifdef CONFIG_PM extern int ufshcd_runtime_suspend(struct device *dev); extern int ufshcd_runtime_resume(struct device *dev); -#endif -#ifdef CONFIG_PM_SLEEP extern int ufshcd_system_suspend(struct device *dev); extern int ufshcd_system_resume(struct device *dev); -#endif extern int ufshcd_shutdown(struct ufs_hba *hba); extern int ufshcd_dme_configure_adapt(struct ufs_hba *hba, int agreed_gear, > .
On Mon, Jul 25, 2022 at 5:38 AM Ren Zhijie <renzhijie2@huawei.com> wrote: > 在 2022/7/13 16:48, Arnd Bergmann 写道: > > I try to use the new marcos SYSTEM_SLEEP_PM_OPS and RUNTIME_PM_OPS to > replace the old ones, and remove #ifdef around the declarations in the > header, my local changes attach below. > > But it seems that doesn't work, which has ld errors: > > aarch64-linux-gnu-ld: Unexpected GOT/PLT entries detected! > aarch64-linux-gnu-ld: Unexpected run-time procedure linkages detected! > drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_resume': > ufs-mediatek.c:(.text+0x1d0c): undefined reference to > `ufshcd_runtime_resume' > drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_suspend': > ufs-mediatek.c:(.text+0x1d64): undefined reference to > `ufshcd_runtime_suspend' > Makefile:1255: recipe for target 'vmlinux' failed > make: *** [vmlinux] Error 1 > > (CONFIG_PM and CONFIG_PM_SLEEP are both not set, and > It appears that there is a mistake in the RUNTIME_PM_OPS() macro definition, can you try this patch on top? diff --git a/include/linux/pm.h b/include/linux/pm.h index 871c9c49ec9d..84592229d754 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -334,9 +334,9 @@ struct dev_pm_ops { .restore_noirq = pm_sleep_ptr(resume_fn), #define RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \ - .runtime_suspend = suspend_fn, \ - .runtime_resume = resume_fn, \ - .runtime_idle = idle_fn, + .runtime_suspend = pm_ptr(suspend_fn), \ + .runtime_resume = pm_ptr(resume_fn), \ + .runtime_idle = pm_ptr(idle_fn), #ifdef CONFIG_PM_SLEEP #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ Commit 1a3c7bb08826 ("PM: core: Add new *_PM_OPS macros, deprecate old ones") introduced all the macros with the intent of using pm_ptr() and pm_sleep_ptr() in them, and I think Paul accidentally forgot to add those in this instance. Arnd
在 2022/7/25 14:34, Arnd Bergmann 写道: > On Mon, Jul 25, 2022 at 5:38 AM Ren Zhijie <renzhijie2@huawei.com> wrote: >> 在 2022/7/13 16:48, Arnd Bergmann 写道: >> >> I try to use the new marcos SYSTEM_SLEEP_PM_OPS and RUNTIME_PM_OPS to >> replace the old ones, and remove #ifdef around the declarations in the >> header, my local changes attach below. >> >> But it seems that doesn't work, which has ld errors: >> >> aarch64-linux-gnu-ld: Unexpected GOT/PLT entries detected! >> aarch64-linux-gnu-ld: Unexpected run-time procedure linkages detected! >> drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_resume': >> ufs-mediatek.c:(.text+0x1d0c): undefined reference to >> `ufshcd_runtime_resume' >> drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_suspend': >> ufs-mediatek.c:(.text+0x1d64): undefined reference to >> `ufshcd_runtime_suspend' >> Makefile:1255: recipe for target 'vmlinux' failed >> make: *** [vmlinux] Error 1 >> >> (CONFIG_PM and CONFIG_PM_SLEEP are both not set, and >> > It appears that there is a mistake in the RUNTIME_PM_OPS() > macro definition, can you try this patch on top? Hi Arnd, I apply this patch and it worked. When will this patch be merged? Thanks, Ren > > diff --git a/include/linux/pm.h b/include/linux/pm.h > index 871c9c49ec9d..84592229d754 100644 > --- a/include/linux/pm.h > +++ b/include/linux/pm.h > @@ -334,9 +334,9 @@ struct dev_pm_ops { > .restore_noirq = pm_sleep_ptr(resume_fn), > > #define RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \ > - .runtime_suspend = suspend_fn, \ > - .runtime_resume = resume_fn, \ > - .runtime_idle = idle_fn, > + .runtime_suspend = pm_ptr(suspend_fn), \ > + .runtime_resume = pm_ptr(resume_fn), \ > + .runtime_idle = pm_ptr(idle_fn), > > #ifdef CONFIG_PM_SLEEP > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ > > Commit 1a3c7bb08826 ("PM: core: Add new *_PM_OPS macros, deprecate > old ones") introduced all the macros with the intent of using pm_ptr() > and pm_sleep_ptr() in them, and I think Paul accidentally forgot to add > those in this instance. > > Arnd > > .
On Mon, Jul 25, 2022 at 9:00 AM Ren Zhijie <renzhijie2@huawei.com> wrote: > 在 2022/7/25 14:34, Arnd Bergmann 写道: > > On Mon, Jul 25, 2022 at 5:38 AM Ren Zhijie <renzhijie2@huawei.com> wrote: > >> 在 2022/7/13 16:48, Arnd Bergmann 写道: > >> > >> I try to use the new marcos SYSTEM_SLEEP_PM_OPS and RUNTIME_PM_OPS to > >> replace the old ones, and remove #ifdef around the declarations in the > >> header, my local changes attach below. > >> > >> But it seems that doesn't work, which has ld errors: > >> > >> aarch64-linux-gnu-ld: Unexpected GOT/PLT entries detected! > >> aarch64-linux-gnu-ld: Unexpected run-time procedure linkages detected! > >> drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_resume': > >> ufs-mediatek.c:(.text+0x1d0c): undefined reference to > >> `ufshcd_runtime_resume' > >> drivers/ufs/host/ufs-mediatek.o: In function `ufs_mtk_runtime_suspend': > >> ufs-mediatek.c:(.text+0x1d64): undefined reference to > >> `ufshcd_runtime_suspend' > >> Makefile:1255: recipe for target 'vmlinux' failed > >> make: *** [vmlinux] Error 1 > >> > >> (CONFIG_PM and CONFIG_PM_SLEEP are both not set, and > >> > > It appears that there is a mistake in the RUNTIME_PM_OPS() > > macro definition, can you try this patch on top? > > Hi Arnd, > > I apply this patch and it worked. > > When will this patch be merged? So far nobody has submitted this, I was just guessing and I had no idea if this would do the right thing. Can you send this patch to the power management maintainers with a proper changelog and a Suggested-by: Arnd Bergmann <arnd@arndb.de> line? Thanks, Arnd
Hi Arnd, Le lun., juil. 25 2022 at 08:34:32 +0200, Arnd Bergmann <arnd@arndb.de> a écrit : > On Mon, Jul 25, 2022 at 5:38 AM Ren Zhijie <renzhijie2@huawei.com> > wrote: >> 在 2022/7/13 16:48, Arnd Bergmann 写道: >> >> I try to use the new marcos SYSTEM_SLEEP_PM_OPS and RUNTIME_PM_OPS >> to >> replace the old ones, and remove #ifdef around the declarations in >> the >> header, my local changes attach below. >> >> But it seems that doesn't work, which has ld errors: >> >> aarch64-linux-gnu-ld: Unexpected GOT/PLT entries detected! >> aarch64-linux-gnu-ld: Unexpected run-time procedure linkages >> detected! >> drivers/ufs/host/ufs-mediatek.o: In function >> `ufs_mtk_runtime_resume': >> ufs-mediatek.c:(.text+0x1d0c): undefined reference to >> `ufshcd_runtime_resume' >> drivers/ufs/host/ufs-mediatek.o: In function >> `ufs_mtk_runtime_suspend': >> ufs-mediatek.c:(.text+0x1d64): undefined reference to >> `ufshcd_runtime_suspend' >> Makefile:1255: recipe for target 'vmlinux' failed >> make: *** [vmlinux] Error 1 >> >> (CONFIG_PM and CONFIG_PM_SLEEP are both not set, and >> > > It appears that there is a mistake in the RUNTIME_PM_OPS() > macro definition, can you try this patch on top? > > diff --git a/include/linux/pm.h b/include/linux/pm.h > index 871c9c49ec9d..84592229d754 100644 > --- a/include/linux/pm.h > +++ b/include/linux/pm.h > @@ -334,9 +334,9 @@ struct dev_pm_ops { > .restore_noirq = pm_sleep_ptr(resume_fn), > > #define RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \ > - .runtime_suspend = suspend_fn, \ > - .runtime_resume = resume_fn, \ > - .runtime_idle = idle_fn, > + .runtime_suspend = pm_ptr(suspend_fn), \ > + .runtime_resume = pm_ptr(resume_fn), \ > + .runtime_idle = pm_ptr(idle_fn), Not a mistake, the pm_ptr() macros are not needed here, because the dev_pm_ops pointer must always be wrapped with pm_ptr(). The proper fix is: - .pm = &ufs_mtk_pm_ops, + .pm = pm_ptr(&ufs_mtk_pm_ops), Cheers, -Paul > > #ifdef CONFIG_PM_SLEEP > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ > > Commit 1a3c7bb08826 ("PM: core: Add new *_PM_OPS macros, deprecate > old ones") introduced all the macros with the intent of using pm_ptr() > and pm_sleep_ptr() in them, and I think Paul accidentally forgot to > add > those in this instance. > > Arnd
diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c index c958279bdd8f..e006c2528a3a 100644 --- a/drivers/ufs/host/ufs-mediatek.c +++ b/drivers/ufs/host/ufs-mediatek.c @@ -1427,6 +1427,7 @@ static int ufs_mtk_system_resume(struct device *dev) } #endif +#ifdef CONFIG_PM static int ufs_mtk_runtime_suspend(struct device *dev) { struct ufs_hba *hba = dev_get_drvdata(dev); @@ -1449,6 +1450,7 @@ static int ufs_mtk_runtime_resume(struct device *dev) return ufshcd_runtime_resume(dev); } +#endif static const struct dev_pm_ops ufs_mtk_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(ufs_mtk_system_suspend,
If CONFIG_PM is not set, make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-, will be failed, like this: drivers/ufs/host/ufs-mediatek.c: In function ‘ufs_mtk_runtime_suspend’: drivers/ufs/host/ufs-mediatek.c:1435:8: error: implicit declaration of function ‘ufshcd_runtime_suspend’; did you mean ‘ufs_mtk_runtime_suspend’? [-Werror=implicit-function-declaration] ret = ufshcd_runtime_suspend(dev); ^~~~~~~~~~~~~~~~~~~~~~ ufs_mtk_runtime_suspend drivers/ufs/host/ufs-mediatek.c: In function ‘ufs_mtk_runtime_resume’: drivers/ufs/host/ufs-mediatek.c:1450:9: error: implicit declaration of function ‘ufshcd_runtime_resume’; did you mean ‘ufs_mtk_runtime_resume’? [-Werror=implicit-function-declaration] return ufshcd_runtime_resume(dev); ^~~~~~~~~~~~~~~~~~~~~ ufs_mtk_runtime_resume At top level: drivers/ufs/host/ufs-mediatek.c:1444:12: error: ‘ufs_mtk_runtime_resume’ defined but not used [-Werror=unused-function] static int ufs_mtk_runtime_resume(struct device *dev) ^~~~~~~~~~~~~~~~~~~~~~ drivers/ufs/host/ufs-mediatek.c:1430:12: error: ‘ufs_mtk_runtime_suspend’ defined but not used [-Werror=unused-function] static int ufs_mtk_runtime_suspend(struct device *dev) ^~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors The declarations of func "ufshcd_runtime_suspend()" and "ufshcd_runtime_resume()" depended on CONFIG_PM, so the function wrappers "ufs_mtk_runtime_suspend()" and "ufs_mtk_runtime_resume()" both should warpped by CONFIG_PM too. Reported-by: Hulk Robot <hulkci@huawei.com> Fixes: e7bf1d50063c ("scsi: ufs: ufs-mediatek: Fix build warnings") Signed-off-by: Ren Zhijie <renzhijie2@huawei.com> --- drivers/ufs/host/ufs-mediatek.c | 2 ++ 1 file changed, 2 insertions(+)