Skip to contentSkip to conversation

usb: chipidea: tegra: Delay PHY suspending

PATCH In review Dmitry Osipenko 2 participants 4 messages Jun 9, 2021 – Jun 12, 2021

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 usb: chipidea: tegra: Delay PHY suspending

The ChipIdea driver enters into suspend immediately after seeing a VBUS disconnection. Some devices need an extra delay after losing VBUS, otherwise VBUS may be floating, preventing the PHY's suspending by the VBUS detection sensors. This problem was found on Tegra30 Asus Transformer TF700T tablet device, where the USB PHY wakes up immediately from suspend because VBUS sensor continues to detect VBUS as active after disconnection. A minimum delay of 20ms is needed in order to fix this issue, hence add 25ms delay before suspending the PHY.

---
 drivers/usb/chipidea/ci_hdrc_tegra.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/usb/chipidea/ci_hdrc_tegra.c +8 -0
index 60361141ac04..d1359b76a0e8 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -255,6 +256,13 @@ static int tegra_ehci_hub_control(struct ci_hdrc *ci, u16 typeReq, u16 wValue,
 
 static void tegra_usb_enter_lpm(struct ci_hdrc *ci, bool enable)
 {
+	/*
+	 * Give hardware time to settle down after VBUS disconnection,
+	 * otherwise PHY may wake up from suspend immediately.
+	 */
+	if (enable)
+		msleep(25);
+
 	/*
 	 * Touching any register which belongs to AHB clock domain will
 	 * hang CPU if USB controller is put into low power mode because
Signature · 1 line
2.30.2
#1 lore raw

09.06.2021 15:04, Dmitry Osipenko пишет:

Quoted 33 lines

The ChipIdea driver enters into suspend immediately after seeing a VBUS disconnection. Some devices need an extra delay after losing VBUS, otherwise VBUS may be floating, preventing the PHY's suspending by the VBUS detection sensors. This problem was found on Tegra30 Asus Transformer TF700T tablet device, where the USB PHY wakes up immediately from suspend because VBUS sensor continues to detect VBUS as active after disconnection. A minimum delay of 20ms is needed in order to fix this issue, hence add 25ms delay before suspending the PHY.

---
 drivers/usb/chipidea/ci_hdrc_tegra.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/usb/chipidea/ci_hdrc_tegra.c +1 -0
index 60361141ac04..d1359b76a0e8 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -255,6 +256,13 @@ static int tegra_ehci_hub_control(struct ci_hdrc *ci, u16 typeReq, u16 wValue,
 
 static void tegra_usb_enter_lpm(struct ci_hdrc *ci, bool enable)
 {

drivers/usb/chipidea/ci_hdrc_tegra.c static int tegra_ehci_hub_control(struct ci_hdrc *ci, u16 typeReq, u16 wValue, · line 268

Quoted 10 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +7 -0
+	/*
+	 * Give hardware time to settle down after VBUS disconnection,
+	 * otherwise PHY may wake up from suspend immediately.
+	 */
+	if (enable)
+		msleep(25);
+
 	/*
 	 * Touching any register which belongs to AHB clock domain will
 	 * hang CPU if USB controller is put into low power mode because

I missed that Peter's email address changed. Making this reply to the new address for visibility.

maintainer
#2 lore raw
In reply to Dmitry Osipenko
Quoted 29 lines · Dmitry Osipenko

The ChipIdea driver enters into suspend immediately after seeing a VBUS disconnection. Some devices need an extra delay after losing VBUS, otherwise VBUS may be floating, preventing the PHY's suspending by the VBUS detection sensors. This problem was found on Tegra30 Asus Transformer TF700T tablet device, where the USB PHY wakes up immediately from suspend because VBUS sensor continues to detect VBUS as active after disconnection. A minimum delay of 20ms is needed in order to fix this issue, hence add 25ms delay before suspending the PHY.

---
 drivers/usb/chipidea/ci_hdrc_tegra.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/usb/chipidea/ci_hdrc_tegra.c +1 -0
index 60361141ac04..d1359b76a0e8 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_device.h>

drivers/usb/chipidea/ci_hdrc_tegra.c static int tegra_ehci_hub_control(struct ci_hdrc *ci, u16 typeReq, u16 wValue, · line 265

Quoted 10 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +7 -0
@@ -255,6 +256,13 @@ static int tegra_ehci_hub_control(struct ci_hdrc *ci, u16 typeReq, u16 wValue,
 
 static void tegra_usb_enter_lpm(struct ci_hdrc *ci, bool enable)
 {
+	/*
+	 * Give hardware time to settle down after VBUS disconnection,
+	 * otherwise PHY may wake up from suspend immediately.
+	 */
+	if (enable)
+		msleep(25);
+

How could you know 25ms is enough for other Tegra designs? Could you poll VBUS wakeup threshold register to ensure the wakeup will not occur? The similar design exists at function:
hw_wait_vbus_lower_bsv.

Signature · 2 lines
Thanks,
Peter Chen
#3 lore raw

12.06.2021 10:34, Peter Chen пишет:

Quoted 31 lines · Dmitry Osipenko
Quoted 30 lines

The ChipIdea driver enters into suspend immediately after seeing a VBUS disconnection. Some devices need an extra delay after losing VBUS, otherwise VBUS may be floating, preventing the PHY's suspending by the VBUS detection sensors. This problem was found on Tegra30 Asus Transformer TF700T tablet device, where the USB PHY wakes up immediately from suspend because VBUS sensor continues to detect VBUS as active after disconnection. A minimum delay of 20ms is needed in order to fix this issue, hence add 25ms delay before suspending the PHY.

---
 drivers/usb/chipidea/ci_hdrc_tegra.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/usb/chipidea/ci_hdrc_tegra.c +1 -0
index 60361141ac04..d1359b76a0e8 100644
@@ -4,6 +4,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -255,6 +256,13 @@ static int tegra_ehci_hub_control(struct ci_hdrc *ci, u16 typeReq, u16 wValue,
 
Quoted 11 lines

drivers/usb/chipidea/ci_hdrc_tegra.c static int tegra_ehci_hub_control(struct ci_hdrc *ci, u16 typeReq, u16 wValue, · line 265

Quoted 9 lines
drivers/usb/chipidea/ci_hdrc_tegra.c +7 -0
 static void tegra_usb_enter_lpm(struct ci_hdrc *ci, bool enable)
 {
+	/*
+	 * Give hardware time to settle down after VBUS disconnection,
+	 * otherwise PHY may wake up from suspend immediately.
+	 */
+	if (enable)
+		msleep(25);
+

How could you know 25ms is enough for other Tegra designs?

I don't know what is the maximum timeout could be, but it shouldn't be a problem to bump the timeout if somebody will report the need to do so.

Quoted 2 lines

Could you poll VBUS wakeup threshold register to ensure the wakeup will not occur?

We indeed can poll the wakeup threshold status in the PHY driver, it works too. I'll make the patch for the PHY driver, thank you for the suggestion.

Quoted 2 lines

The similar design exists at function:
hw_wait_vbus_lower_bsv.

The hw_wait_vbus_lower_bsv uses 5sec timeout, which should be too much. I'll set the polling timeout to 100ms.