mbox series

[v2,0/2] arm64: dts: qcom: x1e80100: fix USB OTG regressions

Message ID 20241209111905.31017-1-johan+linaro@kernel.org (mailing list archive)
Headers show
Series arm64: dts: qcom: x1e80100: fix USB OTG regressions | expand

Message

Johan Hovold Dec. 9, 2024, 11:19 a.m. UTC
A recent change enabling OTG mode on the Lenovo ThinkPad T14s USB-C
ports can break SuperSpeed device hotplugging.

Abel noticed that the corresponding commit for the CRD also triggers a
hard reset during resume from suspend.

With retimer (and orientation detection) support not even merged yet,
let's revert at least until we have stable host mode in mainline.

Note that Stephan and Dmitry have already identified other problems with
the offending commits here:

	https://lore.kernel.org/all/ZxZO6Prrm2ITUZMQ@linaro.org/
	https://lore.kernel.org/all/hw2pdof4ajadjsjrb44f2q4cz4yh5qcqz5d3l7gjt2koycqs3k@xx5xvd26uyef

Johan


Changes in v2
 - revert also the corresponding patch for the CRD which breaks suspend


Johan Hovold (2):
  Revert "arm64: dts: qcom: x1e78100-t14s: enable otg on usb-c ports"
  Revert "arm64: dts: qcom: x1e80100-crd: enable otg on usb ports"

 .../boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts  |  8 ++++++++
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts            | 12 ++++++++++++
 2 files changed, 20 insertions(+)

Comments

Dmitry Baryshkov Dec. 9, 2024, 1:23 p.m. UTC | #1
On Mon, Dec 09, 2024 at 12:19:03PM +0100, Johan Hovold wrote:
> A recent change enabling OTG mode on the Lenovo ThinkPad T14s USB-C
> ports can break SuperSpeed device hotplugging.
> 
> Abel noticed that the corresponding commit for the CRD also triggers a
> hard reset during resume from suspend.
> 
> With retimer (and orientation detection) support not even merged yet,
> let's revert at least until we have stable host mode in mainline.
> 
> Note that Stephan and Dmitry have already identified other problems with
> the offending commits here:
> 
> 	https://lore.kernel.org/all/ZxZO6Prrm2ITUZMQ@linaro.org/
> 	https://lore.kernel.org/all/hw2pdof4ajadjsjrb44f2q4cz4yh5qcqz5d3l7gjt2koycqs3k@xx5xvd26uyef
> 
> Johan
> 
> 
> Changes in v2
>  - revert also the corresponding patch for the CRD which breaks suspend

As you are reverting two commits, please revert the third one too, it
breaks pmic-glink.

> 
> 
> Johan Hovold (2):
>   Revert "arm64: dts: qcom: x1e78100-t14s: enable otg on usb-c ports"
>   Revert "arm64: dts: qcom: x1e80100-crd: enable otg on usb ports"
> 
>  .../boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts  |  8 ++++++++
>  arch/arm64/boot/dts/qcom/x1e80100-crd.dts            | 12 ++++++++++++
>  2 files changed, 20 insertions(+)
> 
> -- 
> 2.45.2
>
Johan Hovold Dec. 9, 2024, 3:30 p.m. UTC | #2
On Mon, Dec 09, 2024 at 03:23:05PM +0200, Dmitry Baryshkov wrote:
> On Mon, Dec 09, 2024 at 12:19:03PM +0100, Johan Hovold wrote:
> > A recent change enabling OTG mode on the Lenovo ThinkPad T14s USB-C
> > ports can break SuperSpeed device hotplugging.
> > 
> > Abel noticed that the corresponding commit for the CRD also triggers a
> > hard reset during resume from suspend.
> > 
> > With retimer (and orientation detection) support not even merged yet,
> > let's revert at least until we have stable host mode in mainline.
> > 
> > Note that Stephan and Dmitry have already identified other problems with
> > the offending commits here:
> > 
> > 	https://lore.kernel.org/all/ZxZO6Prrm2ITUZMQ@linaro.org/
> > 	https://lore.kernel.org/all/hw2pdof4ajadjsjrb44f2q4cz4yh5qcqz5d3l7gjt2koycqs3k@xx5xvd26uyef

> > Changes in v2
> >  - revert also the corresponding patch for the CRD which breaks suspend
> 
> As you are reverting two commits, please revert the third one too, it
> breaks pmic-glink.

Can you be more specific? 

I was gonna say that pmic_glink works since hotplug and orientation
detection still works, but I tested now with DP altmode and that is
indeed broken unless I revert the third commit (f042bc234c2e ("arm64:
dts: qcom: x1e80100: enable OTG on USB-C controllers")).

Was that what you had in mind? Can you explain why that breaks?

I'll respin with a v3, but please answer the above first.

> > Johan Hovold (2):
> >   Revert "arm64: dts: qcom: x1e78100-t14s: enable otg on usb-c ports"
> >   Revert "arm64: dts: qcom: x1e80100-crd: enable otg on usb ports"

Johan
Dmitry Baryshkov Dec. 9, 2024, 4 p.m. UTC | #3
On Mon, 9 Dec 2024 at 17:30, Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Dec 09, 2024 at 03:23:05PM +0200, Dmitry Baryshkov wrote:
> > On Mon, Dec 09, 2024 at 12:19:03PM +0100, Johan Hovold wrote:
> > > A recent change enabling OTG mode on the Lenovo ThinkPad T14s USB-C
> > > ports can break SuperSpeed device hotplugging.
> > >
> > > Abel noticed that the corresponding commit for the CRD also triggers a
> > > hard reset during resume from suspend.
> > >
> > > With retimer (and orientation detection) support not even merged yet,
> > > let's revert at least until we have stable host mode in mainline.
> > >
> > > Note that Stephan and Dmitry have already identified other problems with
> > > the offending commits here:
> > >
> > >     https://lore.kernel.org/all/ZxZO6Prrm2ITUZMQ@linaro.org/
> > >     https://lore.kernel.org/all/hw2pdof4ajadjsjrb44f2q4cz4yh5qcqz5d3l7gjt2koycqs3k@xx5xvd26uyef
>
> > > Changes in v2
> > >  - revert also the corresponding patch for the CRD which breaks suspend
> >
> > As you are reverting two commits, please revert the third one too, it
> > breaks pmic-glink.
>
> Can you be more specific?
>
> I was gonna say that pmic_glink works since hotplug and orientation
> detection still works, but I tested now with DP altmode and that is
> indeed broken unless I revert the third commit (f042bc234c2e ("arm64:
> dts: qcom: x1e80100: enable OTG on USB-C controllers")).
>
> Was that what you had in mind? Can you explain why that breaks?

See https://linux-regtracking.leemhuis.info/regzbot/regression/lore/hw2pdof4ajadjsjrb44f2q4cz4yh5qcqz5d3l7gjt2koycqs3k@xx5xvd26uyef/

For some reason commit f042bc234c2e ("arm64: dts: qcom: x1e80100: enable
OTG on USB-C controllers") seems to break UCSI on X1E80100 CRD:

[   34.479352] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: PPM init
failed, stop trying

>
> I'll respin with a v3, but please answer the above first.
>
> > > Johan Hovold (2):
> > >   Revert "arm64: dts: qcom: x1e78100-t14s: enable otg on usb-c ports"
> > >   Revert "arm64: dts: qcom: x1e80100-crd: enable otg on usb ports"
>
> Johan
Johan Hovold Dec. 10, 2024, 10:45 a.m. UTC | #4
On Mon, Dec 09, 2024 at 06:00:21PM +0200, Dmitry Baryshkov wrote:
> On Mon, 9 Dec 2024 at 17:30, Johan Hovold <johan@kernel.org> wrote:
> > On Mon, Dec 09, 2024 at 03:23:05PM +0200, Dmitry Baryshkov wrote:
> > > On Mon, Dec 09, 2024 at 12:19:03PM +0100, Johan Hovold wrote:
> > > > A recent change enabling OTG mode on the Lenovo ThinkPad T14s USB-C
> > > > ports can break SuperSpeed device hotplugging.
> > > >
> > > > Abel noticed that the corresponding commit for the CRD also triggers a
> > > > hard reset during resume from suspend.
> > > >
> > > > With retimer (and orientation detection) support not even merged yet,
> > > > let's revert at least until we have stable host mode in mainline.
> > > >
> > > > Note that Stephan and Dmitry have already identified other problems with
> > > > the offending commits here:
> > > >
> > > >     https://lore.kernel.org/all/ZxZO6Prrm2ITUZMQ@linaro.org/
> > > >     https://lore.kernel.org/all/hw2pdof4ajadjsjrb44f2q4cz4yh5qcqz5d3l7gjt2koycqs3k@xx5xvd26uyef
> >
> > > > Changes in v2
> > > >  - revert also the corresponding patch for the CRD which breaks suspend
> > >
> > > As you are reverting two commits, please revert the third one too, it
> > > breaks pmic-glink.
> >
> > Can you be more specific?
> >
> > I was gonna say that pmic_glink works since hotplug and orientation
> > detection still works, but I tested now with DP altmode and that is
> > indeed broken unless I revert the third commit (f042bc234c2e ("arm64:
> > dts: qcom: x1e80100: enable OTG on USB-C controllers")).
> >
> > Was that what you had in mind? Can you explain why that breaks?
> 
> See https://linux-regtracking.leemhuis.info/regzbot/regression/lore/hw2pdof4ajadjsjrb44f2q4cz4yh5qcqz5d3l7gjt2koycqs3k@xx5xvd26uyef/
> 
> For some reason commit f042bc234c2e ("arm64: dts: qcom: x1e80100: enable
> OTG on USB-C controllers") seems to break UCSI on X1E80100 CRD:
> 
> [   34.479352] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: PPM init
> failed, stop trying

Ok, so you're referring to your UCSI report that I linked to above.

With the full series applied, I typically do *not* see the PPM init
error message, so it does not seem directly related to the regressions I
reported. DP still works even when SS hotplug does not.

With only the first patch applied, I do see the PPM init error on every
boot and DP is broken. Hotplug and orientation detection still works
though:

[   19.698560] ucsi_glink.pmic_glink_ucsi pmic_glink.ucsi.0: PPM init failed, stop trying
[   45.100142] r8152-cfgselector 4-1: USB disconnect, device number 2
[   47.380999] usb 4-1: new SuperSpeed USB device number 3 using xhci-hcd
[   47.549862] r8152-cfgselector 4-1: reset SuperSpeed USB device number 3 using xhci-hcd
[   47.630710] r8152 4-1:1.0 eth0: v1.12.13
[   50.591050] r8152 4-1:1.0 eth0: carrier on
[   50.892296] r8152-cfgselector 4-1: USB disconnect, device number 3
[   55.517111] usb 4-1: new SuperSpeed USB device number 4 using xhci-hcd
[   55.694021] r8152-cfgselector 4-1: reset SuperSpeed USB device number 4 using xhci-hcd

> > I'll respin with a v3, but please answer the above first.

I'll revert the first patch as well.

Johan