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] [day] [month] [year] [list]
Message-ID: <16b78c39-c2a0-423f-8849-8b4c93e7ce84@quicinc.com>
Date: Mon, 28 Oct 2024 20:04:41 +0530
From: Krishna Kurapati <quic_kriskura@...cinc.com>
To: 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 10/28/2024 5:08 PM, Konrad Dybcio wrote:
> 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?
> 

QCS8300 is Gen-2, so that quirk is not needed.

Regards,
Krishna,