On Tue, May 21, 2019 at 04:37:27PM +0300, Heikki Krogerus wrote: > 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; > > +} Oh yes, and do you really need to supply all of those stubs? thanks, -- heikki