Hi, On Mon, May 20, 2019 at 11:37:48AM -0700, Ajay Gupta wrote: > +static int ucsi_ccg_resume(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct ucsi_ccg *uc = i2c_get_clientdata(client); > + struct ucsi *ucsi = uc->ucsi; > + struct ucsi_control c; > + int ret; > + > + /* restore UCSI notification enable mask */ > + UCSI_CMD_SET_NTFY_ENABLE(c, UCSI_ENABLE_NTFY_ALL); > + ret = ucsi_send_command(ucsi, &c, NULL, 0); > + if (ret < 0) { > + dev_err(uc->dev, "%s: failed to set notification enable - %d\n", > + __func__, ret); > + } > + return 0; > +} I would prefer that we did this for all methods in ucsi.c, not just ccgx. Could you add resume callback to struct ucsi_ppm, and then call it here. > +static int ucsi_ccg_runtime_suspend(struct device *dev) > +{ > + return 0; > +} > + > +static int ucsi_ccg_runtime_resume(struct device *dev) > +{ > + return 0; > +} > + > +static int ucsi_ccg_runtime_idle(struct device *dev) > +{ > + return 0; > +} > + > +static const struct dev_pm_ops ucsi_ccg_pm = { > + .suspend = ucsi_ccg_suspend, > + .resume = ucsi_ccg_resume, > + .runtime_suspend = ucsi_ccg_runtime_suspend, > + .runtime_resume = ucsi_ccg_runtime_resume, > + .runtime_idle = ucsi_ccg_runtime_idle, > +}; > + > static struct i2c_driver ucsi_ccg_driver = { > .driver = { > .name = "ucsi_ccg", > + .pm = &ucsi_ccg_pm, > }, > .probe = ucsi_ccg_probe, > .remove = ucsi_ccg_remove, thanks, -- heikki