2022-02-25 11:24:29

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v8 0/8] PCI: imx6: refine codes and add compliance tests mode support

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
- Disable the regulators and clocks when link never comes up
- Add the compliance tests mode support

Main changes from v7 to v8:
Regarding Bjorn's review comments.
- Align the format of the dev_info message and refine commit log of
#6/7/8 patches.
- Rename the err_reset_phy label, since there is no PHY reset in the out.

Main changes from v6 to v7:
- Keep the regulator usage counter balance in the #5 patch of v6 series.

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 | 223 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------
drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++-
drivers/pci/controller/dwc/pcie-designware.h | 1 +
3 files changed, 150 insertions(+), 79 deletions(-)

[PATCH v8 1/8] PCI: imx6: Encapsulate the clock enable into one
[PATCH v8 2/8] PCI: imx6: Add the error propagation from host_init
[PATCH v8 3/8] PCI: imx6: Move imx6_pcie_clk_disable() earlier
[PATCH v8 4/8] PCI: imx6: Disable iMX6QDL PCIe REF clock when disable
[PATCH v8 5/8] PCI: imx6: Refine the regulator usage
[PATCH v8 6/8] PCI: dwc: Add dw_pcie_host_ops.host_exit() callback
[PATCH v8 7/8] PCI: imx6: Disable clocks and regulators after link is
[PATCH v8 8/8] PCI: imx6: Add compliance tests mode support


2022-02-25 13:08:49

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v8 7/8] PCI: imx6: Disable clocks and regulators after link is down

Since i.MX PCIe doesn't support hot-plug, reduce power consumption
as much as possible by disabling clocks and regulators and returning
error when the link is down.

Add a new host_exit() callback for i.MX PCIe driver to disable the 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 | 34 +++++++++++++++++++++++----
1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 160a0bd02098..f8b15f0bc9ce 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -849,7 +849,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_out;

if (pci->link_gen == 2) {
/* Allow Gen2 mode after the link is up. */
@@ -880,12 +882,14 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
ret = imx6_pcie_wait_for_speed_change(imx6_pcie);
if (ret) {
dev_err(dev, "Failed to bring link up!\n");
- goto err_reset_phy;
+ goto err_out;
}
}

/* 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_out;
} else {
dev_info(dev, "Link: Gen2 disabled\n");
}
@@ -894,11 +898,10 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS);
return 0;

-err_reset_phy:
+err_out:
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;
}

@@ -922,8 +925,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

2022-02-25 16:22:10

by Richard Zhu

[permalink] [raw]
Subject: [PATCH v8 3/8] PCI: imx6: Move imx6_pcie_clk_disable() earlier

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 | 48 +++++++++++++--------------
1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 3ca2eef39617..99fc22d1d55e 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -533,6 +533,30 @@ 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:
+ case IMX8MM:
+ 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 +989,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

2022-03-14 12:26:24

by Richard Zhu

[permalink] [raw]
Subject: RE: [PATCH v8 0/8] PCI: imx6: refine codes and add compliance tests mode support

Hi Lucas:
Since this v8 series patches are pending for a while, and no review comments anymore.
Can you help to take look at the other patches of this set?

Best Regards
Richard Zhu

> -----Original Message-----
> From: Richard Zhu <[email protected]>
> Sent: 2022??2??25?? 11:44
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Cc: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>
> Subject: [PATCH v8 0/8] PCI: imx6: refine codes and add compliance tests
> mode support
>
> 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
> - Disable the regulators and clocks when link never comes up
> - Add the compliance tests mode support
>
> Main changes from v7 to v8:
> Regarding Bjorn's review comments.
> - Align the format of the dev_info message and refine commit log of
> #6/7/8 patches.
> - Rename the err_reset_phy label, since there is no PHY reset in the out.
>
> Main changes from v6 to v7:
> - Keep the regulator usage counter balance in the #5 patch of v6 series.
>
> Main changes from v5 to v6:
> - Refer to the following discussion with Fabio, fix the dump by his patch.
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.kernel.org%2Fproject%2Flinux-pci%2Fpatch%2F1641368602-20401-6-git-s
> end-email-hongxing.zhu%40nxp.com%2F&amp;data=04%7C01%7Chongxing.z
> hu%40nxp.com%7C100772798bed44ec362f08d9f8124d9c%7C686ea1d3bc2b
> 4c6fa92cd99c5c301635%7C0%7C0%7C637813579753331370%7CUnknown%
> 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000&amp;sdata=m4XhLgO0e7H1MzS1KPskWhK1Lc1g
> uUqzYpudrL03pCQ%3D&amp;reserved=0
> 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 | 223
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> -------------------------------
> drivers/pci/controller/dwc/pcie-designware-host.c | 5 ++-
> drivers/pci/controller/dwc/pcie-designware.h | 1 +
> 3 files changed, 150 insertions(+), 79 deletions(-)
>
> [PATCH v8 1/8] PCI: imx6: Encapsulate the clock enable into one [PATCH v8 2/8]
> PCI: imx6: Add the error propagation from host_init [PATCH v8 3/8] PCI: imx6:
> Move imx6_pcie_clk_disable() earlier [PATCH v8 4/8] PCI: imx6: Disable
> iMX6QDL PCIe REF clock when disable [PATCH v8 5/8] PCI: imx6: Refine the
> regulator usage [PATCH v8 6/8] PCI: dwc: Add dw_pcie_host_ops.host_exit()
> callback [PATCH v8 7/8] PCI: imx6: Disable clocks and regulators after link is
> [PATCH v8 8/8] PCI: imx6: Add compliance tests mode support