[PATCH v2 1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers

[PATCH v2 1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers

[Jonathan Marek]

These 3 controllers support OTG and the driver requires the usb-role-switch
property to enable OTG. Add the property to enable OTG by default.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index 7778e17fb2610..fb16047d803c9 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -4199,6 +4199,8 @@ usb_1_ss2_dwc3: usb@a000000 {
 
 				dma-coherent;
 
+				usb-role-switch;
+
 				ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
@@ -4452,6 +4454,8 @@ usb_1_ss0_dwc3: usb@a600000 {
 
 				dma-coherent;
 
+				usb-role-switch;
+
 				ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
@@ -4550,6 +4554,8 @@ usb_1_ss1_dwc3: usb@a800000 {
 
 				dma-coherent;
 
+				usb-role-switch;
+
 				ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
-- 
2.45.1

[PATCH v2 2/3] arm64: dts: qcom: x1e80100-crd: enable otg on usb ports

[Jonathan Marek]

The 3 USB ports on x1e80100-crd are OTG-capable, remove the dr_mode
override to enable OTG.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
index eb6b735c41453..bc66f4713b231 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
@@ -1568,10 +1568,6 @@ &usb_1_ss0 {
 	status = "okay";
 };
 
-&usb_1_ss0_dwc3 {
-	dr_mode = "host";
-};
-
 &usb_1_ss0_dwc3_hs {
 	remote-endpoint = <&pmic_glink_ss0_hs_in>;
 };
@@ -1600,10 +1596,6 @@ &usb_1_ss1 {
 	status = "okay";
 };
 
-&usb_1_ss1_dwc3 {
-	dr_mode = "host";
-};
-
 &usb_1_ss1_dwc3_hs {
 	remote-endpoint = <&pmic_glink_ss1_hs_in>;
 };
@@ -1632,10 +1624,6 @@ &usb_1_ss2 {
 	status = "okay";
 };
 
-&usb_1_ss2_dwc3 {
-	dr_mode = "host";
-};
-
 &usb_1_ss2_dwc3_hs {
 	remote-endpoint = <&pmic_glink_ss2_hs_in>;
 };
-- 
2.45.1

Re: [PATCH v2 2/3] arm64: dts: qcom: x1e80100-crd: enable otg on usb ports

[Dmitry Baryshkov]

On Fri, Oct 11, 2024 at 07:16:22PM -0400, Jonathan Marek wrote:
> The 3 USB ports on x1e80100-crd are OTG-capable, remove the dr_mode
> override to enable OTG.
> 
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 12 ------------
>  1 file changed, 12 deletions(-)

I can not verify this on the CRD, but at least it matches the
data-role="dual".

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/3] arm64: dts: qcom: x1e80100-crd: enable otg on usb ports

[Stephan Gerhold]

+Cc Abel and Johan

FYI, this landed in qcom for-next last week for CRD and T14s.

On Fri, Oct 11, 2024 at 07:16:22PM -0400, Jonathan Marek wrote:
> The 3 USB ports on x1e80100-crd are OTG-capable, remove the dr_mode
> override to enable OTG.
> 
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>

This is a bit problematic, because dr_mode = "otg" seems to imply
gadget/peripheral mode by default and we are currently unable to detect
the role at runtime until the ADSP is started. Being in peripheral mode
by default will break USB installers; they won't be able find the rootfs
via USB. Unfortunately, they wouldn't be able to detect it once in the
rootfs either, because usually you first need to copy the ADSP firmware
from Windows (at least on the laptops).

I think the best quick fix would be to set

	role-switch-default-mode = "host";

for now to restore the old behavior in initrd, while still allowing to
switch to peripheral mode once detected by the ADSP later.

It would be nice to have gadget mode in initrd as well, since e.g.
postmarketOS needs that to set up the USB debug shell. But I'm not sure
how we could support that:

 - We could designate some of the ports as "peripheral by default" and
   some as "host by default". E.g. usb_1_ss0 is also used for EDL and
   Fastboot on CRD, so it's more likely to be used in peripheral mode.
   But there still would be users confused about why they cannot plug in
   their USB installer into one of the ports...

 - Long term, I wonder if there is any way we could reuse the reduced
   ADSP firmware from UEFI for USB detection until we start the full one
   later? Perhaps it provides a similar interface?

Thanks,
Stephan

> ---
>  arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
> index eb6b735c41453..bc66f4713b231 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-crd.dts
> @@ -1568,10 +1568,6 @@ &usb_1_ss0 {
>  	status = "okay";
>  };
>  
> -&usb_1_ss0_dwc3 {
> -	dr_mode = "host";
> -};
> -
>  &usb_1_ss0_dwc3_hs {
>  	remote-endpoint = <&pmic_glink_ss0_hs_in>;
>  };
> @@ -1600,10 +1596,6 @@ &usb_1_ss1 {
>  	status = "okay";
>  };
>  
> -&usb_1_ss1_dwc3 {
> -	dr_mode = "host";
> -};
> -
>  &usb_1_ss1_dwc3_hs {
>  	remote-endpoint = <&pmic_glink_ss1_hs_in>;
>  };
> @@ -1632,10 +1624,6 @@ &usb_1_ss2 {
>  	status = "okay";
>  };
>  
> -&usb_1_ss2_dwc3 {
> -	dr_mode = "host";
> -};
> -
>  &usb_1_ss2_dwc3_hs {
>  	remote-endpoint = <&pmic_glink_ss2_hs_in>;
>  };
> -- 
> 2.45.1
> 

Re: [PATCH v2 2/3] arm64: dts: qcom: x1e80100-crd: enable otg on usb ports

[Jonathan Marek]

On 10/21/24 8:54 AM, Stephan Gerhold wrote:
> +Cc Abel and Johan
> 
> FYI, this landed in qcom for-next last week for CRD and T14s.
> 
> On Fri, Oct 11, 2024 at 07:16:22PM -0400, Jonathan Marek wrote:
>> The 3 USB ports on x1e80100-crd are OTG-capable, remove the dr_mode
>> override to enable OTG.
>>
>> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> 
> This is a bit problematic, because dr_mode = "otg" seems to imply
> gadget/peripheral mode by default and we are currently unable to detect
> the role at runtime until the ADSP is started. Being in peripheral mode
> by default will break USB installers; they won't be able find the rootfs
> via USB. Unfortunately, they wouldn't be able to detect it once in the
> rootfs either, because usually you first need to copy the ADSP firmware
> from Windows (at least on the laptops).
> 
> I think the best quick fix would be to set
> 
> 	role-switch-default-mode = "host";
> 

I have no objection to this but its a hack to workaround qcom's broken 
design and perhaps should include a comment along those lines. The 
situation is also the same on anything sm8350 and newer.

FYI upstream doesn't support a rootfs on USB because loading a new ADSP 
firmware breaks it (cuts off vbus for a moment I guess), but I guess 
that doesn't apply to the USB installer case. (maybe the people making 
these USB installers should just have to carry a patch with this?)

> for now to restore the old behavior in initrd, while still allowing to
> switch to peripheral mode once detected by the ADSP later.
> 
> It would be nice to have gadget mode in initrd as well, since e.g.
> postmarketOS needs that to set up the USB debug shell. But I'm not sure
> how we could support that:
> 
>   - We could designate some of the ports as "peripheral by default" and
>     some as "host by default". E.g. usb_1_ss0 is also used for EDL and
>     Fastboot on CRD, so it's more likely to be used in peripheral mode.
>     But there still would be users confused about why they cannot plug in
>     their USB installer into one of the ports...
> 
>   - Long term, I wonder if there is any way we could reuse the reduced
>     ADSP firmware from UEFI for USB detection until we start the full one
>     later? Perhaps it provides a similar interface?

This is what I do (minus the "start the full one later" part), with a 
hack [1] to make the remoteproc driver skip loading any firmware and 
trying to boot the DSP. The UEFI-loaded ADSP firmware has the same 
charging/usb functionality as the full ADSP firmware.

[1] 
https://github.com/flto/linux/commit/36921742d28b55dc02d8e5a8d6598e567e7874ab

[PATCH v2 3/3] arm64: dts: qcom: x1e78100-t14s: enable otg on usb-c ports

[Jonathan Marek]

The 2 USB-C ports on x1e78100-t14s are OTG-capable, remove the dr_mode
override to enable OTG.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 .../arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts | 8 --------
 1 file changed, 8 deletions(-)

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 f05523cb51cd4..6343f34b978ad 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts
@@ -1152,10 +1152,6 @@ &usb_1_ss0 {
 	status = "okay";
 };
 
-&usb_1_ss0_dwc3 {
-	dr_mode = "host";
-};
-
 &usb_1_ss0_dwc3_hs {
 	remote-endpoint = <&pmic_glink_ss0_hs_in>;
 };
@@ -1184,10 +1180,6 @@ &usb_1_ss1 {
 	status = "okay";
 };
 
-&usb_1_ss1_dwc3 {
-	dr_mode = "host";
-};
-
 &usb_1_ss1_dwc3_hs {
 	remote-endpoint = <&pmic_glink_ss1_hs_in>;
 };
-- 
2.45.1

Re: [PATCH v2 3/3] arm64: dts: qcom: x1e78100-t14s: enable otg on usb-c ports

[Dmitry Baryshkov]

On Fri, Oct 11, 2024 at 07:16:23PM -0400, Jonathan Marek wrote:
> The 2 USB-C ports on x1e78100-t14s are OTG-capable, remove the dr_mode
> override to enable OTG.
> 
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
>  .../arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dts | 8 --------
>  1 file changed, 8 deletions(-)
> 

Same comment,

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers

[Dmitry Baryshkov]

On Fri, Oct 11, 2024 at 07:16:21PM -0400, Jonathan Marek wrote:
> These 3 controllers support OTG and the driver requires the usb-role-switch
> property to enable OTG. Add the property to enable OTG by default.
> 
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: (subset) [PATCH v2 1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers

[Bjorn Andersson]

On Fri, 11 Oct 2024 19:16:21 -0400, Jonathan Marek wrote:
> These 3 controllers support OTG and the driver requires the usb-role-switch
> property to enable OTG. Add the property to enable OTG by default.
> 
> 

Applied, thanks!

[1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers
      commit: f042bc234c2e00764b8aa2c9e2f8177cdc63f664
[2/3] arm64: dts: qcom: x1e80100-crd: enable otg on usb ports
      commit: 2dd3250191bcfe93b0c9da46624af830310400a7
[3/3] arm64: dts: qcom: x1e78100-t14s: enable otg on usb-c ports
      commit: 1a48dd7b9ac809d1bd0fd2fef509abba83433846

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

Re: [PATCH v2 1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers

[Dmitry Baryshkov]

On Fri, Oct 11, 2024 at 07:16:21PM -0400, Jonathan Marek wrote:
> These 3 controllers support OTG and the driver requires the usb-role-switch
> property to enable OTG. Add the property to enable OTG by default.
> 
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 6 ++++++
>  1 file changed, 6 insertions(+)

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

> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 7778e17fb2610..fb16047d803c9 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -4199,6 +4199,8 @@ usb_1_ss2_dwc3: usb@a000000 {
>  
>  				dma-coherent;
>  
> +				usb-role-switch;
> +
>  				ports {
>  					#address-cells = <1>;
>  					#size-cells = <0>;
> @@ -4452,6 +4454,8 @@ usb_1_ss0_dwc3: usb@a600000 {
>  
>  				dma-coherent;
>  
> +				usb-role-switch;
> +
>  				ports {
>  					#address-cells = <1>;
>  					#size-cells = <0>;
> @@ -4550,6 +4554,8 @@ usb_1_ss1_dwc3: usb@a800000 {
>  
>  				dma-coherent;
>  
> +				usb-role-switch;
> +
>  				ports {
>  					#address-cells = <1>;
>  					#size-cells = <0>;
> -- 
> 2.45.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers

[Dmitry Baryshkov]

On Wed, Nov 06, 2024 at 02:37:43PM +0200, Dmitry Baryshkov wrote:
> On Fri, Oct 11, 2024 at 07:16:21PM -0400, Jonathan Marek wrote:
> > These 3 controllers support OTG and the driver requires the usb-role-switch
> > property to enable OTG. Add the property to enable OTG by default.
> > 
> > Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> > ---
> >  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 6 ++++++
> >  1 file changed, 6 insertions(+)
> 
> 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

#regzbot ^introduced: f042bc234c2e

> 
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > index 7778e17fb2610..fb16047d803c9 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> > @@ -4199,6 +4199,8 @@ usb_1_ss2_dwc3: usb@a000000 {
> >  
> >  				dma-coherent;
> >  
> > +				usb-role-switch;
> > +
> >  				ports {
> >  					#address-cells = <1>;
> >  					#size-cells = <0>;
> > @@ -4452,6 +4454,8 @@ usb_1_ss0_dwc3: usb@a600000 {
> >  
> >  				dma-coherent;
> >  
> > +				usb-role-switch;
> > +
> >  				ports {
> >  					#address-cells = <1>;
> >  					#size-cells = <0>;
> > @@ -4550,6 +4554,8 @@ usb_1_ss1_dwc3: usb@a800000 {
> >  
> >  				dma-coherent;
> >  
> > +				usb-role-switch;
> > +
> >  				ports {
> >  					#address-cells = <1>;
> >  					#size-cells = <0>;
> > -- 
> > 2.45.1
> > 
> 
> -- 
> With best wishes
> Dmitry

-- 
With best wishes
Dmitry