Hi Doug, 2017-12-11 22:45 GMT+01:00 Douglas Anderson <dianders@xxxxxxxxxxxx>: > Bind / unbind stress testing of the USB controller on rk3399 found > that we'd often end up with lots of failures that looked like this: > > 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 > > Those errors were sometimes seen at bootup too, in which case USB > peripherals wouldn't work until unplugged and re-plugged in. > > I spent some time trying to figure out why the PHY was failing to > power on but I wasn't able to. Possibly this has to do with the fact > that the PHY docs say that the USB controller "needs to be held in > reset to hold pipe power state in P2 before initializing the Type C > PHY" but that doesn't appear to be easy to do with the dwc3 driver > today. Messing around with the ordering of the reset vs. the PHY > initialization in the dwc3 driver didn't seem to fix things. > > I did, however, find that if I simply retry the power on it seems to > have a good chance of working. So let's add some retries. I ran a > pretty tight bind/unbind loop overnight. When I did so, I found that > I need to retry between 1% and 2% of the time. Overnight I found only > a small handful of times where I needed 2 retries. I never found a > case where I needed 3 retries. > > I'm completely aware of the fact that this is quite an ugly hack and I > wish I didn't have to resort to it, but I have no other real idea how > to make this hardware reliable. If Rockchip in the future can come up > with a solution we can always revert this hack. Until then, let's at > least have something that works. > > This patch is tested atop Enric's latest dwc3 patch series ending at: > https://patchwork.kernel.org/patch/10095527/ > ...but it could be applied independently of that series without any > bad effects. > > For some more details on this bug, you can refer to: > https://bugs.chromium.org/p/chromium/issues/detail?id=783464 > > Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> > --- > > drivers/phy/rockchip/phy-rockchip-typec.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c > index ee85fa0ca4b0..5c2157156ce1 100644 > --- a/drivers/phy/rockchip/phy-rockchip-typec.c > +++ b/drivers/phy/rockchip/phy-rockchip-typec.c > @@ -349,6 +349,8 @@ > #define MODE_DFP_USB BIT(1) > #define MODE_DFP_DP BIT(2) > > +#define POWER_ON_TRIES 5 > + I did the test of increase the number of tries to 100 because unfortunately, even with this patch applied, I can see the problem on my kevin with current mainline. [ 244.309094] rockchip-typec-phy ff800000.phy: Turn on failed after 100 loops That's an extra debug print ^ [ 244.317019] phy phy-ff800000.phy.8: phy poweron failed --> -110 [ 244.323824] dwc3 fe900000.dwc3: failed to initialize core [ 244.330057] dwc3: probe of fe900000.dwc3 failed with error -110 So I'm wondering if there is something else that I need to apply to really fix this as you didn't reproduce the issue doing lots of tests and I can reproduce the issue very easily. > struct usb3phy_reg { > u32 offset; > u32 enable_bit; > @@ -818,9 +820,8 @@ static int tcphy_get_mode(struct rockchip_typec_phy *tcphy) > return mode; > } > > -static int rockchip_usb3_phy_power_on(struct phy *phy) > +static int _rockchip_usb3_phy_power_on(struct rockchip_typec_phy *tcphy) > { > - struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy); > struct rockchip_usb3phy_port_cfg *cfg = &tcphy->port_cfgs; > const struct usb3phy_reg *reg = &cfg->pipe_status; > int timeout, new_mode, ret = 0; > @@ -867,6 +868,25 @@ static int rockchip_usb3_phy_power_on(struct phy *phy) > return ret; > } > > +static int rockchip_usb3_phy_power_on(struct phy *phy) > +{ > + struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy); > + int ret; > + int tries; > + > + for (tries = 0; tries < POWER_ON_TRIES; tries++) { > + ret = _rockchip_usb3_phy_power_on(tcphy); > + if (!ret) > + break; > + } > + > + if (tries && !ret) > + dev_info(tcphy->dev, "Needed %d loops to turn on\n", tries); > + It's curious that in my case I never see this message, or it works or it fails after 100 retries. I'll do more longer tests and continue investigating a little bit. Regards, Enric > + return ret; > +} > + > + > static int rockchip_usb3_phy_power_off(struct phy *phy) > { > struct rockchip_typec_phy *tcphy = phy_get_drvdata(phy); > -- > 2.15.1.424.g9478a66081-goog > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
