diff mbox series

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

Message ID 20241011231624.30628-1-jonathan@marek.ca
State New
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
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(+)

Comments

Dmitry Baryshkov Oct. 12, 2024, 6:17 a.m. UTC | #1
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>
Bjorn Andersson Oct. 16, 2024, 3:32 p.m. UTC | #2
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,
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
Dmitry Baryshkov Nov. 6, 2024, 12:37 p.m. UTC | #4
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
>
Dmitry Baryshkov Nov. 6, 2024, 3:17 p.m. UTC | #5
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
diff mbox series

Patch

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