| Message ID | 20200402121148.1767-2-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Multiple fixes in PCIe qcom driver | expand |
Ansuel, On 4/2/20 3:11 PM, Ansuel Smith wrote: > Aux and Ref clk are missing in pcie qcom driver. > Add support in the driver to fix pcie inizialization in ipq806x. > > Fixes: 82a82383 PCI: qcom: Add Qualcomm PCIe controller driver this should be: Fixes: 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver and add: Cc: stable@vger.kernel.org # v4.5+ But, I wonder, as apq8064 shares the same ops_2_1_0 how it worked until now. Something more I cannot find such clocks for apq8064, which means that this patch will break it. One option is to use those new clocks only for ipq806x. > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > --- > drivers/pci/controller/dwc/pcie-qcom.c | 38 ++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c > index 5ea527a6bd9f..f958c535de6e 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -88,6 +88,8 @@ struct qcom_pcie_resources_2_1_0 { > struct clk *iface_clk; > struct clk *core_clk; > struct clk *phy_clk; > + struct clk *aux_clk; > + struct clk *ref_clk; > struct reset_control *pci_reset; > struct reset_control *axi_reset; > struct reset_control *ahb_reset; > @@ -246,6 +248,14 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie) > if (IS_ERR(res->phy_clk)) > return PTR_ERR(res->phy_clk); > > + res->aux_clk = devm_clk_get(dev, "aux"); > + if (IS_ERR(res->aux_clk)) > + return PTR_ERR(res->aux_clk); > + > + res->ref_clk = devm_clk_get(dev, "ref"); > + if (IS_ERR(res->ref_clk)) > + return PTR_ERR(res->ref_clk); > + > res->pci_reset = devm_reset_control_get_exclusive(dev, "pci"); > if (IS_ERR(res->pci_reset)) > return PTR_ERR(res->pci_reset); > @@ -278,6 +288,8 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie) > clk_disable_unprepare(res->iface_clk); > clk_disable_unprepare(res->core_clk); > clk_disable_unprepare(res->phy_clk); > + clk_disable_unprepare(res->aux_clk); > + clk_disable_unprepare(res->ref_clk); > regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies); > } > > @@ -307,16 +319,28 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie) > goto err_assert_ahb; > } > > + ret = clk_prepare_enable(res->core_clk); > + if (ret) { > + dev_err(dev, "cannot prepare/enable core clock\n"); > + goto err_clk_core; > + } > + > ret = clk_prepare_enable(res->phy_clk); > if (ret) { > dev_err(dev, "cannot prepare/enable phy clock\n"); > goto err_clk_phy; > } > > - ret = clk_prepare_enable(res->core_clk); > + ret = clk_prepare_enable(res->aux_clk); > if (ret) { > - dev_err(dev, "cannot prepare/enable core clock\n"); > - goto err_clk_core; > + dev_err(dev, "cannot prepare/enable aux clock\n"); > + goto err_clk_aux; > + } > + > + ret = clk_prepare_enable(res->ref_clk); > + if (ret) { > + dev_err(dev, "cannot prepare/enable ref clock\n"); > + goto err_clk_ref; > } > > ret = reset_control_deassert(res->ahb_reset); > @@ -372,10 +396,14 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie) > return 0; > > err_deassert_ahb: > - clk_disable_unprepare(res->core_clk); > -err_clk_core: > + clk_disable_unprepare(res->ref_clk); > +err_clk_ref: > + clk_disable_unprepare(res->aux_clk); > +err_clk_aux: > clk_disable_unprepare(res->phy_clk); > err_clk_phy: > + clk_disable_unprepare(res->core_clk); > +err_clk_core: > clk_disable_unprepare(res->iface_clk); > err_assert_ahb: > regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies); >
> PCIe driver > > Ansuel, > > On 4/2/20 3:11 PM, Ansuel Smith wrote: > > Aux and Ref clk are missing in pcie qcom driver. > > Add support in the driver to fix pcie inizialization in ipq806x. > > > > Fixes: 82a82383 PCI: qcom: Add Qualcomm PCIe controller driver > > this should be: > > Fixes: 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver > > and add: > > Cc: stable@vger.kernel.org # v4.5+ > > But, I wonder, as apq8064 shares the same ops_2_1_0 how it worked until > now. Something more I cannot find such clocks for apq8064, which means > that this patch will break it. > > One option is to use those new clocks only for ipq806x. > How to add this new clocks only for ipq806x? Check the compatible and add them accordingly? > > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > --- > > drivers/pci/controller/dwc/pcie-qcom.c | 38 > ++++++++++++++++++++++---- > > 1 file changed, 33 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c > b/drivers/pci/controller/dwc/pcie-qcom.c > > index 5ea527a6bd9f..f958c535de6e 100644 > > --- a/drivers/pci/controller/dwc/pcie-qcom.c > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > > @@ -88,6 +88,8 @@ struct qcom_pcie_resources_2_1_0 { > > struct clk *iface_clk; > > struct clk *core_clk; > > struct clk *phy_clk; > > + struct clk *aux_clk; > > + struct clk *ref_clk; > > struct reset_control *pci_reset; > > struct reset_control *axi_reset; > > struct reset_control *ahb_reset; > > @@ -246,6 +248,14 @@ static int > qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie) > > if (IS_ERR(res->phy_clk)) > > return PTR_ERR(res->phy_clk); > > > > + res->aux_clk = devm_clk_get(dev, "aux"); > > + if (IS_ERR(res->aux_clk)) > > + return PTR_ERR(res->aux_clk); > > + > > + res->ref_clk = devm_clk_get(dev, "ref"); > > + if (IS_ERR(res->ref_clk)) > > + return PTR_ERR(res->ref_clk); > > + > > res->pci_reset = devm_reset_control_get_exclusive(dev, "pci"); > > if (IS_ERR(res->pci_reset)) > > return PTR_ERR(res->pci_reset); > > @@ -278,6 +288,8 @@ static void qcom_pcie_deinit_2_1_0(struct > qcom_pcie *pcie) > > clk_disable_unprepare(res->iface_clk); > > clk_disable_unprepare(res->core_clk); > > clk_disable_unprepare(res->phy_clk); > > + clk_disable_unprepare(res->aux_clk); > > + clk_disable_unprepare(res->ref_clk); > > regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies); > > } > > > > @@ -307,16 +319,28 @@ static int qcom_pcie_init_2_1_0(struct > qcom_pcie *pcie) > > goto err_assert_ahb; > > } > > > > + ret = clk_prepare_enable(res->core_clk); > > + if (ret) { > > + dev_err(dev, "cannot prepare/enable core clock\n"); > > + goto err_clk_core; > > + } > > + > > ret = clk_prepare_enable(res->phy_clk); > > if (ret) { > > dev_err(dev, "cannot prepare/enable phy clock\n"); > > goto err_clk_phy; > > } > > > > - ret = clk_prepare_enable(res->core_clk); > > + ret = clk_prepare_enable(res->aux_clk); > > if (ret) { > > - dev_err(dev, "cannot prepare/enable core clock\n"); > > - goto err_clk_core; > > + dev_err(dev, "cannot prepare/enable aux clock\n"); > > + goto err_clk_aux; > > + } > > + > > + ret = clk_prepare_enable(res->ref_clk); > > + if (ret) { > > + dev_err(dev, "cannot prepare/enable ref clock\n"); > > + goto err_clk_ref; > > } > > > > ret = reset_control_deassert(res->ahb_reset); > > @@ -372,10 +396,14 @@ static int qcom_pcie_init_2_1_0(struct > qcom_pcie *pcie) > > return 0; > > > > err_deassert_ahb: > > - clk_disable_unprepare(res->core_clk); > > -err_clk_core: > > + clk_disable_unprepare(res->ref_clk); > > +err_clk_ref: > > + clk_disable_unprepare(res->aux_clk); > > +err_clk_aux: > > clk_disable_unprepare(res->phy_clk); > > err_clk_phy: > > + clk_disable_unprepare(res->core_clk); > > +err_clk_core: > > clk_disable_unprepare(res->iface_clk); > > err_assert_ahb: > > regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies); > > > > -- > regards, > Stan
Hi Ansuel, On 4/8/20 3:36 PM, ansuelsmth@gmail.com wrote: >> PCIe driver >> >> Ansuel, >> >> On 4/2/20 3:11 PM, Ansuel Smith wrote: >>> Aux and Ref clk are missing in pcie qcom driver. >>> Add support in the driver to fix pcie inizialization in ipq806x. >>> >>> Fixes: 82a82383 PCI: qcom: Add Qualcomm PCIe controller driver >> >> this should be: >> >> Fixes: 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver >> >> and add: >> >> Cc: stable@vger.kernel.org # v4.5+ >> >> But, I wonder, as apq8064 shares the same ops_2_1_0 how it worked until >> now. Something more I cannot find such clocks for apq8064, which means >> that this patch will break it. >> >> One option is to use those new clocks only for ipq806x. >> > > How to add this new clocks only for ipq806x? Check the compatible and add > them accordingly? > Yes, through of_device_is_compatible(). See how we done this in qcom_pcie_get_resources_2_4_0. I thought about second option though - encoder what clocks we have for any SoC but if you take into that direction you have to change the whole driver :) Another option is to use clk_get_optional() for the clocks which you have on ipq806x (and don't have on apq8064). Please research this one first.
> in PCIe driver > > Hi Ansuel, > > On 4/8/20 3:36 PM, ansuelsmth@gmail.com wrote: > >> PCIe driver > >> > >> Ansuel, > >> > >> On 4/2/20 3:11 PM, Ansuel Smith wrote: > >>> Aux and Ref clk are missing in pcie qcom driver. > >>> Add support in the driver to fix pcie inizialization in ipq806x. > >>> > >>> Fixes: 82a82383 PCI: qcom: Add Qualcomm PCIe controller driver > >> > >> this should be: > >> > >> Fixes: 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver > >> > >> and add: > >> > >> Cc: stable@vger.kernel.org # v4.5+ > >> > >> But, I wonder, as apq8064 shares the same ops_2_1_0 how it worked > until > >> now. Something more I cannot find such clocks for apq8064, which > means > >> that this patch will break it. > >> > >> One option is to use those new clocks only for ipq806x. > >> > > > > How to add this new clocks only for ipq806x? Check the compatible and > add > > them accordingly? > > > > Yes, through of_device_is_compatible(). See how we done this in > qcom_pcie_get_resources_2_4_0. > > I thought about second option though - encoder what clocks we have for > any SoC but if you take into that direction you have to change the whole > driver :) > > Another option is to use clk_get_optional() for the clocks which you > have on ipq806x (and don't have on apq8064). Please research this one > first. > > -- > regards, > Stan Ok I will use get optional for the extra clocks. Should I add a warning if they are not present? Also what about the extra reset? Should I follow the same approach? Thx for the suggestions.
On 4/8/20 3:55 PM, ansuelsmth@gmail.com wrote: >> in PCIe driver >> >> Hi Ansuel, >> >> On 4/8/20 3:36 PM, ansuelsmth@gmail.com wrote: >>>> PCIe driver >>>> >>>> Ansuel, >>>> >>>> On 4/2/20 3:11 PM, Ansuel Smith wrote: >>>>> Aux and Ref clk are missing in pcie qcom driver. >>>>> Add support in the driver to fix pcie inizialization in ipq806x. >>>>> >>>>> Fixes: 82a82383 PCI: qcom: Add Qualcomm PCIe controller driver >>>> >>>> this should be: >>>> >>>> Fixes: 82a823833f4e PCI: qcom: Add Qualcomm PCIe controller driver >>>> >>>> and add: >>>> >>>> Cc: stable@vger.kernel.org # v4.5+ >>>> >>>> But, I wonder, as apq8064 shares the same ops_2_1_0 how it worked >> until >>>> now. Something more I cannot find such clocks for apq8064, which >> means >>>> that this patch will break it. >>>> >>>> One option is to use those new clocks only for ipq806x. >>>> >>> >>> How to add this new clocks only for ipq806x? Check the compatible and >> add >>> them accordingly? >>> >> >> Yes, through of_device_is_compatible(). See how we done this in >> qcom_pcie_get_resources_2_4_0. >> >> I thought about second option though - encoder what clocks we have for >> any SoC but if you take into that direction you have to change the whole >> driver :) >> >> Another option is to use clk_get_optional() for the clocks which you >> have on ipq806x (and don't have on apq8064). Please research this one >> first. >> >> -- >> regards, >> Stan > > Ok I will use get optional for the extra clocks. Should I add a warning if they > are not present? Also what about the extra reset? Should I follow the same > approach? > Thx for the suggestions. > No warnings please. You should follow the same rules for resets.
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 5ea527a6bd9f..f958c535de6e 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -88,6 +88,8 @@ struct qcom_pcie_resources_2_1_0 { struct clk *iface_clk; struct clk *core_clk; struct clk *phy_clk; + struct clk *aux_clk; + struct clk *ref_clk; struct reset_control *pci_reset; struct reset_control *axi_reset; struct reset_control *ahb_reset; @@ -246,6 +248,14 @@ static int qcom_pcie_get_resources_2_1_0(struct qcom_pcie *pcie) if (IS_ERR(res->phy_clk)) return PTR_ERR(res->phy_clk); + res->aux_clk = devm_clk_get(dev, "aux"); + if (IS_ERR(res->aux_clk)) + return PTR_ERR(res->aux_clk); + + res->ref_clk = devm_clk_get(dev, "ref"); + if (IS_ERR(res->ref_clk)) + return PTR_ERR(res->ref_clk); + res->pci_reset = devm_reset_control_get_exclusive(dev, "pci"); if (IS_ERR(res->pci_reset)) return PTR_ERR(res->pci_reset); @@ -278,6 +288,8 @@ static void qcom_pcie_deinit_2_1_0(struct qcom_pcie *pcie) clk_disable_unprepare(res->iface_clk); clk_disable_unprepare(res->core_clk); clk_disable_unprepare(res->phy_clk); + clk_disable_unprepare(res->aux_clk); + clk_disable_unprepare(res->ref_clk); regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies); } @@ -307,16 +319,28 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie) goto err_assert_ahb; } + ret = clk_prepare_enable(res->core_clk); + if (ret) { + dev_err(dev, "cannot prepare/enable core clock\n"); + goto err_clk_core; + } + ret = clk_prepare_enable(res->phy_clk); if (ret) { dev_err(dev, "cannot prepare/enable phy clock\n"); goto err_clk_phy; } - ret = clk_prepare_enable(res->core_clk); + ret = clk_prepare_enable(res->aux_clk); if (ret) { - dev_err(dev, "cannot prepare/enable core clock\n"); - goto err_clk_core; + dev_err(dev, "cannot prepare/enable aux clock\n"); + goto err_clk_aux; + } + + ret = clk_prepare_enable(res->ref_clk); + if (ret) { + dev_err(dev, "cannot prepare/enable ref clock\n"); + goto err_clk_ref; } ret = reset_control_deassert(res->ahb_reset); @@ -372,10 +396,14 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie) return 0; err_deassert_ahb: - clk_disable_unprepare(res->core_clk); -err_clk_core: + clk_disable_unprepare(res->ref_clk); +err_clk_ref: + clk_disable_unprepare(res->aux_clk); +err_clk_aux: clk_disable_unprepare(res->phy_clk); err_clk_phy: + clk_disable_unprepare(res->core_clk); +err_clk_core: clk_disable_unprepare(res->iface_clk); err_assert_ahb: regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);