| Message ID | 20241206-gs101-phy-lanes-orientation-phy-v4-7-f5961268b149@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | USB31DRD phy updates for Google Tensor gs101 (orientation & DWC3 rpm) | expand |
Hi André, On Fri, 6 Dec 2024 at 16:31, André Draszik <andre.draszik@linaro.org> wrote: > > To make USB runtime suspend work when a UDC has been bound, the phy > needs to inform the USBDRD controller (DWC3) that Vbus and bvalid are > gone, so that it can in turn raise the respective gadget interrupt with > event == DWC3_DEVICE_EVENT_DISCONNECT, which will cause the USB stack > to clean up, allowing DWC3 to enter runtime suspend. > > On e850 and gs101 this isn't working, as the respective signals are not > directly connected, and instead this driver uses override bits in the > PHY IP to set those signals. It currently forcefully sets them to 'on', > so the above mentioned interrupt will not be raised, preventing runtime > suspend. > > To detect that state, update this driver to act on the TCPC's > orientation signal - when orientation == NONE, Vbus is gone and we can > clear the respective bits. Similarly, for other orientation values we > re-enable them. > > This makes runtime suspend work on platforms with a TCPC (like Pixel6), > while keeping compatibility with platforms without (e850-96). > > With runtime suspend working, USB-C cable orientation detection now > also fully works on such platforms, and the link comes up as Superspeed > as expected irrespective of the cable orientation and whether UDC / > gadget are configured and active. > > Signed-off-by: André Draszik <andre.draszik@linaro.org> As mentioned on the last patch, in my testing cable orientation detection is working, but Pixel is detected as a superspeed device in one orientation, and high speed device in the other orientation. So you should either change the wording of the last paragraph in the commit message (assuming you get the same results as me) or make it detect as superspeed in both orientations. Thanks, Peter.
Hi André, On Sat, 7 Dec 2024 at 22:03, Peter Griffin <peter.griffin@linaro.org> wrote: > > Hi André, > > On Fri, 6 Dec 2024 at 16:31, André Draszik <andre.draszik@linaro.org> wrote: > > > > To make USB runtime suspend work when a UDC has been bound, the phy > > needs to inform the USBDRD controller (DWC3) that Vbus and bvalid are > > gone, so that it can in turn raise the respective gadget interrupt with > > event == DWC3_DEVICE_EVENT_DISCONNECT, which will cause the USB stack > > to clean up, allowing DWC3 to enter runtime suspend. > > > > On e850 and gs101 this isn't working, as the respective signals are not > > directly connected, and instead this driver uses override bits in the > > PHY IP to set those signals. It currently forcefully sets them to 'on', > > so the above mentioned interrupt will not be raised, preventing runtime > > suspend. > > > > To detect that state, update this driver to act on the TCPC's > > orientation signal - when orientation == NONE, Vbus is gone and we can > > clear the respective bits. Similarly, for other orientation values we > > re-enable them. > > > > This makes runtime suspend work on platforms with a TCPC (like Pixel6), > > while keeping compatibility with platforms without (e850-96). > > > > With runtime suspend working, USB-C cable orientation detection now > > also fully works on such platforms, and the link comes up as Superspeed > > as expected irrespective of the cable orientation and whether UDC / > > gadget are configured and active. > > > > Signed-off-by: André Draszik <andre.draszik@linaro.org> > > As mentioned on the last patch, in my testing cable orientation > detection is working, but Pixel is detected as a superspeed device in > one orientation, and high speed device in the other orientation. So > you should either change the wording of the last paragraph in the > commit message (assuming you get the same results as me) or make it > detect as superspeed in both orientations. You can disregard this point, I had a typo in my test setup :( I just confirmed that it is detected as SuperSpeed in both orientations. Thanks, Peter
On 12/06/2024, André Draszik wrote: > To make USB runtime suspend work when a UDC has been bound, the phy > needs to inform the USBDRD controller (DWC3) that Vbus and bvalid are > gone, so that it can in turn raise the respective gadget interrupt with > event == DWC3_DEVICE_EVENT_DISCONNECT, which will cause the USB stack > to clean up, allowing DWC3 to enter runtime suspend. > > On e850 and gs101 this isn't working, as the respective signals are not > directly connected, and instead this driver uses override bits in the > PHY IP to set those signals. It currently forcefully sets them to 'on', > so the above mentioned interrupt will not be raised, preventing runtime > suspend. > > To detect that state, update this driver to act on the TCPC's > orientation signal - when orientation == NONE, Vbus is gone and we can > clear the respective bits. Similarly, for other orientation values we > re-enable them. > > This makes runtime suspend work on platforms with a TCPC (like Pixel6), > while keeping compatibility with platforms without (e850-96). > > With runtime suspend working, USB-C cable orientation detection now > also fully works on such platforms, and the link comes up as Superspeed > as expected irrespective of the cable orientation and whether UDC / > gadget are configured and active. > > Signed-off-by: André Draszik <andre.draszik@linaro.org> Verified on my Pixel 6 Pro. Tested-by: Will McVicker <willmcvicker@google.com> Thanks, Will <snip>
diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c index 8fc15847cfd8..bac1dc927b26 100644 --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c @@ -1137,13 +1137,15 @@ static void exynos850_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd) reg |= LINKCTRL_BUS_FILTER_BYPASS(0xf); writel(reg, regs_base + EXYNOS850_DRD_LINKCTRL); - reg = readl(regs_base + EXYNOS850_DRD_UTMI); - reg |= UTMI_FORCE_BVALID | UTMI_FORCE_VBUSVALID; - writel(reg, regs_base + EXYNOS850_DRD_UTMI); - - reg = readl(regs_base + EXYNOS850_DRD_HSP); - reg |= HSP_VBUSVLDEXT | HSP_VBUSVLDEXTSEL; - writel(reg, regs_base + EXYNOS850_DRD_HSP); + if (!phy_drd->sw) { + reg = readl(regs_base + EXYNOS850_DRD_UTMI); + reg |= UTMI_FORCE_BVALID | UTMI_FORCE_VBUSVALID; + writel(reg, regs_base + EXYNOS850_DRD_UTMI); + + reg = readl(regs_base + EXYNOS850_DRD_HSP); + reg |= HSP_VBUSVLDEXT | HSP_VBUSVLDEXTSEL; + writel(reg, regs_base + EXYNOS850_DRD_HSP); + } reg = readl(regs_base + EXYNOS850_DRD_SSPPLLCTL); reg &= ~SSPPLLCTL_FSEL; @@ -1404,9 +1406,41 @@ static int exynos5_usbdrd_orien_sw_set(struct typec_switch_dev *sw, enum typec_orientation orientation) { struct exynos5_usbdrd_phy *phy_drd = typec_switch_get_drvdata(sw); + int ret; + + ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks); + if (ret) { + dev_err(phy_drd->dev, "Failed to enable PHY clocks(s)\n"); + return ret; + } + + scoped_guard(mutex, &phy_drd->phy_mutex) { + void __iomem * const regs_base = phy_drd->reg_phy; + unsigned int reg; + + if (orientation == TYPEC_ORIENTATION_NONE) { + reg = readl(regs_base + EXYNOS850_DRD_UTMI); + reg &= ~(UTMI_FORCE_VBUSVALID | UTMI_FORCE_BVALID); + writel(reg, regs_base + EXYNOS850_DRD_UTMI); + + reg = readl(regs_base + EXYNOS850_DRD_HSP); + reg |= HSP_VBUSVLDEXTSEL; + reg &= ~HSP_VBUSVLDEXT; + writel(reg, regs_base + EXYNOS850_DRD_HSP); + } else { + reg = readl(regs_base + EXYNOS850_DRD_UTMI); + reg |= UTMI_FORCE_VBUSVALID | UTMI_FORCE_BVALID; + writel(reg, regs_base + EXYNOS850_DRD_UTMI); + + reg = readl(regs_base + EXYNOS850_DRD_HSP); + reg |= HSP_VBUSVLDEXTSEL | HSP_VBUSVLDEXT; + writel(reg, regs_base + EXYNOS850_DRD_HSP); + } - scoped_guard(mutex, &phy_drd->phy_mutex) phy_drd->orientation = orientation; + } + + clk_bulk_disable(phy_drd->drv_data->n_clks, phy_drd->clks); return 0; }
To make USB runtime suspend work when a UDC has been bound, the phy needs to inform the USBDRD controller (DWC3) that Vbus and bvalid are gone, so that it can in turn raise the respective gadget interrupt with event == DWC3_DEVICE_EVENT_DISCONNECT, which will cause the USB stack to clean up, allowing DWC3 to enter runtime suspend. On e850 and gs101 this isn't working, as the respective signals are not directly connected, and instead this driver uses override bits in the PHY IP to set those signals. It currently forcefully sets them to 'on', so the above mentioned interrupt will not be raised, preventing runtime suspend. To detect that state, update this driver to act on the TCPC's orientation signal - when orientation == NONE, Vbus is gone and we can clear the respective bits. Similarly, for other orientation values we re-enable them. This makes runtime suspend work on platforms with a TCPC (like Pixel6), while keeping compatibility with platforms without (e850-96). With runtime suspend working, USB-C cable orientation detection now also fully works on such platforms, and the link comes up as Superspeed as expected irrespective of the cable orientation and whether UDC / gadget are configured and active. Signed-off-by: André Draszik <andre.draszik@linaro.org> --- v3: * update exynos5_usbdrd_orien_sw_set() to not test against previous orientation --- drivers/phy/samsung/phy-exynos5-usbdrd.c | 50 +++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 8 deletions(-)