Skip to contentSkip to conversation

sunxi: musb: Power off OTG port VBUS when disabled

PATCH New Chen-Yu Tsai 3 participants 3 messages Sep 7, 2016 – Sep 17, 2016

Subsystems (4)
Arm (32-bit) USB Arm sub-architectures Arm/Allwinner sunXi SoC support
lore
Share & tools
lore

Chen-Yu Tsai

author
#0 lore raw

PATCH 1/1 sunxi: musb: Power off OTG port VBUS when disabled

The Linux kernel musb driver expects VBUS to be off while initializing musb. Having it on results in a repeating string of warnings, followed by an unusable peripheral. The peripheral is only usable after physically removing the OTG adapter, letting musb reset its state.

This partially reverts commit c9f8947e6604 ("sunxi: usb-phy: Never power off the usb ports")

Signed-off-by Chen-Yu Tsai <[email protected]>
---
 arch/arm/include/asm/arch-sunxi/usb_phy.h |  1 -
 arch/arm/mach-sunxi/usb_phy.c             |  7 -------
 drivers/usb/musb-new/sunxi.c              | 28 ++++++++++++----------------
 3 files changed, 12 insertions(+), 24 deletions(-)
arch/arm/include/asm/arch-sunxi/usb_phy.h +0 -1
index 6a14cad3ff39..cef6c985bc8d 100644
@@ -16,7 +16,6 @@ void sunxi_usb_phy_init(int index);
 void sunxi_usb_phy_exit(int index);
 void sunxi_usb_phy_power_on(int index);
 void sunxi_usb_phy_power_off(int index);
-int sunxi_usb_phy_power_is_on(int index);
 int sunxi_usb_phy_vbus_detect(int index);
 int sunxi_usb_phy_id_detect(int index);
 void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
arch/arm/mach-sunxi/usb_phy.c +0 -7
index f9993d287551..bd1bbee410ba 100644
@@ -296,13 +296,6 @@ void sunxi_usb_phy_power_off(int index)
 		gpio_set_value(phy->gpio_vbus, 0);
 }
 
-int sunxi_usb_phy_power_is_on(int index)
-{
-	struct sunxi_usb_phy *phy = &sunxi_usb_phy[index];
-
-	return phy->power_on_count > 0;
-}
-
 int sunxi_usb_phy_vbus_detect(int index)
 {
 	struct sunxi_usb_phy *phy = &sunxi_usb_phy[index];
drivers/usb/musb-new/sunxi.c +12 -16
index c016a0bb544d..dece7818dc3a 100644
@@ -205,6 +205,8 @@ static struct musb *sunxi_musb;
 
 static int sunxi_musb_enable(struct musb *musb)
 {
+	int ret;
+
 	pr_debug("%s():\n", __func__);
 
 	musb_ep_select(musb->mregs, 0);

@@ -217,26 +219,17 @@ static int sunxi_musb_enable(struct musb *musb)
 	musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0);
 
 	if (is_host_enabled(musb)) {
-		int id = sunxi_usb_phy_id_detect(0);
-
-		if (id == 1 && sunxi_usb_phy_power_is_on(0))
-			sunxi_usb_phy_power_off(0);
-
-		if (!sunxi_usb_phy_power_is_on(0)) {
-			int vbus = sunxi_usb_phy_vbus_detect(0);
-			if (vbus == 1) {
-				printf("A charger is plugged into the OTG: ");
-				return -ENODEV;
-			}
+		ret = sunxi_usb_phy_vbus_detect(0);
+		if (ret == 1) {
+			printf("A charger is plugged into the OTG: ");
+			return -ENODEV;
 		}
-
-		if (id == 1) {
+		ret = sunxi_usb_phy_id_detect(0);
+		if (ret == 1) {
 			printf("No host cable detected: ");
 			return -ENODEV;
 		}
-
-		if (!sunxi_usb_phy_power_is_on(0))
-			sunxi_usb_phy_power_on(0);
+		sunxi_usb_phy_power_on(0); /* port power on */
 	}
 
 	USBC_ForceVbusValidToHigh(musb->mregs);

@@ -252,6 +245,9 @@ static void sunxi_musb_disable(struct musb *musb)
 	if (!enabled)
 		return;
 
+	if (is_host_enabled(musb))
+		sunxi_usb_phy_power_off(0); /* port power off */
+
 	USBC_ForceVbusValidToLow(musb->mregs);
 	mdelay(200); /* Wait for the current session to timeout */
Signature · 1 line
2.9.3
#1 lore raw

PATCH 1/1 sunxi: musb: Power off OTG port VBUS when disabled

Quoted 1 line · Chen-Yu Tsai

The Linux kernel musb driver expects...

I don't much like this line of reasoning/explanation for changes to u- boot. I appreciate that it might seem like all the world is Linux (especially in sunxi context) but the right way to descibe these changes IMHO is "A sensible behaviour for firmware is to ... because ..." i.e. to consider the general case.

Otherwise I might be tempted to say things like...

Quoted 3 lines

VBUS to be off while initializing
musb. Having it on results in a repeating string of warnings, followed

Quoted 1 line

by an unusable peripheral.

... this sounds like it might be a Linux bug, perhaps it should be fixed there?

Ian.

#2 lore raw

PATCH 1/1 sunxi: musb: Power off OTG port VBUS when disabled

In reply to Chen-Yu Tsai

Hi,

Quoted 9 lines · Chen-Yu Tsai

The Linux kernel musb driver expects VBUS to be off while initializing musb. Having it on results in a repeating string of warnings, followed by an unusable peripheral. The peripheral is only usable after physically removing the OTG adapter, letting musb reset its state.

This partially reverts commit c9f8947e6604 ("sunxi: usb-phy: Never power off the usb ports")

Signed-off-by Chen-Yu Tsai <[email protected]>

Thank you, LGTM:

Reviewed-by Hans de Goede <[email protected]>

I've added this to my local queue and this will go out with the pull-req I plan todo later today.

Regards,

Hans

Quoted 94 lines
---
 arch/arm/include/asm/arch-sunxi/usb_phy.h |  1 -
 arch/arm/mach-sunxi/usb_phy.c             |  7 -------
 drivers/usb/musb-new/sunxi.c              | 28 ++++++++++++----------------
 3 files changed, 12 insertions(+), 24 deletions(-)
arch/arm/include/asm/arch-sunxi/usb_phy.h +0 -1
index 6a14cad3ff39..cef6c985bc8d 100644
@@ -16,7 +16,6 @@ void sunxi_usb_phy_init(int index);
 void sunxi_usb_phy_exit(int index);
 void sunxi_usb_phy_power_on(int index);
 void sunxi_usb_phy_power_off(int index);
-int sunxi_usb_phy_power_is_on(int index);
 int sunxi_usb_phy_vbus_detect(int index);
 int sunxi_usb_phy_id_detect(int index);
 void sunxi_usb_phy_enable_squelch_detect(int index, int enable);
arch/arm/mach-sunxi/usb_phy.c +0 -7
index f9993d287551..bd1bbee410ba 100644
@@ -296,13 +296,6 @@ void sunxi_usb_phy_power_off(int index)
 		gpio_set_value(phy->gpio_vbus, 0);
 }
 
-int sunxi_usb_phy_power_is_on(int index)
-{
-	struct sunxi_usb_phy *phy = &sunxi_usb_phy[index];
-
-	return phy->power_on_count > 0;
-}
-
 int sunxi_usb_phy_vbus_detect(int index)
 {
 	struct sunxi_usb_phy *phy = &sunxi_usb_phy[index];
drivers/usb/musb-new/sunxi.c +12 -16
index c016a0bb544d..dece7818dc3a 100644
@@ -205,6 +205,8 @@ static struct musb *sunxi_musb;
 
 static int sunxi_musb_enable(struct musb *musb)
 {
+	int ret;
+
 	pr_debug("%s():\n", __func__);
 
 	musb_ep_select(musb->mregs, 0);

@@ -217,26 +219,17 @@ static int sunxi_musb_enable(struct musb *musb)
 	musb_writeb(musb->mregs, USBC_REG_o_VEND0, 0);
 
 	if (is_host_enabled(musb)) {
-		int id = sunxi_usb_phy_id_detect(0);
-
-		if (id == 1 && sunxi_usb_phy_power_is_on(0))
-			sunxi_usb_phy_power_off(0);
-
-		if (!sunxi_usb_phy_power_is_on(0)) {
-			int vbus = sunxi_usb_phy_vbus_detect(0);
-			if (vbus == 1) {
-				printf("A charger is plugged into the OTG: ");
-				return -ENODEV;
-			}
+		ret = sunxi_usb_phy_vbus_detect(0);
+		if (ret == 1) {
+			printf("A charger is plugged into the OTG: ");
+			return -ENODEV;
 		}
-
-		if (id == 1) {
+		ret = sunxi_usb_phy_id_detect(0);
+		if (ret == 1) {
 			printf("No host cable detected: ");
 			return -ENODEV;
 		}
-
-		if (!sunxi_usb_phy_power_is_on(0))
-			sunxi_usb_phy_power_on(0);
+		sunxi_usb_phy_power_on(0); /* port power on */
 	}
 
 	USBC_ForceVbusValidToHigh(musb->mregs);

@@ -252,6 +245,9 @@ static void sunxi_musb_disable(struct musb *musb)
 	if (!enabled)
 		return;
 
+	if (is_host_enabled(musb))
+		sunxi_usb_phy_power_off(0); /* port power off */
+
 	USBC_ForceVbusValidToLow(musb->mregs);
 	mdelay(200); /* Wait for the current session to timeout */