linux-kernel - Re: [PATCH v2 1/2] arm64: dts: qcom: Add support for usb nodes on QCS8300

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <5800abe0-19e6-4364-a305-1be63c28c6d9@oss.qualcomm.com>
Date: Mon, 28 Oct 2024 12:38:35 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Krishna Kurapati <quic_kriskura@...cinc.com>,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Bjorn Andersson <andersson@...nel.org>, Rob Herring <robh@...nel.org>,
        devicetree@...r.kernel.org, Conor Dooley <conor+dt@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>, quic_ppratap@...cinc.com,
        quic_jackp@...cinc.com
Subject: Re: [PATCH v2 1/2] arm64: dts: qcom: Add support for usb nodes on
 QCS8300

On 26.10.2024 6:56 PM, Krishna Kurapati wrote:
> 
> 
> On 10/25/2024 11:58 PM, Konrad Dybcio wrote:
>> On 11.10.2024 9:46 AM, Krishna Kurapati wrote:
>>
>> The commit title should include a `qcs8300: ` part, like others in
>> the directory (see git log --oneline arch/arm64/boot/dts/qcom).
>>
>>> Add support for USB controllers on QCS8300. The second
>>> controller is only High Speed capable.
>>>
>>> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
>>> ---
>>>   arch/arm64/boot/dts/qcom/qcs8300.dtsi | 168 ++++++++++++++++++++++++++
>>>   1 file changed, 168 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>> index 2c35f96c3f28..4e6ba9f49b95 100644
>>> --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi
>>> @@ -1363,6 +1363,174 @@ IPCC_MPROC_SIGNAL_GLINK_QMP
>>>                   qcom,remote-pid = <5>;
>>>               };
>>>           };
>>> +
>>> +        usb_1_hsphy: phy@...4000 {
>>> +            compatible = "qcom,qcs8300-usb-hs-phy",
>>> +                     "qcom,usb-snps-hs-7nm-phy";
>>> +            reg = <0x0 0x8904000 0x0 0x400>;
>>
>> Please pad the address parts to 8 hex digits with leading zeroes.
>>
>>> +
>>> +            clocks = <&rpmhcc RPMH_CXO_CLK>;
>>> +            clock-names = "ref";
>>> +
>>> +            resets = <&gcc GCC_USB2_PHY_PRIM_BCR>;
>>> +
>>> +            #phy-cells = <0>;
>>> +
>>> +            status = "disabled";
>>> +        };
>>> +
>>> +        usb_2_hsphy: phy@...6000 {
>>> +            compatible = "qcom,qcs8300-usb-hs-phy",
>>> +                     "qcom,usb-snps-hs-7nm-phy";
>>> +            reg = <0x0 0x08906000 0x0 0x400>;
>>> +
>>> +            clocks = <&rpmhcc RPMH_CXO_CLK>;
>>> +            clock-names = "ref";
>>> +
>>> +            resets = <&gcc GCC_USB2_PHY_SEC_BCR>;
>>> +
>>> +            #phy-cells = <0>;
>>> +
>>> +            status = "disabled";
>>> +        };
>>> +
>>> +        usb_qmpphy: phy@...7000 {
>>> +            compatible = "qcom,qcs8300-qmp-usb3-uni-phy";
>>> +            reg = <0x0 0x8907000 0x0 0x2000>;
>>> +
>>> +            clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
>>> +                 <&gcc GCC_USB_CLKREF_EN>,
>>> +                 <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
>>> +                 <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
>>> +            clock-names = "aux", "ref", "com_aux", "pipe";
>>
>> Please make this a vertical list like in the node below.
>>
>> [...]
>>
>>> +            interconnects = <&aggre1_noc MASTER_USB3_0 0 &mc_virt SLAVE_EBI1 0>,
>>
>> QCOM_ICC_TAG_ALWAYS, see x1e80100.dtsi
>>
>>> +                    <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_0 0>;
>>> +            interconnect-names = "usb-ddr", "apps-usb";
>>> +
>>> +            wakeup-source;
>>> +
>>> +            status = "disabled";
>>> +
>>> +            usb_1_dwc3: usb@...0000 {
>>> +                compatible = "snps,dwc3";
>>> +                reg = <0x0 0x0a600000 0x0 0xe000>;
>>> +                interrupts = <GIC_SPI 292 IRQ_TYPE_LEVEL_HIGH>;
>>> +                iommus = <&apps_smmu 0x80 0x0>;
>>> +                phys = <&usb_1_hsphy>, <&usb_qmpphy>;
>>> +                phy-names = "usb2-phy", "usb3-phy";
>>> +                snps,dis_u2_susphy_quirk;
>>> +                snps,dis_enblslpm_quirk;
>>
>> That's a very low number of quirks.. Should we have some more?
>>
> 
> snps,dis-u1-entry-quirk;
> snps,dis-u2-entry-quirk;
> snps,dis_u2_susphy_quirk;
> snps,ssp-u3-u0-quirk;
> 
> I would actually like to add these as well, but there is no precedent in upstream as to what quirks to add for usb nodes

Every single one that applies to the hardware ;)

> , so I kept only a couple of them. Ideally downstream we disable u1u2 for almost all targets because of some issues in the past. (atleast during tethering use cases, but I need to double check though).

Does

5b8baed4b881 ("arm64: dts: qcom: sc7180: Disable SuperSpeed instances in park mode")

apply here too?

Konrad