[PATCH v2 0/2] arm64: dts: qcom: x1e80100: qcp/t14s: Enable USB multi-port controller related ports

[PATCH v2 0/2] arm64: dts: qcom: x1e80100: qcp/t14s: Enable USB multi-port controller related ports

[Abel Vesa]

Both QCP and T14s have 2 NXP PTN3222 eUSB-to-USB2 redrivers used to level-shift
between the USB multi-port controller High-Speed PHY eUSB2 and the USB
Type-A ports connected to it. The QCP has a 3rd instance of this
redriver but is connected to the USB2S controller.

Describe the redrivers used for the USB multi-port and enable each pair
of PHYS and controller in order to enable USB support on those 2 Type-A ports
on each board.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Changes in v2:
- Reordered alphabetically the HS PHYs and QMP PHYs in each dtsi, as per
  Konrad's request.
- Picked up Konrad's R-b tags
- Link to v1: https://lore.kernel.org/r/20241105-x1e80100-qcp-t14-enable-usb-type-a-ports-v1-0-b4386def91d8@linaro.org

---
Abel Vesa (2):
      arm64: dts: qcom: x1e78100-t14s: Enable support for both Type-A USB ports
      arm64: dts: qcom: x1e78100-qcp: Enable Type-A USB ports labeled 3 and 4/6

 .../dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts     | 86 ++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts          | 86 ++++++++++++++++++++++
 2 files changed, 172 insertions(+)
---
base-commit: f486c8aa16b8172f63bddc70116a0c897a7f3f02
change-id: 20241105-x1e80100-qcp-t14-enable-usb-type-a-ports-552ced3b4430

Best regards,
-- 
Abel Vesa <abel.vesa@linaro.org>

[PATCH v2 1/2] arm64: dts: qcom: x1e78100-t14s: Enable support for both Type-A USB ports

[Abel Vesa]

The Thinkpad T14s has 2 USB-A ports, both connected to the USB
multiport controller, each one via a separate NXP PTN3222 eUSB2-to-USB2
redriver to the eUSB2 PHY for High-Speed support, with a dedicated QMP
PHY for SuperSpeed support.

Describe each redriver and then enable each pair of PHYs and the
USB controller itself, in order to enable support for the 2 USB-A ports.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 .../dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts     | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
index 975550139e1024420ed335a2a46e4d54df7ee423..f936e3246ec87972746a60080c3a48d646a356f2 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
@@ -495,6 +495,40 @@ keyboard@3a {
 	};
 };
 
+&i2c5 {
+	clock-frequency = <400000>;
+
+	status = "okay";
+
+	eusb3_repeater: redriver@47 {
+		compatible = "nxp,ptn3222";
+		reg = <0x47>;
+		#phy-cells = <0>;
+
+		vdd3v3-supply = <&vreg_l13b_3p0>;
+		vdd1v8-supply = <&vreg_l4b_1p8>;
+
+		reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
+
+		pinctrl-0 = <&eusb3_reset_n>;
+		pinctrl-names = "default";
+	};
+
+	eusb6_repeater: redriver@4f {
+		compatible = "nxp,ptn3222";
+		reg = <0x4f>;
+		#phy-cells = <0>;
+
+		vdd3v3-supply = <&vreg_l13b_3p0>;
+		vdd1v8-supply = <&vreg_l4b_1p8>;
+
+		reset-gpios = <&tlmm 184 GPIO_ACTIVE_LOW>;
+
+		pinctrl-0 = <&eusb6_reset_n>;
+		pinctrl-names = "default";
+	};
+};
+
 &i2c8 {
 	clock-frequency = <400000>;
 
@@ -651,6 +685,22 @@ &tlmm {
 			       <72 2>, /* Secure EC I2C connection (?) */
 			       <238 1>; /* UFS Reset */
 
+	eusb3_reset_n: eusb3-reset-n-state {
+		pins = "gpio6";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-low;
+	};
+
+	eusb6_reset_n: eusb6-reset-n-state {
+		pins = "gpio184";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-low;
+	};
+
 	tpad_default: tpad-default-state {
 		pins = "gpio3";
 		function = "gpio";
@@ -808,3 +858,39 @@ &usb_1_ss1_dwc3_hs {
 &usb_1_ss1_qmpphy_out {
 	remote-endpoint = <&pmic_glink_ss1_ss_in>;
 };
+
+&usb_mp {
+	status = "okay";
+};
+
+&usb_mp_hsphy0 {
+	vdd-supply = <&vreg_l2e_0p8>;
+	vdda12-supply = <&vreg_l3e_1p2>;
+
+	phys = <&eusb6_repeater>;
+
+	status = "okay";
+};
+
+&usb_mp_hsphy1 {
+	vdd-supply = <&vreg_l2e_0p8>;
+	vdda12-supply = <&vreg_l3e_1p2>;
+
+	phys = <&eusb3_repeater>;
+
+	status = "okay";
+};
+
+&usb_mp_qmpphy0 {
+	vdda-phy-supply = <&vreg_l3e_1p2>;
+	vdda-pll-supply = <&vreg_l3c_0p8>;
+
+	status = "okay";
+};
+
+&usb_mp_qmpphy1 {
+	vdda-phy-supply = <&vreg_l3e_1p2>;
+	vdda-pll-supply = <&vreg_l3c_0p8>;
+
+	status = "okay";
+};

-- 
2.34.1

[PATCH v2 2/2] arm64: dts: qcom: x1e78100-qcp: Enable Type-A USB ports labeled 3 and 4/6

[Abel Vesa]

The X Elite QCP board has 3 USB-A ports. The ones labed as USB3 and
USB4/6 are both connected to the multiport controller, each one via a
separate NXP PTN3222 eUSB2-to-USB2 redriver to the eUSB2 PHY for
High-Speed support, with a dedicated QMP PHY for SuperSpeed support.

Describe these two redrivers and enable each pair of PHYs along with the
USB controller, all in order to enable support for these 2 USB-A ports.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 arch/arm64/boot/dts/qcom/x1e80100-qcp.dts | 86 +++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
index 5ef030c60abe2998d093ee60a6754a90cd5aaf72..7c133e60e6f00f65f94adc5863cf935954989589 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-qcp.dts
@@ -616,6 +616,40 @@ zap-shader {
 	};
 };
 
+&i2c5 {
+	clock-frequency = <400000>;
+
+	status = "okay";
+
+	eusb3_repeater: redriver@47 {
+		compatible = "nxp,ptn3222";
+		reg = <0x47>;
+		#phy-cells = <0>;
+
+		vdd3v3-supply = <&vreg_l13b_3p0>;
+		vdd1v8-supply = <&vreg_l4b_1p8>;
+
+		reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
+
+		pinctrl-0 = <&eusb3_reset_n>;
+		pinctrl-names = "default";
+	};
+
+	eusb6_repeater: redriver@4f {
+		compatible = "nxp,ptn3222";
+		reg = <0x4f>;
+		#phy-cells = <0>;
+
+		vdd3v3-supply = <&vreg_l13b_3p0>;
+		vdd1v8-supply = <&vreg_l4b_1p8>;
+
+		reset-gpios = <&tlmm 184 GPIO_ACTIVE_LOW>;
+
+		pinctrl-0 = <&eusb6_reset_n>;
+		pinctrl-names = "default";
+	};
+};
+
 &lpass_tlmm {
 	spkr_01_sd_n_active: spkr-01-sd-n-active-state {
 		pins = "gpio12";
@@ -819,6 +853,22 @@ edp_reg_en: edp-reg-en-state {
 		bias-disable;
 	};
 
+	eusb3_reset_n: eusb3-reset-n-state {
+		pins = "gpio6";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-low;
+	};
+
+	eusb6_reset_n: eusb6-reset-n-state {
+		pins = "gpio184";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-low;
+	};
+
 	nvme_reg_en: nvme-reg-en-state {
 		pins = "gpio18";
 		function = "gpio";
@@ -981,3 +1031,39 @@ &usb_1_ss2_dwc3_hs {
 &usb_1_ss2_qmpphy_out {
 	remote-endpoint = <&pmic_glink_ss2_ss_in>;
 };
+
+&usb_mp {
+	status = "okay";
+};
+
+&usb_mp_hsphy0 {
+	vdd-supply = <&vreg_l2e_0p8>;
+	vdda12-supply = <&vreg_l3e_1p2>;
+
+	phys = <&eusb6_repeater>;
+
+	status = "okay";
+};
+
+&usb_mp_hsphy1 {
+	vdd-supply = <&vreg_l2e_0p8>;
+	vdda12-supply = <&vreg_l3e_1p2>;
+
+	phys = <&eusb3_repeater>;
+
+	status = "okay";
+};
+
+&usb_mp_qmpphy0 {
+	vdda-phy-supply = <&vreg_l3e_1p2>;
+	vdda-pll-supply = <&vreg_l3c_0p8>;
+
+	status = "okay";
+};
+
+&usb_mp_qmpphy1 {
+	vdda-phy-supply = <&vreg_l3e_1p2>;
+	vdda-pll-supply = <&vreg_l3c_0p8>;
+
+	status = "okay";
+};

-- 
2.34.1

Re: [PATCH v2 1/2] arm64: dts: qcom: x1e78100-t14s: Enable support for both Type-A USB ports

[Johan Hovold]

On Mon, Dec 02, 2024 at 11:23:17AM +0200, Abel Vesa wrote:
> The Thinkpad T14s has 2 USB-A ports, both connected to the USB
> multiport controller, each one via a separate NXP PTN3222 eUSB2-to-USB2
> redriver to the eUSB2 PHY for High-Speed support, with a dedicated QMP
> PHY for SuperSpeed support.
> 
> Describe each redriver and then enable each pair of PHYs and the
> USB controller itself, in order to enable support for the 2 USB-A ports.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  .../dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts     | 86 ++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> index 975550139e1024420ed335a2a46e4d54df7ee423..f936e3246ec87972746a60080c3a48d646a356f2 100644
> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> @@ -495,6 +495,40 @@ keyboard@3a {
>  	};
>  };
>  
> +&i2c5 {
> +	clock-frequency = <400000>;
> +
> +	status = "okay";
> +
> +	eusb3_repeater: redriver@47 {
> +		compatible = "nxp,ptn3222";
> +		reg = <0x47>;

The driver doesn't seem to actually communicate with these devices
currently and the addresses you specify here do not match what the
schematics says.

Have you verified that these addresses are correct?

> +		#phy-cells = <0>;
> +
> +		vdd3v3-supply = <&vreg_l13b_3p0>;
> +		vdd1v8-supply = <&vreg_l4b_1p8>;
> +
> +		reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
> +
> +		pinctrl-0 = <&eusb3_reset_n>;
> +		pinctrl-names = "default";
> +	};
> +
> +	eusb6_repeater: redriver@4f {
> +		compatible = "nxp,ptn3222";
> +		reg = <0x4f>;

Same here.

> +		#phy-cells = <0>;
> +
> +		vdd3v3-supply = <&vreg_l13b_3p0>;
> +		vdd1v8-supply = <&vreg_l4b_1p8>;
> +
> +		reset-gpios = <&tlmm 184 GPIO_ACTIVE_LOW>;
> +
> +		pinctrl-0 = <&eusb6_reset_n>;
> +		pinctrl-names = "default";
> +	};
> +};
> +
>  &i2c8 {
>  	clock-frequency = <400000>;
>  
> @@ -651,6 +685,22 @@ &tlmm {
>  			       <72 2>, /* Secure EC I2C connection (?) */
>  			       <238 1>; /* UFS Reset */
>  
> +	eusb3_reset_n: eusb3-reset-n-state {
> +		pins = "gpio6";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-low;

I don't think the pin configuration should assert reset, that should be
left up to the driver to decide, that is,  when (and if) it's an
appropriate thing to do.

> +	};
> +
> +	eusb6_reset_n: eusb6-reset-n-state {
> +		pins = "gpio184";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-low;

Same here.

> +	};
> +
>  	tpad_default: tpad-default-state {
>  		pins = "gpio3";
>  		function = "gpio";
> @@ -808,3 +858,39 @@ &usb_1_ss1_dwc3_hs {
>  &usb_1_ss1_qmpphy_out {
>  	remote-endpoint = <&pmic_glink_ss1_ss_in>;
>  };

And last, but not least, the T14s may hard reset if you disconnect a
thumb drive connected to one of these ports while suspended (6.13-rc1).

Once it survived with a lockdep splat indicating a circular locking
dependency. I see that on the CRD as well, so possibly not related to
the hard reset.

No such issues with a FullSpeed keyboard.

Johan

Re: [PATCH v2 1/2] arm64: dts: qcom: x1e78100-t14s: Enable support for both Type-A USB ports

[Abel Vesa]

On 24-12-02 16:18:37, Johan Hovold wrote:
> On Mon, Dec 02, 2024 at 11:23:17AM +0200, Abel Vesa wrote:
> > The Thinkpad T14s has 2 USB-A ports, both connected to the USB
> > multiport controller, each one via a separate NXP PTN3222 eUSB2-to-USB2
> > redriver to the eUSB2 PHY for High-Speed support, with a dedicated QMP
> > PHY for SuperSpeed support.
> > 
> > Describe each redriver and then enable each pair of PHYs and the
> > USB controller itself, in order to enable support for the 2 USB-A ports.
> > 
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> >  .../dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts     | 86 ++++++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> > index 975550139e1024420ed335a2a46e4d54df7ee423..f936e3246ec87972746a60080c3a48d646a356f2 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> > +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
> > @@ -495,6 +495,40 @@ keyboard@3a {
> >  	};
> >  };
> >  
> > +&i2c5 {
> > +	clock-frequency = <400000>;
> > +
> > +	status = "okay";
> > +
> > +	eusb3_repeater: redriver@47 {
> > +		compatible = "nxp,ptn3222";
> > +		reg = <0x47>;
> 
> The driver doesn't seem to actually communicate with these devices
> currently and the addresses you specify here do not match what the
> schematics says.

Schematics have the addressess left shifted for the wr/rd bit.

> 
> Have you verified that these addresses are correct?

Reading the chip id regs confirms the addresses are correct.

> 
> > +		#phy-cells = <0>;
> > +
> > +		vdd3v3-supply = <&vreg_l13b_3p0>;
> > +		vdd1v8-supply = <&vreg_l4b_1p8>;
> > +
> > +		reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
> > +
> > +		pinctrl-0 = <&eusb3_reset_n>;
> > +		pinctrl-names = "default";
> > +	};
> > +
> > +	eusb6_repeater: redriver@4f {
> > +		compatible = "nxp,ptn3222";
> > +		reg = <0x4f>;
> 
> Same here.
> 
> > +		#phy-cells = <0>;
> > +
> > +		vdd3v3-supply = <&vreg_l13b_3p0>;
> > +		vdd1v8-supply = <&vreg_l4b_1p8>;
> > +
> > +		reset-gpios = <&tlmm 184 GPIO_ACTIVE_LOW>;
> > +
> > +		pinctrl-0 = <&eusb6_reset_n>;
> > +		pinctrl-names = "default";
> > +	};
> > +};
> > +
> >  &i2c8 {
> >  	clock-frequency = <400000>;
> >  
> > @@ -651,6 +685,22 @@ &tlmm {
> >  			       <72 2>, /* Secure EC I2C connection (?) */
> >  			       <238 1>; /* UFS Reset */
> >  
> > +	eusb3_reset_n: eusb3-reset-n-state {
> > +		pins = "gpio6";
> > +		function = "gpio";
> > +		drive-strength = <2>;
> > +		bias-disable;
> > +		output-low;
> 
> I don't think the pin configuration should assert reset, that should be
> left up to the driver to decide, that is,  when (and if) it's an
> appropriate thing to do.

Yep. The driver needs changes for that.

> 
> > +	};
> > +
> > +	eusb6_reset_n: eusb6-reset-n-state {
> > +		pins = "gpio184";
> > +		function = "gpio";
> > +		drive-strength = <2>;
> > +		bias-disable;
> > +		output-low;
> 
> Same here.
> 
> > +	};
> > +
> >  	tpad_default: tpad-default-state {
> >  		pins = "gpio3";
> >  		function = "gpio";
> > @@ -808,3 +858,39 @@ &usb_1_ss1_dwc3_hs {
> >  &usb_1_ss1_qmpphy_out {
> >  	remote-endpoint = <&pmic_glink_ss1_ss_in>;
> >  };
> 
> And last, but not least, the T14s may hard reset if you disconnect a
> thumb drive connected to one of these ports while suspended (6.13-rc1).

Wasn't able to reproduce this issue yet. Will spend some more time on it
in the following days.

> 
> Once it survived with a lockdep splat indicating a circular locking
> dependency. I see that on the CRD as well, so possibly not related to
> the hard reset.

This is most definitely the same splat triggered by the repeater PHY ops
being called from the eUSB2 PHY driver. We are already in discussion
with Vinod on how to handle multi PHY levels in the generic framework.

> 
> No such issues with a FullSpeed keyboard.
> 
> Johan

Thanks for reviewing,

Abel

Re: [PATCH v2 1/2] arm64: dts: qcom: x1e78100-t14s: Enable support for both Type-A USB ports

[Johan Hovold]

On Wed, Dec 04, 2024 at 10:56:59AM +0200, Abel Vesa wrote:
> On 24-12-02 16:18:37, Johan Hovold wrote:
> > On Mon, Dec 02, 2024 at 11:23:17AM +0200, Abel Vesa wrote:

> > > +&i2c5 {
> > > +	clock-frequency = <400000>;
> > > +
> > > +	status = "okay";
> > > +
> > > +	eusb3_repeater: redriver@47 {
> > > +		compatible = "nxp,ptn3222";
> > > +		reg = <0x47>;
> > 
> > The driver doesn't seem to actually communicate with these devices
> > currently and the addresses you specify here do not match what the
> > schematics says.
> 
> Schematics have the addressess left shifted for the wr/rd bit.
> 
> > Have you verified that these addresses are correct?
> 
> Reading the chip id regs confirms the addresses are correct.

Thanks for confirming.

> > And last, but not least, the T14s may hard reset if you disconnect a
> > thumb drive connected to one of these ports while suspended (6.13-rc1).
> 
> Wasn't able to reproduce this issue yet. Will spend some more time on it
> in the following days.

Just triggered what appears to be a deadlock in the block layer by
disconnecting a thumb drive while suspended. Not sure if that could
have triggered the reset, but it is likely related to the lockdep splat
I mentioned below.

> > Once it survived with a lockdep splat indicating a circular locking
> > dependency. I see that on the CRD as well, so possibly not related to
> > the hard reset.
> 
> This is most definitely the same splat triggered by the repeater PHY ops
> being called from the eUSB2 PHY driver. We are already in discussion
> with Vinod on how to handle multi PHY levels in the generic framework.

No, if it was the false-positive PHY splat I would have said so. This
appears to be a new block-layer splat with 6.13-rc1. Don't see it with
6.12.

I'll send a report.

Johan

Re: [PATCH v2 0/2] arm64: dts: qcom: x1e80100: qcp/t14s: Enable USB multi-port controller related ports

[Bjorn Andersson]

On Mon, 02 Dec 2024 11:23:16 +0200, Abel Vesa wrote:
> Both QCP and T14s have 2 NXP PTN3222 eUSB-to-USB2 redrivers used to level-shift
> between the USB multi-port controller High-Speed PHY eUSB2 and the USB
> Type-A ports connected to it. The QCP has a 3rd instance of this
> redriver but is connected to the USB2S controller.
> 
> Describe the redrivers used for the USB multi-port and enable each pair
> of PHYS and controller in order to enable USB support on those 2 Type-A ports
> on each board.
> 
> [...]

Applied, thanks!

[1/2] arm64: dts: qcom: x1e78100-t14s: Enable support for both Type-A USB ports
      commit: ffbf3a8be76613d83c41de40312235cb7cb2cbe4
[2/2] arm64: dts: qcom: x1e78100-qcp: Enable Type-A USB ports labeled 3 and 4/6
      commit: 9f53c3611960a97d2b71825477e96fd8c2fbb050

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>