On 4/26/22 01:17, liuxp11@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx> > Reviewed-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx> > --- > 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