On 8/11/19 4:23 AM, Jonathan Cameron wrote:
On Wed, 7 Aug 2019 14:40:21 -0500
David Lechner <david@xxxxxxxxxxxxxx> wrote:
+
+ pm_runtime_enable(dev);
+ pm_runtime_get(dev);
I'm a little confused on the flow here.
pm_runtime_enable turns on runtime pm in general.
pm_runtime_get basically calls runtime_resume to ensrue the
device has power.
+
+ return devm_counter_register(dev, &priv->counter);
This registers the userspace interfaces and exposes the device.
+}
+
+static int ti_eqep_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ pm_runtime_put(dev),
+ pm_runtime_disable(dev);
pm_runtime_put notifies the system that the device is idle
(and hence potentially turns it off).
Not good if the counter is still registered.
I'm assuming the presence of runtime pm at all is to interact
with a parent driver and hence stop that turning off if this
driver is probed? That's probably fine, but add a few comments
to make it clear that this driver itself doesn't really do
runtime pm at all.
To be honest, despite reading the runtime PM docs more than once,
I still don't feel like I have a good grasp on how it is supposed
to work. I just know that we get a crash if it is omitted because
the IP block is not powered on.
I will fix the ordering in _remove() and add a comment in v3.
