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

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

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