[...]

quoted

quoted

quoted

 	pm_runtime_set_autosuspend_delay(&pdev->dev,

I2C_PM_TIMEOUT);

quoted

 	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_get_noresume(&pdev->dev);
-	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);

+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		dev_err(&pdev->dev, "failed to enable clock\n");
+		return ret;
+	}

Can't current clk control via rpm work well?
Please describe why need change.

I think the previous patch maker might want to use the return value of
pm_runtime_get_sync to check whether the clock has been turned on
correctly to
avoid the kernel panic.
Maybe I can change to the method like this.
	pm_runtime_get_noresume(&pdev->dev);
	ret = pm_runtime_set_active(&pdev->dev);
	if (ret < 0)
		goto out;
	pm_runtime_enable(&pdev->dev);

Best Regards,
Clark Wang

Sorry, I missed the point before.
If we use pm_runtime_get_noresume(&pdev->dev); and
pm_runtime_set_active(&pdev->dev); here, the clk should be enabled by using
clk_prepare_enable() in the probe function. However, the call of
clk_prepare_enable() is already in lpi2c_runtime_resume().
Using get_sync() here can help to reduce the repetitive code, especially ipg
clk will be added later.

Let's not do for this negligible improvement unless there're any other good benefits.
If really care, move clk operation into an inline function instead.
Another benefit if not doing that is the driver still can work even RPM not enabled.

Regards
Aisheng

Shall we change to use pm_runtime_get_sync() here?

Regards,
Clark Wang

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel