[RFT] arm64: dts: qcom: sc7280: drop incorrect EUD port on SoC side
Commit Message
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(-)
Comments
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.
-Doug
@@ -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 */
@@ -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";
@@ -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>;
- };
- };
};
};