| Message ID | 20230725084633.67179-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFT] arm64: dts: qcom: sc7280: drop incorrect EUD port on SoC side | expand |
On 25/07/2023 21:35, Doug Anderson wrote: > Hi, > > On Tue, Jul 25, 2023 at 1:46 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> Qualcomm Embedded USB Debugger (EUD) second port should point to Type-C >> USB connector. Such connector was defined directly in root node of >> sc7280.dtsi which is clearly wrong. SC7280 is a chip, so physically it >> does not have USB Type-C port. The connector is usually accessible >> through some USB switch or controller. >> >> Correct the EUD/USB connector topology by removing the top-level fake >> USB connector and adding appropriate ports in boards having actual USB >> Type-C connector defined (Herobrine, IDP). All other boards will have >> this EUD port missing. >> >> This fixes also dtbs_check warnings: >> >> sc7280-herobrine-crd.dtb: connector: ports:port@0: 'reg' is a required property >> >> Fixes: 9ee402ccfeb1 ("arm64: dts: qcom: sc7280: Fix EUD dt node syntax") >> Cc: Bhupesh Sharma <bhupesh.sharma@linaro.org> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> --- >> >> Not tested on hardware. >> --- >> .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 15 +++++++++++++ >> .../arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi | 15 +++++++++++++ >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 21 +------------------ >> 3 files changed, 31 insertions(+), 20 deletions(-) > > FWIW, I've always been very intrigued about the embedded USB port but > never managed to find any way to get it actually enabled. :( ...so I'm > probably not the best person to actually review this. That being said: > > 1. I'm nearly certain that this is completely unusable on herobrine > boards. Specifically on herobrine there's a USB hub between the SoC > and all the physical ports on the device and (I think?) that prevents > EUD from working. It is possible that hoglin/zoglin is an exception > here and Qualcomm might have some backdoor way to access EUD on these > devices since this is hardware that they built. > > 2. I've always been pretty baffled about the sc7280 EUD stuff since > the device tree shows the EUD on "usb_2". For some background: there > are two USB controllers on sc7280. There's "usb_1" which is USB > 2.0/3.0 capable and, at an SoC level, is the "Type C" port. > Specifically the pins on the SoC for the USB 3.0 signals are the same > pins on the SoC as two of the DisplayPort lanes. Then there's "usb_2" > which is USB 2.0 only. If you'll notice, "usb_2" is not set to status > "okay" on any boards except "sc7280-idp.dts". > > I asked Qualcomm at least a few times in the past if the EUD is truly > on the USB 2.0 port (which means it isn't connected to anything on > herobrine boards) or if it's actually on the "type C" port (which > means there's a hub in between) and never got a ton of clarify... > > Given how baffling everything is, I wouldn't be opposed to just > deleting the EUD from the device tree until there is more clarity > here. If you don't want to just delete it, at least I'd say that it > shouldn't be hooked up for herobrine. > Thanks Doug. I forgot to Cc the original author of the code - Souradeep - but anyway disabling incomplete node seems reasonable. Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi index 9ea6636125ad..2a4f239c5632 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi @@ -573,6 +573,12 @@ usb_c0: connector@0 { power-role = "dual"; data-role = "host"; try-power-role = "source"; + + port { + usb_c0_ep: endpoint { + remote-endpoint = <&eud_con>; + }; + }; }; usb_c1: connector@1 { @@ -590,6 +596,15 @@ usb_c1: connector@1 { #include <arm/cros-ec-keyboard.dtsi> #include <arm/cros-ec-sbs.dtsi> +&eud_ports { + port@1 { + reg = <1>; + eud_con: endpoint { + remote-endpoint = <&usb_c0_ep>; + }; + }; +}; + &keyboard_controller { function-row-physmap = < MATRIX_KEY(0x00, 0x02, 0) /* T1 */ diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi index ebae545c587c..ffc469431340 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi @@ -44,6 +44,12 @@ usb_c0: connector@0 { power-role = "dual"; data-role = "host"; try-power-role = "source"; + + port { + usb_c0_ep: endpoint { + remote-endpoint = <&eud_con>; + }; + }; }; usb_c1: connector@1 { @@ -78,6 +84,15 @@ cr50: tpm@0 { }; }; +&eud_ports { + port@1 { + reg = <1>; + eud_con: endpoint { + remote-endpoint = <&usb_c0_ep>; + }; + }; +}; + &tlmm { ap_ec_int_l: ap-ec-int-l-state { pins = "gpio18"; diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index 925428a5f6ae..af9bb2ebcaa1 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -649,18 +649,6 @@ cpu7_opp_3014mhz: opp-3014400000 { }; }; - eud_typec: connector { - compatible = "usb-c-connector"; - - ports { - port@0 { - con_eud: endpoint { - remote-endpoint = <&eud_con>; - }; - }; - }; - }; - memory@80000000 { device_type = "memory"; /* We expect the bootloader to fill in the size */ @@ -3624,7 +3612,7 @@ eud: eud@88e0000 { <0 0x88e2000 0 0x1000>; interrupts-extended = <&pdc 11 IRQ_TYPE_LEVEL_HIGH>; - ports { + eud_ports: ports { #address-cells = <1>; #size-cells = <0>; @@ -3634,13 +3622,6 @@ eud_ep: endpoint { remote-endpoint = <&usb2_role_switch>; }; }; - - port@1 { - reg = <1>; - eud_con: endpoint { - remote-endpoint = <&con_eud>; - }; - }; }; };
Qualcomm Embedded USB Debugger (EUD) second port should point to Type-C USB connector. Such connector was defined directly in root node of sc7280.dtsi which is clearly wrong. SC7280 is a chip, so physically it does not have USB Type-C port. The connector is usually accessible through some USB switch or controller. Correct the EUD/USB connector topology by removing the top-level fake USB connector and adding appropriate ports in boards having actual USB Type-C connector defined (Herobrine, IDP). All other boards will have this EUD port missing. This fixes also dtbs_check warnings: sc7280-herobrine-crd.dtb: connector: ports:port@0: 'reg' is a required property Fixes: 9ee402ccfeb1 ("arm64: dts: qcom: sc7280: Fix EUD dt node syntax") Cc: Bhupesh Sharma <bhupesh.sharma@linaro.org> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Not tested on hardware. --- .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 15 +++++++++++++ .../arm64/boot/dts/qcom/sc7280-idp-ec-h1.dtsi | 15 +++++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 21 +------------------ 3 files changed, 31 insertions(+), 20 deletions(-)