[PATCH v2 0/2] phy: freescale: imx8m-pcie: fix and cleanup phy

[PATCH v2 0/2] phy: freescale: imx8m-pcie: fix and cleanup phy

[Stefan Eichenberger]

The imx8m-pcie PHY PLL fails to lock under certain conditions when
returning from suspend. This is resolved by asserting the PHY reset when
powering off the PHY during suspend. This ensures that the PHY is
properly reset when powering on again in resume.

Changes in v2:
- Remove unnecessary check if perst is not null (Philipp)

Stefan Eichenberger (2):
  phy: freescale: imx8m-pcie: cleanup reset logic
  phy: freescale: imx8m-pcie: assert phy reset and perst in power off

 drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 23 +++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

-- 
2.45.2

[PATCH v2 1/2] phy: freescale: imx8m-pcie: cleanup reset logic

[Stefan Eichenberger]

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Remove the switch statement and base perst release on whether it is
found in the device tree. The probe function fails without the reset
property, making it mandatory. Therefore, always release reset
independent of the variant.

This does not change the behavior of the driver but reduces driver
complexity and allows for easier future modifications.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index e98361dcdeadf..5b505e34ca364 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -141,15 +141,9 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
 			   IMX8MM_GPR_PCIE_REF_CLK_PLL);
 	usleep_range(100, 200);
 
-	switch (imx8_phy->drvdata->variant) {
-	case IMX8MP:
-		reset_control_deassert(imx8_phy->perst);
-		fallthrough;
-	case IMX8MM:
-		reset_control_deassert(imx8_phy->reset);
-		usleep_range(200, 500);
-		break;
-	}
+	reset_control_deassert(imx8_phy->perst);
+	reset_control_deassert(imx8_phy->reset);
+	usleep_range(200, 500);
 
 	/* Do the PHY common block reset */
 	regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
-- 
2.45.2

[PATCH v2 2/2] phy: freescale: imx8m-pcie: assert phy reset and perst in power off

[Stefan Eichenberger]

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Ensure the PHY reset and perst is asserted during power-off to
guarantee it is in a reset state upon repeated power-on calls. This
resolves an issue where the PHY may not properly initialize during
subsequent power-on cycles. Power-on will deassert the reset at the
appropriate time after tuning the PHY parameters.

During suspend/resume cycles, we observed that the PHY PLL failed to
lock during resume when the CPU temperature increased from 65C to 75C.
The observed errors were:
  phy phy-32f00000.pcie-phy.3: phy poweron failed --> -110
  imx6q-pcie 33800000.pcie: waiting for PHY ready timeout!
  imx6q-pcie 33800000.pcie: PM: dpm_run_callback(): genpd_resume_noirq+0x0/0x80 returns -110
  imx6q-pcie 33800000.pcie: PM: failed to resume noirq: error -110

This resulted in a complete CPU freeze, which is resolved by ensuring
the PHY is in reset during power-on, thus preventing PHY PLL failures.

Cc: stable@vger.kernel.org
Fixes: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver")
Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index 5b505e34ca364..7355d9921b646 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -156,6 +156,16 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
 	return ret;
 }
 
+static int imx8_pcie_phy_power_off(struct phy *phy)
+{
+	struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
+
+	reset_control_assert(imx8_phy->reset);
+	reset_control_assert(imx8_phy->perst);
+
+	return 0;
+}
+
 static int imx8_pcie_phy_init(struct phy *phy)
 {
 	struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
@@ -176,6 +186,7 @@ static const struct phy_ops imx8_pcie_phy_ops = {
 	.init		= imx8_pcie_phy_init,
 	.exit		= imx8_pcie_phy_exit,
 	.power_on	= imx8_pcie_phy_power_on,
+	.power_off	= imx8_pcie_phy_power_off,
 	.owner		= THIS_MODULE,
 };
 
-- 
2.45.2

Re: [PATCH v2 1/2] phy: freescale: imx8m-pcie: cleanup reset logic

[Frank Li]

On Wed, Mar 05, 2025 at 03:43:15PM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Remove the switch statement and base perst release on whether it is
> found in the device tree. The probe function fails without the reset
> property, making it mandatory. Therefore, always release reset
> independent of the variant.

Thanks for clean up this.

"Remove the switch statement for releasing 'perst' and 'reset' since they
are already correctly set at probe and are no-ops for
reset_control_deassert(NULL). Call these unconditionally."

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> This does not change the behavior of the driver but reduces driver
> complexity and allows for easier future modifications.
>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> ---
>  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> index e98361dcdeadf..5b505e34ca364 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> @@ -141,15 +141,9 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
>  			   IMX8MM_GPR_PCIE_REF_CLK_PLL);
>  	usleep_range(100, 200);
>
> -	switch (imx8_phy->drvdata->variant) {
> -	case IMX8MP:
> -		reset_control_deassert(imx8_phy->perst);
> -		fallthrough;
> -	case IMX8MM:
> -		reset_control_deassert(imx8_phy->reset);
> -		usleep_range(200, 500);
> -		break;
> -	}
> +	reset_control_deassert(imx8_phy->perst);
> +	reset_control_deassert(imx8_phy->reset);
> +	usleep_range(200, 500);
>
>  	/* Do the PHY common block reset */
>  	regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> --
> 2.45.2
>

Re: [PATCH v2 2/2] phy: freescale: imx8m-pcie: assert phy reset and perst in power off

[Frank Li]

On Wed, Mar 05, 2025 at 03:43:16PM +0100, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
>
> Ensure the PHY reset and perst is asserted during power-off to
> guarantee it is in a reset state upon repeated power-on calls. This
> resolves an issue where the PHY may not properly initialize during
> subsequent power-on cycles. Power-on will deassert the reset at the
> appropriate time after tuning the PHY parameters.
>
> During suspend/resume cycles, we observed that the PHY PLL failed to
> lock during resume when the CPU temperature increased from 65C to 75C.
> The observed errors were:
>   phy phy-32f00000.pcie-phy.3: phy poweron failed --> -110
>   imx6q-pcie 33800000.pcie: waiting for PHY ready timeout!
>   imx6q-pcie 33800000.pcie: PM: dpm_run_callback(): genpd_resume_noirq+0x0/0x80 returns -110
>   imx6q-pcie 33800000.pcie: PM: failed to resume noirq: error -110
>
> This resulted in a complete CPU freeze, which is resolved by ensuring
> the PHY is in reset during power-on, thus preventing PHY PLL failures.
>
> Cc: stable@vger.kernel.org
> Fixes: 1aa97b002258 ("phy: freescale: pcie: Initialize the imx8 pcie standalone phy driver")
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> index 5b505e34ca364..7355d9921b646 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> @@ -156,6 +156,16 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
>  	return ret;
>  }
>
> +static int imx8_pcie_phy_power_off(struct phy *phy)
> +{
> +	struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
> +
> +	reset_control_assert(imx8_phy->reset);
> +	reset_control_assert(imx8_phy->perst);
> +
> +	return 0;
> +}
> +
>  static int imx8_pcie_phy_init(struct phy *phy)
>  {
>  	struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);
> @@ -176,6 +186,7 @@ static const struct phy_ops imx8_pcie_phy_ops = {
>  	.init		= imx8_pcie_phy_init,
>  	.exit		= imx8_pcie_phy_exit,
>  	.power_on	= imx8_pcie_phy_power_on,
> +	.power_off	= imx8_pcie_phy_power_off,
>  	.owner		= THIS_MODULE,
>  };
>
> --
> 2.45.2
>

RE: [PATCH v2 0/2] phy: freescale: imx8m-pcie: fix and cleanup phy

[Hongxing Zhu]

Hi Stefan:
Thanks for the clean up and bug fix. Please update the commit with Frank's comments.
For this series, Acked-by: Richard Zhu <hongxing.zhu@nxp.com>

Best Regards
Richard Zhu

> -----Original Message-----
> From: Stefan Eichenberger <eichest@gmail.com>
> Sent: 2025年3月5日 22:43
> To: vkoul@kernel.org; kishon@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> p.zabel@pengutronix.de; tharvey@gateworks.com; Hongxing Zhu
> <hongxing.zhu@nxp.com>; Francesco Dolcini <francesco.dolcini@toradex.com>
> Cc: linux-phy@lists.infradead.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [PATCH v2 0/2] phy: freescale: imx8m-pcie: fix and cleanup phy
> 
> The imx8m-pcie PHY PLL fails to lock under certain conditions when returning
> from suspend. This is resolved by asserting the PHY reset when powering off the
> PHY during suspend. This ensures that the PHY is properly reset when powering
> on again in resume.
> 
> Changes in v2:
> - Remove unnecessary check if perst is not null (Philipp)
> 
> Stefan Eichenberger (2):
>   phy: freescale: imx8m-pcie: cleanup reset logic
>   phy: freescale: imx8m-pcie: assert phy reset and perst in power off
> 
>  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 23 +++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> --
> 2.45.2

Re: [PATCH v2 0/2] phy: freescale: imx8m-pcie: fix and cleanup phy

[Vinod Koul]

On Wed, 05 Mar 2025 15:43:14 +0100, Stefan Eichenberger wrote:
> The imx8m-pcie PHY PLL fails to lock under certain conditions when
> returning from suspend. This is resolved by asserting the PHY reset when
> powering off the PHY during suspend. This ensures that the PHY is
> properly reset when powering on again in resume.
> 
> Changes in v2:
> - Remove unnecessary check if perst is not null (Philipp)
> 
> [...]

Applied, thanks!

[1/2] phy: freescale: imx8m-pcie: cleanup reset logic
      commit: 97e8a2ff28a3dc36ca3cdc94f37359852d7e1c8b
[2/2] phy: freescale: imx8m-pcie: assert phy reset and perst in power off
      commit: aecb63e88c5e5fb9afb782a1577264c76f179af9

Best regards,
-- 
~Vinod