[PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[Kamil Debski]

The Exynos4412 USB 2.0 PHY hardware differs from the description provided
in the documentation. Some register bits have different function. This
patch fixes the defines of register bits and changes the way how phys are
powered on and off.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
---
 drivers/phy/phy-exynos4x12-usb2.c |  112 +++++++++++++++++++++++++------------
 drivers/phy/phy-samsung-usb2.h    |    3 +-
 2 files changed, 77 insertions(+), 38 deletions(-)

diff --git a/drivers/phy/phy-exynos4x12-usb2.c b/drivers/phy/phy-exynos4x12-usb2.c
index d92a7cc..59d8dd3 100644
--- a/drivers/phy/phy-exynos4x12-usb2.c
+++ b/drivers/phy/phy-exynos4x12-usb2.c
@@ -86,13 +86,23 @@
 #define EXYNOS_4x12_URSTCON_OTG_HLINK		BIT(1)
 #define EXYNOS_4x12_URSTCON_OTG_PHYLINK		BIT(2)
 #define EXYNOS_4x12_URSTCON_HOST_PHY		BIT(3)
+/* The following bit defines are presented in the
+ * order taken from the Exynos4412 reference manual.
+ *
+ * During experiments with the hardware and debugging
+ * it was determined that the hardware behaves contrary
+ * to the manual.
+ *
+ * The following bit values were chaned accordingly to the
+ * results of real hardware experiments.
+ */
 #define EXYNOS_4x12_URSTCON_PHY1		BIT(4)
-#define EXYNOS_4x12_URSTCON_HSIC0		BIT(5)
-#define EXYNOS_4x12_URSTCON_HSIC1		BIT(6)
+#define EXYNOS_4x12_URSTCON_HSIC0		BIT(6)
+#define EXYNOS_4x12_URSTCON_HSIC1		BIT(5)
 #define EXYNOS_4x12_URSTCON_HOST_LINK_ALL	BIT(7)
-#define EXYNOS_4x12_URSTCON_HOST_LINK_P0	BIT(8)
+#define EXYNOS_4x12_URSTCON_HOST_LINK_P0	BIT(10)
 #define EXYNOS_4x12_URSTCON_HOST_LINK_P1	BIT(9)
-#define EXYNOS_4x12_URSTCON_HOST_LINK_P2	BIT(10)
+#define EXYNOS_4x12_URSTCON_HOST_LINK_P2	BIT(8)
 
 /* Isolation, configured in the power management unit */
 #define EXYNOS_4x12_USB_ISOL_OFFSET		0x704
@@ -188,6 +198,7 @@ static void exynos4x12_setup_clk(struct samsung_usb2_phy_instance *inst)
 	clk = readl(drv->reg_phy + EXYNOS_4x12_UPHYCLK);
 	clk &= ~EXYNOS_4x12_UPHYCLK_PHYFSEL_MASK;
 	clk |= drv->ref_reg_val << EXYNOS_4x12_UPHYCLK_PHYFSEL_OFFSET;
+	clk |= EXYNOS_4x12_UPHYCLK_PHY1_COMMON_ON;
 	writel(clk, drv->reg_phy + EXYNOS_4x12_UPHYCLK);
 }
 
@@ -198,27 +209,22 @@ static void exynos4x12_phy_pwr(struct samsung_usb2_phy_instance *inst, bool on)
 	u32 phypwr = 0;
 	u32 rst;
 	u32 pwr;
-	u32 mode = 0;
-	u32 switch_mode = 0;
 
 	switch (inst->cfg->id) {
 	case EXYNOS4x12_DEVICE:
 		phypwr =	EXYNOS_4x12_UPHYPWR_PHY0;
 		rstbits =	EXYNOS_4x12_URSTCON_PHY0;
-		mode =		EXYNOS_4x12_MODE_SWITCH_DEVICE;
-		switch_mode =	1;
 		break;
 	case EXYNOS4x12_HOST:
 		phypwr =	EXYNOS_4x12_UPHYPWR_PHY1;
-		rstbits =	EXYNOS_4x12_URSTCON_HOST_PHY;
-		mode =		EXYNOS_4x12_MODE_SWITCH_HOST;
-		switch_mode =	1;
+		rstbits =	EXYNOS_4x12_URSTCON_HOST_PHY |
+				EXYNOS_4x12_URSTCON_PHY1 |
+				EXYNOS_4x12_URSTCON_HOST_LINK_P0;
 		break;
 	case EXYNOS4x12_HSIC0:
 		phypwr =	EXYNOS_4x12_UPHYPWR_HSIC0;
-		rstbits =	EXYNOS_4x12_URSTCON_HSIC1 |
-				EXYNOS_4x12_URSTCON_HOST_LINK_P0 |
-				EXYNOS_4x12_URSTCON_HOST_PHY;
+		rstbits =	EXYNOS_4x12_URSTCON_HSIC0 |
+				EXYNOS_4x12_URSTCON_HOST_LINK_P1 ;
 		break;
 	case EXYNOS4x12_HSIC1:
 		phypwr =	EXYNOS_4x12_UPHYPWR_HSIC1;
@@ -228,11 +234,6 @@ static void exynos4x12_phy_pwr(struct samsung_usb2_phy_instance *inst, bool on)
 	};
 
 	if (on) {
-		if (switch_mode)
-			regmap_update_bits(drv->reg_sys,
-					   EXYNOS_4x12_MODE_SWITCH_OFFSET,
-					   EXYNOS_4x12_MODE_SWITCH_MASK, mode);
-
 		pwr = readl(drv->reg_phy + EXYNOS_4x12_UPHYPWR);
 		pwr &= ~phypwr;
 		writel(pwr, drv->reg_phy + EXYNOS_4x12_UPHYPWR);
@@ -253,41 +254,78 @@ static void exynos4x12_phy_pwr(struct samsung_usb2_phy_instance *inst, bool on)
 	}
 }
 
-static int exynos4x12_power_on(struct samsung_usb2_phy_instance *inst)
+static void exynos4x12_power_on_internal(struct samsung_usb2_phy_instance *inst)
 {
-	struct samsung_usb2_phy_driver *drv = inst->drv;
+	if (inst->int_cnt++ > 0)
+		return;
 
-	inst->enabled = 1;
 	exynos4x12_setup_clk(inst);
-	exynos4x12_phy_pwr(inst, 1);
 	exynos4x12_isol(inst, 0);
+	exynos4x12_phy_pwr(inst, 1);
+}
+
+static int exynos4x12_power_on(struct samsung_usb2_phy_instance *inst)
+{
+	struct samsung_usb2_phy_driver *drv = inst->drv;
+
+	if (inst->ext_cnt++ > 0)
+		return 0;
 
-	/* Power on the device, as it is necessary for HSIC to work */
-	if (inst->cfg->id == EXYNOS4x12_HSIC0) {
-		struct samsung_usb2_phy_instance *device =
-					&drv->instances[EXYNOS4x12_DEVICE];
-		exynos4x12_phy_pwr(device, 1);
-		exynos4x12_isol(device, 0);
+	if (inst->cfg->id == EXYNOS4x12_HOST) {
+		regmap_update_bits(drv->reg_sys, EXYNOS_4x12_MODE_SWITCH_OFFSET,
+						EXYNOS_4x12_MODE_SWITCH_MASK,
+						EXYNOS_4x12_MODE_SWITCH_HOST);
+		exynos4x12_power_on_internal(&drv->instances[EXYNOS4x12_DEVICE]);
 	}
 
+	if (inst->cfg->id == EXYNOS4x12_DEVICE)
+		regmap_update_bits(drv->reg_sys, EXYNOS_4x12_MODE_SWITCH_OFFSET,
+						EXYNOS_4x12_MODE_SWITCH_MASK,
+						EXYNOS_4x12_MODE_SWITCH_DEVICE);
+
+	if (inst->cfg->id == EXYNOS4x12_HSIC0 ||
+		inst->cfg->id == EXYNOS4x12_HSIC1) {
+		exynos4x12_power_on_internal(&drv->instances[EXYNOS4x12_DEVICE]);
+		exynos4x12_power_on_internal(&drv->instances[EXYNOS4x12_HOST]);
+	}
+
+	exynos4x12_power_on_internal(inst);
+
 	return 0;
 }
 
-static int exynos4x12_power_off(struct samsung_usb2_phy_instance *inst)
+static void exynos4x12_power_off_internal(struct samsung_usb2_phy_instance *inst)
 {
-	struct samsung_usb2_phy_driver *drv = inst->drv;
-	struct samsung_usb2_phy_instance *device =
-					&drv->instances[EXYNOS4x12_DEVICE];
+	if (inst->int_cnt-- > 1)
+		return;
 
-	inst->enabled = 0;
 	exynos4x12_isol(inst, 1);
 	exynos4x12_phy_pwr(inst, 0);
+}
 
-	if (inst->cfg->id == EXYNOS4x12_HSIC0 && !device->enabled) {
-		exynos4x12_isol(device, 1);
-		exynos4x12_phy_pwr(device, 0);
+static int exynos4x12_power_off(struct samsung_usb2_phy_instance *inst)
+{
+	struct samsung_usb2_phy_driver *drv = inst->drv;
+
+	if (inst->ext_cnt-- > 1)
+		return 0;
+
+	if (inst->cfg->id == EXYNOS4x12_DEVICE)
+		regmap_update_bits(drv->reg_sys, EXYNOS_4x12_MODE_SWITCH_OFFSET,
+						EXYNOS_4x12_MODE_SWITCH_MASK,
+						EXYNOS_4x12_MODE_SWITCH_HOST);
+
+	if (inst->cfg->id == EXYNOS4x12_HOST)
+		exynos4x12_power_off_internal(&drv->instances[EXYNOS4x12_DEVICE]);
+
+	if (inst->cfg->id == EXYNOS4x12_HSIC0 ||
+		inst->cfg->id == EXYNOS4x12_HSIC1) {
+		exynos4x12_power_off_internal(&drv->instances[EXYNOS4x12_DEVICE]);
+		exynos4x12_power_off_internal(&drv->instances[EXYNOS4x12_HOST]);
 	}
 
+	exynos4x12_power_off_internal(inst);
+
 	return 0;
 }
 
diff --git a/drivers/phy/phy-samsung-usb2.h b/drivers/phy/phy-samsung-usb2.h
index 45b3170..9188478 100644
--- a/drivers/phy/phy-samsung-usb2.h
+++ b/drivers/phy/phy-samsung-usb2.h
@@ -29,7 +29,8 @@ struct samsung_usb2_phy_instance {
 	const struct samsung_usb2_common_phy *cfg;
 	struct phy *phy;
 	struct samsung_usb2_phy_driver *drv;
-	bool enabled;
+	int int_cnt;
+	int ext_cnt;
 };
 
 struct samsung_usb2_phy_driver {
-- 
1.7.9.5

Re: [PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[Daniel Drake]

On Tue, Jun 24, 2014 at 1:54 PM, Kamil Debski <k.debski@samsung.com> wrote:
> The Exynos4412 USB 2.0 PHY hardware differs from the description provided
> in the documentation. Some register bits have different function. This
> patch fixes the defines of register bits and changes the way how phys are
> powered on and off.

I guess this replaces the patch titled "drivers: phy: exynos4x12-phy:
fix HSIC1 power on/off sequence"

Tested on ODROID-U2. Seems to be working as well as the previous patch:
- Internal SMSC hub works on boot and after reboot, tested with USB mouse
- Internal SMSC ethernet device works on boot, but disappears upon
reboot. (same as previous patch, also reproduced by Marek)

Thanks,
Daniel

RE: [PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[Kamil Debski]

Hi Daniel,

> From: Daniel Drake [mailto:drake@endlessm.com]
> Sent: Tuesday, June 24, 2014 5:09 PM
> 
> On Tue, Jun 24, 2014 at 1:54 PM, Kamil Debski <k.debski@samsung.com>
> wrote:
> > The Exynos4412 USB 2.0 PHY hardware differs from the description
> > provided in the documentation. Some register bits have different
> > function. This patch fixes the defines of register bits and changes
> > the way how phys are powered on and off.
> 
> I guess this replaces the patch titled "drivers: phy: exynos4x12-phy:
> fix HSIC1 power on/off sequence"

Yes, indeed it replaces this patch. I did some more research on how the
hardware actually behaves.

> 
> Tested on ODROID-U2. Seems to be working as well as the previous patch:

Thank you very much for testing.

> - Internal SMSC hub works on boot and after reboot, tested with USB
> mouse
> - Internal SMSC ethernet device works on boot, but disappears upon
> reboot. (same as previous patch, also reproduced by Marek)

By reboot I guess that you mean typing "reboot" or by using SysRq magic
and not power cycling?

If so, I had experienced the same symptoms. I guess that the Ethernet
chip is not reset properly and fails to enumerate without power cycling
(it's nRESET pin is connected to P3V3).

I found that removing regulator-always-on from buck8_reg: BUCK8 in the
dts file fixes this problem.

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland

Re: [PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[Marek Szyprowski]

Hello,

On 2014-06-24 17:35, Kamil Debski wrote:
> On Tue, Jun 24, 2014 at 1:54 PM, Kamil Debski <k.debski@samsung.com> 
> wrote:
>>> The Exynos4412 USB 2.0 PHY hardware differs from the description
>>> provided in the documentation. Some register bits have different
>>> function. This patch fixes the defines of register bits and changes
>>> the way how phys are powered on and off.
>> I guess this replaces the patch titled "drivers: phy: exynos4x12-phy:
>> fix HSIC1 power on/off sequence"
> Yes, indeed it replaces this patch. I did some more research on how the
> hardware actually behaves.

My patch was a quick hack to get HSIC1 working. Kamil spent much more time
trying to figure out how the hardware actually behaves. Please drop my patch
and replace it with this one.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

>
>> Tested on ODROID-U2. Seems to be working as well as the previous patch:
> Thank you very much for testing.
>
>> - Internal SMSC hub works on boot and after reboot, tested with USB
>> mouse
>> - Internal SMSC ethernet device works on boot, but disappears upon
>> reboot. (same as previous patch, also reproduced by Marek)
> By reboot I guess that you mean typing "reboot" or by using SysRq magic
> and not power cycling?
>
> If so, I had experienced the same symptoms. I guess that the Ethernet
> chip is not reset properly and fails to enumerate without power cycling
> (it's nRESET pin is connected to P3V3).
>
> I found that removing regulator-always-on from buck8_reg: BUCK8 in the
> dts file fixes this problem.

Thanks for investigating this issue! I will add this fix to next version
of Odroid patches.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[Daniel Drake]

On Tue, Jun 24, 2014 at 4:35 PM, Kamil Debski <k.debski@samsung.com> wrote:
> By reboot I guess that you mean typing "reboot" or by using SysRq magic
> and not power cycling?
>
> If so, I had experienced the same symptoms. I guess that the Ethernet
> chip is not reset properly and fails to enumerate without power cycling
> (it's nRESET pin is connected to P3V3).
>
> I found that removing regulator-always-on from buck8_reg: BUCK8 in the
> dts file fixes this problem.

Yes, that fixes the problem. Thanks!

Tested-by: Daniel Drake <drake@endlessm.com>

Re: [PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[Kishon Vijay Abraham I]

Hi Kamil,

On Tuesday 24 June 2014 06:24 PM, Kamil Debski wrote:
> The Exynos4412 USB 2.0 PHY hardware differs from the description provided
> in the documentation. Some register bits have different function. This
> patch fixes the defines of register bits and changes the way how phys are
> powered on and off.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>

Can you fix these checkpatch warnings?

WARNING: space prohibited before semicolon
#149: FILE: drivers/phy/phy-exynos4x12-usb2.c:227:
+				EXYNOS_4x12_URSTCON_HOST_LINK_P1 ;

WARNING: line over 80 characters
#200: FILE: drivers/phy/phy-exynos4x12-usb2.c:278:
+		exynos4x12_power_on_internal(&drv->instances[EXYNOS4x12_DEVICE]);

WARNING: line over 80 characters
#210: FILE: drivers/phy/phy-exynos4x12-usb2.c:288:
+		exynos4x12_power_on_internal(&drv->instances[EXYNOS4x12_DEVICE]);

WARNING: line over 80 characters
#220: FILE: drivers/phy/phy-exynos4x12-usb2.c:297:
+static void exynos4x12_power_off_internal(struct samsung_usb2_phy_instance *inst)

WARNING: line over 80 characters
#249: FILE: drivers/phy/phy-exynos4x12-usb2.c:319:
+		exynos4x12_power_off_internal(&drv->instances[EXYNOS4x12_DEVICE]);

WARNING: line over 80 characters
#253: FILE: drivers/phy/phy-exynos4x12-usb2.c:323:
+		exynos4x12_power_off_internal(&drv->instances[EXYNOS4x12_DEVICE]);

total: 0 errors, 6 warnings, 182 lines checked

Thanks
Kishon

Re: [PATCH] phy: phy-samsung-usb2: Change phy power on/power off sequence

[Kishon Vijay Abraham I]

Hi Kamil,

On Tuesday 24 June 2014 06:24 PM, Kamil Debski wrote:
> The Exynos4412 USB 2.0 PHY hardware differs from the description provided
> in the documentation. Some register bits have different function. This
> patch fixes the defines of register bits and changes the way how phys are
> powered on and off.
> 
> Signed-off-by: Kamil Debski <k.debski@samsung.com>

Can you fix these checkpatch warnings?

WARNING: space prohibited before semicolon
#149: FILE: drivers/phy/phy-exynos4x12-usb2.c:227:
+				EXYNOS_4x12_URSTCON_HOST_LINK_P1 ;

WARNING: line over 80 characters
#200: FILE: drivers/phy/phy-exynos4x12-usb2.c:278:
+		exynos4x12_power_on_internal(&drv->instances[EXYNOS4x12_DEVICE]);

WARNING: line over 80 characters
#210: FILE: drivers/phy/phy-exynos4x12-usb2.c:288:
+		exynos4x12_power_on_internal(&drv->instances[EXYNOS4x12_DEVICE]);

WARNING: line over 80 characters
#220: FILE: drivers/phy/phy-exynos4x12-usb2.c:297:
+static void exynos4x12_power_off_internal(struct samsung_usb2_phy_instance *inst)

WARNING: line over 80 characters
#249: FILE: drivers/phy/phy-exynos4x12-usb2.c:319:
+		exynos4x12_power_off_internal(&drv->instances[EXYNOS4x12_DEVICE]);

WARNING: line over 80 characters
#253: FILE: drivers/phy/phy-exynos4x12-usb2.c:323:
+		exynos4x12_power_off_internal(&drv->instances[EXYNOS4x12_DEVICE]);

total: 0 errors, 6 warnings, 182 lines checked

Thanks
Kishon