Messages in this thread | | | Subject | Re: [PATCH v2 3/5] counter: new TI eQEP driver | From | David Lechner <> | Date | Sun, 11 Aug 2019 14:15:14 -0500 |
| |
On 8/11/19 4:23 AM, Jonathan Cameron wrote: > On Wed, 7 Aug 2019 14:40:21 -0500 > David Lechner <david@lechnology.com> 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.
| |