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().
This was pointed out by Johan as well in [1]
[1] https://lkml.org/lkml/2018/5/28/1705
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
