| Message ID | 20241114095409.2682558-5-quic_ziyuzhan@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pci: qcom: Add QCS8300 PCIe support | expand |
On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: > Add configurations in devicetree for PCIe0, including registers, clocks, > interrupts and phy setting sequence. > > Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> > --- > arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- > arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ > 2 files changed, 219 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > index 7eed19a694c3..9d7c8555ed38 100644 > --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { > &gcc { The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there any reason to have the clocks property in the board data file? > clocks = <&rpmhcc RPMH_CXO_CLK>, > <&sleep_clk>, > - <0>, > + <&pcie0_phy>, > <0>, > <0>, > <0>, > @@ -223,6 +223,23 @@ &gcc { > <0>; > }; > > +&pcie0 { > + perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>; > + wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>; > + > + pinctrl-0 = <&pcie0_default_state>; > + pinctrl-names = "default"; > + > + status = "okay"; > +}; > + > +&pcie0_phy { > + vdda-phy-supply = <&vreg_l6a>; > + vdda-pll-supply = <&vreg_l5a>; > + > + status = "okay"; > +}; > + > &qupv3_id_0 { > status = "okay"; > }; > @@ -247,6 +264,31 @@ &rpmhcc { > clock-names = "xo"; > }; > > +&tlmm { > + pcie0_default_state: pcie0-default-state { > + perst-pins { > + pins = "gpio2"; > + function = "gpio"; > + drive-strength = <2>; > + bias-pull-down; > + }; > + > + clkreq-pins { > + pins = "gpio1"; > + function = "pcie0_clkreq"; > + drive-strength = <2>; > + bias-pull-up; > + }; > + > + wake-pins { > + pins = "gpio0"; > + function = "gpio"; > + drive-strength = <2>; > + bias-pull-up; > + }; > + }; > +}; > + > &uart7 { > status = "okay"; > }; > diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi > index 2c35f96c3f28..d4924f48b347 100644 > --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi > +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi > @@ -637,6 +637,182 @@ mmss_noc: interconnect@17a0000 { > qcom,bcm-voters = <&apps_bcm_voter>; > }; > > + pcie0: pci@1c00000 { > + compatible = "qcom,pcie-qcs8300","qcom,pcie-sa8775p"; > + reg = <0x0 0x01c00000 0x0 0x3000>, > + <0x0 0x40000000 0x0 0xf20>, > + <0x0 0x40000f20 0x0 0xa8>, > + <0x0 0x40001000 0x0 0x4000>, > + <0x0 0x40100000 0x0 0x100000>, > + <0x0 0x01c03000 0x0 0x1000>; > + > + reg-names = "parf", > + "dbi", > + "elbi", > + "atu", > + "config", > + "mhi"; > + > + device_type = "pci"; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges = <0x01000000 0x0 0x00000000 0x0 0x40200000 0x0 0x100000>, > + <0x02000000 0x0 0x40300000 0x0 0x40300000 0x0 0x1fd00000>; > + bus-range = <0x00 0xff>; > + > + dma-coherent; > + > + linux,pci-domain = <0>; > + num-lanes = <2>; > + > + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>; > + > + interrupt-names = "msi0", > + "msi1", > + "msi2", > + "msi3", > + "msi4", > + "msi5", > + "msi6", > + "msi7"; > + > + #interrupt-cells = <1>; > + interrupt-map-mask = <0 0 0 0x7>; > + interrupt-map = <0 0 0 1 &intc GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>, > + <0 0 0 2 &intc GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>, > + <0 0 0 3 &intc GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>, > + <0 0 0 4 &intc GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>; > + > + 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>; > + > + clock-names = "aux", > + "cfg", > + "bus_master", > + "bus_slave", > + "slave_q2a"; > + > + assigned-clocks = <&gcc GCC_PCIE_0_AUX_CLK>; > + assigned-clock-rates = <19200000>; > + > + interconnects = <&pcie_anoc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>, > + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_PCIE_0 0>; > + > + interconnect-names = "pcie-mem", "cpu-pcie"; > + > + iommu-map = <0x0 &pcie_smmu 0x0000 0x1>, > + <0x100 &pcie_smmu 0x0001 0x1>; > + > + resets = <&gcc GCC_PCIE_0_BCR>; > + reset-names = "pci"; > + power-domains = <&gcc GCC_PCIE_0_GDSC>; > + > + phys = <&pcie0_phy>; > + phy-names = "pciephy"; > + > + status = "disabled"; > + > + pcie3_opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + /* GEN 1 x1 */ > + opp-2500000 { > + opp-hz = /bits/ 64 <2500000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + opp-peak-kBps = <250000 1>; > + }; > + > + /* GEN 1 x2 and GEN 2 x1 */ > + opp-5000000 { > + opp-hz = /bits/ 64 <5000000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + opp-peak-kBps = <500000 1>; > + }; > + > + /* GEN 2 x2 */ > + opp-10000000 { > + opp-hz = /bits/ 64 <10000000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + opp-peak-kBps = <1000000 1>; > + }; > + > + /* GEN 3 x1 */ > + opp-8000000 { > + opp-hz = /bits/ 64 <8000000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + opp-peak-kBps = <984500 1>; > + }; > + > + /* GEN 3 x2 and GEN 4 x1 */ > + opp-16000000 { > + opp-hz = /bits/ 64 <16000000>; > + required-opps = <&rpmhpd_opp_nom>; > + opp-peak-kBps = <1969000 1>; > + }; > + > + /* GEN 4 x2 */ > + opp-32000000 { > + opp-hz = /bits/ 64 <32000000>; > + required-opps = <&rpmhpd_opp_nom>; > + opp-peak-kBps = <3938000 1>; > + }; > + }; > + > + pcieport0: pcie@0 { > + device_type = "pci"; > + reg = <0x0 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + bus-range = <0x01 0xff>; > + }; > + }; > + > + pcie0_phy: phy@1c04000 { > + compatible = "qcom,qcs8300-qmp-gen4x2-pcie-phy"; > + reg = <0x0 0x1c04000 0x0 0x2000>; > + > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > + <&gcc GCC_PCIE_CLKREF_EN>, > + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>, > + <&gcc GCC_PCIE_0_PIPE_CLK>, > + <&gcc GCC_PCIE_0_PIPEDIV2_CLK>, > + <&gcc GCC_PCIE_0_PHY_AUX_CLK>; > + > + clock-names = "aux", > + "cfg_ahb", > + "ref", > + "rchng", > + "pipe", > + "pipediv2", > + "phy_aux"; > + > + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; > + assigned-clock-rates = <100000000>; > + > + resets = <&gcc GCC_PCIE_0_PHY_BCR>; > + reset-names = "phy"; > + > + #clock-cells = <0>; > + clock-output-names = "pcie_0_pipe_clk"; > + > + #phy-cells = <0>; > + > + status = "disabled"; > + }; > + > ufs_mem_hc: ufs@1d84000 { > compatible = "qcom,qcs8300-ufshc", "qcom,ufshc", "jedec,ufs-2.0"; > reg = <0x0 0x01d84000 0x0 0x3000>; > -- > 2.34.1 >
On 14.11.2024 10:54 AM, Ziyue Zhang wrote: > Add configurations in devicetree for PCIe0, including registers, clocks, > interrupts and phy setting sequence. > > Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> > --- > arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- > arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ This implies this patch should be two separate ones [...] > +&tlmm { > + pcie0_default_state: pcie0-default-state { > + perst-pins { > + pins = "gpio2"; > + function = "gpio"; > + drive-strength = <2>; > + bias-pull-down; > + }; > + > + clkreq-pins { > + pins = "gpio1"; > + function = "pcie0_clkreq"; > + drive-strength = <2>; > + bias-pull-up; > + }; > + > + wake-pins { > + pins = "gpio0"; Sorting these in an increasing order would be welcome > > + pcie0: pci@1c00000 { > + compatible = "qcom,pcie-qcs8300","qcom,pcie-sa8775p"; Missing ' ' after ',' > + reg = <0x0 0x01c00000 0x0 0x3000>, > + <0x0 0x40000000 0x0 0xf20>, > + <0x0 0x40000f20 0x0 0xa8>, > + <0x0 0x40001000 0x0 0x4000>, > + <0x0 0x40100000 0x0 0x100000>, > + <0x0 0x01c03000 0x0 0x1000>; > + > + reg-names = "parf", > + "dbi", > + "elbi", > + "atu", > + "config", > + "mhi"; > + > + device_type = "pci"; Please try to match the style in x1e80100, it's mostly coherent but things like newlines differ, which is tiny but mildly annoying > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges = <0x01000000 0x0 0x00000000 0x0 0x40200000 0x0 0x100000>, > + <0x02000000 0x0 0x40300000 0x0 0x40300000 0x0 0x1fd00000>; Looks like there's a bit more space in there > + bus-range = <0x00 0xff>; > + > + dma-coherent; > + > + linux,pci-domain = <0>; > + num-lanes = <2>; > + > + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>; > + > + interrupt-names = "msi0", > + "msi1", > + "msi2", > + "msi3", > + "msi4", > + "msi5", > + "msi6", > + "msi7"; Please also add a "global" interrupt.. looks like it's GIC_SPI 166, but please confirm > + > + #interrupt-cells = <1>; > + interrupt-map-mask = <0 0 0 0x7>; > + interrupt-map = <0 0 0 1 &intc GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>, > + <0 0 0 2 &intc GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>, > + <0 0 0 3 &intc GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>, > + <0 0 0 4 &intc GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>; > + > + 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>; > + > + clock-names = "aux", > + "cfg", > + "bus_master", > + "bus_slave", > + "slave_q2a"; > + > + assigned-clocks = <&gcc GCC_PCIE_0_AUX_CLK>; > + assigned-clock-rates = <19200000>; > + > + interconnects = <&pcie_anoc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>, QCOM_ICC_TAG_ALWAYS > + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_PCIE_0 0>; QCOM_ICC_TAG_ACTIVE_ONLY [...] > + > + pcieport0: pcie@0 { > + device_type = "pci"; > + reg = <0x0 0x0 0x0 0x0 0x0>; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + bus-range = <0x01 0xff>; > + }; Are you going to use this? If not, please drop > + }; > + > + pcie0_phy: phy@1c04000 { > + compatible = "qcom,qcs8300-qmp-gen4x2-pcie-phy"; > + reg = <0x0 0x1c04000 0x0 0x2000>; > + > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, This clock goes to the RC, it should be _PHY_AUX (which you put below as phy_aux), please replace it. > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > + <&gcc GCC_PCIE_CLKREF_EN>, > + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>, > + <&gcc GCC_PCIE_0_PIPE_CLK>, > + <&gcc GCC_PCIE_0_PIPEDIV2_CLK>, > + <&gcc GCC_PCIE_0_PHY_AUX_CLK>; > + > + clock-names = "aux", > + "cfg_ahb", > + "ref", > + "rchng", > + "pipe", > + "pipediv2", > + "phy_aux"; Konrad
On 14.11.2024 1:10 PM, Dmitry Baryshkov wrote: > On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: >> Add configurations in devicetree for PCIe0, including registers, clocks, >> interrupts and phy setting sequence. >> >> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- >> arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ >> 2 files changed, 219 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >> index 7eed19a694c3..9d7c8555ed38 100644 >> --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >> +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >> @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { >> &gcc { > > The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there > any reason to have the clocks property in the board data file? Definitely not. Ziyue, please move that change to the soc dtsi Konrad
On 11/14/2024 9:03 PM, Konrad Dybcio wrote: > On 14.11.2024 1:10 PM, Dmitry Baryshkov wrote: >> On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: >>> Add configurations in devicetree for PCIe0, including registers, clocks, >>> interrupts and phy setting sequence. >>> >>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> >>> --- >>> arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- >>> arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ >>> 2 files changed, 219 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>> index 7eed19a694c3..9d7c8555ed38 100644 >>> --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>> +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>> @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { >>> &gcc { >> >> The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there >> any reason to have the clocks property in the board data file? > > Definitely not. Ziyue, please move that change to the soc dtsi Gcc node is updated in board device tree due to sleep_clk is defined in board device tree. Sleep_clk is from PMIC instead SoC so we were requested to move sleep_clk to board device tree in previous review [1]. [1]https://lore.kernel.org/all/10914199-1e86-4a2e-aec8-2a48cc49ef14@kernel.org/ > > Konrad
On Fri, Nov 15, 2024 at 12:59:12PM +0800, Tingwei Zhang wrote: > On 11/14/2024 9:03 PM, Konrad Dybcio wrote: > > On 14.11.2024 1:10 PM, Dmitry Baryshkov wrote: > > > On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: > > > > Add configurations in devicetree for PCIe0, including registers, clocks, > > > > interrupts and phy setting sequence. > > > > > > > > Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> > > > > --- > > > > arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- > > > > arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ > > > > 2 files changed, 219 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > index 7eed19a694c3..9d7c8555ed38 100644 > > > > --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { > > > > &gcc { > > > > > > The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there > > > any reason to have the clocks property in the board data file? > > > > Definitely not. Ziyue, please move that change to the soc dtsi > > Gcc node is updated in board device tree due to sleep_clk is defined in > board device tree. Sleep_clk is from PMIC instead SoC so we were requested > to move sleep_clk to board device tree in previous review [1]. Note, the review doesn't talk about sleep_clk at all. The recent examples (sm8650, x1e80100, sa8775p) still pull the clocks into the SoC dtsi, but without the freq. > > [1]https://lore.kernel.org/all/10914199-1e86-4a2e-aec8-2a48cc49ef14@kernel.org/ > > > > Konrad > > > -- > Thanks, > Tingwei > > -- > linux-phy mailing list > linux-phy@lists.infradead.org > https://lists.infradead.org/mailman/listinfo/linux-phy
On 11/15/2024 2:26 PM, Dmitry Baryshkov wrote: > On Fri, Nov 15, 2024 at 12:59:12PM +0800, Tingwei Zhang wrote: >> On 11/14/2024 9:03 PM, Konrad Dybcio wrote: >>> On 14.11.2024 1:10 PM, Dmitry Baryshkov wrote: >>>> On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: >>>>> Add configurations in devicetree for PCIe0, including registers, clocks, >>>>> interrupts and phy setting sequence. >>>>> >>>>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> >>>>> --- >>>>> arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- >>>>> arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ >>>>> 2 files changed, 219 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>>>> index 7eed19a694c3..9d7c8555ed38 100644 >>>>> --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>>>> +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>>>> @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { >>>>> &gcc { >>>> >>>> The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there >>>> any reason to have the clocks property in the board data file? >>> >>> Definitely not. Ziyue, please move that change to the soc dtsi >> >> Gcc node is updated in board device tree due to sleep_clk is defined in >> board device tree. Sleep_clk is from PMIC instead SoC so we were requested >> to move sleep_clk to board device tree in previous review [1]. > > Note, the review doesn't talk about sleep_clk at all. The recent > examples (sm8650, x1e80100, sa8775p) still pull the clocks into the SoC > dtsi, but without the freq. > It's begining of the discussion of the PMIC clock for SoC. Sleep clock specific discussion is here [2]. [2]https://lore.kernel.org/all/be8b573c-db4e-4eec-a9a6-3cd83d04156d@kernel.org/ >> >> [1]https://lore.kernel.org/all/10914199-1e86-4a2e-aec8-2a48cc49ef14@kernel.org/ >>> >>> Konrad >> >> >> -- >> Thanks, >> Tingwei >> >> -- >> linux-phy mailing list >> linux-phy@lists.infradead.org >> https://lists.infradead.org/mailman/listinfo/linux-phy >
On Thu, Nov 14, 2024 at 02:02:48PM +0100, Konrad Dybcio wrote: [...] > > + > > + pcieport0: pcie@0 { > > + device_type = "pci"; > > + reg = <0x0 0x0 0x0 0x0 0x0>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + ranges; > > + bus-range = <0x01 0xff>; > > + }; > > Are you going to use this? If not, please drop > Absolutely not! This describes the IP that is present in the SoC and that IP is being used. You can however keep it disabled in the soc.dtsi and enable in board dts when PCIe controller is enabled. Moreover, I plan to move the slot supplies to this node soon, so it will be used mostly. - Mani
On Fri, Nov 15, 2024 at 02:42:47PM +0800, Tingwei Zhang wrote: > On 11/15/2024 2:26 PM, Dmitry Baryshkov wrote: > > On Fri, Nov 15, 2024 at 12:59:12PM +0800, Tingwei Zhang wrote: > > > On 11/14/2024 9:03 PM, Konrad Dybcio wrote: > > > > On 14.11.2024 1:10 PM, Dmitry Baryshkov wrote: > > > > > On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: > > > > > > Add configurations in devicetree for PCIe0, including registers, clocks, > > > > > > interrupts and phy setting sequence. > > > > > > > > > > > > Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> > > > > > > --- > > > > > > arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- > > > > > > arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ > > > > > > 2 files changed, 219 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > > > index 7eed19a694c3..9d7c8555ed38 100644 > > > > > > --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > > > +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > > > @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { > > > > > > &gcc { > > > > > > > > > > The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there > > > > > any reason to have the clocks property in the board data file? > > > > > > > > Definitely not. Ziyue, please move that change to the soc dtsi > > > > > > Gcc node is updated in board device tree due to sleep_clk is defined in > > > board device tree. Sleep_clk is from PMIC instead SoC so we were requested > > > to move sleep_clk to board device tree in previous review [1]. > > > > Note, the review doesn't talk about sleep_clk at all. The recent > > examples (sm8650, x1e80100, sa8775p) still pull the clocks into the SoC > > dtsi, but without the freq. > > > It's begining of the discussion of the PMIC clock for SoC. Sleep clock > specific discussion is here [2]. > [2]https://lore.kernel.org/all/be8b573c-db4e-4eec-a9a6-3cd83d04156d@kernel.org/ Please note how the recent platforms describe those clocks: the node in the SoC dtsi, the frequency in the board dtsi. X1E80100 is a step backwards, the clock are completely defined in the x1e80100.dtsi. There seems to be no strict rule on how to handle board clocks. I've sent an RFC patchset, trying to move them to a single logical location. Let's see what kind of response it will get. We probably need to define and follow a common rule for all Qualcomm platforms. Please give it a couple of days for the dust to settle. However, I think there should be no reason to keep GCC's clock definitions in the board DTS. > > > > > > [1]https://lore.kernel.org/all/10914199-1e86-4a2e-aec8-2a48cc49ef14@kernel.org/ > > > > > > > > Konrad > > > > > > > > > -- > > > Thanks, > > > Tingwei > > > > > > -- > > > linux-phy mailing list > > > linux-phy@lists.infradead.org > > > https://lists.infradead.org/mailman/listinfo/linux-phy > > > > > -- > Thanks, > Tingwei
On 11/15/2024 3:03 PM, Dmitry Baryshkov wrote: > On Fri, Nov 15, 2024 at 02:42:47PM +0800, Tingwei Zhang wrote: >> On 11/15/2024 2:26 PM, Dmitry Baryshkov wrote: >>> On Fri, Nov 15, 2024 at 12:59:12PM +0800, Tingwei Zhang wrote: >>>> On 11/14/2024 9:03 PM, Konrad Dybcio wrote: >>>>> On 14.11.2024 1:10 PM, Dmitry Baryshkov wrote: >>>>>> On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: >>>>>>> Add configurations in devicetree for PCIe0, including registers, clocks, >>>>>>> interrupts and phy setting sequence. >>>>>>> >>>>>>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> >>>>>>> --- >>>>>>> arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- >>>>>>> arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ >>>>>>> 2 files changed, 219 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>>>>>> index 7eed19a694c3..9d7c8555ed38 100644 >>>>>>> --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>>>>>> +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts >>>>>>> @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { >>>>>>> &gcc { >>>>>> >>>>>> The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there >>>>>> any reason to have the clocks property in the board data file? >>>>> >>>>> Definitely not. Ziyue, please move that change to the soc dtsi >>>> >>>> Gcc node is updated in board device tree due to sleep_clk is defined in >>>> board device tree. Sleep_clk is from PMIC instead SoC so we were requested >>>> to move sleep_clk to board device tree in previous review [1]. >>> >>> Note, the review doesn't talk about sleep_clk at all. The recent >>> examples (sm8650, x1e80100, sa8775p) still pull the clocks into the SoC >>> dtsi, but without the freq. >>> >> It's begining of the discussion of the PMIC clock for SoC. Sleep clock >> specific discussion is here [2]. >> [2]https://lore.kernel.org/all/be8b573c-db4e-4eec-a9a6-3cd83d04156d@kernel.org/ > > Please note how the recent platforms describe those clocks: the node in > the SoC dtsi, the frequency in the board dtsi. X1E80100 is a step > backwards, the clock are completely defined in the x1e80100.dtsi. There > seems to be no strict rule on how to handle board clocks. I've sent an > RFC patchset, trying to move them to a single logical location. Let's > see what kind of response it will get. We probably need to define and > follow a common rule for all Qualcomm platforms. Please give it a couple > of days for the dust to settle. However, I think there should be no > reason to keep GCC's clock definitions in the board DTS. > Thanks for the clean up patch and make it consistent. Is it reasonable for GCC's clock definition to refer xo_clk/sleep_clk in board device tree? Theoretically, can we have another board has different xo_clk say xo1_clk defined in board device tree? >>>> >>>> [1]https://lore.kernel.org/all/10914199-1e86-4a2e-aec8-2a48cc49ef14@kernel.org/ >>>>> >>>>> Konrad >>>> >>>> >>>> -- >>>> Thanks, >>>> Tingwei >>>> >>>> -- >>>> linux-phy mailing list >>>> linux-phy@lists.infradead.org >>>> https://lists.infradead.org/mailman/listinfo/linux-phy >>> >> >> >> -- >> Thanks, >> Tingwei >
On Fri, Nov 15, 2024 at 03:16:29PM +0800, Tingwei Zhang wrote: > On 11/15/2024 3:03 PM, Dmitry Baryshkov wrote: > > On Fri, Nov 15, 2024 at 02:42:47PM +0800, Tingwei Zhang wrote: > > > On 11/15/2024 2:26 PM, Dmitry Baryshkov wrote: > > > > On Fri, Nov 15, 2024 at 12:59:12PM +0800, Tingwei Zhang wrote: > > > > > On 11/14/2024 9:03 PM, Konrad Dybcio wrote: > > > > > > On 14.11.2024 1:10 PM, Dmitry Baryshkov wrote: > > > > > > > On Thu, Nov 14, 2024 at 05:54:08PM +0800, Ziyue Zhang wrote: > > > > > > > > Add configurations in devicetree for PCIe0, including registers, clocks, > > > > > > > > interrupts and phy setting sequence. > > > > > > > > > > > > > > > > Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> > > > > > > > > --- > > > > > > > > arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- > > > > > > > > arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ > > > > > > > > 2 files changed, 219 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > > > > > index 7eed19a694c3..9d7c8555ed38 100644 > > > > > > > > --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > > > > > +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts > > > > > > > > @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { > > > > > > > > &gcc { > > > > > > > > > > > > > > The patch doesn't seem to update the gcc node in qcs8300.dtsi. Is there > > > > > > > any reason to have the clocks property in the board data file? > > > > > > > > > > > > Definitely not. Ziyue, please move that change to the soc dtsi > > > > > > > > > > Gcc node is updated in board device tree due to sleep_clk is defined in > > > > > board device tree. Sleep_clk is from PMIC instead SoC so we were requested > > > > > to move sleep_clk to board device tree in previous review [1]. > > > > > > > > Note, the review doesn't talk about sleep_clk at all. The recent > > > > examples (sm8650, x1e80100, sa8775p) still pull the clocks into the SoC > > > > dtsi, but without the freq. > > > > > > > It's begining of the discussion of the PMIC clock for SoC. Sleep clock > > > specific discussion is here [2]. > > > [2]https://lore.kernel.org/all/be8b573c-db4e-4eec-a9a6-3cd83d04156d@kernel.org/ > > > > Please note how the recent platforms describe those clocks: the node in > > the SoC dtsi, the frequency in the board dtsi. X1E80100 is a step > > backwards, the clock are completely defined in the x1e80100.dtsi. There > > seems to be no strict rule on how to handle board clocks. I've sent an > > RFC patchset, trying to move them to a single logical location. Let's > > see what kind of response it will get. We probably need to define and > > follow a common rule for all Qualcomm platforms. Please give it a couple > > of days for the dust to settle. However, I think there should be no > > reason to keep GCC's clock definitions in the board DTS. > > > Thanks for the clean up patch and make it consistent. > > Is it reasonable for GCC's clock definition to refer xo_clk/sleep_clk in > board device tree? Theoretically, can we have another board has different > xo_clk say xo1_clk defined in board device tree? That's a question for that series. I'd say, no. Some older platforms had separate CXO and PXO clocks, newer platforms have single CXO. > > > > > > > > > > [1]https://lore.kernel.org/all/10914199-1e86-4a2e-aec8-2a48cc49ef14@kernel.org/ > > > > > > > > > > > > Konrad > > > > > > > > > > > > > > > -- > > > > > Thanks, > > > > > Tingwei > > > > > > > > > > -- > > > > > linux-phy mailing list > > > > > linux-phy@lists.infradead.org > > > > > https://lists.infradead.org/mailman/listinfo/linux-phy > > > > > > > > > > > > > -- > > > Thanks, > > > Tingwei > > > > > -- > Thanks, > Tingwei
diff --git a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts index 7eed19a694c3..9d7c8555ed38 100644 --- a/arch/arm64/boot/dts/qcom/qcs8300-ride.dts +++ b/arch/arm64/boot/dts/qcom/qcs8300-ride.dts @@ -213,7 +213,7 @@ vreg_l9c: ldo9 { &gcc { clocks = <&rpmhcc RPMH_CXO_CLK>, <&sleep_clk>, - <0>, + <&pcie0_phy>, <0>, <0>, <0>, @@ -223,6 +223,23 @@ &gcc { <0>; }; +&pcie0 { + perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>; + wake-gpios = <&tlmm 0 GPIO_ACTIVE_HIGH>; + + pinctrl-0 = <&pcie0_default_state>; + pinctrl-names = "default"; + + status = "okay"; +}; + +&pcie0_phy { + vdda-phy-supply = <&vreg_l6a>; + vdda-pll-supply = <&vreg_l5a>; + + status = "okay"; +}; + &qupv3_id_0 { status = "okay"; }; @@ -247,6 +264,31 @@ &rpmhcc { clock-names = "xo"; }; +&tlmm { + pcie0_default_state: pcie0-default-state { + perst-pins { + pins = "gpio2"; + function = "gpio"; + drive-strength = <2>; + bias-pull-down; + }; + + clkreq-pins { + pins = "gpio1"; + function = "pcie0_clkreq"; + drive-strength = <2>; + bias-pull-up; + }; + + wake-pins { + pins = "gpio0"; + function = "gpio"; + drive-strength = <2>; + bias-pull-up; + }; + }; +}; + &uart7 { status = "okay"; }; diff --git a/arch/arm64/boot/dts/qcom/qcs8300.dtsi b/arch/arm64/boot/dts/qcom/qcs8300.dtsi index 2c35f96c3f28..d4924f48b347 100644 --- a/arch/arm64/boot/dts/qcom/qcs8300.dtsi +++ b/arch/arm64/boot/dts/qcom/qcs8300.dtsi @@ -637,6 +637,182 @@ mmss_noc: interconnect@17a0000 { qcom,bcm-voters = <&apps_bcm_voter>; }; + pcie0: pci@1c00000 { + compatible = "qcom,pcie-qcs8300","qcom,pcie-sa8775p"; + reg = <0x0 0x01c00000 0x0 0x3000>, + <0x0 0x40000000 0x0 0xf20>, + <0x0 0x40000f20 0x0 0xa8>, + <0x0 0x40001000 0x0 0x4000>, + <0x0 0x40100000 0x0 0x100000>, + <0x0 0x01c03000 0x0 0x1000>; + + reg-names = "parf", + "dbi", + "elbi", + "atu", + "config", + "mhi"; + + device_type = "pci"; + + #address-cells = <3>; + #size-cells = <2>; + ranges = <0x01000000 0x0 0x00000000 0x0 0x40200000 0x0 0x100000>, + <0x02000000 0x0 0x40300000 0x0 0x40300000 0x0 0x1fd00000>; + bus-range = <0x00 0xff>; + + dma-coherent; + + linux,pci-domain = <0>; + num-lanes = <2>; + + interrupts = <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>; + + interrupt-names = "msi0", + "msi1", + "msi2", + "msi3", + "msi4", + "msi5", + "msi6", + "msi7"; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0x7>; + interrupt-map = <0 0 0 1 &intc GIC_SPI 434 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 2 &intc GIC_SPI 435 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 3 &intc GIC_SPI 438 IRQ_TYPE_LEVEL_HIGH>, + <0 0 0 4 &intc GIC_SPI 439 IRQ_TYPE_LEVEL_HIGH>; + + 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>; + + clock-names = "aux", + "cfg", + "bus_master", + "bus_slave", + "slave_q2a"; + + assigned-clocks = <&gcc GCC_PCIE_0_AUX_CLK>; + assigned-clock-rates = <19200000>; + + interconnects = <&pcie_anoc MASTER_PCIE_0 0 &mc_virt SLAVE_EBI1 0>, + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_PCIE_0 0>; + + interconnect-names = "pcie-mem", "cpu-pcie"; + + iommu-map = <0x0 &pcie_smmu 0x0000 0x1>, + <0x100 &pcie_smmu 0x0001 0x1>; + + resets = <&gcc GCC_PCIE_0_BCR>; + reset-names = "pci"; + power-domains = <&gcc GCC_PCIE_0_GDSC>; + + phys = <&pcie0_phy>; + phy-names = "pciephy"; + + status = "disabled"; + + pcie3_opp_table: opp-table { + compatible = "operating-points-v2"; + + /* GEN 1 x1 */ + opp-2500000 { + opp-hz = /bits/ 64 <2500000>; + required-opps = <&rpmhpd_opp_svs_l1>; + opp-peak-kBps = <250000 1>; + }; + + /* GEN 1 x2 and GEN 2 x1 */ + opp-5000000 { + opp-hz = /bits/ 64 <5000000>; + required-opps = <&rpmhpd_opp_svs_l1>; + opp-peak-kBps = <500000 1>; + }; + + /* GEN 2 x2 */ + opp-10000000 { + opp-hz = /bits/ 64 <10000000>; + required-opps = <&rpmhpd_opp_svs_l1>; + opp-peak-kBps = <1000000 1>; + }; + + /* GEN 3 x1 */ + opp-8000000 { + opp-hz = /bits/ 64 <8000000>; + required-opps = <&rpmhpd_opp_svs_l1>; + opp-peak-kBps = <984500 1>; + }; + + /* GEN 3 x2 and GEN 4 x1 */ + opp-16000000 { + opp-hz = /bits/ 64 <16000000>; + required-opps = <&rpmhpd_opp_nom>; + opp-peak-kBps = <1969000 1>; + }; + + /* GEN 4 x2 */ + opp-32000000 { + opp-hz = /bits/ 64 <32000000>; + required-opps = <&rpmhpd_opp_nom>; + opp-peak-kBps = <3938000 1>; + }; + }; + + pcieport0: pcie@0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + bus-range = <0x01 0xff>; + }; + }; + + pcie0_phy: phy@1c04000 { + compatible = "qcom,qcs8300-qmp-gen4x2-pcie-phy"; + reg = <0x0 0x1c04000 0x0 0x2000>; + + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, + <&gcc GCC_PCIE_CLKREF_EN>, + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>, + <&gcc GCC_PCIE_0_PIPE_CLK>, + <&gcc GCC_PCIE_0_PIPEDIV2_CLK>, + <&gcc GCC_PCIE_0_PHY_AUX_CLK>; + + clock-names = "aux", + "cfg_ahb", + "ref", + "rchng", + "pipe", + "pipediv2", + "phy_aux"; + + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; + assigned-clock-rates = <100000000>; + + resets = <&gcc GCC_PCIE_0_PHY_BCR>; + reset-names = "phy"; + + #clock-cells = <0>; + clock-output-names = "pcie_0_pipe_clk"; + + #phy-cells = <0>; + + status = "disabled"; + }; + ufs_mem_hc: ufs@1d84000 { compatible = "qcom,qcs8300-ufshc", "qcom,ufshc", "jedec,ufs-2.0"; reg = <0x0 0x01d84000 0x0 0x3000>;
Add configurations in devicetree for PCIe0, including registers, clocks, interrupts and phy setting sequence. Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com> --- arch/arm64/boot/dts/qcom/qcs8300-ride.dts | 44 +++++- arch/arm64/boot/dts/qcom/qcs8300.dtsi | 176 ++++++++++++++++++++++ 2 files changed, 219 insertions(+), 1 deletion(-)