On Tue, May 21, 2019 at 05:44:50PM +0000, Ajay Gupta wrote: > Hi Heikki > > > > +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. > struct ucsi_ppm currently have .sync() and .cmd() callback which is implemented by > ucsi_ccg and ucsi_acpi and invoked by usci.c. > > Is it okay to add a callback in this structure and implement inside ucsi.c and invoke > from ucsi_ccg and ucsi_acpi? OR we can just add a function in ucsi.c and export it > and use it from ucsi_ccg and ucsi_acpi? Right! Export the function. Sorry. thanks, -- heikki