Re: [PATCH 2/4] phy: rockchip-usb: use rockchip_usb_phy_reset to reset phy during wakeup

Am Montag, 22. August 2016, 17:17:41 schrieb Kishon Vijay Abraham I:

Hi,

On Sunday 21 August 2016 02:02 AM, Randy Li wrote:

quoted

It is a hardware bug in RK3288, the only way to solve it is to
reset the phy.

Signed-off-by: Randy Li <redacted>
---

 drivers/phy/phy-rockchip-usb.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/phy/phy-rockchip-usb.c

b/drivers/phy/phy-rockchip-usb.c index 2a7381f..734987f 100644

--- a/drivers/phy/phy-rockchip-usb.c
+++ b/drivers/phy/phy-rockchip-usb.c

@@ -29,6 +29,7 @@

 #include <linux/reset.h>
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>

+#include <linux/delay.h>

 static int enable_usb_uart;

@@ -64,6 +65,7 @@ struct rockchip_usb_phy {

 	struct clk_hw	clk480m_hw;
 	struct phy	*phy;
 	bool		uart_enabled;

+	struct reset_control *reset;

 };
 
 static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,

@@ -144,9 +146,23 @@ static int rockchip_usb_phy_power_on(struct phy

*_phy)

 	return clk_prepare_enable(phy->clk480m);
 
 }

+static int rockchip_usb_phy_reset(struct phy *_phy)
+{
+	struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
+
+	if (phy->reset) {
+		reset_control_assert(phy->reset);
+		udelay(10);
+		reset_control_deassert(phy->reset);
+	}
+
+	return 0;
+}
+

 static const struct phy_ops ops = {
 
 	.power_on	= rockchip_usb_phy_power_on,
 	.power_off	= rockchip_usb_phy_power_off,

+	.reset		= rockchip_usb_phy_reset,

why not just reuse the .init ops? reset can be done during initialization
right?

The naming of power_on + power_off and init + exit probably suggests that they 
are supposed to be used in pairs. (aka module_init + module_exit and probably 
more)

But in fact I've seen different combinations so far (phy_init + phy_power_on 
... phy_power_off + phy_exit but also phy_power_on + phy_init ... phy_exit + 
phy_power_off), so I guess the semantics are not that strictly defined.