| Message ID | 20190502001955.10575-4-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Qualcomm QCS404 PCIe support | expand |
On 01-05-19, 17:19, Bjorn Andersson wrote: > The QCS404 platform contains a PCIe controller of version 2.4.0 and a > Qualcomm PCIe2 PHY. The driver already supports version 2.4.0, for the > IPQ4019, but this support touches clocks and resets related to the PHY > as well, and there's no upstream driver for the PHY. > > On QCS404 we must initialize the PHY, so a separate PHY driver is > implemented to take care of this and the controller driver is updated to > not require the PHY related resources. This is done by relying on the > fact that operations in both the clock and reset framework are nops when > passed NULL, so we can isolate this change to only the get_resource > function. > > For QCS404 we also need to enable the AHB (iface) clock, in order to > access the register space of the controller, but as this is not part of > the IPQ4019 DT binding this is only added for new users of the 2.4.0 > controller. Reviewed-by: Vinod Koul <vkoul@kernel.org>
Hi Bjorn, On 5/2/19 3:19 AM, Bjorn Andersson wrote: > The QCS404 platform contains a PCIe controller of version 2.4.0 and a > Qualcomm PCIe2 PHY. The driver already supports version 2.4.0, for the > IPQ4019, but this support touches clocks and resets related to the PHY > as well, and there's no upstream driver for the PHY. > > On QCS404 we must initialize the PHY, so a separate PHY driver is > implemented to take care of this and the controller driver is updated to > not require the PHY related resources. This is done by relying on the > fact that operations in both the clock and reset framework are nops when > passed NULL, so we can isolate this change to only the get_resource > function. > > For QCS404 we also need to enable the AHB (iface) clock, in order to > access the register space of the controller, but as this is not part of > the IPQ4019 DT binding this is only added for new users of the 2.4.0 > controller. > > Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Changes since v2: > - None > > drivers/pci/controller/dwc/pcie-qcom.c | 64 +++++++++++++++----------- > 1 file changed, 38 insertions(+), 26 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index d740cbe0e56d..d101bc5c0def 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -112,7 +112,7 @@ struct qcom_pcie_resources_2_3_2 { > struct regulator_bulk_data supplies[QCOM_PCIE_2_3_2_MAX_SUPPLY]; > }; > > -#define QCOM_PCIE_2_4_0_MAX_CLOCKS 3 > +#define QCOM_PCIE_2_4_0_MAX_CLOCKS 4 > struct qcom_pcie_resources_2_4_0 { > struct clk_bulk_data clks[QCOM_PCIE_2_4_0_MAX_CLOCKS]; > int num_clks; > @@ -638,13 +638,16 @@ static int qcom_pcie_get_resources_2_4_0(struct qcom_pcie *pcie) > struct qcom_pcie_resources_2_4_0 *res = &pcie->res.v2_4_0; > struct dw_pcie *pci = pcie->pci; > struct device *dev = pci->dev; > + bool is_ipq = of_device_is_compatible(dev->of_node, "qcom,pcie-ipq4019"); > int ret; > > res->clks[0].id = "aux"; > res->clks[1].id = "master_bus"; > res->clks[2].id = "slave_bus"; > + res->clks[3].id = "iface"; > > - res->num_clks = 3; > + /* qcom,pcie-ipq4019 is defined without "iface" */ > + res->num_clks = is_ipq ? 3 : 4; This is ugly but I don't have better idea except having static const resource structures where we can describe num_clks and select the right resource from compatible string, but lets leave that for the future. Otherwise: Acked-by: Stanimir Varbanov <svarbanov@mm-sol.com>
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index d740cbe0e56d..d101bc5c0def 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -112,7 +112,7 @@ struct qcom_pcie_resources_2_3_2 { struct regulator_bulk_data supplies[QCOM_PCIE_2_3_2_MAX_SUPPLY]; }; -#define QCOM_PCIE_2_4_0_MAX_CLOCKS 3 +#define QCOM_PCIE_2_4_0_MAX_CLOCKS 4 struct qcom_pcie_resources_2_4_0 { struct clk_bulk_data clks[QCOM_PCIE_2_4_0_MAX_CLOCKS]; int num_clks; @@ -638,13 +638,16 @@ static int qcom_pcie_get_resources_2_4_0(struct qcom_pcie *pcie) struct qcom_pcie_resources_2_4_0 *res = &pcie->res.v2_4_0; struct dw_pcie *pci = pcie->pci; struct device *dev = pci->dev; + bool is_ipq = of_device_is_compatible(dev->of_node, "qcom,pcie-ipq4019"); int ret; res->clks[0].id = "aux"; res->clks[1].id = "master_bus"; res->clks[2].id = "slave_bus"; + res->clks[3].id = "iface"; - res->num_clks = 3; + /* qcom,pcie-ipq4019 is defined without "iface" */ + res->num_clks = is_ipq ? 3 : 4; ret = devm_clk_bulk_get(dev, res->num_clks, res->clks); if (ret < 0) @@ -658,27 +661,33 @@ static int qcom_pcie_get_resources_2_4_0(struct qcom_pcie *pcie) if (IS_ERR(res->axi_s_reset)) return PTR_ERR(res->axi_s_reset); - res->pipe_reset = devm_reset_control_get_exclusive(dev, "pipe"); - if (IS_ERR(res->pipe_reset)) - return PTR_ERR(res->pipe_reset); - - res->axi_m_vmid_reset = devm_reset_control_get_exclusive(dev, - "axi_m_vmid"); - if (IS_ERR(res->axi_m_vmid_reset)) - return PTR_ERR(res->axi_m_vmid_reset); - - res->axi_s_xpu_reset = devm_reset_control_get_exclusive(dev, - "axi_s_xpu"); - if (IS_ERR(res->axi_s_xpu_reset)) - return PTR_ERR(res->axi_s_xpu_reset); - - res->parf_reset = devm_reset_control_get_exclusive(dev, "parf"); - if (IS_ERR(res->parf_reset)) - return PTR_ERR(res->parf_reset); - - res->phy_reset = devm_reset_control_get_exclusive(dev, "phy"); - if (IS_ERR(res->phy_reset)) - return PTR_ERR(res->phy_reset); + if (is_ipq) { + /* + * These resources relates to the PHY or are secure clocks, but + * are controlled here for IPQ4019 + */ + res->pipe_reset = devm_reset_control_get_exclusive(dev, "pipe"); + if (IS_ERR(res->pipe_reset)) + return PTR_ERR(res->pipe_reset); + + res->axi_m_vmid_reset = devm_reset_control_get_exclusive(dev, + "axi_m_vmid"); + if (IS_ERR(res->axi_m_vmid_reset)) + return PTR_ERR(res->axi_m_vmid_reset); + + res->axi_s_xpu_reset = devm_reset_control_get_exclusive(dev, + "axi_s_xpu"); + if (IS_ERR(res->axi_s_xpu_reset)) + return PTR_ERR(res->axi_s_xpu_reset); + + res->parf_reset = devm_reset_control_get_exclusive(dev, "parf"); + if (IS_ERR(res->parf_reset)) + return PTR_ERR(res->parf_reset); + + res->phy_reset = devm_reset_control_get_exclusive(dev, "phy"); + if (IS_ERR(res->phy_reset)) + return PTR_ERR(res->phy_reset); + } res->axi_m_sticky_reset = devm_reset_control_get_exclusive(dev, "axi_m_sticky"); @@ -698,9 +707,11 @@ static int qcom_pcie_get_resources_2_4_0(struct qcom_pcie *pcie) if (IS_ERR(res->ahb_reset)) return PTR_ERR(res->ahb_reset); - res->phy_ahb_reset = devm_reset_control_get_exclusive(dev, "phy_ahb"); - if (IS_ERR(res->phy_ahb_reset)) - return PTR_ERR(res->phy_ahb_reset); + if (is_ipq) { + res->phy_ahb_reset = devm_reset_control_get_exclusive(dev, "phy_ahb"); + if (IS_ERR(res->phy_ahb_reset)) + return PTR_ERR(res->phy_ahb_reset); + } return 0; } @@ -1268,6 +1279,7 @@ static const struct of_device_id qcom_pcie_match[] = { { .compatible = "qcom,pcie-msm8996", .data = &ops_2_3_2 }, { .compatible = "qcom,pcie-ipq8074", .data = &ops_2_3_3 }, { .compatible = "qcom,pcie-ipq4019", .data = &ops_2_4_0 }, + { .compatible = "qcom,pcie-qcs404", .data = &ops_2_4_0 }, { } };