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

On 08/09/16 20:52, Martin Blumenstingl wrote:

On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman [off-list ref] wrote:

quoted

quoted

+     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.

So, in phy_meson_usb2_power_on() you could do:

pm_runtime_get_sync(pdev);

and in phy_meson_usb2_power_off

pm_runtime_put(pdev);

https://www.kernel.org/doc/Documentation/power/runtime_pm.txt

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html