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?
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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
