Re: [PATCH v2] PCI: qcom: fix IPQ8074 Gen2 support

[Robert Marko <robimarko@gmail.com>]

On Tue, 21 Jun 2022 at 19:29, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Tue, 21 Jun 2022 at 14:23, Robert Marko <robimarko@gmail.com> wrote:
> >
> > IPQ8074 has one Gen2 and one Gen3 port, currently the Gen2 port will
> > cause the system to hang as its using DBI registers in the .init
> > and those are only accesible after phy_power_on().
> >
> > So solve this by splitting the DBI read/writes to .post_init.
> >
> > Fixes: a0fd361db8e5 ("PCI: dwc: Move "dbi", "dbi2", and "addr_space" resource setup into common code")
>
> Any elaboration for the Fixes tag? I think the follow one is more
> logical, isn't it?
>
> Fixes: 5d76117f070d ("PCI: qcom: Add support for IPQ8074 PCIe controller")

Hi,
My logic was that it was working before the commit a0fd361db8e5 as it
moved PHY init
later and indirectly broke IPQ8074 gen2.

Regards,
Robert
>
> > Signed-off-by: Robert Marko <robimarko@gmail.com>
> > ---
> > Changes in v2:
> > * Rebase onto next-20220621
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 48 +++++++++++++++-----------
> >  1 file changed, 28 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 51fed83484af..da6d79d61397 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -1061,9 +1061,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
> >         struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
> >         struct dw_pcie *pci = pcie->pci;
> >         struct device *dev = pci->dev;
> > -       u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> >         int i, ret;
> > -       u32 val;
> >
> >         for (i = 0; i < ARRAY_SIZE(res->rst); i++) {
> >                 ret = reset_control_assert(res->rst[i]);
> > @@ -1120,6 +1118,33 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
> >                 goto err_clk_aux;
> >         }
> >
> > +       return 0;
> > +
> > +err_clk_aux:
> > +       clk_disable_unprepare(res->ahb_clk);
> > +err_clk_ahb:
> > +       clk_disable_unprepare(res->axi_s_clk);
> > +err_clk_axi_s:
> > +       clk_disable_unprepare(res->axi_m_clk);
> > +err_clk_axi_m:
> > +       clk_disable_unprepare(res->iface);
> > +err_clk_iface:
> > +       /*
> > +        * Not checking for failure, will anyway return
> > +        * the original failure in 'ret'.
> > +        */
> > +       for (i = 0; i < ARRAY_SIZE(res->rst); i++)
> > +               reset_control_assert(res->rst[i]);
> > +
> > +       return ret;
> > +}
> > +
> > +static int qcom_pcie_post_init_2_3_3(struct qcom_pcie *pcie)
> > +{
> > +       struct dw_pcie *pci = pcie->pci;
> > +       u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> > +       u32 val;
> > +
> >         writel(SLV_ADDR_SPACE_SZ,
> >                 pcie->parf + PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE);
> >
> > @@ -1147,24 +1172,6 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie)
> >                 PCI_EXP_DEVCTL2);
> >
> >         return 0;
> > -
> > -err_clk_aux:
> > -       clk_disable_unprepare(res->ahb_clk);
> > -err_clk_ahb:
> > -       clk_disable_unprepare(res->axi_s_clk);
> > -err_clk_axi_s:
> > -       clk_disable_unprepare(res->axi_m_clk);
> > -err_clk_axi_m:
> > -       clk_disable_unprepare(res->iface);
> > -err_clk_iface:
> > -       /*
> > -        * Not checking for failure, will anyway return
> > -        * the original failure in 'ret'.
> > -        */
> > -       for (i = 0; i < ARRAY_SIZE(res->rst); i++)
> > -               reset_control_assert(res->rst[i]);
> > -
> > -       return ret;
> >  }
> >
> >  static int qcom_pcie_get_resources_2_7_0(struct qcom_pcie *pcie)
> > @@ -1598,6 +1605,7 @@ static const struct qcom_pcie_ops ops_2_4_0 = {
> >  static const struct qcom_pcie_ops ops_2_3_3 = {
> >         .get_resources = qcom_pcie_get_resources_2_3_3,
> >         .init = qcom_pcie_init_2_3_3,
> > +       .post_init = qcom_pcie_post_init_2_3_3,
> >         .deinit = qcom_pcie_deinit_2_3_3,
> >         .ltssm_enable = qcom_pcie_2_3_2_ltssm_enable,
> >  };
> > --
> > 2.36.1
> >
>
>
> --
> With best wishes
> Dmitry