Message ID | 20230514054917.21318-8-quic_kriskura@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp6133091vqo; Sat, 13 May 2023 22:52:41 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4I9+ihrH9RnFhe0wZEEWoIiFvXD8zM2YARIg398QGX7rOPi09St0YRBgbQSXkvyszKvtAl X-Received: by 2002:a05:6a21:6d8a:b0:101:282c:2b with SMTP id wl10-20020a056a216d8a00b00101282c002bmr24137094pzb.32.1684043561512; Sat, 13 May 2023 22:52:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684043561; cv=none; d=google.com; s=arc-20160816; b=mME8VpfHrd56+87s2P/2fI3m21R1ou95ghw2OaxyxqMFHkjJiR3a1ncwX2j3nG7AWy rRtyN/DawU486CFZSI4NLJ6LnBIRlVYWTkcdTzJzLhmVEhTWXPectScbqHDhXDcnHdRT eiygw68KDrhxycL/tjrA6ET3gpSoChVZwDU8juqTYOj6is9EdZ9C3gU+eo2vX0ADjyR5 01OQuypxdpQ7Ih6a1SNKY6w3OfjCZoChSS8PNlEcP58w5uSJpSexq3IOM3mwN9kYnXFv KLsrON6cdiwYNELDbFjjFFn7c4TVTIIvEQ6L00CcnkmTmp91RPVA+Fz53hcI3j088H4V g1mQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=wJ9Vc6ehQfoDKHhoTHMU2B3s0lYNjvU/4yKdkQIX1As=; b=VYg51M5I7BEG0VkHafRMrBBz1A/znI82JLPzmH9byYFcqgZPUzFUrPBwvKVXH6imLG iwyN/xX2nNDr9H1f40ZYCnWDiUmlCqidC/qaQqn9uJtloRT0R/MORf+T7pcyC54Bp+yQ P9SifpHOSOfsszdQAy4HBlx9QSrZVfM8+RQ8lQhp9wVt9tN6KYEDXcF2WQpQgkMEU4nF BZ/6MNJ+9HjHxFUK7OCbRjGlFC6su/5C2luuPSTFRMqCfmJ/YF4IVcmJPRJUb6fdbpct t9Da8LC2V01yqvd1PLv4r3iq9D57o/sjCrvT1ytQKwU+hvKuiM4LfivmsCrUbh/9yKE9 z8Uw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=VVH1GOat; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b188-20020a6334c5000000b00524fcd54553si12419332pga.485.2023.05.13.22.52.28; Sat, 13 May 2023 22:52:41 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=VVH1GOat; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235216AbjENFvU (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Sun, 14 May 2023 01:51:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37306 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233871AbjENFuv (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 14 May 2023 01:50:51 -0400 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CD6F7273E; Sat, 13 May 2023 22:50:27 -0700 (PDT) Received: from pps.filterd (m0279864.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 34E5oHPi021442; Sun, 14 May 2023 05:50:17 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=qcppdkim1; bh=wJ9Vc6ehQfoDKHhoTHMU2B3s0lYNjvU/4yKdkQIX1As=; b=VVH1GOatNvi1EVBv0FSqmcag7q7JZ+Ibe4arrEYcYsrnIU3mMhW5uRYKo6O83MzIATcw UaZZ223qXQWWew2h4QBxBqi1SJbYSJyvSI5A8kte9jRtjG6lFy0c/G3xsdByEpDxQ0zC gVTFT9y0B/sqPdN/Vt7uD1w54D01wbQ1NXJOBMJfg9WXh8OddXCuH+uGder5ggZUVti/ kRhUf4FmGS9eJn8tjSPh2mMONKd7mWHGBEkHAfATnYhCDboJ3IoAe8Yibx1FVwfyVqao mVal2FHGYrMn/+gDrGmNgz8Go3sfZ2tUjHy7bpT4yF0WK1kAFbxQix+FMqh3eG6Kw0I6 jQ== Received: from nalasppmta05.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3qj3qw1bme-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 14 May 2023 05:50:17 +0000 Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA05.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 34E5oG67021303 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 14 May 2023 05:50:16 GMT Received: from hu-kriskura-hyd.qualcomm.com (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.42; Sat, 13 May 2023 22:50:10 -0700 From: Krishna Kurapati <quic_kriskura@quicinc.com> To: Thinh Nguyen <Thinh.Nguyen@synopsys.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Philipp Zabel <p.zabel@pengutronix.de>, "Andy Gross" <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, "Konrad Dybcio" <konrad.dybcio@linaro.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Felipe Balbi <balbi@kernel.org> CC: <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>, <quic_pkondeti@quicinc.com>, <quic_ppratap@quicinc.com>, <quic_wcheng@quicinc.com>, <quic_jackp@quicinc.com>, <quic_harshq@quicinc.com>, <ahalaney@redhat.com>, Krishna Kurapati <quic_kriskura@quicinc.com> Subject: [PATCH v8 7/9] arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280 Date: Sun, 14 May 2023 11:19:15 +0530 Message-ID: <20230514054917.21318-8-quic_kriskura@quicinc.com> X-Mailer: git-send-email 2.40.0 In-Reply-To: <20230514054917.21318-1-quic_kriskura@quicinc.com> References: <20230514054917.21318-1-quic_kriskura@quicinc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: 7dvMPE7PCbSOnTM_N6hG_EOyfBsgC3Ub X-Proofpoint-GUID: 7dvMPE7PCbSOnTM_N6hG_EOyfBsgC3Ub X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-05-14_03,2023-05-05_01,2023-02-09_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 phishscore=0 adultscore=0 impostorscore=0 bulkscore=0 suspectscore=0 priorityscore=1501 mlxscore=0 spamscore=0 lowpriorityscore=0 mlxlogscore=999 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000 definitions=main-2305140052 X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765847661869018686?= X-GMAIL-MSGID: =?utf-8?q?1765847661869018686?= |
Series |
Add multiport support for DWC3 controllers
|
|
Commit Message
Krishna Kurapati
May 14, 2023, 5:49 a.m. UTC
Add USB and DWC3 node for tertiary port of SC8280 along with multiport
IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride
platforms.
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 66 ++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
Comments
On Sun, May 14, 2023 at 11:19:15AM +0530, Krishna Kurapati wrote: > Add USB and DWC3 node for tertiary port of SC8280 along with multiport > IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride > platforms. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 66 ++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > index 8fa9fbfe5d00..50f6a8424537 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi > @@ -3133,6 +3133,72 @@ usb_1_role_switch: endpoint { > }; > }; > > + usb_2: usb@a4f8800 { As I believe someone already pointed out, this node is not in sort order (i.e. it should go before usb@a6f8800). > + compatible = "qcom,sc8280xp-dwc3-mp", "qcom,dwc3"; > + reg = <0 0x0a4f8800 0 0x400>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>, > + <&gcc GCC_USB30_MP_MASTER_CLK>, > + <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>, > + <&gcc GCC_USB30_MP_SLEEP_CLK>, > + <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>, > + <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>, > + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; > + clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi", > + "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys"; > + > + assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, > + <&gcc GCC_USB30_MP_MASTER_CLK>; > + assigned-clock-rates = <19200000>, <200000000>; > + > + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, > + <&pdc 126 IRQ_TYPE_EDGE_RISING>, > + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>, > + <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>, > + <&intc GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>, > + <&intc GIC_SPI 857 IRQ_TYPE_LEVEL_HIGH>, > + <&intc GIC_SPI 856 IRQ_TYPE_LEVEL_HIGH>; > + > + interrupt-names = "dp_hs_phy_irq", > + "dm_hs_phy_irq", > + "ss_phy_irq", > + "pwr_event_1", > + "pwr_event_2", > + "pwr_event_3", > + "pwr_event_4"; > + > + power-domains = <&gcc USB30_MP_GDSC>; > + required-opps = <&rpmhpd_opp_nom>; > + > + resets = <&gcc GCC_USB30_MP_BCR>; > + > + interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>, > + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>; This is not the correct interconnect master and slave; it should be MASTER_USB3_MP and SLAVE_USB3_MP. > + interconnect-names = "usb-ddr", "apps-usb"; Looks like 'wakeup-source' is missing here too. > + > + status = "disabled"; > + > + usb_2_dwc3: usb@a400000 { > + compatible = "snps,dwc3"; > + reg = <0 0x0a400000 0 0xcd00>; > + interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>; > + iommus = <&apps_smmu 0x800 0x0>; > + phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>, > + <&usb_2_hsphy1>, <&usb_2_qmpphy1>, > + <&usb_2_hsphy2>, > + <&usb_2_hsphy3>; > + phy-names = "usb2-port0", "usb3-port0", > + "usb2-port1", "usb3-port1", > + "usb2-port2", > + "usb2-port3"; The phys and phy-names continuation lines above are still not aligned. > + }; > + }; > + > mdss0: display-subsystem@ae00000 { > compatible = "qcom,sc8280xp-mdss"; > reg = <0 0x0ae00000 0 0x1000>; Johan
On 5/15/2023 7:56 PM, Johan Hovold wrote: > On Sun, May 14, 2023 at 11:19:15AM +0530, Krishna Kurapati wrote: >> Add USB and DWC3 node for tertiary port of SC8280 along with multiport >> IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride >> platforms. >> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 66 ++++++++++++++++++++++++++ >> 1 file changed, 66 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> index 8fa9fbfe5d00..50f6a8424537 100644 >> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi >> @@ -3133,6 +3133,72 @@ usb_1_role_switch: endpoint { >> }; >> }; >> >> + usb_2: usb@a4f8800 { > > As I believe someone already pointed out, this node is not in sort order > (i.e. it should go before usb@a6f8800). > Hi Johan, I missed that message, but since I named it usb_2, so I placed it in order after usb_1. Hope that is fine !! >> + compatible = "qcom,sc8280xp-dwc3-mp", "qcom,dwc3"; >> + reg = <0 0x0a4f8800 0 0x400>; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>, >> + <&gcc GCC_USB30_MP_MASTER_CLK>, >> + <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>, >> + <&gcc GCC_USB30_MP_SLEEP_CLK>, >> + <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, >> + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, >> + <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>, >> + <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>, >> + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; >> + clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi", >> + "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys"; >> + >> + assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, >> + <&gcc GCC_USB30_MP_MASTER_CLK>; >> + assigned-clock-rates = <19200000>, <200000000>; >> + >> + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, >> + <&pdc 126 IRQ_TYPE_EDGE_RISING>, >> + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>, >> + <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>, >> + <&intc GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>, >> + <&intc GIC_SPI 857 IRQ_TYPE_LEVEL_HIGH>, >> + <&intc GIC_SPI 856 IRQ_TYPE_LEVEL_HIGH>; >> + >> + interrupt-names = "dp_hs_phy_irq", >> + "dm_hs_phy_irq", >> + "ss_phy_irq", >> + "pwr_event_1", >> + "pwr_event_2", >> + "pwr_event_3", >> + "pwr_event_4"; >> + >> + power-domains = <&gcc USB30_MP_GDSC>; >> + required-opps = <&rpmhpd_opp_nom>; >> + >> + resets = <&gcc GCC_USB30_MP_BCR>; >> + >> + interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>, >> + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>; > > This is not the correct interconnect master and slave; it should be > MASTER_USB3_MP and SLAVE_USB3_MP. > Thanks for pointing it out. I need to check how it was working all these days. (Probably since both of them vote for the same NOC, it didn't show any affect) >> + interconnect-names = "usb-ddr", "apps-usb"; > > Looks like 'wakeup-source' is missing here too. > I believe this property was added to enable wakeup from system suspend in host mode. I didn't add this property as currently I don't need to support wakeup. If any requirement comes in future, then I might need to add dp/dm interrupts (if any) for other ports as well and then need to change driver code to enable/disable them on suspend/resume. >> + >> + status = "disabled"; >> + >> + usb_2_dwc3: usb@a400000 { >> + compatible = "snps,dwc3"; >> + reg = <0 0x0a400000 0 0xcd00>; >> + interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>; >> + iommus = <&apps_smmu 0x800 0x0>; >> + phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>, >> + <&usb_2_hsphy1>, <&usb_2_qmpphy1>, >> + <&usb_2_hsphy2>, >> + <&usb_2_hsphy3>; >> + phy-names = "usb2-port0", "usb3-port0", >> + "usb2-port1", "usb3-port1", >> + "usb2-port2", >> + "usb2-port3"; > > The phys and phy-names continuation lines above are still not aligned. > Missed it. Will fix it in next version. Thanks, Krishna, >> + }; >> + }; >> + >> mdss0: display-subsystem@ae00000 { >> compatible = "qcom,sc8280xp-mdss"; >> reg = <0 0x0ae00000 0 0x1000>; > > Johan
On Mon, May 15, 2023 at 09:02:13PM +0530, Krishna Kurapati PSSNV wrote: > On 5/15/2023 7:56 PM, Johan Hovold wrote: > > On Sun, May 14, 2023 at 11:19:15AM +0530, Krishna Kurapati wrote: > >> @@ -3133,6 +3133,72 @@ usb_1_role_switch: endpoint { > >> }; > >> }; > >> > >> + usb_2: usb@a4f8800 { > > > > As I believe someone already pointed out, this node is not in sort order > > (i.e. it should go before usb@a6f8800). > I missed that message, but since I named it usb_2, so I placed it in > order after usb_1. Hope that is fine !! No, the nodes should be sorted by unit address so you need to move it. > >> + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, > >> + <&pdc 126 IRQ_TYPE_EDGE_RISING>, > >> + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>, > >> + <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>, > >> + <&intc GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>, > >> + <&intc GIC_SPI 857 IRQ_TYPE_LEVEL_HIGH>, > >> + <&intc GIC_SPI 856 IRQ_TYPE_LEVEL_HIGH>; > >> + > >> + interrupt-names = "dp_hs_phy_irq", > >> + "dm_hs_phy_irq", > >> + "ss_phy_irq", > >> + "pwr_event_1", > >> + "pwr_event_2", > >> + "pwr_event_3", > >> + "pwr_event_4"; > >> + interconnect-names = "usb-ddr", "apps-usb"; > > > > Looks like 'wakeup-source' is missing here too. > > > > I believe this property was added to enable wakeup from system suspend > in host mode. I didn't add this property as currently I don't need to > support wakeup. If any requirement comes in future, then I might need to > add dp/dm interrupts (if any) for other ports as well and then need to > change driver code to enable/disable them on suspend/resume. If there are dp/dm/ss interrupts per ports then those need to be defined in the binding and devicetree from the start. Similar for 'wakeup-source' which indicates that the controller *can* be used to wakeup the system from suspend (which those pdc interrupts indicates). Remember that the devicetree is supposed to describe the hardware, and which features are currently supported in some version of software is mostly irrelevant. Johan
On 5/16/2023 4:24 PM, Johan Hovold wrote: > On Mon, May 15, 2023 at 09:02:13PM +0530, Krishna Kurapati PSSNV wrote: >> On 5/15/2023 7:56 PM, Johan Hovold wrote: >>> On Sun, May 14, 2023 at 11:19:15AM +0530, Krishna Kurapati wrote: > >>>> @@ -3133,6 +3133,72 @@ usb_1_role_switch: endpoint { >>>> }; >>>> }; >>>> >>>> + usb_2: usb@a4f8800 { >>> >>> As I believe someone already pointed out, this node is not in sort order >>> (i.e. it should go before usb@a6f8800). > >> I missed that message, but since I named it usb_2, so I placed it in >> order after usb_1. Hope that is fine !! > > No, the nodes should be sorted by unit address so you need to move it. > Sure, in that case will put it in between usb_0 and usb_1 nodes. >>>> + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, >>>> + <&pdc 126 IRQ_TYPE_EDGE_RISING>, >>>> + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>, >>>> + <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>, >>>> + <&intc GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>, >>>> + <&intc GIC_SPI 857 IRQ_TYPE_LEVEL_HIGH>, >>>> + <&intc GIC_SPI 856 IRQ_TYPE_LEVEL_HIGH>; >>>> + >>>> + interrupt-names = "dp_hs_phy_irq", >>>> + "dm_hs_phy_irq", >>>> + "ss_phy_irq", >>>> + "pwr_event_1", >>>> + "pwr_event_2", >>>> + "pwr_event_3", >>>> + "pwr_event_4"; > >>>> + interconnect-names = "usb-ddr", "apps-usb"; >>> >>> Looks like 'wakeup-source' is missing here too. >>> >> >> I believe this property was added to enable wakeup from system suspend >> in host mode. I didn't add this property as currently I don't need to >> support wakeup. If any requirement comes in future, then I might need to >> add dp/dm interrupts (if any) for other ports as well and then need to >> change driver code to enable/disable them on suspend/resume. > > If there are dp/dm/ss interrupts per ports then those need to be defined > in the binding and devicetree from the start. > > Similar for 'wakeup-source' which indicates that the controller *can* be > used to wakeup the system from suspend (which those pdc interrupts > indicates). > > Remember that the devicetree is supposed to describe the hardware, and > which features are currently supported in some version of software is > mostly irrelevant. > > Johan Can I take this up as a separate series (Wakeup support for multiport) once this series is merged. If I am adding interrupts for other ports, I can add driver code to handle those interrupts as well. Regards, Krishna,
On Tue, May 16, 2023 at 07:54:00PM +0530, Krishna Kurapati PSSNV wrote: > > > On 5/16/2023 4:24 PM, Johan Hovold wrote: > > On Mon, May 15, 2023 at 09:02:13PM +0530, Krishna Kurapati PSSNV wrote: > >> On 5/15/2023 7:56 PM, Johan Hovold wrote: > >>> On Sun, May 14, 2023 at 11:19:15AM +0530, Krishna Kurapati wrote: > > > >>>> @@ -3133,6 +3133,72 @@ usb_1_role_switch: endpoint { > >>>> }; > >>>> }; > >>>> > >>>> + usb_2: usb@a4f8800 { > >>> > >>> As I believe someone already pointed out, this node is not in sort order > >>> (i.e. it should go before usb@a6f8800). > > > >> I missed that message, but since I named it usb_2, so I placed it in > >> order after usb_1. Hope that is fine !! > > > > No, the nodes should be sorted by unit address so you need to move it. > > > > Sure, in that case will put it in between usb_0 and usb_1 nodes. No, it goes before usb_0 on sc8280xp. usb_2: usb@a4f8800 { usb_0: usb@a6f8800 { usb_1: usb@a8f8800 { > >>>> + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, > >>>> + <&pdc 126 IRQ_TYPE_EDGE_RISING>, > >>>> + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>, > >>>> + <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>, > >>>> + <&intc GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>, > >>>> + <&intc GIC_SPI 857 IRQ_TYPE_LEVEL_HIGH>, > >>>> + <&intc GIC_SPI 856 IRQ_TYPE_LEVEL_HIGH>; > >>>> + > >>>> + interrupt-names = "dp_hs_phy_irq", > >>>> + "dm_hs_phy_irq", > >>>> + "ss_phy_irq", > >>>> + "pwr_event_1", > >>>> + "pwr_event_2", > >>>> + "pwr_event_3", > >>>> + "pwr_event_4"; > > > >>>> + interconnect-names = "usb-ddr", "apps-usb"; > >>> > >>> Looks like 'wakeup-source' is missing here too. > >>> > >> > >> I believe this property was added to enable wakeup from system suspend > >> in host mode. I didn't add this property as currently I don't need to > >> support wakeup. If any requirement comes in future, then I might need to > >> add dp/dm interrupts (if any) for other ports as well and then need to > >> change driver code to enable/disable them on suspend/resume. > > > > If there are dp/dm/ss interrupts per ports then those need to be defined > > in the binding and devicetree from the start. > > > > Similar for 'wakeup-source' which indicates that the controller *can* be > > used to wakeup the system from suspend (which those pdc interrupts > > indicates). > > > > Remember that the devicetree is supposed to describe the hardware, and > > which features are currently supported in some version of software is > > mostly irrelevant. > Can I take this up as a separate series (Wakeup support for multiport) > once this series is merged. If I am adding interrupts for other ports, I > can add driver code to handle those interrupts as well. Nope. You can possibly add driver support later, but the binding and dtsi need to be correct from the start (and it may be easier to do it all at once). Johan
On 5/16/2023 8:12 PM, Johan Hovold wrote: > On Tue, May 16, 2023 at 07:54:00PM +0530, Krishna Kurapati PSSNV wrote: >> >> >> On 5/16/2023 4:24 PM, Johan Hovold wrote: >>> On Mon, May 15, 2023 at 09:02:13PM +0530, Krishna Kurapati PSSNV wrote: >>>> On 5/15/2023 7:56 PM, Johan Hovold wrote: >>>>> On Sun, May 14, 2023 at 11:19:15AM +0530, Krishna Kurapati wrote: >>> >>>>>> @@ -3133,6 +3133,72 @@ usb_1_role_switch: endpoint { >>>>>> }; >>>>>> }; >>>>>> >>>>>> + usb_2: usb@a4f8800 { >>>>> >>>>> As I believe someone already pointed out, this node is not in sort order >>>>> (i.e. it should go before usb@a6f8800). >>> >>>> I missed that message, but since I named it usb_2, so I placed it in >>>> order after usb_1. Hope that is fine !! >>> >>> No, the nodes should be sorted by unit address so you need to move it. >>> >> >> Sure, in that case will put it in between usb_0 and usb_1 nodes. > > No, it goes before usb_0 on sc8280xp. > > usb_2: usb@a4f8800 { > usb_0: usb@a6f8800 { > usb_1: usb@a8f8800 { > >>>>>> + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, >>>>>> + <&pdc 126 IRQ_TYPE_EDGE_RISING>, >>>>>> + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>, >>>>>> + <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>, >>>>>> + <&intc GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>, >>>>>> + <&intc GIC_SPI 857 IRQ_TYPE_LEVEL_HIGH>, >>>>>> + <&intc GIC_SPI 856 IRQ_TYPE_LEVEL_HIGH>; >>>>>> + >>>>>> + interrupt-names = "dp_hs_phy_irq", >>>>>> + "dm_hs_phy_irq", >>>>>> + "ss_phy_irq", >>>>>> + "pwr_event_1", >>>>>> + "pwr_event_2", >>>>>> + "pwr_event_3", >>>>>> + "pwr_event_4"; >>> >>>>>> + interconnect-names = "usb-ddr", "apps-usb"; >>>>> >>>>> Looks like 'wakeup-source' is missing here too. >>>>> >>>> >>>> I believe this property was added to enable wakeup from system suspend >>>> in host mode. I didn't add this property as currently I don't need to >>>> support wakeup. If any requirement comes in future, then I might need to >>>> add dp/dm interrupts (if any) for other ports as well and then need to >>>> change driver code to enable/disable them on suspend/resume. >>> >>> If there are dp/dm/ss interrupts per ports then those need to be defined >>> in the binding and devicetree from the start. >>> >>> Similar for 'wakeup-source' which indicates that the controller *can* be >>> used to wakeup the system from suspend (which those pdc interrupts >>> indicates). >>> >>> Remember that the devicetree is supposed to describe the hardware, and >>> which features are currently supported in some version of software is >>> mostly irrelevant. > >> Can I take this up as a separate series (Wakeup support for multiport) >> once this series is merged. If I am adding interrupts for other ports, I >> can add driver code to handle those interrupts as well. > > Nope. You can possibly add driver support later, but the binding and > dtsi need to be correct from the start (and it may be easier to do it > all at once). > Ok, will add this in v9. Regards, Krishna,
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi index 8fa9fbfe5d00..50f6a8424537 100644 --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi @@ -3133,6 +3133,72 @@ usb_1_role_switch: endpoint { }; }; + usb_2: usb@a4f8800 { + compatible = "qcom,sc8280xp-dwc3-mp", "qcom,dwc3"; + reg = <0 0x0a4f8800 0 0x400>; + #address-cells = <2>; + #size-cells = <2>; + ranges; + + clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>, + <&gcc GCC_USB30_MP_MASTER_CLK>, + <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>, + <&gcc GCC_USB30_MP_SLEEP_CLK>, + <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, + <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>, + <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>, + <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>, + <&gcc GCC_SYS_NOC_USB_AXI_CLK>; + clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi", + "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys"; + + assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>, + <&gcc GCC_USB30_MP_MASTER_CLK>; + assigned-clock-rates = <19200000>, <200000000>; + + interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>, + <&pdc 126 IRQ_TYPE_EDGE_RISING>, + <&pdc 16 IRQ_TYPE_LEVEL_HIGH>, + <&intc GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>, + <&intc GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>, + <&intc GIC_SPI 857 IRQ_TYPE_LEVEL_HIGH>, + <&intc GIC_SPI 856 IRQ_TYPE_LEVEL_HIGH>; + + interrupt-names = "dp_hs_phy_irq", + "dm_hs_phy_irq", + "ss_phy_irq", + "pwr_event_1", + "pwr_event_2", + "pwr_event_3", + "pwr_event_4"; + + power-domains = <&gcc USB30_MP_GDSC>; + required-opps = <&rpmhpd_opp_nom>; + + resets = <&gcc GCC_USB30_MP_BCR>; + + interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>, + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>; + interconnect-names = "usb-ddr", "apps-usb"; + + status = "disabled"; + + usb_2_dwc3: usb@a400000 { + compatible = "snps,dwc3"; + reg = <0 0x0a400000 0 0xcd00>; + interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>; + iommus = <&apps_smmu 0x800 0x0>; + phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>, + <&usb_2_hsphy1>, <&usb_2_qmpphy1>, + <&usb_2_hsphy2>, + <&usb_2_hsphy3>; + phy-names = "usb2-port0", "usb3-port0", + "usb2-port1", "usb3-port1", + "usb2-port2", + "usb2-port3"; + }; + }; + mdss0: display-subsystem@ae00000 { compatible = "qcom,sc8280xp-mdss"; reg = <0 0x0ae00000 0 0x1000>;