2022-06-23 15:56:33

by Robert Marko

[permalink] [raw]
Subject: [PATCH v3 1/2] PCI: qcom: fix IPQ8074 Gen2 support

Currently the Gen2 port in IPQ8074 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")
Signed-off-by: Robert Marko <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
Changes in v3:
* Make sure it applies onto 5.19-rc3
* Update the commit description to make it clear this only affects the
Gen2 port

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 a1f1aca2fb59..24708d5d817d 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)
@@ -1596,6 +1603,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


2022-06-23 16:22:50

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] PCI: qcom: fix IPQ8074 Gen2 support

On Thu, Jun 23, 2022 at 05:50:03PM +0200, Robert Marko wrote:
> Currently the Gen2 port in IPQ8074 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")

Does v5.11 or some v5.11-based stable kernel contain IPQ8074 support?
If so, I'll mark this for backporting to stable.

> Signed-off-by: Robert Marko <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> ---
> Changes in v3:
> * Make sure it applies onto 5.19-rc3
> * Update the commit description to make it clear this only affects the
> Gen2 port
>
> 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 a1f1aca2fb59..24708d5d817d 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)
> @@ -1596,6 +1603,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
>

2022-06-23 16:46:34

by Robert Marko

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] PCI: qcom: fix IPQ8074 Gen2 support

On Thu, 23 Jun 2022 at 18:03, Bjorn Helgaas <[email protected]> wrote:
>
> On Thu, Jun 23, 2022 at 05:50:03PM +0200, Robert Marko wrote:
> > Currently the Gen2 port in IPQ8074 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")
>
> Does v5.11 or some v5.11-based stable kernel contain IPQ8074 support?
> If so, I'll mark this for backporting to stable.

Hi Bjorn,
Initial IPQ8074 support was added way back in 4.14.

Note that DTS nodes for both QMP PCI PHY-s and PCIe itself
for IPQ8074 are broken as well.

That is my TODO to clean up and send upstream as well.

Regards,
Robert
>
> > Signed-off-by: Robert Marko <[email protected]>
> > Reviewed-by: Dmitry Baryshkov <[email protected]>
> > ---
> > Changes in v3:
> > * Make sure it applies onto 5.19-rc3
> > * Update the commit description to make it clear this only affects the
> > Gen2 port
> >
> > 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 a1f1aca2fb59..24708d5d817d 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)
> > @@ -1596,6 +1603,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
> >