This series patches refine pci-imx6 driver and do the following changes.
- Encapsulate the clock enable into one standalone function
- Add the error propagation from host_init
- Balance the usage of the regulator and clocks when link never came up
- Add the compliance tests mode support
Main changes from v5 to v6:
- Refer to the following discussion with Fabio, fix the dump by his patch.
https://patchwork.kernel.org/project/linux-pci/patch/[email protected]/
Refine and rebase this patch-set after Fabio' dump fix patch is merged.
- Add one new #4 patch to disable i.MX6QDL REF clock too when disable clocks
- Split the regulator refine codes into one standalone patch #5 in this version.
Main changes from v4 to v5:
- Since i.MX8MM PCIe support had been merged. Based on Lorenzo's git repos,
resend the patch-set after rebase.
Main changes from v3 to v4:
- Regarding Mark's comments, delete the regulator_is_enabled() check.
- Squash #3 and #6 of v3 patch into #5 patch of v4 set.
Main changes from v2 to v3:
- Add "Reviewed-by: Lucas Stach <[email protected]>" tag into
first two patches.
- Add a Fixes tag into #3 patch.
- Split the #4 of v2 to two patches, one is clock disable codes move,
the other one is the acutal clock unbalance fix.
- Add a new host_exit() callback into dw_pcie_host_ops, then it could be
invoked to handle the unbalance issue in the error handling after
host_init() function when link is down.
- Add a new host_exit() callback for i.MX PCIe driver to handle this case
in the error handling after host_init.
Main changes from v1 to v2:
Regarding Lucas' comments.
- Move the placement of the new imx6_pcie_clk_enable() to avoid the
forward declarition.
- Seperate the second patch of v1 patch-set to three patches.
- Use the module_param to replace the kernel command line.
Regarding Bjorn's comments:
- Use the cover-letter for a multi-patch series.
- Correct the subject line, and refine the commit logs. For example,
remove the timestamp of the logs.
drivers/pci/controller/dwc/pci-imx6.c | 213 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++-
drivers/pci/controller/dwc/pcie-designware.h | 1 +
3 files changed, 142 insertions(+), 77 deletions(-)
[PATCH v6 1/8] PCI: imx6: Encapsulate the clock enable into one
[PATCH v6 2/8] PCI: imx6: Add the error propagation from host_init
[PATCH v6 3/8] PCI: imx6: Move imx6_pcie_clk_disable() earlier
[PATCH v6 4/8] PCI: imx6: Disable imx6qdl pcie ref clk
[PATCH v6 5/8] PCI: imx6: Refine the regulator usage
[PATCH v6 6/8] PCI: dwc: Add dw_pcie_host_ops.host_exit() callback
[PATCH v6 7/8] PCI: imx6: Disable enabled clocks and regulators after
[PATCH v6 8/8] PCI: imx6: Add the compliance tests mode support
Just move the imx6_pcie_clk_disable() to an earlier place without function
changes, since it wouldn't be only used in imx6_pcie_suspend_noirq() later.
Signed-off-by: Richard Zhu <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 47 +++++++++++++--------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 3ca2eef39617..e157f0bec37e 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -533,6 +533,29 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
return ret;
}
+static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
+{
+ clk_disable_unprepare(imx6_pcie->pcie);
+ clk_disable_unprepare(imx6_pcie->pcie_phy);
+ clk_disable_unprepare(imx6_pcie->pcie_bus);
+
+ switch (imx6_pcie->drvdata->variant) {
+ case IMX6SX:
+ clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
+ break;
+ case IMX7D:
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
+ IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
+ break;
+ case IMX8MQ:
+ clk_disable_unprepare(imx6_pcie->pcie_aux);
+ break;
+ default:
+ break;
+ }
+}
+
static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
{
u32 val;
@@ -965,30 +988,6 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
usleep_range(1000, 10000);
}
-static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
-{
- clk_disable_unprepare(imx6_pcie->pcie);
- clk_disable_unprepare(imx6_pcie->pcie_phy);
- clk_disable_unprepare(imx6_pcie->pcie_bus);
-
- switch (imx6_pcie->drvdata->variant) {
- case IMX6SX:
- clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
- break;
- case IMX7D:
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
- IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
- break;
- case IMX8MQ:
- case IMX8MM:
- clk_disable_unprepare(imx6_pcie->pcie_aux);
- break;
- default:
- break;
- }
-}
-
static int imx6_pcie_suspend_noirq(struct device *dev)
{
struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
--
2.25.1
Since i.MX PCIe doesn't support the hot-plug, and to save power
consumption as much as possible. Return error and disable the enabled
clocks and regulators when link is down,.
Add a new host_exit() callback for i.MX PCIe driver to disable the
enabled clocks, regulators and so on in the error handling after
host_init is finished.
Signed-off-by: Richard Zhu <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 30 ++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index e165ad00989c..7a7d9204c6bc 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -848,7 +848,9 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
/* Start LTSSM. */
imx6_pcie_ltssm_enable(dev);
- dw_pcie_wait_for_link(pci);
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret)
+ goto err_reset_phy;
if (pci->link_gen == 2) {
/* Allow Gen2 mode after the link is up. */
@@ -884,7 +886,9 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
}
/* Make sure link training is finished as well! */
- dw_pcie_wait_for_link(pci);
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret)
+ goto err_reset_phy;
} else {
dev_info(dev, "Link: Gen2 disabled\n");
}
@@ -897,7 +901,6 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
dev_dbg(dev, "PHY DEBUG_R0=0x%08x DEBUG_R1=0x%08x\n",
dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
- imx6_pcie_reset_phy(imx6_pcie);
return ret;
}
@@ -921,8 +924,29 @@ static int imx6_pcie_host_init(struct pcie_port *pp)
return 0;
}
+static void imx6_pcie_host_exit(struct pcie_port *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct device *dev = pci->dev;
+ struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
+
+ imx6_pcie_reset_phy(imx6_pcie);
+ imx6_pcie_clk_disable(imx6_pcie);
+ switch (imx6_pcie->drvdata->variant) {
+ case IMX8MM:
+ if (phy_power_off(imx6_pcie->phy))
+ dev_err(dev, "unable to power off phy\n");
+ break;
+ default:
+ break;
+ }
+ if (imx6_pcie->vpcie)
+ regulator_disable(imx6_pcie->vpcie);
+}
+
static const struct dw_pcie_host_ops imx6_pcie_host_ops = {
.host_init = imx6_pcie_host_init,
+ .host_exit = imx6_pcie_host_exit,
};
static const struct dw_pcie_ops dw_pcie_ops = {
--
2.25.1
Hello Richard,
On Tue, Feb 08, 2022 at 11:25:30AM +0800, Richard Zhu wrote:
> Just move the imx6_pcie_clk_disable() to an earlier place without function
> changes, since it wouldn't be only used in imx6_pcie_suspend_noirq() later.
In which patch are you going to use imx6_pcie_clk_disable()? I could not
see it in this patch series.
Francesco
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 47 +++++++++++++--------------
> 1 file changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 3ca2eef39617..e157f0bec37e 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -533,6 +533,29 @@ static int imx6_pcie_clk_enable(struct imx6_pcie *imx6_pcie)
> return ret;
> }
>
> +static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
> +{
> + clk_disable_unprepare(imx6_pcie->pcie);
> + clk_disable_unprepare(imx6_pcie->pcie_phy);
> + clk_disable_unprepare(imx6_pcie->pcie_bus);
> +
> + switch (imx6_pcie->drvdata->variant) {
> + case IMX6SX:
> + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> + break;
> + case IMX7D:
> + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> + break;
> + case IMX8MQ:
> + clk_disable_unprepare(imx6_pcie->pcie_aux);
> + break;
> + default:
> + break;
> + }
> +}
> +
> static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> {
> u32 val;
> @@ -965,30 +988,6 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
> usleep_range(1000, 10000);
> }
>
> -static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie)
> -{
> - clk_disable_unprepare(imx6_pcie->pcie);
> - clk_disable_unprepare(imx6_pcie->pcie_phy);
> - clk_disable_unprepare(imx6_pcie->pcie_bus);
> -
> - switch (imx6_pcie->drvdata->variant) {
> - case IMX6SX:
> - clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> - break;
> - case IMX7D:
> - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> - break;
> - case IMX8MQ:
> - case IMX8MM:
> - clk_disable_unprepare(imx6_pcie->pcie_aux);
> - break;
> - default:
> - break;
> - }
> -}
> -
> static int imx6_pcie_suspend_noirq(struct device *dev)
> {
> struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> --
> 2.25.1
>
> -----Original Message-----
> From: Francesco Dolcini <[email protected]>
> Sent: 2022??2??12?? 0:30
> To: Hongxing Zhu <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>;
> [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH v6 3/8] PCI: imx6: Move imx6_pcie_clk_disable() earlier
>
> Hello Richard,
>
> On Tue, Feb 08, 2022 at 11:25:30AM +0800, Richard Zhu wrote:
> > Just move the imx6_pcie_clk_disable() to an earlier place without
> > function changes, since it wouldn't be only used in
> imx6_pcie_suspend_noirq() later.
>
> In which patch are you going to use imx6_pcie_clk_disable()? I could not see it
> in this patch series.
Hi Francesco:
Thanks for your review.
The imx6_pcie_clk_disable() would be invoked in imx6_pcie_host_exit() in #7
patch later.
Best Regards
Richard Zhu
>
> Francesco
>
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 47
> > +++++++++++++--------------
> > 1 file changed, 23 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 3ca2eef39617..e157f0bec37e 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -533,6 +533,29 @@ static int imx6_pcie_clk_enable(struct imx6_pcie
> *imx6_pcie)
> > return ret;
> > }
> >
> > +static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie) {
> > + clk_disable_unprepare(imx6_pcie->pcie);
> > + clk_disable_unprepare(imx6_pcie->pcie_phy);
> > + clk_disable_unprepare(imx6_pcie->pcie_bus);
> > +
> > + switch (imx6_pcie->drvdata->variant) {
> > + case IMX6SX:
> > + clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > + break;
> > + case IMX7D:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > + IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > + break;
> > + case IMX8MQ:
> > + clk_disable_unprepare(imx6_pcie->pcie_aux);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie
> > *imx6_pcie) {
> > u32 val;
> > @@ -965,30 +988,6 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie
> *imx6_pcie)
> > usleep_range(1000, 10000);
> > }
> >
> > -static void imx6_pcie_clk_disable(struct imx6_pcie *imx6_pcie) -{
> > - clk_disable_unprepare(imx6_pcie->pcie);
> > - clk_disable_unprepare(imx6_pcie->pcie_phy);
> > - clk_disable_unprepare(imx6_pcie->pcie_bus);
> > -
> > - switch (imx6_pcie->drvdata->variant) {
> > - case IMX6SX:
> > - clk_disable_unprepare(imx6_pcie->pcie_inbound_axi);
> > - break;
> > - case IMX7D:
> > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL,
> > - IMX7D_GPR12_PCIE_PHY_REFCLK_SEL);
> > - break;
> > - case IMX8MQ:
> > - case IMX8MM:
> > - clk_disable_unprepare(imx6_pcie->pcie_aux);
> > - break;
> > - default:
> > - break;
> > - }
> > -}
> > -
> > static int imx6_pcie_suspend_noirq(struct device *dev) {
> > struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> > --
> > 2.25.1
> >