Roger Quadros <rogerq@xxxxxx> writes:
> On 13/06/18 11:05, Felipe Balbi wrote:
>> Roger Quadros <rogerq@xxxxxx> writes:
>>
>>> Hi Johan,
>>>
>>> On 31/05/18 17:45, Johan Hovold wrote:
>>>> The clocks have already been explicitly disabled and put as part of
>>>> remove() so the runtime suspend callback must not be run when balancing
>>>> the runtime PM usage count before returning.
>>>>
>>>> Fixes: 16adc674d0d6 ("usb: dwc3: add generic OF glue layer")
>>>> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
>>>> ---
>>>>
>>>> Changes in v2
>>>> - balance usage count only after disabling runtime PM to avoid racing
>>>> with pm_runtime_suspend() as suggested by Alan
>>>>
>>>>
>>>> drivers/usb/dwc3/dwc3-of-simple.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
>>>> index cb2ee96fd3e8..048922d549dd 100644
>>>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>>>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>>>> @@ -165,8 +165,9 @@ static int dwc3_of_simple_remove(struct platform_device *pdev)
>>>>
>>>> reset_control_put(simple->resets);
>>>>
>>>> - pm_runtime_put_sync(dev);
>>>
>>> Wasn't this call there to balance out the pm_runtime_get_sync() call in probe()?
>>> The pm_runtime_get_sync() call is still there in probe().
>>
>> that's now balanced by put_noidle below
>>
>>
>
> OK.
>
> I'm still trying to get my head around this.
>
> in probe() we do
> {
> enable all clocks;
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
> pm_runtime_get_sync(dev);
> }
>
> How will runtime suspend work at all?
> We're holding a positive RPM count in probe().
echo auto > /path/to/dwc3/power/control
Granted, that get_sync() would've been better as a pm_runtime_forbid()
--
balbi
Attachment:
signature.asc
Description: PGP signature
