Re: [PATCH RFT] arm64: dts: qcom: sc7280: drop incorrect EUD port on SoC side

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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