designware platdrv and runtime pm?

designware platdrv and runtime pm?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

Hi guys,

while handling the merge conflict for the designware-platdrv, I noticed
an asymmetry in the runtime PM handling. Currently, code looks like
this:

	if (dev->pm_runtime_disabled) {
		pm_runtime_forbid(&pdev->dev);
	} else {
		pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
		pm_runtime_use_autosuspend(&pdev->dev);
		pm_runtime_set_active(&pdev->dev);
		pm_runtime_enable(&pdev->dev);
	}

	r = i2c_dw_probe(dev);
	if (r) {
		pm_runtime_disable(&pdev->dev);
		return r;
	}

But shouldn't the above error path (and the remove path) also take
dev->pm_runtime_disabled into account and act accordingly?

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: designware platdrv and runtime pm?

[Jarkko Nikula]

On 09.11.2015 23:15, Wolfram Sang wrote:
> Hi guys,
>
> while handling the merge conflict for the designware-platdrv, I noticed
> an asymmetry in the runtime PM handling. Currently, code looks like
> this:
>
> 	if (dev->pm_runtime_disabled) {
> 		pm_runtime_forbid(&pdev->dev);
> 	} else {
> 		pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
> 		pm_runtime_use_autosuspend(&pdev->dev);
> 		pm_runtime_set_active(&pdev->dev);
> 		pm_runtime_enable(&pdev->dev);
> 	}
>
> 	r = i2c_dw_probe(dev);
> 	if (r) {
> 		pm_runtime_disable(&pdev->dev);
> 		return r;
> 	}
>
> But shouldn't the above error path (and the remove path) also take
> dev->pm_runtime_disabled into account and act accordingly?
>
I think you are right. Which brings another question to my mind do we 
need to have a patch to linux-stable too?

David: Your original commit 894acb2f823b ("i2c: designware: Add Intel 
Baytrail PMIC I2C bus support") doesn't add pm_runtime_disabled test to 
dw_i2c_remove(). I guess there is possibility power down the shared 
controller by having CONFIG_I2C_DESIGNWARE_PLATFORM=m and then unloading 
the driver?

-- 
Jarkko