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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4b6c8f86-d879-fa69-ca28-cb684877932e@linaro.org>
Date:   Sun, 20 Aug 2023 09:46:34 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     cros-qcom-dts-watchers@...omium.org,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Bhupesh Sharma <bhupesh.sharma@...aro.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Souradeep Chowdhury <quic_schowdhu@...cinc.com>
Subject: Re: [PATCH RFT] arm64: dts: qcom: sc7280: drop incorrect EUD port on
 SoC side

On 25/07/2023 21:35, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 25, 2023 at 1:46 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@...aro.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@...aro.org>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.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