Message ID | 20230119140453.3942340-12-abel.vesa@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp347105wrn; Thu, 19 Jan 2023 06:08:52 -0800 (PST) X-Google-Smtp-Source: AMrXdXvwcHYRqy4VY9NDErF4LFz2BEydrgG+7+McMZlrtEQqVq92TIghL9z6rO7Ahxk8fQwf/Q0r X-Received: by 2002:a17:902:7049:b0:194:c241:f604 with SMTP id h9-20020a170902704900b00194c241f604mr4825348plt.57.1674137332636; Thu, 19 Jan 2023 06:08:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674137332; cv=none; d=google.com; s=arc-20160816; b=l+20z1pb7T/XQuDVSZ1l8bDvGvDEy0eGTv7RgxYhluYe93zDTjaiSaA5Iqg8QtrMP0 Dt59P738aU1dZntX/Q3Tzr+K3GAFW4Q6I8XojSNgXw/35txbyUEiISBmq9cv7oua6Qxq icziz5nUcYoz8Sv5nwPG5BPWaBt2nQaJ8v/oCyhX2fX/XWsQv3w/WY4ehwae2UoqVjhh TTRW/RnBPpvM+tfVRz0Qn3q3zsZJLe3gv9QVWxYSOe7dCWmjxNhcvKeXGsOZplz7MytN rQLm/Q2g8e4kldbnXE31ngZtxj6Rg3T9AjKGqhoE9WMxCK/TJ00lu7MKJP+867Nat5Mh FLEQ== 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=NAv8RcQ1Bsx8gUbogF4715qXGJcbfiJKjivsfn1yiu0=; b=Ba8VJtjkAr26Hhl0+bzie1cz4HgdonXERZZLAwhqJGo4OioLdj4st4Fzar1+LP4pMw DIlpjj3x7F0KdJJXQtcagYlFvJIXmCP0xQRL/OEBovchF5cX7/dCaQA8tkkSgutnBARz +oE3BoJanB4ybC5PPcdZYbAM/ksIzR+F/dnOlxZLZOMoEA6Dx1oI+ki1R3vmJI2jO3mk SF0SWwo9dsOTAgOTWvoOj540zUwJkAwXz5Ot1PHcOmZZ0eVfkAHMMP7SuIS4gCbVcRNH 8WTMkjI71h5K8OEBuEenGRqx/O1DIz1/NkUPI+5VEL2votfi3LAlm3zimyRQPj2JuxWP jpFA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Un5qh1G6; 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=linaro.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l12-20020a63da4c000000b004ced9993991si9132037pgj.369.2023.01.19.06.08.39; Thu, 19 Jan 2023 06:08:52 -0800 (PST) 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=@linaro.org header.s=google header.b=Un5qh1G6; 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=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231158AbjASOGp (ORCPT <rfc822;pfffrao@gmail.com> + 99 others); Thu, 19 Jan 2023 09:06:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33566 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231416AbjASOF5 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 19 Jan 2023 09:05:57 -0500 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 84DE6808B0 for <linux-kernel@vger.kernel.org>; Thu, 19 Jan 2023 06:05:25 -0800 (PST) Received: by mail-wm1-x32e.google.com with SMTP id l8so1615722wms.3 for <linux-kernel@vger.kernel.org>; Thu, 19 Jan 2023 06:05:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=NAv8RcQ1Bsx8gUbogF4715qXGJcbfiJKjivsfn1yiu0=; b=Un5qh1G61U2quJ4YHwSunBiI5bgC2y7hlWhl5OcJWpUokh8XTKIfJQEhIbMWzQYsWw IZuusnyHBXsiDSC+4bzZKWiCDmGwF1c8vzsTd1Ox5Ea7FbNLEB6yxOpQoZWWmDP3hxrL UYiFIb0rvKLjuZzLqUN5kKwAY8sZuAYtGzkFVMoMDqsSPNkb4Y9aV2t/2hlWojIgVceG OWyyaW8IPZzjl4Xr1w3S118riPD9eCHppjVmYui+h6iqJVIFO+sea2qvIhKwsSJ/aK90 rom/7t75Dj47VPzUYMfmVEvAJzzzoP0p9a7JOZFPtZN5zx885WN9+Qrwd3kJ9PcwctZ0 5qJw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NAv8RcQ1Bsx8gUbogF4715qXGJcbfiJKjivsfn1yiu0=; b=QVVXi7FlfsErpsr0aK7LCcwpGEIEV8XlDmH3vCI9Jcp/vSbn9xXkbxMqdVN84eR50O VQRmRvaeg/I6406pst1bsvXvJmsdbSh9lFFNZid6GSPKgRSHxedCZaikftuXXOn3+rWT MfQSuSfe7ZzjdzdZAbUI1JCTEDG435T+IZ2JEhTsXrdJTROQ6iHwCrvxZlzv3kisA+SF sQF1UmDtvJn6Fs9ET13ORa5kkvQk2SOwMIOYg/fOqH+9abaE7AbmrT3uj+A4XQknKwyX 3yoQOCWz8QkuH2pxDgziC1dk87yjevstVqCxKB9H/qiP81HwRLqKIKSeRzeGvqZgNMFD Yg3g== X-Gm-Message-State: AFqh2koFayUpsw/ZlIHbpd0wIZpsWu/Xtrj8nfilrP3CILELGFd6SQ/z 5k6MdPfHSUhTj4DGH39vwwirHA== X-Received: by 2002:a05:600c:3514:b0:3c6:c6c9:d75e with SMTP id h20-20020a05600c351400b003c6c6c9d75emr10922437wmq.0.1674137124983; Thu, 19 Jan 2023 06:05:24 -0800 (PST) Received: from hackbox.lan ([94.52.112.99]) by smtp.gmail.com with ESMTPSA id m10-20020a05600c4f4a00b003d96efd09b7sm5263883wmq.19.2023.01.19.06.05.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Jan 2023 06:05:24 -0800 (PST) From: Abel Vesa <abel.vesa@linaro.org> To: Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>, Rob Herring <robh@kernel.org>, =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= <kw@linux.com>, Bjorn Helgaas <bhelgaas@google.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Lorenzo Pieralisi <lpieralisi@kernel.org>, "vkoul@kernel.org" <vkoul@kernel.org>, Kishon Vijay Abraham I <kishon@kernel.org>, Manivannan Sadhasivam <mani@kernel.org> Cc: linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org, linux-phy@lists.infradead.org, devicetree@vger.kernel.org, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: [PATCH v4 11/12] arm64: dts: qcom: sm8550: Add PCIe PHYs and controllers nodes Date: Thu, 19 Jan 2023 16:04:52 +0200 Message-Id: <20230119140453.3942340-12-abel.vesa@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230119140453.3942340-1-abel.vesa@linaro.org> References: <20230119140453.3942340-1-abel.vesa@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS 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?1755460227387904553?= X-GMAIL-MSGID: =?utf-8?q?1755460227387904553?= |
Series |
sm8550: Add PCIe HC and PHY support
|
|
Commit Message
Abel Vesa
Jan. 19, 2023, 2:04 p.m. UTC
Add PCIe controllers and PHY nodes.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
This patch does not have a v3, but since it is now part of the same
patchset with the controller and the phy drivers patches, I had to
bump the version to 4.
Latest version was here (v2):
https://lore.kernel.org/all/20230118230526.1499328-2-abel.vesa@linaro.org/
Changes since latest version (v2):
* renamed the pcie_1_link_down_reset to simply link_down
* dropped the pipe from clock-names
* renamed aggre clock-names to noc_aggr_4
* dropped the _pcie infix from cnoc_pcie_sf_axi
* dropped the aux_phy clock from the pcie1
Changes since v1:
* ordered pcie related nodes alphabetically in MTP dts
* dropped the pipe_mux, phy_pipe and ref clocks from the pcie nodes
* dropped the child node from the phy nodes, like Johan suggested,
and updated to use the sc8280xp binding scheme
* changed "pcie_1_nocsr_com_phy_reset" 2nd reset name of pcie1_phy
to "nocsr"
* reordered all pcie nodes properties to look similar to the ones
from sc8280xp
arch/arm64/boot/dts/qcom/sm8550.dtsi | 207 ++++++++++++++++++++++++++-
1 file changed, 204 insertions(+), 3 deletions(-)
Comments
On Thu, Jan 19, 2023 at 04:04:52PM +0200, Abel Vesa wrote: > Add PCIe controllers and PHY nodes. > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > > This patch does not have a v3, but since it is now part of the same > patchset with the controller and the phy drivers patches, I had to > bump the version to 4. > > Latest version was here (v2): > https://lore.kernel.org/all/20230118230526.1499328-2-abel.vesa@linaro.org/ > > Changes since latest version (v2): > * renamed the pcie_1_link_down_reset to simply link_down > * dropped the pipe from clock-names > * renamed aggre clock-names to noc_aggr_4 > * dropped the _pcie infix from cnoc_pcie_sf_axi > * dropped the aux_phy clock from the pcie1 > > Changes since v1: > * ordered pcie related nodes alphabetically in MTP dts > * dropped the pipe_mux, phy_pipe and ref clocks from the pcie nodes > * dropped the child node from the phy nodes, like Johan suggested, > and updated to use the sc8280xp binding scheme > * changed "pcie_1_nocsr_com_phy_reset" 2nd reset name of pcie1_phy > to "nocsr" > * reordered all pcie nodes properties to look similar to the ones > from sc8280xp > > > arch/arm64/boot/dts/qcom/sm8550.dtsi | 207 ++++++++++++++++++++++++++- > 1 file changed, 204 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi > index 3d47281a276b..8df226530d76 100644 > --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi > @@ -646,9 +646,9 @@ gcc: clock-controller@100000 { > #reset-cells = <1>; > #power-domain-cells = <1>; > clocks = <&bi_tcxo_div2>, <&sleep_clk>, > - <0>, > - <0>, > - <0>, > + <&pcie0_phy>, > + <&pcie1_phy>, > + <&pcie_1_phy_aux_clk>, > <&ufs_mem_phy 0>, > <&ufs_mem_phy 1>, > <&ufs_mem_phy 2>, > @@ -1547,6 +1547,207 @@ mmss_noc: interconnect@1780000 { > qcom,bcm-voters = <&apps_bcm_voter>; > }; > > + pcie0: pci@1c00000 { > + device_type = "pci"; > + compatible = "qcom,pcie-sm8550"; > + reg = <0 0x01c00000 0 0x3000>, > + <0 0x60000000 0 0xf1d>, > + <0 0x60000f20 0 0xa8>, > + <0 0x60001000 0 0x1000>, > + <0 0x60100000 0 0x100000>; > + reg-names = "parf", "dbi", "elbi", "atu", "config"; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>, > + <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>; > + bus-range = <0x00 0xff>; > + > + dma-coherent; > + > + linux,pci-domain = <0>; > + num-lanes = <2>; > + > + interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "msi"; > + > + #interrupt-cells = <1>; > + interrupt-map-mask = <0 0 0 0x7>; > + interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */ > + <0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */ > + <0 0 0 3 &intc 0 0 0 151 IRQ_TYPE_LEVEL_HIGH>, /* int_c */ > + <0 0 0 4 &intc 0 0 0 152 IRQ_TYPE_LEVEL_HIGH>; /* int_d */ > + > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > + <&gcc GCC_PCIE_0_MSTR_AXI_CLK>, > + <&gcc GCC_PCIE_0_SLV_AXI_CLK>, > + <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>, > + <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>, > + <&gcc GCC_AGGRE_NOC_PCIE_AXI_CLK>; > + clock-names = "aux", > + "cfg", > + "bus_master", > + "bus_slave", > + "slave_q2a", > + "ddrss_sf_tbu", You're reusing a clock name which doesn't seem to match this SoC. I don't know what "QTB" refers to here and if it's just some Qualcomm alternate name for "TBU" which could make this ok. > + "noc_aggr_4"; The 4 here comes from the fact that the clock was named this way on sc8280xp. Perhaps 'noc_aggr' would have been a better generic name for the interconnect clock. > + > + interconnects = <&pcie_noc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>; > + interconnect-names = "pcie-mem"; > + > + iommus = <&apps_smmu 0x1400 0x7f>; > + iommu-map = <0x0 &apps_smmu 0x1400 0x1>, > + <0x100 &apps_smmu 0x1401 0x1>; > + > + resets = <&gcc GCC_PCIE_0_BCR>; > + reset-names = "pci"; > + > + power-domains = <&gcc PCIE_0_GDSC>; > + > + phys = <&pcie0_phy>; > + phy-names = "pciephy"; > + > + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; > + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; > + > + pinctrl-names = "default"; > + pinctrl-0 = <&pcie0_default_state>; For sc8280xp we decided to keep all pin configuration (and the gpios properties above) in the dts file. I believe this should be done also for any new SoCs. Either way, the pin nodes should be added along with the consumer. > + > + status = "disabled"; > + }; > + > + pcie0_phy: phy@1c06000 { > + compatible = "qcom,sm8550-qmp-gen3x2-pcie-phy"; > + reg = <0 0x01c06000 0 0x2000>; > + > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > + <&tcsr TCSR_PCIE_0_CLKREF_EN>, > + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>, > + <&gcc GCC_PCIE_0_PIPE_CLK>; > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > + "pipe"; > + > + resets = <&gcc GCC_PCIE_0_PHY_BCR>; > + reset-names = "phy"; > + > + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; > + assigned-clock-rates = <100000000>; > + > + power-domains = <&gcc PCIE_0_PHY_GDSC>; > + > + #clock-cells = <0>; > + clock-output-names = "pcie0_pipe_clk"; > + > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + pcie1_phy: phy@1c0e000 { > + compatible = "qcom,sm8550-qmp-gen4x2-pcie-phy"; > + reg = <0x0 0x01c0e000 0x0 0x2000>; > + > + clocks = <&gcc GCC_PCIE_1_PHY_AUX_CLK>, > + <&gcc GCC_PCIE_1_CFG_AHB_CLK>, > + <&tcsr TCSR_PCIE_1_CLKREF_EN>, > + <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>, > + <&gcc GCC_PCIE_1_PIPE_CLK>; > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > + "pipe"; > + > + resets = <&gcc GCC_PCIE_1_PHY_BCR>, > + <&gcc GCC_PCIE_1_NOCSR_COM_PHY_BCR>; > + reset-names = "phy", "nocsr"; Do you know why only the second PHY uses two resets here? Did you intend to add it also for the first PHY? Both of these resets exists also on sc8280xp, and I believe downstream used the NOCSR_COM variant, which does not reset all registers in the PHY so you could unknowingly be relying on firmware to setup things up for you. I did a fair bit of reverse engineering to determine the init sequences and opted to use the full reset for the PHYs here in the end. I don't think you should be using both, but someone with access to documentation may provide more insight. Have you tested both pci0 and 1 by the way? > + > + assigned-clocks = <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>; > + assigned-clock-rates = <100000000>; > + > + power-domains = <&gcc PCIE_1_PHY_GDSC>; > + > + #clock-cells = <0>; > + clock-output-names = "pcie1_pipe_clk"; > + > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > cryptobam: dma-controller@1dc4000 { > compatible = "qcom,bam-v1.7.0"; > reg = <0x0 0x01dc4000 0x0 0x28000>; Johan
On 23-01-23 09:51:20, Johan Hovold wrote: > On Thu, Jan 19, 2023 at 04:04:52PM +0200, Abel Vesa wrote: > > Add PCIe controllers and PHY nodes. > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > --- > > > > This patch does not have a v3, but since it is now part of the same > > patchset with the controller and the phy drivers patches, I had to > > bump the version to 4. > > > > Latest version was here (v2): > > https://lore.kernel.org/all/20230118230526.1499328-2-abel.vesa@linaro.org/ > > > > Changes since latest version (v2): > > * renamed the pcie_1_link_down_reset to simply link_down > > * dropped the pipe from clock-names > > * renamed aggre clock-names to noc_aggr_4 > > * dropped the _pcie infix from cnoc_pcie_sf_axi > > * dropped the aux_phy clock from the pcie1 > > > > Changes since v1: > > * ordered pcie related nodes alphabetically in MTP dts > > * dropped the pipe_mux, phy_pipe and ref clocks from the pcie nodes > > * dropped the child node from the phy nodes, like Johan suggested, > > and updated to use the sc8280xp binding scheme > > * changed "pcie_1_nocsr_com_phy_reset" 2nd reset name of pcie1_phy > > to "nocsr" > > * reordered all pcie nodes properties to look similar to the ones > > from sc8280xp > > > > > > arch/arm64/boot/dts/qcom/sm8550.dtsi | 207 ++++++++++++++++++++++++++- > > 1 file changed, 204 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi > > index 3d47281a276b..8df226530d76 100644 > > --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi > > @@ -646,9 +646,9 @@ gcc: clock-controller@100000 { > > #reset-cells = <1>; > > #power-domain-cells = <1>; > > clocks = <&bi_tcxo_div2>, <&sleep_clk>, > > - <0>, > > - <0>, > > - <0>, > > + <&pcie0_phy>, > > + <&pcie1_phy>, > > + <&pcie_1_phy_aux_clk>, > > <&ufs_mem_phy 0>, > > <&ufs_mem_phy 1>, > > <&ufs_mem_phy 2>, > > @@ -1547,6 +1547,207 @@ mmss_noc: interconnect@1780000 { > > qcom,bcm-voters = <&apps_bcm_voter>; > > }; > > > > + pcie0: pci@1c00000 { > > + device_type = "pci"; > > + compatible = "qcom,pcie-sm8550"; > > + reg = <0 0x01c00000 0 0x3000>, > > + <0 0x60000000 0 0xf1d>, > > + <0 0x60000f20 0 0xa8>, > > + <0 0x60001000 0 0x1000>, > > + <0 0x60100000 0 0x100000>; > > + reg-names = "parf", "dbi", "elbi", "atu", "config"; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>, > > + <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>; > > + bus-range = <0x00 0xff>; > > + > > + dma-coherent; > > + > > + linux,pci-domain = <0>; > > + num-lanes = <2>; > > + > > + interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>; > > + interrupt-names = "msi"; > > + > > + #interrupt-cells = <1>; > > + interrupt-map-mask = <0 0 0 0x7>; > > + interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */ > > + <0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */ > > + <0 0 0 3 &intc 0 0 0 151 IRQ_TYPE_LEVEL_HIGH>, /* int_c */ > > + <0 0 0 4 &intc 0 0 0 152 IRQ_TYPE_LEVEL_HIGH>; /* int_d */ > > + > > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > > + <&gcc GCC_PCIE_0_MSTR_AXI_CLK>, > > + <&gcc GCC_PCIE_0_SLV_AXI_CLK>, > > + <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>, > > + <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>, > > + <&gcc GCC_AGGRE_NOC_PCIE_AXI_CLK>; > > + clock-names = "aux", > > + "cfg", > > + "bus_master", > > + "bus_slave", > > + "slave_q2a", > > + "ddrss_sf_tbu", > > You're reusing a clock name which doesn't seem to match this SoC. I > don't know what "QTB" refers to here and if it's just some Qualcomm > alternate name for "TBU" which could make this ok. I'll come back later with an answer here, once I know exactly what QTB means. > > > + "noc_aggr_4"; > > The 4 here comes from the fact that the clock was named this way on > sc8280xp. Perhaps 'noc_aggr' would have been a better generic name for > the interconnect clock. > So should I rename it to noc_aggr as part of this patchset then? > > + > > + interconnects = <&pcie_noc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>; > > + interconnect-names = "pcie-mem"; > > + > > + iommus = <&apps_smmu 0x1400 0x7f>; > > + iommu-map = <0x0 &apps_smmu 0x1400 0x1>, > > + <0x100 &apps_smmu 0x1401 0x1>; > > + > > + resets = <&gcc GCC_PCIE_0_BCR>; > > + reset-names = "pci"; > > + > > + power-domains = <&gcc PCIE_0_GDSC>; > > + > > + phys = <&pcie0_phy>; > > + phy-names = "pciephy"; > > + > > + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; > > + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&pcie0_default_state>; > > For sc8280xp we decided to keep all pin configuration (and the gpios > properties above) in the dts file. I believe this should be done also > for any new SoCs. Right, I'll move the pinctrl properties to the dts node instead. > > Either way, the pin nodes should be added along with the consumer. > The pin nodes have been added already, back when the initial dtsi was sent. > > + > > + status = "disabled"; > > + }; > > + > > + pcie0_phy: phy@1c06000 { > > + compatible = "qcom,sm8550-qmp-gen3x2-pcie-phy"; > > + reg = <0 0x01c06000 0 0x2000>; > > + > > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > > + <&tcsr TCSR_PCIE_0_CLKREF_EN>, > > + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>, > > + <&gcc GCC_PCIE_0_PIPE_CLK>; > > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > > + "pipe"; > > + > > + resets = <&gcc GCC_PCIE_0_PHY_BCR>; > > + reset-names = "phy"; > > + > > + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; > > + assigned-clock-rates = <100000000>; > > + > > + power-domains = <&gcc PCIE_0_PHY_GDSC>; > > + > > + #clock-cells = <0>; > > + clock-output-names = "pcie0_pipe_clk"; > > + > > + #phy-cells = <0>; > > + > > + status = "disabled"; > > + }; > > > + pcie1_phy: phy@1c0e000 { > > + compatible = "qcom,sm8550-qmp-gen4x2-pcie-phy"; > > + reg = <0x0 0x01c0e000 0x0 0x2000>; > > + > > + clocks = <&gcc GCC_PCIE_1_PHY_AUX_CLK>, > > + <&gcc GCC_PCIE_1_CFG_AHB_CLK>, > > + <&tcsr TCSR_PCIE_1_CLKREF_EN>, > > + <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>, > > + <&gcc GCC_PCIE_1_PIPE_CLK>; > > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > > + "pipe"; > > + > > + resets = <&gcc GCC_PCIE_1_PHY_BCR>, > > + <&gcc GCC_PCIE_1_NOCSR_COM_PHY_BCR>; > > + reset-names = "phy", "nocsr"; > > Do you know why only the second PHY uses two resets here? Did you intend > to add it also for the first PHY? Please notice that this is a g4x2 phy. The documentation specifically says that both the pciephy_reset and pciephy_nocsr_reset should be asserted on power-up. Now, even the g3x2 has the nocsr reset (at least in GCC) but its documentation doesn't seem to say anything about nocsr needed to be asserted (ever). > > Both of these resets exists also on sc8280xp, and I believe downstream > used the NOCSR_COM variant, which does not reset all registers in the > PHY so you could unknowingly be relying on firmware to setup things up > for you. That is also the case for the g3x2 phy on sm8550. > > I did a fair bit of reverse engineering to determine the init sequences > and opted to use the full reset for the PHYs here in the end. > > I don't think you should be using both, but someone with access to > documentation may provide more insight. Again, the documentation I have access to, seems to suggest otherwise. > > Have you tested both pci0 and 1 by the way? Only the pcie0 can be tested with the MTP I have access to. So only pcie0 was tested. > > > + > > + assigned-clocks = <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>; > > + assigned-clock-rates = <100000000>; > > + > > + power-domains = <&gcc PCIE_1_PHY_GDSC>; > > + > > + #clock-cells = <0>; > > + clock-output-names = "pcie1_pipe_clk"; > > + > > + #phy-cells = <0>; > > + > > + status = "disabled"; > > + }; > > + > > cryptobam: dma-controller@1dc4000 { > > compatible = "qcom,bam-v1.7.0"; > > reg = <0x0 0x01dc4000 0x0 0x28000>; > > Johan
On 23-01-23 14:39:55, Abel Vesa wrote: > On 23-01-23 09:51:20, Johan Hovold wrote: > > On Thu, Jan 19, 2023 at 04:04:52PM +0200, Abel Vesa wrote: > > > Add PCIe controllers and PHY nodes. > > > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > > --- > > > > > > This patch does not have a v3, but since it is now part of the same > > > patchset with the controller and the phy drivers patches, I had to > > > bump the version to 4. > > > > > > Latest version was here (v2): > > > https://lore.kernel.org/all/20230118230526.1499328-2-abel.vesa@linaro.org/ > > > > > > Changes since latest version (v2): > > > * renamed the pcie_1_link_down_reset to simply link_down > > > * dropped the pipe from clock-names > > > * renamed aggre clock-names to noc_aggr_4 > > > * dropped the _pcie infix from cnoc_pcie_sf_axi > > > * dropped the aux_phy clock from the pcie1 > > > > > > Changes since v1: > > > * ordered pcie related nodes alphabetically in MTP dts > > > * dropped the pipe_mux, phy_pipe and ref clocks from the pcie nodes > > > * dropped the child node from the phy nodes, like Johan suggested, > > > and updated to use the sc8280xp binding scheme > > > * changed "pcie_1_nocsr_com_phy_reset" 2nd reset name of pcie1_phy > > > to "nocsr" > > > * reordered all pcie nodes properties to look similar to the ones > > > from sc8280xp > > > > > > > > > arch/arm64/boot/dts/qcom/sm8550.dtsi | 207 ++++++++++++++++++++++++++- > > > 1 file changed, 204 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi > > > index 3d47281a276b..8df226530d76 100644 > > > --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi > > > @@ -646,9 +646,9 @@ gcc: clock-controller@100000 { > > > #reset-cells = <1>; > > > #power-domain-cells = <1>; > > > clocks = <&bi_tcxo_div2>, <&sleep_clk>, > > > - <0>, > > > - <0>, > > > - <0>, > > > + <&pcie0_phy>, > > > + <&pcie1_phy>, > > > + <&pcie_1_phy_aux_clk>, > > > <&ufs_mem_phy 0>, > > > <&ufs_mem_phy 1>, > > > <&ufs_mem_phy 2>, > > > @@ -1547,6 +1547,207 @@ mmss_noc: interconnect@1780000 { > > > qcom,bcm-voters = <&apps_bcm_voter>; > > > }; > > > > > > + pcie0: pci@1c00000 { > > > + device_type = "pci"; > > > + compatible = "qcom,pcie-sm8550"; > > > + reg = <0 0x01c00000 0 0x3000>, > > > + <0 0x60000000 0 0xf1d>, > > > + <0 0x60000f20 0 0xa8>, > > > + <0 0x60001000 0 0x1000>, > > > + <0 0x60100000 0 0x100000>; > > > + reg-names = "parf", "dbi", "elbi", "atu", "config"; > > > + #address-cells = <3>; > > > + #size-cells = <2>; > > > + ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>, > > > + <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>; > > > + bus-range = <0x00 0xff>; > > > + > > > + dma-coherent; > > > + > > > + linux,pci-domain = <0>; > > > + num-lanes = <2>; > > > + > > > + interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>; > > > + interrupt-names = "msi"; > > > + > > > + #interrupt-cells = <1>; > > > + interrupt-map-mask = <0 0 0 0x7>; > > > + interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */ > > > + <0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */ > > > + <0 0 0 3 &intc 0 0 0 151 IRQ_TYPE_LEVEL_HIGH>, /* int_c */ > > > + <0 0 0 4 &intc 0 0 0 152 IRQ_TYPE_LEVEL_HIGH>; /* int_d */ > > > + > > > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > > > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > > > + <&gcc GCC_PCIE_0_MSTR_AXI_CLK>, > > > + <&gcc GCC_PCIE_0_SLV_AXI_CLK>, > > > + <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>, > > > + <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>, > > > + <&gcc GCC_AGGRE_NOC_PCIE_AXI_CLK>; > > > + clock-names = "aux", > > > + "cfg", > > > + "bus_master", > > > + "bus_slave", > > > + "slave_q2a", > > > + "ddrss_sf_tbu", > > > > You're reusing a clock name which doesn't seem to match this SoC. I > > don't know what "QTB" refers to here and if it's just some Qualcomm > > alternate name for "TBU" which could make this ok. > > I'll come back later with an answer here, once I know exactly what QTB > means. So, AFAICT, they replaced the TBU with QTB, which basically does the same thing. It is part of the SMMU. So, yes, it is just an alternate name, at least from the clock point of view. > > > > > > + "noc_aggr_4"; > > > > The 4 here comes from the fact that the clock was named this way on > > sc8280xp. Perhaps 'noc_aggr' would have been a better generic name for > > the interconnect clock. > > > > So should I rename it to noc_aggr as part of this patchset then? > > > > + > > > + interconnects = <&pcie_noc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>; > > > + interconnect-names = "pcie-mem"; > > > + > > > + iommus = <&apps_smmu 0x1400 0x7f>; > > > + iommu-map = <0x0 &apps_smmu 0x1400 0x1>, > > > + <0x100 &apps_smmu 0x1401 0x1>; > > > + > > > + resets = <&gcc GCC_PCIE_0_BCR>; > > > + reset-names = "pci"; > > > + > > > + power-domains = <&gcc PCIE_0_GDSC>; > > > + > > > + phys = <&pcie0_phy>; > > > + phy-names = "pciephy"; > > > + > > > + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; > > > + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; > > > + > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&pcie0_default_state>; > > > > For sc8280xp we decided to keep all pin configuration (and the gpios > > properties above) in the dts file. I believe this should be done also > > for any new SoCs. > > Right, I'll move the pinctrl properties to the dts node instead. > > > > > Either way, the pin nodes should be added along with the consumer. > > > > The pin nodes have been added already, back when the initial dtsi was sent. > > > > + > > > + status = "disabled"; > > > + }; > > > + > > > + pcie0_phy: phy@1c06000 { > > > + compatible = "qcom,sm8550-qmp-gen3x2-pcie-phy"; > > > + reg = <0 0x01c06000 0 0x2000>; > > > + > > > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > > > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > > > + <&tcsr TCSR_PCIE_0_CLKREF_EN>, > > > + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>, > > > + <&gcc GCC_PCIE_0_PIPE_CLK>; > > > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > > > + "pipe"; > > > + > > > + resets = <&gcc GCC_PCIE_0_PHY_BCR>; > > > + reset-names = "phy"; > > > + > > > + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; > > > + assigned-clock-rates = <100000000>; > > > + > > > + power-domains = <&gcc PCIE_0_PHY_GDSC>; > > > + > > > + #clock-cells = <0>; > > > + clock-output-names = "pcie0_pipe_clk"; > > > + > > > + #phy-cells = <0>; > > > + > > > + status = "disabled"; > > > + }; > > > > > + pcie1_phy: phy@1c0e000 { > > > + compatible = "qcom,sm8550-qmp-gen4x2-pcie-phy"; > > > + reg = <0x0 0x01c0e000 0x0 0x2000>; > > > + > > > + clocks = <&gcc GCC_PCIE_1_PHY_AUX_CLK>, > > > + <&gcc GCC_PCIE_1_CFG_AHB_CLK>, > > > + <&tcsr TCSR_PCIE_1_CLKREF_EN>, > > > + <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>, > > > + <&gcc GCC_PCIE_1_PIPE_CLK>; > > > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > > > + "pipe"; > > > + > > > + resets = <&gcc GCC_PCIE_1_PHY_BCR>, > > > + <&gcc GCC_PCIE_1_NOCSR_COM_PHY_BCR>; > > > + reset-names = "phy", "nocsr"; > > > > Do you know why only the second PHY uses two resets here? Did you intend > > to add it also for the first PHY? > > Please notice that this is a g4x2 phy. The documentation specifically > says that both the pciephy_reset and pciephy_nocsr_reset should be > asserted on power-up. Now, even the g3x2 has the nocsr reset (at least > in GCC) but its documentation doesn't seem to say anything about > nocsr needed to be asserted (ever). > > > > > Both of these resets exists also on sc8280xp, and I believe downstream > > used the NOCSR_COM variant, which does not reset all registers in the > > PHY so you could unknowingly be relying on firmware to setup things up > > for you. > > That is also the case for the g3x2 phy on sm8550. > > > > > I did a fair bit of reverse engineering to determine the init sequences > > and opted to use the full reset for the PHYs here in the end. > > > > I don't think you should be using both, but someone with access to > > documentation may provide more insight. > > Again, the documentation I have access to, seems to suggest otherwise. > > > > > Have you tested both pci0 and 1 by the way? > > Only the pcie0 can be tested with the MTP I have access to. So only > pcie0 was tested. > > > > > > + > > > + assigned-clocks = <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>; > > > + assigned-clock-rates = <100000000>; > > > + > > > + power-domains = <&gcc PCIE_1_PHY_GDSC>; > > > + > > > + #clock-cells = <0>; > > > + clock-output-names = "pcie1_pipe_clk"; > > > + > > > + #phy-cells = <0>; > > > + > > > + status = "disabled"; > > > + }; > > > + > > > cryptobam: dma-controller@1dc4000 { > > > compatible = "qcom,bam-v1.7.0"; > > > reg = <0x0 0x01dc4000 0x0 0x28000>; > > > > Johan
On Mon, Jan 23, 2023 at 02:39:55PM +0200, Abel Vesa wrote: > On 23-01-23 09:51:20, Johan Hovold wrote: > > On Thu, Jan 19, 2023 at 04:04:52PM +0200, Abel Vesa wrote: > > > Add PCIe controllers and PHY nodes. > > > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > > --- > > > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > > > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > > > + <&gcc GCC_PCIE_0_MSTR_AXI_CLK>, > > > + <&gcc GCC_PCIE_0_SLV_AXI_CLK>, > > > + <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>, > > > + <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>, > > > + <&gcc GCC_AGGRE_NOC_PCIE_AXI_CLK>; > > > + clock-names = "aux", > > > + "cfg", > > > + "bus_master", > > > + "bus_slave", > > > + "slave_q2a", > > > + "ddrss_sf_tbu", > > > > You're reusing a clock name which doesn't seem to match this SoC. I > > don't know what "QTB" refers to here and if it's just some Qualcomm > > alternate name for "TBU" which could make this ok. > > I'll come back later with an answer here, once I know exactly what QTB > means. > > > > > > + "noc_aggr_4"; > > > > The 4 here comes from the fact that the clock was named this way on > > sc8280xp. Perhaps 'noc_aggr' would have been a better generic name for > > the interconnect clock. > > > > So should I rename it to noc_aggr as part of this patchset then? Yes, or rather add that as the name this (and possible coming) SoCs use. > > > + > > > + interconnects = <&pcie_noc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>; > > > + interconnect-names = "pcie-mem"; > > > + > > > + iommus = <&apps_smmu 0x1400 0x7f>; > > > + iommu-map = <0x0 &apps_smmu 0x1400 0x1>, > > > + <0x100 &apps_smmu 0x1401 0x1>; > > > + > > > + resets = <&gcc GCC_PCIE_0_BCR>; > > > + reset-names = "pci"; > > > + > > > + power-domains = <&gcc PCIE_0_GDSC>; > > > + > > > + phys = <&pcie0_phy>; > > > + phy-names = "pciephy"; > > > + > > > + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; > > > + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; > > > + > > > + pinctrl-names = "default"; > > > + pinctrl-0 = <&pcie0_default_state>; > > > > For sc8280xp we decided to keep all pin configuration (and the gpios > > properties above) in the dts file. I believe this should be done also > > for any new SoCs. > > Right, I'll move the pinctrl properties to the dts node instead. > > > > > Either way, the pin nodes should be added along with the consumer. > > > > The pin nodes have been added already, back when the initial dtsi was sent. Ok. > > > + pcie1_phy: phy@1c0e000 { > > > + compatible = "qcom,sm8550-qmp-gen4x2-pcie-phy"; > > > + reg = <0x0 0x01c0e000 0x0 0x2000>; > > > + > > > + clocks = <&gcc GCC_PCIE_1_PHY_AUX_CLK>, > > > + <&gcc GCC_PCIE_1_CFG_AHB_CLK>, > > > + <&tcsr TCSR_PCIE_1_CLKREF_EN>, > > > + <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>, > > > + <&gcc GCC_PCIE_1_PIPE_CLK>; > > > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > > > + "pipe"; > > > + > > > + resets = <&gcc GCC_PCIE_1_PHY_BCR>, > > > + <&gcc GCC_PCIE_1_NOCSR_COM_PHY_BCR>; > > > + reset-names = "phy", "nocsr"; > > > > Do you know why only the second PHY uses two resets here? Did you intend > > to add it also for the first PHY? > > Please notice that this is a g4x2 phy. The documentation specifically > says that both the pciephy_reset and pciephy_nocsr_reset should be > asserted on power-up. Now, even the g3x2 has the nocsr reset (at least > in GCC) but its documentation doesn't seem to say anything about > nocsr needed to be asserted (ever). Ok. Thanks for confirming. I did not notice the difference in generation at first. > > Both of these resets exists also on sc8280xp, and I believe downstream > > used the NOCSR_COM variant, which does not reset all registers in the > > PHY so you could unknowingly be relying on firmware to setup things up > > for you. > > That is also the case for the g3x2 phy on sm8550. > > > > > I did a fair bit of reverse engineering to determine the init sequences > > and opted to use the full reset for the PHYs here in the end. > > > > I don't think you should be using both, but someone with access to > > documentation may provide more insight. > > Again, the documentation I have access to, seems to suggest otherwise. If that's what the documentation says then let's go with that. > > Have you tested both pci0 and 1 by the way? > > Only the pcie0 can be tested with the MTP I have access to. So only > pcie0 was tested. Ok. Johan
On Mon, Jan 23, 2023 at 03:11:40PM +0200, Abel Vesa wrote: > On 23-01-23 14:39:55, Abel Vesa wrote: > > On 23-01-23 09:51:20, Johan Hovold wrote: > > > On Thu, Jan 19, 2023 at 04:04:52PM +0200, Abel Vesa wrote: > > > > Add PCIe controllers and PHY nodes. > > > > > > > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > > > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > > > > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > > > > + <&gcc GCC_PCIE_0_MSTR_AXI_CLK>, > > > > + <&gcc GCC_PCIE_0_SLV_AXI_CLK>, > > > > + <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>, > > > > + <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>, > > > > + <&gcc GCC_AGGRE_NOC_PCIE_AXI_CLK>; > > > > + clock-names = "aux", > > > > + "cfg", > > > > + "bus_master", > > > > + "bus_slave", > > > > + "slave_q2a", > > > > + "ddrss_sf_tbu", > > > > > > You're reusing a clock name which doesn't seem to match this SoC. I > > > don't know what "QTB" refers to here and if it's just some Qualcomm > > > alternate name for "TBU" which could make this ok. > > > > I'll come back later with an answer here, once I know exactly what QTB > > means. > > So, AFAICT, they replaced the TBU with QTB, which basically does the > same thing. It is part of the SMMU. So, yes, it is just an alternate > name, at least from the clock point of view. Good, thanks for checking. Johan
On Mon, Jan 23, 2023 at 03:16:09PM +0100, Johan Hovold wrote: > On Mon, Jan 23, 2023 at 02:39:55PM +0200, Abel Vesa wrote: > > On 23-01-23 09:51:20, Johan Hovold wrote: > > > > + pcie1_phy: phy@1c0e000 { > > > > + compatible = "qcom,sm8550-qmp-gen4x2-pcie-phy"; > > > > + reg = <0x0 0x01c0e000 0x0 0x2000>; > > > > + > > > > + clocks = <&gcc GCC_PCIE_1_PHY_AUX_CLK>, > > > > + <&gcc GCC_PCIE_1_CFG_AHB_CLK>, > > > > + <&tcsr TCSR_PCIE_1_CLKREF_EN>, > > > > + <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>, > > > > + <&gcc GCC_PCIE_1_PIPE_CLK>; > > > > + clock-names = "aux", "cfg_ahb", "ref", "rchng", > > > > + "pipe"; > > > > + > > > > + resets = <&gcc GCC_PCIE_1_PHY_BCR>, > > > > + <&gcc GCC_PCIE_1_NOCSR_COM_PHY_BCR>; > > > > + reset-names = "phy", "nocsr"; > > > > > > Do you know why only the second PHY uses two resets here? Did you intend > > > to add it also for the first PHY? > > > > Please notice that this is a g4x2 phy. The documentation specifically > > says that both the pciephy_reset and pciephy_nocsr_reset should be > > asserted on power-up. Now, even the g3x2 has the nocsr reset (at least > > in GCC) but its documentation doesn't seem to say anything about > > nocsr needed to be asserted (ever). > > Ok. Thanks for confirming. I did not notice the difference in generation > at first. > > > > Both of these resets exists also on sc8280xp, and I believe downstream > > > used the NOCSR_COM variant, which does not reset all registers in the > > > PHY so you could unknowingly be relying on firmware to setup things up > > > for you. > > > > That is also the case for the g3x2 phy on sm8550. One more thing: Shouldn't the second reset be named 'nocsr_com' or similar? Johan
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi index 3d47281a276b..8df226530d76 100644 --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi @@ -646,9 +646,9 @@ gcc: clock-controller@100000 { #reset-cells = <1>; #power-domain-cells = <1>; clocks = <&bi_tcxo_div2>, <&sleep_clk>, - <0>, - <0>, - <0>, + <&pcie0_phy>, + <&pcie1_phy>, + <&pcie_1_phy_aux_clk>, <&ufs_mem_phy 0>, <&ufs_mem_phy 1>, <&ufs_mem_phy 2>, @@ -1547,6 +1547,207 @@ mmss_noc: interconnect@1780000 { qcom,bcm-voters = <&apps_bcm_voter>; }; + pcie0: pci@1c00000 { + device_type = "pci"; + compatible = "qcom,pcie-sm8550"; + reg = <0 0x01c00000 0 0x3000>, + <0 0x60000000 0 0xf1d>, + <0 0x60000f20 0 0xa8>, + <0 0x60001000 0 0x1000>, + <0 0x60100000 0 0x100000>; + reg-names = "parf", "dbi", "elbi", "atu", "config"; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x01000000 0x0 0x60200000 0 0x60200000 0x0 0x100000>, + <0x02000000 0x0 0x60300000 0 0x60300000 0x0 0x3d00000>; + bus-range = <0x00 0xff>; + + dma-coherent; + + linux,pci-domain = <0>; + num-lanes = <2>; + + interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi"; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0x7>; + interrupt-map = <0 0 0 1 &intc 0 0 0 149 IRQ_TYPE_LEVEL_HIGH>, /* int_a */ + <0 0 0 2 &intc 0 0 0 150 IRQ_TYPE_LEVEL_HIGH>, /* int_b */ + <0 0 0 3 &intc 0 0 0 151 IRQ_TYPE_LEVEL_HIGH>, /* int_c */ + <0 0 0 4 &intc 0 0 0 152 IRQ_TYPE_LEVEL_HIGH>; /* int_d */ + + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, + <&gcc GCC_PCIE_0_MSTR_AXI_CLK>, + <&gcc GCC_PCIE_0_SLV_AXI_CLK>, + <&gcc GCC_PCIE_0_SLV_Q2A_AXI_CLK>, + <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>, + <&gcc GCC_AGGRE_NOC_PCIE_AXI_CLK>; + clock-names = "aux", + "cfg", + "bus_master", + "bus_slave", + "slave_q2a", + "ddrss_sf_tbu", + "noc_aggr_4"; + + interconnects = <&pcie_noc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>; + interconnect-names = "pcie-mem"; + + iommus = <&apps_smmu 0x1400 0x7f>; + iommu-map = <0x0 &apps_smmu 0x1400 0x1>, + <0x100 &apps_smmu 0x1401 0x1>; + + resets = <&gcc GCC_PCIE_0_BCR>; + reset-names = "pci"; + + power-domains = <&gcc PCIE_0_GDSC>; + + phys = <&pcie0_phy>; + phy-names = "pciephy"; + + perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>; + wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>; + + pinctrl-names = "default"; + pinctrl-0 = <&pcie0_default_state>; + + status = "disabled"; + }; + + pcie0_phy: phy@1c06000 { + compatible = "qcom,sm8550-qmp-gen3x2-pcie-phy"; + reg = <0 0x01c06000 0 0x2000>; + + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, + <&tcsr TCSR_PCIE_0_CLKREF_EN>, + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>, + <&gcc GCC_PCIE_0_PIPE_CLK>; + clock-names = "aux", "cfg_ahb", "ref", "rchng", + "pipe"; + + resets = <&gcc GCC_PCIE_0_PHY_BCR>; + reset-names = "phy"; + + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; + assigned-clock-rates = <100000000>; + + power-domains = <&gcc PCIE_0_PHY_GDSC>; + + #clock-cells = <0>; + clock-output-names = "pcie0_pipe_clk"; + + #phy-cells = <0>; + + status = "disabled"; + }; + + pcie1: pci@1c08000 { + device_type = "pci"; + compatible = "qcom,pcie-sm8550"; + reg = <0x0 0x01c08000 0x0 0x3000>, + <0x0 0x40000000 0x0 0xf1d>, + <0x0 0x40000f20 0x0 0xa8>, + <0x0 0x40001000 0x0 0x1000>, + <0x0 0x40100000 0x0 0x100000>; + reg-names = "parf", "dbi", "elbi", "atu", "config"; + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x01000000 0x0 0x40200000 0 0x40200000 0x0 0x100000>, + <0x02000000 0x0 0x40300000 0 0x40300000 0x0 0x1fd00000>; + bus-range = <0x00 0xff>; + + dma-coherent; + + linux,pci-domain = <1>; + num-lanes = <2>; + + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "msi"; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0x7>; + interrupt-map = <0 0 0 1 &intc 0 0 0 434 IRQ_TYPE_LEVEL_HIGH>, /* int_a */ + <0 0 0 2 &intc 0 0 0 435 IRQ_TYPE_LEVEL_HIGH>, /* int_b */ + <0 0 0 3 &intc 0 0 0 438 IRQ_TYPE_LEVEL_HIGH>, /* int_c */ + <0 0 0 4 &intc 0 0 0 439 IRQ_TYPE_LEVEL_HIGH>; /* int_d */ + + clocks = <&gcc GCC_PCIE_1_AUX_CLK>, + <&gcc GCC_PCIE_1_CFG_AHB_CLK>, + <&gcc GCC_PCIE_1_MSTR_AXI_CLK>, + <&gcc GCC_PCIE_1_SLV_AXI_CLK>, + <&gcc GCC_PCIE_1_SLV_Q2A_AXI_CLK>, + <&gcc GCC_DDRSS_PCIE_SF_QTB_CLK>, + <&gcc GCC_AGGRE_NOC_PCIE_AXI_CLK>, + <&gcc GCC_CNOC_PCIE_SF_AXI_CLK>; + clock-names = "aux", + "cfg", + "bus_master", + "bus_slave", + "slave_q2a", + "ddrss_sf_tbu", + "noc_aggr_4", + "cnoc_sf_axi"; + + assigned-clocks = <&gcc GCC_PCIE_1_AUX_CLK>; + assigned-clock-rates = <19200000>; + + interconnects = <&pcie_noc MASTER_PCIE_1 0 &mc_virt SLAVE_EBI1 0>; + interconnect-names = "pcie-mem"; + + iommus = <&apps_smmu 0x1480 0x7f>; + iommu-map = <0x0 &apps_smmu 0x1480 0x1>, + <0x100 &apps_smmu 0x1481 0x1>; + + resets = <&gcc GCC_PCIE_1_BCR>, + <&gcc GCC_PCIE_1_LINK_DOWN_BCR>; + reset-names = "pci", "link_down"; + + power-domains = <&gcc PCIE_1_GDSC>; + + phys = <&pcie1_phy>; + phy-names = "pciephy"; + + perst-gpios = <&tlmm 97 GPIO_ACTIVE_LOW>; + enable-gpios = <&tlmm 99 GPIO_ACTIVE_HIGH>; + + pinctrl-names = "default"; + pinctrl-0 = <&pcie1_default_state>; + + status = "disabled"; + }; + + pcie1_phy: phy@1c0e000 { + compatible = "qcom,sm8550-qmp-gen4x2-pcie-phy"; + reg = <0x0 0x01c0e000 0x0 0x2000>; + + clocks = <&gcc GCC_PCIE_1_PHY_AUX_CLK>, + <&gcc GCC_PCIE_1_CFG_AHB_CLK>, + <&tcsr TCSR_PCIE_1_CLKREF_EN>, + <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>, + <&gcc GCC_PCIE_1_PIPE_CLK>; + clock-names = "aux", "cfg_ahb", "ref", "rchng", + "pipe"; + + resets = <&gcc GCC_PCIE_1_PHY_BCR>, + <&gcc GCC_PCIE_1_NOCSR_COM_PHY_BCR>; + reset-names = "phy", "nocsr"; + + assigned-clocks = <&gcc GCC_PCIE_1_PHY_RCHNG_CLK>; + assigned-clock-rates = <100000000>; + + power-domains = <&gcc PCIE_1_PHY_GDSC>; + + #clock-cells = <0>; + clock-output-names = "pcie1_pipe_clk"; + + #phy-cells = <0>; + + status = "disabled"; + }; + cryptobam: dma-controller@1dc4000 { compatible = "qcom,bam-v1.7.0"; reg = <0x0 0x01dc4000 0x0 0x28000>;