Skip to contentSkip to conversation

i2c imx: suspicious use of runtime PM calls

DISCUSSION Heiner Kallweit 3 participants 3 messages Aug 22, 2015 – Aug 24, 2015 2 messages not in the archive I2C subsystem host drivers

lore
Share & tools
lore

Heiner Kallweit

author
#0 lore raw

Just by chance I stumbled across this driver and the recently added runtime PM code. Two things in i2c_imx_probe look suspicious to me:

- You activate the I2C clock and then initialize runtime PM with

    pm_runtime_enable(&pdev->dev);
    pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
    pm_runtime_use_autosuspend(&pdev->dev);

Seems like a call to pm_runtime_set_active is missing to tell runtime PM that the device is in status "active" now.

- You use pm_runtime_put_autosuspend (which decrements the ref count) but you don't get a reference before. I think you would have to use pm_runtime_get_... before or use pm_runtime_autosuspend.

Regards, Heiner

#2 lore raw
In reply to a message not in the archive

From: Heiner Kallweit <mailto:[email protected]> Sent: Sunday, August 23, 2015 5:42 AM

Quoted 8 lines

To: Gao Pan-B54642
Cc: [email protected]; Wolfram Sang
Subject: i2c imx: suspicious use of runtime PM calls

Just by chance I stumbled across this driver and the recently added runtime PM code.
Two things in i2c_imx_probe look suspicious to me:

Quoted 8 lines

- You activate the I2C clock and then initialize runtime PM with

    pm_runtime_enable(&pdev->dev);
    pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
    pm_runtime_use_autosuspend(&pdev->dev);

Seems like a call to pm_runtime_set_active is missing to tell runtime
PM that the device is in status "active" now.

Thanks, you are right. I will fix it in next patch.

Quoted 4 lines

- You use pm_runtime_put_autosuspend (which decrements the ref count) but you don't
get a reference before. I think you would have to use pm_runtime_get_...
before or use pm_runtime_autosuspend.

Thanks for your precise review. I will fix it in next patch.

#4 lore raw
In reply to a message not in the archive
Quoted 7 lines
Quoted 7 lines

Just by chance I stumbled across this driver and the recently added runtime PM code.
Two things in i2c_imx_probe look suspicious to me:

- You activate the I2C clock and then initialize runtime PM with

Quoted 8 lines
Quoted 6 lines
    pm_runtime_enable(&pdev->dev);
    pm_runtime_set_autosuspend_delay(&pdev->dev, I2C_PM_TIMEOUT);
    pm_runtime_use_autosuspend(&pdev->dev);

Seems like a call to pm_runtime_set_active is missing to tell runtime
PM that the device is in status "active" now.

Thanks, you are right. I will fix it in next patch.

Thanks Heiner for the report. I dropped this patch and will wait for a new version.