Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> wrote:
> On 08/09/16 21:42, Kevin Hilman wrote:
>>
>> Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx> writes:
>>
>>> On 08/09/16 20:52, Martin Blumenstingl wrote:
>>>>
>>>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman <khilman@xxxxxxxxxxxx>
>>>> wrote:
>>>>>>
>>>>>> +     phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
>>>>>> +     if (IS_ERR(phy)) {
>>>>>> +             dev_err(&pdev->dev, "failed to create PHY\n");
>>>>>> +             return PTR_ERR(phy);
>>>>>> +     }
>>>>>> +
>>>>>> +     if (usb_reset_refcnt++ == 0) {
>>>>>> +             ret = device_reset(&pdev->dev);
>>>>>> +             if (ret) {
>>>>>> +                     dev_err(&phy->dev, "Failed to reset USB PHY\n");
>>>>>> +                     return ret;
>>>>>> +             }
>>>>>> +     }
>>>>>
>>>>>
>>>>> The ref count + reset here looks like something that could/should be
>>>>> handled in a runtime PM callback.
>>>>
>>>> Unfortunately that doesn't work (as Jerome found out) because both
>>>> PHYs are sharing the same reset line.
>>>> So if the second PHY would call device_reset then it would also reset
>>>> the first PHY!
>>>>
>>>> There's a comment above the declaration of usb_reset_refcnt which
>>>> tries to explain this:
>>>> "The PHYs are sharing a common reset line -> we are only allowed to
>>>> reset once for all PHYs."
>>>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
>>>> {" line to make it easier to see?
>>>>
>>>
>>> pm-runtime has refcounting in it. When one of the nodes turns on,
>>> the pm-runtime will call your driver to say there is a user when
>>> this first use turns up.
>>>
>>> If all the sub-phys turn off and drop their refcount then the driver
>>> is called to say there are no more users and you can go to sleep.
>>
>>
>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
>>
>> The reason is because there are physically two PHY devices[1].  Those 2
>> devices will be treated independely by runtime PM, and have separate
>> use-counting, which means doing what I proposed would cause a reset to
>> happen when either device was probed.
>>
>> So, I think it's OK as it is.
>
>
> Surely you can do pm_runtime_get/put on the phy's parent platform
> device and do it that way?
could you please be more specific with that (do you mean pdev->dev.parent)?
so we would use pm_runtime_{get_sync,put} with the parent, while we
would still define the runtime_resume in our driver.

I'd be happy if that works and we can remove that refcounting hack
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux