diff mbox series

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

Message ID 20241011231624.30628-2-jonathan@marek.ca (mailing list archive)
State Accepted, archived
Commit 2dd3250191bcfe93b0c9da46624af830310400a7
Headers show
Series [v2,1/3] arm64: dts: qcom: x1e80100: enable OTG on USB-C controllers | expand

Commit Message

Jonathan Marek Oct. 11, 2024, 11:16 p.m. UTC
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(-)

Comments

Dmitry Baryshkov Oct. 12, 2024, 7:33 a.m. UTC | #1
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>
Stephan Gerhold Oct. 21, 2024, 12:54 p.m. UTC | #2
+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
>
Jonathan Marek Oct. 22, 2024, 4:25 a.m. UTC | #3
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
diff mbox series

Patch

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>;
 };