Re: [PATCH v2 3/5] usb: typec: ucsi: ccg: enable runtime pm support

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

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