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?
>
> > +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?
We can drop ucsi_ccg_runtime_idle() but we do need
ucsi_ccg_runtime_suspend() and ucsi_ccg_runtime_resume() for
runtime pm functionality.
Thanks
Ajay
> nvpublic
> > +
> > +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
