Message ID | 20211208171442.1327689-1-dmitry.baryshkov@linaro.org |
---|---|
Headers | show |
Series | qcom: add support for PCIe0 on SM8450 platform | expand |
On Wed, Dec 08, 2021 at 08:14:33PM +0300, Dmitry Baryshkov wrote: > Document the PCIe DT bindings for SM8450 SoC.The PCIe IP is similar > to the one used on SM8250. Add the compatible for SM8450. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Note to self: This binding should be converted to YAML very soon. Thanks, Mani > --- > .../devicetree/bindings/pci/qcom,pcie.txt | 21 ++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > index a0ae024c2d0c..73bc763c5009 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > @@ -15,6 +15,7 @@ > - "qcom,pcie-sc8180x" for sc8180x > - "qcom,pcie-sdm845" for sdm845 > - "qcom,pcie-sm8250" for sm8250 > + - "qcom,pcie-sm8450" for sm8450 > - "qcom,pcie-ipq6018" for ipq6018 > > - reg: > @@ -169,6 +170,24 @@ > - "ddrss_sf_tbu" PCIe SF TBU clock > - "pipe" PIPE clock > > +- clock-names: > + Usage: required for sm8450 > + Value type: <stringlist> > + Definition: Should contain the following entries > + - "aux" Auxiliary clock > + - "cfg" Configuration clock > + - "bus_master" Master AXI clock > + - "bus_slave" Slave AXI clock > + - "slave_q2a" Slave Q2A clock > + - "tbu" PCIe TBU clock > + - "ddrss_sf_tbu" PCIe SF TBU clock > + - "pipe" PIPE clock > + - "pipe_mux" PIPE MUX > + - "phy_pipe" PIPE output clock > + - "ref" REFERENCE clock > + - "aggre0" Aggre NoC PCIe0 AXI clock > + - "aggre1" Aggre NoC PCIe1 AXI clock > + > - resets: > Usage: required > Value type: <prop-encoded-array> > @@ -246,7 +265,7 @@ > - "ahb" AHB reset > > - reset-names: > - Usage: required for sc8180x, sdm845 and sm8250 > + Usage: required for sc8180x, sdm845, sm8250 and sm8450 > Value type: <stringlist> > Definition: Should contain the following entries > - "pci" PCIe core reset > -- > 2.33.0 >
On Wed, Dec 08, 2021 at 08:14:36PM +0300, Dmitry Baryshkov wrote: > In preparation to adding more flags to configuration data, use struct > qcom_pcie_cfg directly inside struct qcom_pcie, rather than duplicating > all its fields. This would save us from the boilerplate code that just > copies flags values from one sruct to another one. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 39 +++++++++++--------------- > 1 file changed, 17 insertions(+), 22 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 1c3d1116bb60..51a0475173fb 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -204,8 +204,7 @@ struct qcom_pcie { > union qcom_pcie_resources res; > struct phy *phy; > struct gpio_desc *reset; > - const struct qcom_pcie_ops *ops; > - unsigned int pipe_clk_need_muxing:1; > + const struct qcom_pcie_cfg *cfg; There is no change in this patch that adds "pipe_clk_need_muxing" to qcom_pcie_cfg. Thanks, Mani > }; > > #define to_qcom_pcie(x) dev_get_drvdata((x)->dev) > @@ -229,8 +228,8 @@ static int qcom_pcie_start_link(struct dw_pcie *pci) > struct qcom_pcie *pcie = to_qcom_pcie(pci); > > /* Enable Link Training state machine */ > - if (pcie->ops->ltssm_enable) > - pcie->ops->ltssm_enable(pcie); > + if (pcie->cfg->ops->ltssm_enable) > + pcie->cfg->ops->ltssm_enable(pcie); > > return 0; > } > @@ -1176,7 +1175,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > if (ret < 0) > return ret; > > - if (pcie->pipe_clk_need_muxing) { > + if (pcie->cfg->pipe_clk_need_muxing) { > res->pipe_clk_src = devm_clk_get(dev, "pipe_mux"); > if (IS_ERR(res->pipe_clk_src)) > return PTR_ERR(res->pipe_clk_src); > @@ -1209,7 +1208,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) > } > > /* Set TCXO as clock source for pcie_pipe_clk_src */ > - if (pcie->pipe_clk_need_muxing) > + if (pcie->cfg->pipe_clk_need_muxing) > clk_set_parent(res->pipe_clk_src, res->ref_clk_src); > > ret = clk_bulk_prepare_enable(res->num_clks, res->clks); > @@ -1284,7 +1283,7 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie) > struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; > > /* Set pipe clock as clock source for pcie_pipe_clk_src */ > - if (pcie->pipe_clk_need_muxing) > + if (pcie->cfg->pipe_clk_need_muxing) > clk_set_parent(res->pipe_clk_src, res->phy_pipe_clk); > > return clk_prepare_enable(res->pipe_clk); > @@ -1384,7 +1383,7 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > > qcom_ep_reset_assert(pcie); > > - ret = pcie->ops->init(pcie); > + ret = pcie->cfg->ops->init(pcie); > if (ret) > return ret; > > @@ -1392,16 +1391,16 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > if (ret) > goto err_deinit; > > - if (pcie->ops->post_init) { > - ret = pcie->ops->post_init(pcie); > + if (pcie->cfg->ops->post_init) { > + ret = pcie->cfg->ops->post_init(pcie); > if (ret) > goto err_disable_phy; > } > > qcom_ep_reset_deassert(pcie); > > - if (pcie->ops->config_sid) { > - ret = pcie->ops->config_sid(pcie); > + if (pcie->cfg->ops->config_sid) { > + ret = pcie->cfg->ops->config_sid(pcie); > if (ret) > goto err; > } > @@ -1410,12 +1409,12 @@ static int qcom_pcie_host_init(struct pcie_port *pp) > > err: > qcom_ep_reset_assert(pcie); > - if (pcie->ops->post_deinit) > - pcie->ops->post_deinit(pcie); > + if (pcie->cfg->ops->post_deinit) > + pcie->cfg->ops->post_deinit(pcie); > err_disable_phy: > phy_power_off(pcie->phy); > err_deinit: > - pcie->ops->deinit(pcie); > + pcie->cfg->ops->deinit(pcie); > > return ret; > } > @@ -1531,7 +1530,6 @@ static int qcom_pcie_probe(struct platform_device *pdev) > struct pcie_port *pp; > struct dw_pcie *pci; > struct qcom_pcie *pcie; > - const struct qcom_pcie_cfg *pcie_cfg; > int ret; > > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > @@ -1553,15 +1551,12 @@ static int qcom_pcie_probe(struct platform_device *pdev) > > pcie->pci = pci; > > - pcie_cfg = of_device_get_match_data(dev); > - if (!pcie_cfg || !pcie_cfg->ops) { > + pcie->cfg = of_device_get_match_data(dev); > + if (!pcie->cfg || !pcie->cfg->ops) { > dev_err(dev, "Invalid platform data\n"); > return -EINVAL; > } > > - pcie->ops = pcie_cfg->ops; > - pcie->pipe_clk_need_muxing = pcie_cfg->pipe_clk_need_muxing; > - > pcie->reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH); > if (IS_ERR(pcie->reset)) { > ret = PTR_ERR(pcie->reset); > @@ -1586,7 +1581,7 @@ static int qcom_pcie_probe(struct platform_device *pdev) > goto err_pm_runtime_put; > } > > - ret = pcie->ops->get_resources(pcie); > + ret = pcie->cfg->ops->get_resources(pcie); > if (ret) > goto err_pm_runtime_put; > > -- > 2.33.0 >
On Wed, Dec 08, 2021 at 08:14:37PM +0300, Dmitry Baryshkov wrote: > Qualcomm PCIe driver uses compatible string to check if the ddrss_sf_tbu > clock should be used. Since sc7280 support has added flags, switch to > the new mechanism to check if this clock should be used. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 51a0475173fb..803d3ac18c56 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -194,7 +194,9 @@ struct qcom_pcie_ops { > > struct qcom_pcie_cfg { > const struct qcom_pcie_ops *ops; > + /* flags for ops 2.7.0 and 1.9.0 */ No need of this comment. > unsigned int pipe_clk_need_muxing:1; This should be added in the previous patch. > + unsigned int has_ddrss_sf_tbu_clk:1; Wondering if we could make both the flags "bool" as the values passed to it are of boolean type. I don't think we could save a significant amount of memory using bitfields. Thanks, Mani > }; > > struct qcom_pcie { > @@ -1164,7 +1166,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > res->clks[3].id = "bus_slave"; > res->clks[4].id = "slave_q2a"; > res->clks[5].id = "tbu"; > - if (of_device_is_compatible(dev->of_node, "qcom,pcie-sm8250")) { > + if (pcie->cfg->has_ddrss_sf_tbu_clk) { > res->clks[6].id = "ddrss_sf_tbu"; > res->num_clks = 7; > } else { > @@ -1512,6 +1514,7 @@ static const struct qcom_pcie_cfg sdm845_cfg = { > > static const struct qcom_pcie_cfg sm8250_cfg = { > .ops = &ops_1_9_0, > + .has_ddrss_sf_tbu_clk = true, > }; > > static const struct qcom_pcie_cfg sc7280_cfg = { > -- > 2.33.0 >
On Wed, Dec 08, 2021 at 08:14:39PM +0300, Dmitry Baryshkov wrote: > Add device tree node for the first PCIe PHY device found on the Qualcomm > SM8450 platform. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm64/boot/dts/qcom/sm8450.dtsi | 42 ++++++++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi > index 16a789cacb65..a047d8a22897 100644 > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi > @@ -558,8 +558,12 @@ gcc: clock-controller@100000 { > #clock-cells = <1>; > #reset-cells = <1>; > #power-domain-cells = <1>; > - clock-names = "bi_tcxo", "sleep_clk"; > - clocks = <&rpmhcc RPMH_CXO_CLK>, <&sleep_clk>; > + clocks = <&rpmhcc RPMH_CXO_CLK>, > + <&pcie0_lane>, > + <&sleep_clk>; > + clock-names = "bi_tcxo", > + "pcie_0_pipe_clk", > + "sleep_clk"; > }; > > qupv3_id_0: geniqup@9c0000 { > @@ -625,6 +629,40 @@ i2c14: i2c@a98000 { > }; > }; > > + pcie0_phy: phy@1c06000 { > + compatible = "qcom,sm8450-qmp-gen3x1-pcie-phy"; > + reg = <0 0x01c06000 0 0x200>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, > + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, > + <&gcc GCC_PCIE_0_CLKREF_EN>, > + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; > + clock-names = "aux", "cfg_ahb", "ref", "refgen"; > + > + resets = <&gcc GCC_PCIE_0_PHY_BCR>; > + reset-names = "phy"; > + > + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; > + assigned-clock-rates = <100000000>; > + > + status = "disabled"; > + > + pcie0_lane: lanes@1c06200 { > + reg = <0 0x1c06e00 0 0x200>, /* tx */ > + <0 0x1c07000 0 0x200>, /* rx */ > + <0 0x1c06200 0 0x200>, /* pcs */ Oh, so this platform has "PCS" at the starting offset? This is different compared to other platforms as "TX" always comes first. And the size is "0x200" for all? Thanks, Mani > + <0 0x1c06600 0 0x200>; /* pcs_pcie */ > + clocks = <&gcc GCC_PCIE_0_PIPE_CLK>; > + clock-names = "pipe0"; > + > + #clock-cells = <0>; > + #phy-cells = <0>; > + clock-output-names = "pcie_0_pipe_clk"; > + }; > + }; > + > config_noc: interconnect@1500000 { > compatible = "qcom,sm8450-config-noc"; > reg = <0 0x01500000 0 0x1c000>; > -- > 2.33.0 >
On 10/12/2021 14:22, Manivannan Sadhasivam wrote: > On Wed, Dec 08, 2021 at 08:14:37PM +0300, Dmitry Baryshkov wrote: >> Qualcomm PCIe driver uses compatible string to check if the ddrss_sf_tbu >> clock should be used. Since sc7280 support has added flags, switch to >> the new mechanism to check if this clock should be used. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index 51a0475173fb..803d3ac18c56 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -194,7 +194,9 @@ struct qcom_pcie_ops { >> >> struct qcom_pcie_cfg { >> const struct qcom_pcie_ops *ops; >> + /* flags for ops 2.7.0 and 1.9.0 */ > > No need of this comment. Dropping it > >> unsigned int pipe_clk_need_muxing:1; > > This should be added in the previous patch. It exists already > >> + unsigned int has_ddrss_sf_tbu_clk:1; > > Wondering if we could make both the flags "bool" as the values passed to it > are of boolean type. I don't think we could save a significant amount of > memory using bitfields. I followed the existing pipe_clk_need_muxing. I have no strong preference here, so let's see what Bjorn will prefer. > > Thanks, > Mani >> }; >> >> struct qcom_pcie { >> @@ -1164,7 +1166,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) >> res->clks[3].id = "bus_slave"; >> res->clks[4].id = "slave_q2a"; >> res->clks[5].id = "tbu"; >> - if (of_device_is_compatible(dev->of_node, "qcom,pcie-sm8250")) { >> + if (pcie->cfg->has_ddrss_sf_tbu_clk) { >> res->clks[6].id = "ddrss_sf_tbu"; >> res->num_clks = 7; >> } else { >> @@ -1512,6 +1514,7 @@ static const struct qcom_pcie_cfg sdm845_cfg = { >> >> static const struct qcom_pcie_cfg sm8250_cfg = { >> .ops = &ops_1_9_0, >> + .has_ddrss_sf_tbu_clk = true, >> }; >> >> static const struct qcom_pcie_cfg sc7280_cfg = { >> -- >> 2.33.0 >>
On 10/12/2021 14:37, Manivannan Sadhasivam wrote: > On Wed, Dec 08, 2021 at 08:14:39PM +0300, Dmitry Baryshkov wrote: >> Add device tree node for the first PCIe PHY device found on the Qualcomm >> SM8450 platform. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/sm8450.dtsi | 42 ++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi >> index 16a789cacb65..a047d8a22897 100644 >> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi >> @@ -558,8 +558,12 @@ gcc: clock-controller@100000 { >> #clock-cells = <1>; >> #reset-cells = <1>; >> #power-domain-cells = <1>; >> - clock-names = "bi_tcxo", "sleep_clk"; >> - clocks = <&rpmhcc RPMH_CXO_CLK>, <&sleep_clk>; >> + clocks = <&rpmhcc RPMH_CXO_CLK>, >> + <&pcie0_lane>, >> + <&sleep_clk>; >> + clock-names = "bi_tcxo", >> + "pcie_0_pipe_clk", >> + "sleep_clk"; >> }; >> >> qupv3_id_0: geniqup@9c0000 { >> @@ -625,6 +629,40 @@ i2c14: i2c@a98000 { >> }; >> }; >> >> + pcie0_phy: phy@1c06000 { >> + compatible = "qcom,sm8450-qmp-gen3x1-pcie-phy"; >> + reg = <0 0x01c06000 0 0x200>; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + clocks = <&gcc GCC_PCIE_0_AUX_CLK>, >> + <&gcc GCC_PCIE_0_CFG_AHB_CLK>, >> + <&gcc GCC_PCIE_0_CLKREF_EN>, >> + <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; >> + clock-names = "aux", "cfg_ahb", "ref", "refgen"; >> + >> + resets = <&gcc GCC_PCIE_0_PHY_BCR>; >> + reset-names = "phy"; >> + >> + assigned-clocks = <&gcc GCC_PCIE_0_PHY_RCHNG_CLK>; >> + assigned-clock-rates = <100000000>; >> + >> + status = "disabled"; >> + >> + pcie0_lane: lanes@1c06200 { >> + reg = <0 0x1c06e00 0 0x200>, /* tx */ >> + <0 0x1c07000 0 0x200>, /* rx */ >> + <0 0x1c06200 0 0x200>, /* pcs */ > > Oh, so this platform has "PCS" at the starting offset? This is different > compared to other platforms as "TX" always comes first. > Yes. this is correct. > And the size is "0x200" for all? It is for the PCS block. As you see below, PCS_PCIE starts at 0x600. Initially I thought about extend it further, making it cover few other regions (up to the tx region). However as we do not touch other regions, I decided to keep it as this way. > > Thanks, > Mani > >> + <0 0x1c06600 0 0x200>; /* pcs_pcie */ >> + clocks = <&gcc GCC_PCIE_0_PIPE_CLK>; >> + clock-names = "pipe0"; >> + >> + #clock-cells = <0>; >> + #phy-cells = <0>; >> + clock-output-names = "pcie_0_pipe_clk"; >> + }; >> + }; >> + >> config_noc: interconnect@1500000 { >> compatible = "qcom,sm8450-config-noc"; >> reg = <0 0x01500000 0 0x1c000>; >> -- >> 2.33.0 >>
On Sat, Dec 11, 2021 at 04:59:05AM +0300, Dmitry Baryshkov wrote: > On 10/12/2021 14:22, Manivannan Sadhasivam wrote: > > On Wed, Dec 08, 2021 at 08:14:37PM +0300, Dmitry Baryshkov wrote: > > > Qualcomm PCIe driver uses compatible string to check if the ddrss_sf_tbu > > > clock should be used. Since sc7280 support has added flags, switch to > > > the new mechanism to check if this clock should be used. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > drivers/pci/controller/dwc/pcie-qcom.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > > index 51a0475173fb..803d3ac18c56 100644 > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > > @@ -194,7 +194,9 @@ struct qcom_pcie_ops { > > > struct qcom_pcie_cfg { > > > const struct qcom_pcie_ops *ops; > > > + /* flags for ops 2.7.0 and 1.9.0 */ > > > > No need of this comment. > > Dropping it > > > > > > unsigned int pipe_clk_need_muxing:1; > > > > This should be added in the previous patch. > > It exists already > Ah, my tree was outdated. I do see it in -rc4. > > > > > + unsigned int has_ddrss_sf_tbu_clk:1; > > > > Wondering if we could make both the flags "bool" as the values passed to it > > are of boolean type. I don't think we could save a significant amount of > > memory using bitfields. > > I followed the existing pipe_clk_need_muxing. I have no strong preference > here, so let's see what Bjorn will prefer. > Okay. Thanks, Mani > > > > Thanks, > > Mani > > > }; > > > struct qcom_pcie { > > > @@ -1164,7 +1166,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > > > res->clks[3].id = "bus_slave"; > > > res->clks[4].id = "slave_q2a"; > > > res->clks[5].id = "tbu"; > > > - if (of_device_is_compatible(dev->of_node, "qcom,pcie-sm8250")) { > > > + if (pcie->cfg->has_ddrss_sf_tbu_clk) { > > > res->clks[6].id = "ddrss_sf_tbu"; > > > res->num_clks = 7; > > > } else { > > > @@ -1512,6 +1514,7 @@ static const struct qcom_pcie_cfg sdm845_cfg = { > > > static const struct qcom_pcie_cfg sm8250_cfg = { > > > .ops = &ops_1_9_0, > > > + .has_ddrss_sf_tbu_clk = true, > > > }; > > > static const struct qcom_pcie_cfg sc7280_cfg = { > > > -- > > > 2.33.0 > > > > > > -- > With best wishes > Dmitry