New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 770412 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

kevin: sometimes seeing "wait pma ready timeout" from type C PHY, leading to bad behavior

Project Member Reported by diand...@chromium.org, Sep 29 2017

Issue description

While poking around with type C devices and plugging and unplugging a bunch, a few times I saw:

  rockchip-typec-phy ff800000.phy: wait pma ready timeout
  phy phy-ff800000.phy.9: phy poweron failed --> -110
  dwc3 fe900000.dwc3: failed to initialize core
  dwc3: probe of fe900000.dwc3 failed with error -110
  rockchip-dwc3 usb@fe900000: USB HOST connected

Once that happened everything was in pretty bad shape.

---

I'm not totally sure why the "pma ready timeout" happened in the first place but once it happened the port was pretty borked, even if I unplugged and re-plugged the device.

---

I dug into this and found at least two things:

1. The error handling in the Rockchip Type C PHY is lacking in this case.  I'll post a patch soon.

2. There's a bug in the error handling in the dwc3 code that's since been fixed upstream.  I'll post that soon too.

---

I still have no idea if we'd ever expect the "pma ready timeout" in real life when not plugging / unplugging problematic dongles constantly.  If we do we can dig into it more, but at least let's more the error handling robust.
 
Posted these:

* https://chromium-review.googlesource.com/#/c/chromiumos/third_party/kernel/+/697887 
  UPSTREAM: usb: dwc3: Fix error handling for core init        

* https://chromium-review.googlesource.com/#/c/chromiumos/third_party/kernel/+/697888 
  FROMGIT: phy: rockchip-typec: Check for errors from tcphy_phy_init()        

* https://chromium-review.googlesource.com/#/c/chromiumos/third_party/kernel/+/697889 
  TEST-ONLY: phy: rockchip-typec: Add a fake failure        

I'll do one more check once I'm in the office and then mark the first two ready to go in.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 5 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2f98de8461cb68dbd73b3be1828d42e10c39c863

commit 2f98de8461cb68dbd73b3be1828d42e10c39c863
Author: Vivek Gautam <vivek.gautam@codeaurora.org>
Date: Thu Oct 05 07:34:10 2017

UPSTREAM: usb: dwc3: Fix error handling for core init

Fixing the sequence of events in dwc3_core_init() error exit path.
dwc3_core_exit() call is also removed from the error path since,
whatever it's doing is already done.

BUG= chromium:770412 
TEST=Fake PHY failures and see that things recover now

Change-Id: I9dc05e6835bb77df1e4d2f18ded0a01fce3bffeb
Fixes: c499ff7 usb: dwc3: core: re-factor init and exit paths
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Stable <stable@vger.kernel.org> # 4.8+
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 9b9d7cdd0a20a8c26a022604580f93516ad69c36)
Reviewed-on: https://chromium-review.googlesource.com/697887
Reviewed-by: Guenter Roeck <groeck@chromium.org>

[modify] https://crrev.com/2f98de8461cb68dbd73b3be1828d42e10c39c863/drivers/usb/dwc3/core.c

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2b6395d9d63488ac2825a67a7178671c936808c4

commit 2b6395d9d63488ac2825a67a7178671c936808c4
Author: Douglas Anderson <dianders@chromium.org>
Date: Thu Oct 05 07:34:11 2017

FROMGIT: phy: rockchip-typec: Check for errors from tcphy_phy_init()

The function tcphy_phy_init() could return an error but the callers
weren't checking the return value.  They should.  In at least one case
while testing I saw the message "wait pma ready timeout" which
indicates that tcphy_phy_init() really could return an error and we
should account for it.

BUG= chromium:770412 
TEST=Fake PHY failures and see that things recover now

Change-Id: I4925bbe8a24b00f485a1eaee2feec661512dc0cc
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
(cherry picked from git.kernel.org kishon/linux-phy.git fixes
 commit 2fb850092fd95198a0a4746f07b80077d5a3aa37)
Reviewed-on: https://chromium-review.googlesource.com/697888

[modify] https://crrev.com/2b6395d9d63488ac2825a67a7178671c936808c4/drivers/phy/phy-rockchip-typec.c

Not super critical, but if there's still time to get this in M-62 it will probably fix a person or two.
Labels: M-62 Merge-Request-62
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 5 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: We are only 11 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-62 Merge-Approved-62
Approved for 62. 
Labels: -Merge-Approved-62 Merge-Merged
Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 5 2017

Labels: merge-merged-release-R62-9901.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/05660502a07d9f5cf7eeec5e4b9a6fce4d6bd11b

commit 05660502a07d9f5cf7eeec5e4b9a6fce4d6bd11b
Author: Vivek Gautam <vivek.gautam@codeaurora.org>
Date: Thu Oct 05 18:39:29 2017

UPSTREAM: usb: dwc3: Fix error handling for core init

Fixing the sequence of events in dwc3_core_init() error exit path.
dwc3_core_exit() call is also removed from the error path since,
whatever it's doing is already done.

BUG= chromium:770412 
TEST=Fake PHY failures and see that things recover now

Change-Id: I9dc05e6835bb77df1e4d2f18ded0a01fce3bffeb
Fixes: c499ff7 usb: dwc3: core: re-factor init and exit paths
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Stable <stable@vger.kernel.org> # 4.8+
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
(cherry picked from commit 9b9d7cdd0a20a8c26a022604580f93516ad69c36)
Reviewed-on: https://chromium-review.googlesource.com/697887
Reviewed-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 2f98de8461cb68dbd73b3be1828d42e10c39c863)
Reviewed-on: https://chromium-review.googlesource.com/702735

[modify] https://crrev.com/05660502a07d9f5cf7eeec5e4b9a6fce4d6bd11b/drivers/usb/dwc3/core.c

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/81edf24597794cc0ee7b523ac206d33dd33a4fb3

commit 81edf24597794cc0ee7b523ac206d33dd33a4fb3
Author: Douglas Anderson <dianders@chromium.org>
Date: Thu Oct 05 18:39:57 2017

FROMGIT: phy: rockchip-typec: Check for errors from tcphy_phy_init()

The function tcphy_phy_init() could return an error but the callers
weren't checking the return value.  They should.  In at least one case
while testing I saw the message "wait pma ready timeout" which
indicates that tcphy_phy_init() really could return an error and we
should account for it.

BUG= chromium:770412 
TEST=Fake PHY failures and see that things recover now

Change-Id: I4925bbe8a24b00f485a1eaee2feec661512dc0cc
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
(cherry picked from git.kernel.org kishon/linux-phy.git fixes
 commit 2fb850092fd95198a0a4746f07b80077d5a3aa37)
Reviewed-on: https://chromium-review.googlesource.com/697888
(cherry picked from commit 2b6395d9d63488ac2825a67a7178671c936808c4)
Reviewed-on: https://chromium-review.googlesource.com/703016

[modify] https://crrev.com/81edf24597794cc0ee7b523ac206d33dd33a4fb3/drivers/phy/phy-rockchip-typec.c

Sign in to add a comment