Kukjin, On Wed, Jun 25, 2014 at 4:13 AM, Kukjin Kim <kgene.kim@xxxxxxxxxxx> wrote: > Doug Anderson wrote: >> >> The original code for the exynos i2c controller registered for the >> "noirq" variants. However during review feedback it was moved to >> SIMPLE_DEV_PM_OPS without anyone noticing that it meant we were no >> longer actually "noirq" (despite functions named >> exynos5_i2c_suspend_noirq and exynos5_i2c_resume_noirq). >> >> i2c controllers that might have wakeup sources on them seem to need to >> resume at noirq time so that the individual drivers can actually read >> the i2c bus to handle their wakeup. >> >> NOTE: I took the original review feedback from Wolfram and added >> poweroff, thaw, freeze, restore. >> > Yeah I'm not sure except .suspend_noirq and .resume_noirq but I'm fine if > Wolfram suggested ;-) Sorry, I didn't mean to imply that Wolfram suggested the "noirq" versions. Specifically in <https://lkml.org/lkml/2013/9/8/133> Naveen had: > +static const struct dev_pm_ops exynos5_i2c_dev_pm_ops = { > + .suspend_noirq = exynos5_i2c_suspend_noirq, > + .resume_noirq = exynos5_i2c_resume_noirq, > +}; > + > +#define EXYNOS5_DEV_PM_OPS (&exynos5_i2c_dev_pm_ops) > +#else > +#define EXYNOS5_DEV_PM_OPS NULL > +#endif And Wolfram said: > Isn't there a macro for this? SIMPLE_DEV_PM_OPS*? Not sure, I always mix > them up... That had the side effect of getting freeze, restore, ... Ah, I also see that Felipe Balbi was the one that gave earlier feedback about this also at <https://lkml.org/lkml/2012/11/27/262>. He said "you need to define poweroff, thaw, freeze, restore." >> This patch has only been compile-tested since I don't have all the >> patches needed to make my machine using this i2c driver actually >> suspend/resume. >> >> Signed-off-by: Doug Anderson <dianders@xxxxxxxxxxxx> >> --- >> Changes in v2: >> - Added missing CONFIG_PM_SLEEP >> >> drivers/i2c/busses/i2c-exynos5.c | 12 ++++++++++-- >> 1 file changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c- >> exynos5.c >> index 63d2292..348b1cd 100644 >> --- a/drivers/i2c/busses/i2c-exynos5.c >> +++ b/drivers/i2c/busses/i2c-exynos5.c >> @@ -789,8 +789,16 @@ static int exynos5_i2c_resume_noirq(struct device *dev) >> } >> #endif >> >> -static SIMPLE_DEV_PM_OPS(exynos5_i2c_dev_pm_ops, exynos5_i2c_suspend_noirq, >> - exynos5_i2c_resume_noirq); >> +const struct dev_pm_ops exynos5_i2c_dev_pm_ops = { > > Maybe static const struct...? Duh, right. Fixing and will spin. >> +#ifdef CONFIG_PM_SLEEP >> + .suspend_noirq = exynos5_i2c_suspend_noirq, >> + .resume_noirq = exynos5_i2c_resume_noirq, >> + .freeze_noirq = exynos5_i2c_suspend_noirq, >> + .thaw_noirq = exynos5_i2c_resume_noirq, >> + .poweroff_noirq = exynos5_i2c_suspend_noirq, >> + .restore_noirq = exynos5_i2c_resume_noirq, >> +#endif >> +}; >> >> static struct platform_driver exynos5_i2c_driver = { >> .probe = exynos5_i2c_probe, >> -- >> 2.0.0.526.g5318336 > > Others look good to me, > > Acked-by: Kukjin Kim <kgene.kim@xxxxxxxxxxx> > > Thanks, > Kukjin > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
