| Message ID | 20211208171442.1327689-7-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qcom: add support for PCIe0 on SM8450 platform | expand |
On Wed, Dec 08, 2021 at 08:14:38PM +0300, Dmitry Baryshkov wrote: > On SM8450 platform PCIe hosts do not use all the clocks (and add several > additional clocks), so expand the driver to handle these requirements. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 47 +++++++++++++++++++------- > 1 file changed, 34 insertions(+), 13 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 803d3ac18c56..ada9c816395d 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -161,7 +161,7 @@ struct qcom_pcie_resources_2_3_3 { > > /* 6 clocks typically, 7 for sm8250 */ > struct qcom_pcie_resources_2_7_0 { > - struct clk_bulk_data clks[7]; > + struct clk_bulk_data clks[9]; > int num_clks; > struct regulator_bulk_data supplies[2]; > struct reset_control *pci_reset; > @@ -196,7 +196,10 @@ struct qcom_pcie_cfg { > const struct qcom_pcie_ops *ops; > /* flags for ops 2.7.0 and 1.9.0 */ > unsigned int pipe_clk_need_muxing:1; > + unsigned int has_tbu_clk:1; > unsigned int has_ddrss_sf_tbu_clk:1; > + unsigned int has_aggre0_clk:1; > + unsigned int has_aggre1_clk:1; > }; > > struct qcom_pcie { > @@ -1147,6 +1150,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; > struct dw_pcie *pci = pcie->pci; > struct device *dev = pci->dev; > + unsigned int idx; u32? > int ret; > > res->pci_reset = devm_reset_control_get_exclusive(dev, "pci"); > @@ -1160,18 +1164,22 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > if (ret) > return ret; > > - res->clks[0].id = "aux"; > - res->clks[1].id = "cfg"; > - res->clks[2].id = "bus_master"; > - res->clks[3].id = "bus_slave"; > - res->clks[4].id = "slave_q2a"; > - res->clks[5].id = "tbu"; > - if (pcie->cfg->has_ddrss_sf_tbu_clk) { > - res->clks[6].id = "ddrss_sf_tbu"; > - res->num_clks = 7; > - } else { > - res->num_clks = 6; > - } > + idx = 0; > + res->clks[idx++].id = "aux"; > + res->clks[idx++].id = "cfg"; > + res->clks[idx++].id = "bus_master"; > + res->clks[idx++].id = "bus_slave"; > + res->clks[idx++].id = "slave_q2a"; > + if (pcie->cfg->has_tbu_clk) > + res->clks[idx++].id = "tbu"; > + if (pcie->cfg->has_ddrss_sf_tbu_clk) > + res->clks[idx++].id = "ddrss_sf_tbu"; > + if (pcie->cfg->has_aggre0_clk) > + res->clks[idx++].id = "aggre0"; > + if (pcie->cfg->has_aggre1_clk) > + res->clks[idx++].id = "aggre1"; > + > + res->num_clks = idx; res->num_clks = idx + 1? Thanks, Mani > > ret = devm_clk_bulk_get(dev, res->num_clks, res->clks); > if (ret < 0) > @@ -1510,15 +1518,27 @@ static const struct qcom_pcie_cfg ipq4019_cfg = { > > static const struct qcom_pcie_cfg sdm845_cfg = { > .ops = &ops_2_7_0, > + .has_tbu_clk = true, > }; > > static const struct qcom_pcie_cfg sm8250_cfg = { > .ops = &ops_1_9_0, > + .has_tbu_clk = true, > .has_ddrss_sf_tbu_clk = true, > }; > > +/* Only for the PCIe0! */ > +static const struct qcom_pcie_cfg sm8450_cfg = { > + .ops = &ops_1_9_0, > + .has_ddrss_sf_tbu_clk = true, > + .pipe_clk_need_muxing = true, > + .has_aggre0_clk = true, > + .has_aggre1_clk = true, > +}; > + > static const struct qcom_pcie_cfg sc7280_cfg = { > .ops = &ops_1_9_0, > + .has_tbu_clk = true, > .pipe_clk_need_muxing = true, > }; > > @@ -1626,6 +1646,7 @@ static const struct of_device_id qcom_pcie_match[] = { > { .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg }, > { .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg }, > { .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg }, > + { .compatible = "qcom,pcie-sm8450", .data = &sm8450_cfg }, > { .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg }, > { } > }; > -- > 2.33.0 >
On 10/12/2021 14:30, Manivannan Sadhasivam wrote: > On Wed, Dec 08, 2021 at 08:14:38PM +0300, Dmitry Baryshkov wrote: >> On SM8450 platform PCIe hosts do not use all the clocks (and add several >> additional clocks), so expand the driver to handle these requirements. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 47 +++++++++++++++++++------- >> 1 file changed, 34 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index 803d3ac18c56..ada9c816395d 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -161,7 +161,7 @@ struct qcom_pcie_resources_2_3_3 { >> >> /* 6 clocks typically, 7 for sm8250 */ >> struct qcom_pcie_resources_2_7_0 { >> - struct clk_bulk_data clks[7]; >> + struct clk_bulk_data clks[9]; >> int num_clks; >> struct regulator_bulk_data supplies[2]; >> struct reset_control *pci_reset; >> @@ -196,7 +196,10 @@ struct qcom_pcie_cfg { >> const struct qcom_pcie_ops *ops; >> /* flags for ops 2.7.0 and 1.9.0 */ >> unsigned int pipe_clk_need_muxing:1; >> + unsigned int has_tbu_clk:1; >> unsigned int has_ddrss_sf_tbu_clk:1; >> + unsigned int has_aggre0_clk:1; >> + unsigned int has_aggre1_clk:1; >> }; >> >> struct qcom_pcie { >> @@ -1147,6 +1150,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) >> struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; >> struct dw_pcie *pci = pcie->pci; >> struct device *dev = pci->dev; >> + unsigned int idx; > > u32? Why? it's just a counter. > >> int ret; >> >> res->pci_reset = devm_reset_control_get_exclusive(dev, "pci"); >> @@ -1160,18 +1164,22 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) >> if (ret) >> return ret; >> >> - res->clks[0].id = "aux"; >> - res->clks[1].id = "cfg"; >> - res->clks[2].id = "bus_master"; >> - res->clks[3].id = "bus_slave"; >> - res->clks[4].id = "slave_q2a"; >> - res->clks[5].id = "tbu"; >> - if (pcie->cfg->has_ddrss_sf_tbu_clk) { >> - res->clks[6].id = "ddrss_sf_tbu"; >> - res->num_clks = 7; >> - } else { >> - res->num_clks = 6; >> - } >> + idx = 0; >> + res->clks[idx++].id = "aux"; >> + res->clks[idx++].id = "cfg"; >> + res->clks[idx++].id = "bus_master"; >> + res->clks[idx++].id = "bus_slave"; >> + res->clks[idx++].id = "slave_q2a"; >> + if (pcie->cfg->has_tbu_clk) >> + res->clks[idx++].id = "tbu"; >> + if (pcie->cfg->has_ddrss_sf_tbu_clk) >> + res->clks[idx++].id = "ddrss_sf_tbu"; >> + if (pcie->cfg->has_aggre0_clk) >> + res->clks[idx++].id = "aggre0"; >> + if (pcie->cfg->has_aggre1_clk) >> + res->clks[idx++].id = "aggre1"; >> + >> + res->num_clks = idx; > > res->num_clks = idx + 1? No. the idx is equal to the amount of clocks we added to the array, so this is correct. > > Thanks, > Mani > >> >> ret = devm_clk_bulk_get(dev, res->num_clks, res->clks); >> if (ret < 0) >> @@ -1510,15 +1518,27 @@ static const struct qcom_pcie_cfg ipq4019_cfg = { >> >> static const struct qcom_pcie_cfg sdm845_cfg = { >> .ops = &ops_2_7_0, >> + .has_tbu_clk = true, >> }; >> >> static const struct qcom_pcie_cfg sm8250_cfg = { >> .ops = &ops_1_9_0, >> + .has_tbu_clk = true, >> .has_ddrss_sf_tbu_clk = true, >> }; >> >> +/* Only for the PCIe0! */ >> +static const struct qcom_pcie_cfg sm8450_cfg = { >> + .ops = &ops_1_9_0, >> + .has_ddrss_sf_tbu_clk = true, >> + .pipe_clk_need_muxing = true, >> + .has_aggre0_clk = true, >> + .has_aggre1_clk = true, >> +}; >> + >> static const struct qcom_pcie_cfg sc7280_cfg = { >> .ops = &ops_1_9_0, >> + .has_tbu_clk = true, >> .pipe_clk_need_muxing = true, >> }; >> >> @@ -1626,6 +1646,7 @@ static const struct of_device_id qcom_pcie_match[] = { >> { .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg }, >> { .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg }, >> { .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg }, >> + { .compatible = "qcom,pcie-sm8450", .data = &sm8450_cfg }, >> { .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg }, >> { } >> }; >> -- >> 2.33.0 >>
On Sat, Dec 11, 2021 at 05:01:01AM +0300, Dmitry Baryshkov wrote: > On 10/12/2021 14:30, Manivannan Sadhasivam wrote: > > On Wed, Dec 08, 2021 at 08:14:38PM +0300, Dmitry Baryshkov wrote: > > > On SM8450 platform PCIe hosts do not use all the clocks (and add several > > > additional clocks), so expand the driver to handle these requirements. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > drivers/pci/controller/dwc/pcie-qcom.c | 47 +++++++++++++++++++------- > > > 1 file changed, 34 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > > > index 803d3ac18c56..ada9c816395d 100644 > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > > @@ -161,7 +161,7 @@ struct qcom_pcie_resources_2_3_3 { > > > /* 6 clocks typically, 7 for sm8250 */ > > > struct qcom_pcie_resources_2_7_0 { > > > - struct clk_bulk_data clks[7]; > > > + struct clk_bulk_data clks[9]; > > > int num_clks; > > > struct regulator_bulk_data supplies[2]; > > > struct reset_control *pci_reset; > > > @@ -196,7 +196,10 @@ struct qcom_pcie_cfg { > > > const struct qcom_pcie_ops *ops; > > > /* flags for ops 2.7.0 and 1.9.0 */ > > > unsigned int pipe_clk_need_muxing:1; > > > + unsigned int has_tbu_clk:1; > > > unsigned int has_ddrss_sf_tbu_clk:1; > > > + unsigned int has_aggre0_clk:1; > > > + unsigned int has_aggre1_clk:1; > > > }; > > > struct qcom_pcie { > > > @@ -1147,6 +1150,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > > > struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; > > > struct dw_pcie *pci = pcie->pci; > > > struct device *dev = pci->dev; > > > + unsigned int idx; > > > > u32? > > Why? it's just a counter. > Yeah but IMO using the kernel defined datatype is mostly preferred. This is not touching any MMIO but still it is a good practice. > > > > > int ret; > > > res->pci_reset = devm_reset_control_get_exclusive(dev, "pci"); > > > @@ -1160,18 +1164,22 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) > > > if (ret) > > > return ret; > > > - res->clks[0].id = "aux"; > > > - res->clks[1].id = "cfg"; > > > - res->clks[2].id = "bus_master"; > > > - res->clks[3].id = "bus_slave"; > > > - res->clks[4].id = "slave_q2a"; > > > - res->clks[5].id = "tbu"; > > > - if (pcie->cfg->has_ddrss_sf_tbu_clk) { > > > - res->clks[6].id = "ddrss_sf_tbu"; > > > - res->num_clks = 7; > > > - } else { > > > - res->num_clks = 6; > > > - } > > > + idx = 0; > > > + res->clks[idx++].id = "aux"; > > > + res->clks[idx++].id = "cfg"; > > > + res->clks[idx++].id = "bus_master"; > > > + res->clks[idx++].id = "bus_slave"; > > > + res->clks[idx++].id = "slave_q2a"; > > > + if (pcie->cfg->has_tbu_clk) > > > + res->clks[idx++].id = "tbu"; > > > + if (pcie->cfg->has_ddrss_sf_tbu_clk) > > > + res->clks[idx++].id = "ddrss_sf_tbu"; > > > + if (pcie->cfg->has_aggre0_clk) > > > + res->clks[idx++].id = "aggre0"; > > > + if (pcie->cfg->has_aggre1_clk) > > > + res->clks[idx++].id = "aggre1"; > > > + > > > + res->num_clks = idx; > > > > res->num_clks = idx + 1? > > No. the idx is equal to the amount of clocks we added to the array, so this > is correct. > Oops, brain fade. Sorry, the usual post-increment confusion :) Ignore my comment. Thanks, Mani > > > > Thanks, > > Mani > > > > > ret = devm_clk_bulk_get(dev, res->num_clks, res->clks); > > > if (ret < 0) > > > @@ -1510,15 +1518,27 @@ static const struct qcom_pcie_cfg ipq4019_cfg = { > > > static const struct qcom_pcie_cfg sdm845_cfg = { > > > .ops = &ops_2_7_0, > > > + .has_tbu_clk = true, > > > }; > > > static const struct qcom_pcie_cfg sm8250_cfg = { > > > .ops = &ops_1_9_0, > > > + .has_tbu_clk = true, > > > .has_ddrss_sf_tbu_clk = true, > > > }; > > > +/* Only for the PCIe0! */ > > > +static const struct qcom_pcie_cfg sm8450_cfg = { > > > + .ops = &ops_1_9_0, > > > + .has_ddrss_sf_tbu_clk = true, > > > + .pipe_clk_need_muxing = true, > > > + .has_aggre0_clk = true, > > > + .has_aggre1_clk = true, > > > +}; > > > + > > > static const struct qcom_pcie_cfg sc7280_cfg = { > > > .ops = &ops_1_9_0, > > > + .has_tbu_clk = true, > > > .pipe_clk_need_muxing = true, > > > }; > > > @@ -1626,6 +1646,7 @@ static const struct of_device_id qcom_pcie_match[] = { > > > { .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg }, > > > { .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg }, > > > { .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg }, > > > + { .compatible = "qcom,pcie-sm8450", .data = &sm8450_cfg }, > > > { .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg }, > > > { } > > > }; > > > -- > > > 2.33.0 > > > > > > -- > With best wishes > Dmitry
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 803d3ac18c56..ada9c816395d 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -161,7 +161,7 @@ struct qcom_pcie_resources_2_3_3 { /* 6 clocks typically, 7 for sm8250 */ struct qcom_pcie_resources_2_7_0 { - struct clk_bulk_data clks[7]; + struct clk_bulk_data clks[9]; int num_clks; struct regulator_bulk_data supplies[2]; struct reset_control *pci_reset; @@ -196,7 +196,10 @@ struct qcom_pcie_cfg { const struct qcom_pcie_ops *ops; /* flags for ops 2.7.0 and 1.9.0 */ unsigned int pipe_clk_need_muxing:1; + unsigned int has_tbu_clk:1; unsigned int has_ddrss_sf_tbu_clk:1; + unsigned int has_aggre0_clk:1; + unsigned int has_aggre1_clk:1; }; struct qcom_pcie { @@ -1147,6 +1150,7 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0; struct dw_pcie *pci = pcie->pci; struct device *dev = pci->dev; + unsigned int idx; int ret; res->pci_reset = devm_reset_control_get_exclusive(dev, "pci"); @@ -1160,18 +1164,22 @@ static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie) if (ret) return ret; - res->clks[0].id = "aux"; - res->clks[1].id = "cfg"; - res->clks[2].id = "bus_master"; - res->clks[3].id = "bus_slave"; - res->clks[4].id = "slave_q2a"; - res->clks[5].id = "tbu"; - if (pcie->cfg->has_ddrss_sf_tbu_clk) { - res->clks[6].id = "ddrss_sf_tbu"; - res->num_clks = 7; - } else { - res->num_clks = 6; - } + idx = 0; + res->clks[idx++].id = "aux"; + res->clks[idx++].id = "cfg"; + res->clks[idx++].id = "bus_master"; + res->clks[idx++].id = "bus_slave"; + res->clks[idx++].id = "slave_q2a"; + if (pcie->cfg->has_tbu_clk) + res->clks[idx++].id = "tbu"; + if (pcie->cfg->has_ddrss_sf_tbu_clk) + res->clks[idx++].id = "ddrss_sf_tbu"; + if (pcie->cfg->has_aggre0_clk) + res->clks[idx++].id = "aggre0"; + if (pcie->cfg->has_aggre1_clk) + res->clks[idx++].id = "aggre1"; + + res->num_clks = idx; ret = devm_clk_bulk_get(dev, res->num_clks, res->clks); if (ret < 0) @@ -1510,15 +1518,27 @@ static const struct qcom_pcie_cfg ipq4019_cfg = { static const struct qcom_pcie_cfg sdm845_cfg = { .ops = &ops_2_7_0, + .has_tbu_clk = true, }; static const struct qcom_pcie_cfg sm8250_cfg = { .ops = &ops_1_9_0, + .has_tbu_clk = true, .has_ddrss_sf_tbu_clk = true, }; +/* Only for the PCIe0! */ +static const struct qcom_pcie_cfg sm8450_cfg = { + .ops = &ops_1_9_0, + .has_ddrss_sf_tbu_clk = true, + .pipe_clk_need_muxing = true, + .has_aggre0_clk = true, + .has_aggre1_clk = true, +}; + static const struct qcom_pcie_cfg sc7280_cfg = { .ops = &ops_1_9_0, + .has_tbu_clk = true, .pipe_clk_need_muxing = true, }; @@ -1626,6 +1646,7 @@ static const struct of_device_id qcom_pcie_match[] = { { .compatible = "qcom,pcie-sdm845", .data = &sdm845_cfg }, { .compatible = "qcom,pcie-sm8250", .data = &sm8250_cfg }, { .compatible = "qcom,pcie-sc8180x", .data = &sm8250_cfg }, + { .compatible = "qcom,pcie-sm8450", .data = &sm8450_cfg }, { .compatible = "qcom,pcie-sc7280", .data = &sc7280_cfg }, { } };
On SM8450 platform PCIe hosts do not use all the clocks (and add several additional clocks), so expand the driver to handle these requirements. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/pci/controller/dwc/pcie-qcom.c | 47 +++++++++++++++++++------- 1 file changed, 34 insertions(+), 13 deletions(-)