Messages in this thread | | | Date | Tue, 26 Apr 2022 01:27:28 -0700 | Subject | Re: [PATCH v4 4/4] watchdog: iTCO_wdg: Make code more clearly with macro definition | From | Guenter Roeck <> |
| |
On 4/26/22 01:17, liuxp11@chinatelecom.cn wrote: > On 4/26/22 00:10, Liu Xinpeng wrote: > > Using SET_NOIRQ_SYSTEM_SLEEP_PM_OPS reduces redundant code. > > > > Signed-off-by: Liu Xinpeng <liuxp11@chinatelecom.cn> > > Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org> > > --- > > drivers/watchdog/iTCO_wdt.c | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c > > index 3f2f4343644f..8d24cf4f4ca3 100644 > > --- a/drivers/watchdog/iTCO_wdt.c > > +++ b/drivers/watchdog/iTCO_wdt.c > > @@ -635,22 +635,18 @@ static int iTCO_wdt_resume_noirq(struct device *dev) > > > > return 0; > > } > > +#endif /* CONFIG_PM_SLEEP */ > > > > static const struct dev_pm_ops iTCO_wdt_pm = { > > - .suspend_noirq = iTCO_wdt_suspend_noirq, > > - .resume_noirq = iTCO_wdt_resume_noirq, > > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(iTCO_wdt_suspend_noirq, > > + iTCO_wdt_resume_noirq) > > }; > > > > -#define ITCO_WDT_PM_OPS (&iTCO_wdt_pm) > > -#else > > -#define ITCO_WDT_PM_OPS NULL > > -#endif /* CONFIG_PM_SLEEP */ > > - > > static struct platform_driver iTCO_wdt_driver = { > > .probe = iTCO_wdt_probe, > > .driver = { > > .name = DRV_NAME, > > - .pm = ITCO_WDT_PM_OPS, > > + .pm = &iTCO_wdt_pm, > > }, > > }; > > > Sorry, I fail to see how this is an improvement. > > Guenter > ---> > #define NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ > .suspend_noirq = pm_sleep_ptr(suspend_fn), \ > .resume_noirq = pm_sleep_ptr(resume_fn), \ > .freeze_noirq = pm_sleep_ptr(suspend_fn), \ > .thaw_noirq = pm_sleep_ptr(resume_fn), \ > .poweroff_noirq = pm_sleep_ptr(suspend_fn), \ > .restore_noirq = pm_sleep_ptr(resume_fn), > > NOIRQ_SYSTEM_SLEEP_PM_OPS, defined for CONFIG_PM_SLEEP, will > point ->suspend_noirq, ->freeze_noirq and ->poweroff_noirq to the same > function. Vice versa happens for ->resume_noirq, ->thaw_noirq and ->restore_noirq. > (commit 020af89a41c41fd2c92d0da524968dfaba6269f0) > > Some power management occasion needs freeze_noirq/thaw_noirq /poweroff_noirq /restore_noirq. >
That isn't what the commit description and subject say. Change description and subject to describe what is actually done, use __maybe_unused instead of #ifdef as suggested, and do not invent Reviewed-by: tags.
Also, this is an independent change, and should not be sent together with the other patches.
Guenter
| |