[v2,1/1] phy: freescale: imx8m-pcie: Do CMN_RST just before PHY PLL lock check

Commit Message

Frank Li Oct. 21, 2024, 3:52 p.m. UTC

From: Richard Zhu <[e-mail]>

When enable initcall_debug together with higher debug level below.
CONFIG_CONSOLE_LOGLEVEL_DEFAULT=9
CONFIG_CONSOLE_LOGLEVEL_QUIET=9
CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7

The initialization of i.MX8MP PCIe PHY might be timeout failed randomly.
To fix this issue, adjust the sequence of the resets refer to the power
up sequence listed below.

i.MX8MP PCIe PHY power up sequence:
                          /---------------------------------------------
1.8v supply     ---------/
                    /---------------------------------------------------
0.8v supply     ---/

                ---\ /--------------------------------------------------
                    X        REFCLK Valid
Reference Clock ---/ \--------------------------------------------------
                             -------------------------------------------
                             |
i_init_restn    --------------
                                    ------------------------------------
                                    |
i_cmn_rstn      ---------------------
                                         -------------------------------
                                         |
o_pll_lock_done --------------------------

Logs:
imx6q-pcie 33800000.pcie: host bridge /soc@0/pcie@33800000 ranges:
imx6q-pcie 33800000.pcie:       IO 0x001ff80000..0x001ff8ffff -> 0x0000000000
imx6q-pcie 33800000.pcie:      MEM 0x0018000000..0x001fefffff -> 0x0018000000
probe of clk_imx8mp_audiomix.reset.0 returned 0 after 1052 usecs
probe of 30e20000.clock-controller returned 0 after 32971 usecs
phy phy-32f00000.pcie-phy.4: phy poweron failed --> -110
probe of 30e10000.dma-controller returned 0 after 10235 usecs
imx6q-pcie 33800000.pcie: waiting for PHY ready timeout!
dwhdmi-imx 32fd8000.hdmi: Detected HDMI TX controller v2.13a with HDCP (samsung_dw_hdmi_phy2)
imx6q-pcie 33800000.pcie: probe with driver imx6q-pcie failed with error -110

Fixes: dce9edff16ee ("phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY support")
Cc: [e-mail]
Signed-off-by: Richard Zhu <[e-mail]>
Signed-off-by: Frank Li <[e-mail]>

v2 changes:
- Rebase to latest fixes branch of linux-phy git repo.
- Richard's environment have problem and can't sent out patch. So I help
post this fix patch.
---
 drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Richard Zhu Oct. 22, 2024, 3:49 a.m. UTC | #1

> -----Original Message-----
> From: Frank Li <[e-mail]>
> Sent: 2024年10月21日 23:53
> To: [e-mail]
> Cc: Frank Li <[e-mail]>; [e-mail]; Hongxing Zhu
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Marcel Ziswiler
> <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]
> Subject: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just before
> PHY PLL lock check
> 
> From: Richard Zhu <[e-mail]>
> 
> When enable initcall_debug together with higher debug level below.
> CONFIG_CONSOLE_LOGLEVEL_DEFAULT=9
> CONFIG_CONSOLE_LOGLEVEL_QUIET=9
> CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7
> 
> The initialization of i.MX8MP PCIe PHY might be timeout failed randomly.
> To fix this issue, adjust the sequence of the resets refer to the power up
> sequence listed below.
> 
> i.MX8MP PCIe PHY power up sequence:
>                           /---------------------------------------------
> 1.8v supply     ---------/
>                     /---------------------------------------------------
> 0.8v supply     ---/
> 
>                 ---\ /--------------------------------------------------
>                     X        REFCLK Valid
> Reference Clock ---/ \--------------------------------------------------
>                              -------------------------------------------
>                              |
> i_init_restn    --------------
>                                     ------------------------------------
>                                     |
> i_cmn_rstn      ---------------------
>                                          -------------------------------
>                                          | o_pll_lock_done
> --------------------------
> 
> Logs:
> imx6q-pcie 33800000.pcie: host bridge /soc@0/pcie@33800000 ranges:
> imx6q-pcie 33800000.pcie:       IO 0x001ff80000..0x001ff8ffff ->
> 0x0000000000
> imx6q-pcie 33800000.pcie:      MEM 0x0018000000..0x001fefffff ->
> 0x0018000000
> probe of clk_imx8mp_audiomix.reset.0 returned 0 after 1052 usecs probe of
> 30e20000.clock-controller returned 0 after 32971 usecs phy
> phy-32f00000.pcie-phy.4: phy poweron failed --> -110 probe of
> 30e10000.dma-controller returned 0 after 10235 usecs imx6q-pcie
> 33800000.pcie: waiting for PHY ready timeout!
> dwhdmi-imx 32fd8000.hdmi: Detected HDMI TX controller v2.13a with HDCP
> (samsung_dw_hdmi_phy2) imx6q-pcie 33800000.pcie: probe with driver
> imx6q-pcie failed with error -110
> 
> Fixes: dce9edff16ee ("phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY
> support")
> Cc: [e-mail]
> Signed-off-by: Richard Zhu <[e-mail]>
> Signed-off-by: Frank Li <[e-mail]>
> 
> v2 changes:
> - Rebase to latest fixes branch of linux-phy git repo.
> - Richard's environment have problem and can't sent out patch. So I help post
> this fix patch.
> ---
Hi Frank:
Thanks a lot for your kindly help.
Since my server is down, I can't send out this v2 in the past days.

Hi Vinod:
Sorry for the late reply, and bring you inconvenience.

Best Regards
Richard Zhu

>  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> index 11fcb1867118c..e98361dcdeadf 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> @@ -141,11 +141,6 @@ static int imx8_pcie_phy_power_on(struct phy
> *phy)
>  			   IMX8MM_GPR_PCIE_REF_CLK_PLL);
>  	usleep_range(100, 200);
> 
> -	/* Do the PHY common block reset */
> -	regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> -			   IMX8MM_GPR_PCIE_CMN_RST,
> -			   IMX8MM_GPR_PCIE_CMN_RST);
> -
>  	switch (imx8_phy->drvdata->variant) {
>  	case IMX8MP:
>  		reset_control_deassert(imx8_phy->perst);
> @@ -156,6 +151,11 @@ static int imx8_pcie_phy_power_on(struct phy
> *phy)
>  		break;
>  	}
> 
> +	/* Do the PHY common block reset */
> +	regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> +			   IMX8MM_GPR_PCIE_CMN_RST,
> +			   IMX8MM_GPR_PCIE_CMN_RST);
> +
>  	/* Polling to check the phy is ready or not. */
>  	ret = readl_poll_timeout(imx8_phy->base +
> IMX8MM_PCIE_PHY_CMN_REG075,
>  				 val, val == ANA_PLL_DONE, 10, 20000);
> --
> 2.34.1

Vinod Koul Oct. 22, 2024, 5:31 a.m. UTC | #2

On Mon, 21 Oct 2024 11:52:41 -0400, Frank Li wrote:
> When enable initcall_debug together with higher debug level below.
> CONFIG_CONSOLE_LOGLEVEL_DEFAULT=9
> CONFIG_CONSOLE_LOGLEVEL_QUIET=9
> CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7
> 
> The initialization of i.MX8MP PCIe PHY might be timeout failed randomly.
> To fix this issue, adjust the sequence of the resets refer to the power
> up sequence listed below.
> 
> [...]

Applied, thanks!

[1/1] phy: freescale: imx8m-pcie: Do CMN_RST just before PHY PLL lock check
      commit: f89263b69731e0144d275fff777ee0dd92069200

Best regards,

Adam Ford Nov. 1, 2024, 7:52 p.m. UTC | #3

On Mon, Oct 21, 2024 at 11:06 PM Hongxing Zhu <[e-mail]> wrote:
>
> > -----Original Message-----
> > From: Frank Li <[e-mail]>
> > Sent: 2024年10月21日 23:53
> > To: [e-mail]
> > Cc: Frank Li <[e-mail]>; [e-mail]; Hongxing Zhu
> > <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; Marcel Ziswiler
> > <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail]
> > Subject: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just before
> > PHY PLL lock check
> >
> > From: Richard Zhu <[e-mail]>
> >
> > When enable initcall_debug together with higher debug level below.
> > CONFIG_CONSOLE_LOGLEVEL_DEFAULT=9
> > CONFIG_CONSOLE_LOGLEVEL_QUIET=9
> > CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7
> >
> > The initialization of i.MX8MP PCIe PHY might be timeout failed randomly.
> > To fix this issue, adjust the sequence of the resets refer to the power up
> > sequence listed below.
> >
> > i.MX8MP PCIe PHY power up sequence:
> >                           /---------------------------------------------
> > 1.8v supply     ---------/
> >                     /---------------------------------------------------
> > 0.8v supply     ---/
> >
> >                 ---\ /--------------------------------------------------
> >                     X        REFCLK Valid
> > Reference Clock ---/ \--------------------------------------------------
> >                              -------------------------------------------
> >                              |
> > i_init_restn    --------------
> >                                     ------------------------------------
> >                                     |
> > i_cmn_rstn      ---------------------
> >                                          -------------------------------
> >                                          | o_pll_lock_done
> > --------------------------
> >
> > Logs:
> > imx6q-pcie 33800000.pcie: host bridge /soc@0/pcie@33800000 ranges:
> > imx6q-pcie 33800000.pcie:       IO 0x001ff80000..0x001ff8ffff ->
> > 0x0000000000
> > imx6q-pcie 33800000.pcie:      MEM 0x0018000000..0x001fefffff ->
> > 0x0018000000
> > probe of clk_imx8mp_audiomix.reset.0 returned 0 after 1052 usecs probe of
> > 30e20000.clock-controller returned 0 after 32971 usecs phy
> > phy-32f00000.pcie-phy.4: phy poweron failed --> -110 probe of
> > 30e10000.dma-controller returned 0 after 10235 usecs imx6q-pcie
> > 33800000.pcie: waiting for PHY ready timeout!
> > dwhdmi-imx 32fd8000.hdmi: Detected HDMI TX controller v2.13a with HDCP
> > (samsung_dw_hdmi_phy2) imx6q-pcie 33800000.pcie: probe with driver
> > imx6q-pcie failed with error -110
> >
> > Fixes: dce9edff16ee ("phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY
> > support")
> > Cc: [e-mail]
> > Signed-off-by: Richard Zhu <[e-mail]>
> > Signed-off-by: Frank Li <[e-mail]>
> >
> > v2 changes:
> > - Rebase to latest fixes branch of linux-phy git repo.
> > - Richard's environment have problem and can't sent out patch. So I help post
> > this fix patch.

Even with this patch, I am still seeing an occasional timeout on 8MP.
I looked at some logs on a similarly functioning 8MM and I can't get
this error to appear on Mini that I see on Plus.

The TRM doesn't document the timing of the startup sequence, like this
e-mail patch did nor does it state how long a reasonable timeout
should take. So, I started looking through the code and I noticed that
the Mini asserts the reset at the beginning, then makes all the
changes, and de-asserts the resets toward the end.  Is there any
reason we should not assert one or both of the resets on 8MP before
setting up the reset of the registers like the way Mini does it?

adam

> > ---
> Hi Frank:
> Thanks a lot for your kindly help.
> Since my server is down, I can't send out this v2 in the past days.
>
> Hi Vinod:
> Sorry for the late reply, and bring you inconvenience.
>
> Best Regards
> Richard Zhu
>
> >  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > index 11fcb1867118c..e98361dcdeadf 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > @@ -141,11 +141,6 @@ static int imx8_pcie_phy_power_on(struct phy
> > *phy)
> >                          IMX8MM_GPR_PCIE_REF_CLK_PLL);
> >       usleep_range(100, 200);
> >
> > -     /* Do the PHY common block reset */
> > -     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > -                        IMX8MM_GPR_PCIE_CMN_RST,
> > -                        IMX8MM_GPR_PCIE_CMN_RST);
> > -
> >       switch (imx8_phy->drvdata->variant) {
> >       case IMX8MP:
> >               reset_control_deassert(imx8_phy->perst);
> > @@ -156,6 +151,11 @@ static int imx8_pcie_phy_power_on(struct phy
> > *phy)
> >               break;
> >       }
> >
> > +     /* Do the PHY common block reset */
> > +     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > +                        IMX8MM_GPR_PCIE_CMN_RST,
> > +                        IMX8MM_GPR_PCIE_CMN_RST);
> > +
> >       /* Polling to check the phy is ready or not. */
> >       ret = readl_poll_timeout(imx8_phy->base +
> > IMX8MM_PCIE_PHY_CMN_REG075,
> >                                val, val == ANA_PLL_DONE, 10, 20000);
> > --
> > 2.34.1
>
> --
> linux-phy mailing list
> [e-mail]
> https://lists.infradead.org/mailman/listinfo/linux-phy

Richard Zhu Nov. 4, 2024, 5:19 a.m. UTC | #4

> -----Original Message-----
> From: Adam Ford <[e-mail]>
> Sent: 2024年11月2日 3:53
> To: Hongxing Zhu <[e-mail]>
> Cc: Frank Li <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; Marcel Ziswiler <[e-mail]>;
> [e-mail]; [e-mail]; [e-mail]
> Subject: Re: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just before
> PHY PLL lock check
> 
> On Mon, Oct 21, 2024 at 11:06 PM Hongxing Zhu <[e-mail]>
> wrote:
> >
> > > -----Original Message-----
> > > From: Frank Li <[e-mail]>
> > > Sent: 2024年10月21日 23:53
> > > To: [e-mail]
> > > Cc: Frank Li <[e-mail]>; [e-mail]; Hongxing Zhu
> > > <[e-mail]>; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; Marcel
> > > Ziswiler <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]
> > > Subject: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just
> > > before PHY PLL lock check
> > >
> > > From: Richard Zhu <[e-mail]>
> > >
> > > When enable initcall_debug together with higher debug level below.
> > > CONFIG_CONSOLE_LOGLEVEL_DEFAULT=9
> > > CONFIG_CONSOLE_LOGLEVEL_QUIET=9
> > > CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7
> > >
> > > The initialization of i.MX8MP PCIe PHY might be timeout failed randomly.
> > > To fix this issue, adjust the sequence of the resets refer to the
> > > power up sequence listed below.
> > >
> > > i.MX8MP PCIe PHY power up sequence:
> > >                           /---------------------------------------------
> > > 1.8v supply     ---------/
> > >                     /---------------------------------------------------
> > > 0.8v supply     ---/
> > >
> > >                 ---\ /--------------------------------------------------
> > >                     X        REFCLK Valid
> > > Reference Clock ---/ \--------------------------------------------------
> > >                              -------------------------------------------
> > >                              |
> > > i_init_restn    --------------
> > >                                     ------------------------------------
> > >                                     |
> > > i_cmn_rstn      ---------------------
> > >                                          -------------------------------
> > >                                          | o_pll_lock_done
> > > --------------------------
> > >
> > > Logs:
> > > imx6q-pcie 33800000.pcie: host bridge /soc@0/pcie@33800000 ranges:
> > > imx6q-pcie 33800000.pcie:       IO 0x001ff80000..0x001ff8ffff ->
> > > 0x0000000000
> > > imx6q-pcie 33800000.pcie:      MEM 0x0018000000..0x001fefffff ->
> > > 0x0018000000
> > > probe of clk_imx8mp_audiomix.reset.0 returned 0 after 1052 usecs
> > > probe of 30e20000.clock-controller returned 0 after 32971 usecs phy
> > > phy-32f00000.pcie-phy.4: phy poweron failed --> -110 probe of
> > > 30e10000.dma-controller returned 0 after 10235 usecs imx6q-pcie
> > > 33800000.pcie: waiting for PHY ready timeout!
> > > dwhdmi-imx 32fd8000.hdmi: Detected HDMI TX controller v2.13a with
> > > HDCP
> > > (samsung_dw_hdmi_phy2) imx6q-pcie 33800000.pcie: probe with driver
> > > imx6q-pcie failed with error -110
> > >
> > > Fixes: dce9edff16ee ("phy: freescale: imx8m-pcie: Add i.MX8MP PCIe
> > > PHY
> > > support")
> > > Cc: [e-mail]
> > > Signed-off-by: Richard Zhu <[e-mail]>
> > > Signed-off-by: Frank Li <[e-mail]>
> > >
> > > v2 changes:
> > > - Rebase to latest fixes branch of linux-phy git repo.
> > > - Richard's environment have problem and can't sent out patch. So I
> > > help post this fix patch.
> 
> Even with this patch, I am still seeing an occasional timeout on 8MP.
> I looked at some logs on a similarly functioning 8MM and I can't get this error to
> appear on Mini that I see on Plus.
> 
> The TRM doesn't document the timing of the startup sequence, like this e-mail
> patch did nor does it state how long a reasonable timeout should take. So, I
> started looking through the code and I noticed that the Mini asserts the reset at
> the beginning, then makes all the changes, and de-asserts the resets toward the
> end.  Is there any reason we should not assert one or both of the resets on
> 8MP before setting up the reset of the registers like the way Mini does it?
Yes, I had the similar confusions when I try to bring up i.MX8MP PCIe.
i.MX8MP PCIe resets have the different designs but I don't know the reason and
the details. These resets shouldn't be asserted/de-asserted as Mini does during
 the initialization.

On i.MX8MP, these resets should be configured one-shot. I used to toggle them
in my own experiments. Unfortunately, the PCIe PHY wouldn't be functional.

Best Regards
Richard Zhu
> 
> adam
> 
> > > ---
> > Hi Frank:
> > Thanks a lot for your kindly help.
> > Since my server is down, I can't send out this v2 in the past days.
> >
> > Hi Vinod:
> > Sorry for the late reply, and bring you inconvenience.
> >
> > Best Regards
> > Richard Zhu
> >
> > >  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > index 11fcb1867118c..e98361dcdeadf 100644
> > > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > @@ -141,11 +141,6 @@ static int imx8_pcie_phy_power_on(struct phy
> > > *phy)
> > >                          IMX8MM_GPR_PCIE_REF_CLK_PLL);
> > >       usleep_range(100, 200);
> > >
> > > -     /* Do the PHY common block reset */
> > > -     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > > -                        IMX8MM_GPR_PCIE_CMN_RST,
> > > -                        IMX8MM_GPR_PCIE_CMN_RST);
> > > -
> > >       switch (imx8_phy->drvdata->variant) {
> > >       case IMX8MP:
> > >               reset_control_deassert(imx8_phy->perst);
> > > @@ -156,6 +151,11 @@ static int imx8_pcie_phy_power_on(struct phy
> > > *phy)
> > >               break;
> > >       }
> > >
> > > +     /* Do the PHY common block reset */
> > > +     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > > +                        IMX8MM_GPR_PCIE_CMN_RST,
> > > +                        IMX8MM_GPR_PCIE_CMN_RST);
> > > +
> > >       /* Polling to check the phy is ready or not. */
> > >       ret = readl_poll_timeout(imx8_phy->base +
> > > IMX8MM_PCIE_PHY_CMN_REG075,
> > >                                val, val == ANA_PLL_DONE, 10,
> 20000);
> > > --
> > > 2.34.1
> >
> > --
> > linux-phy mailing list
> > [e-mail]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> >
> s.infradead.org%2Fmailman%2Flistinfo%2Flinux-phy&data=05%7C02%7Chongxi
> >
> ng.zhu%40nxp.com%7C666c8968b3094147ed4408dcfaaec631%7C686ea1d3bc
> 2b4c6f
> >
> a92cd99c5c301635%7C0%7C0%7C638660875771545687%7CUnknown%7CTWF
> pbGZsb3d8
> >
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C0
> > %7C%7C%7C&sdata=3Mg4SqcaA%2FlbScveriGBBMOq1YOTt3okydgHmjmdLps
> %3D&reser
> > ved=0

Adam Ford Nov. 4, 2024, 2:14 p.m. UTC | #5

On Sun, Nov 3, 2024 at 11:19 PM Hongxing Zhu <[e-mail]> wrote:
>
> > -----Original Message-----
> > From: Adam Ford <[e-mail]>
> > Sent: 2024年11月2日 3:53
> > To: Hongxing Zhu <[e-mail]>
> > Cc: Frank Li <[e-mail]>; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; Marcel Ziswiler <[e-mail]>;
> > [e-mail]; [e-mail]; [e-mail]
> > Subject: Re: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just before
> > PHY PLL lock check
> >
> > On Mon, Oct 21, 2024 at 11:06 PM Hongxing Zhu <[e-mail]>
> > wrote:
> > >
> > > > -----Original Message-----
> > > > From: Frank Li <[e-mail]>
> > > > Sent: 2024年10月21日 23:53
> > > > To: [e-mail]
> > > > Cc: Frank Li <[e-mail]>; [e-mail]; Hongxing Zhu
> > > > <[e-mail]>; [e-mail]; [e-mail];
> > > > [e-mail]; [e-mail];
> > > > [e-mail]; [e-mail]; Marcel
> > > > Ziswiler <[e-mail]>; [e-mail];
> > > > [e-mail]; [e-mail]
> > > > Subject: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just
> > > > before PHY PLL lock check
> > > >
> > > > From: Richard Zhu <[e-mail]>
> > > >
> > > > When enable initcall_debug together with higher debug level below.
> > > > CONFIG_CONSOLE_LOGLEVEL_DEFAULT=9
> > > > CONFIG_CONSOLE_LOGLEVEL_QUIET=9
> > > > CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7
> > > >
> > > > The initialization of i.MX8MP PCIe PHY might be timeout failed randomly.
> > > > To fix this issue, adjust the sequence of the resets refer to the
> > > > power up sequence listed below.
> > > >
> > > > i.MX8MP PCIe PHY power up sequence:
> > > >                           /---------------------------------------------
> > > > 1.8v supply     ---------/
> > > >                     /---------------------------------------------------
> > > > 0.8v supply     ---/
> > > >
> > > >                 ---\ /--------------------------------------------------
> > > >                     X        REFCLK Valid
> > > > Reference Clock ---/ \--------------------------------------------------
> > > >                              -------------------------------------------
> > > >                              |
> > > > i_init_restn    --------------
> > > >                                     ------------------------------------
> > > >                                     |
> > > > i_cmn_rstn      ---------------------
> > > >                                          -------------------------------
> > > >                                          | o_pll_lock_done
> > > > --------------------------
> > > >
> > > > Logs:
> > > > imx6q-pcie 33800000.pcie: host bridge /soc@0/pcie@33800000 ranges:
> > > > imx6q-pcie 33800000.pcie:       IO 0x001ff80000..0x001ff8ffff ->
> > > > 0x0000000000
> > > > imx6q-pcie 33800000.pcie:      MEM 0x0018000000..0x001fefffff ->
> > > > 0x0018000000
> > > > probe of clk_imx8mp_audiomix.reset.0 returned 0 after 1052 usecs
> > > > probe of 30e20000.clock-controller returned 0 after 32971 usecs phy
> > > > phy-32f00000.pcie-phy.4: phy poweron failed --> -110 probe of
> > > > 30e10000.dma-controller returned 0 after 10235 usecs imx6q-pcie
> > > > 33800000.pcie: waiting for PHY ready timeout!
> > > > dwhdmi-imx 32fd8000.hdmi: Detected HDMI TX controller v2.13a with
> > > > HDCP
> > > > (samsung_dw_hdmi_phy2) imx6q-pcie 33800000.pcie: probe with driver
> > > > imx6q-pcie failed with error -110
> > > >
> > > > Fixes: dce9edff16ee ("phy: freescale: imx8m-pcie: Add i.MX8MP PCIe
> > > > PHY
> > > > support")
> > > > Cc: [e-mail]
> > > > Signed-off-by: Richard Zhu <[e-mail]>
> > > > Signed-off-by: Frank Li <[e-mail]>
> > > >
> > > > v2 changes:
> > > > - Rebase to latest fixes branch of linux-phy git repo.
> > > > - Richard's environment have problem and can't sent out patch. So I
> > > > help post this fix patch.
> >
> > Even with this patch, I am still seeing an occasional timeout on 8MP.
> > I looked at some logs on a similarly functioning 8MM and I can't get this error to
> > appear on Mini that I see on Plus.
> >
> > The TRM doesn't document the timing of the startup sequence, like this e-mail
> > patch did nor does it state how long a reasonable timeout should take. So, I
> > started looking through the code and I noticed that the Mini asserts the reset at
> > the beginning, then makes all the changes, and de-asserts the resets toward the
> > end.  Is there any reason we should not assert one or both of the resets on
> > 8MP before setting up the reset of the registers like the way Mini does it?
> Yes, I had the similar confusions when I try to bring up i.MX8MP PCIe.
> i.MX8MP PCIe resets have the different designs but I don't know the reason and
> the details. These resets shouldn't be asserted/de-asserted as Mini does during
>  the initialization.
>
> On i.MX8MP, these resets should be configured one-shot. I used to toggle them
> in my own experiments. Unfortunately, the PCIe PHY wouldn't be functional.

I started testing adding the resets before I asked, because it appears
to be working ok, but if you're suggesting it's a bad idea, I won't
continue down that path.  Do you have any other suggestions on how to
eliminate the occasional timeout error?  I still see it at times on a
cold-boot even with this latest patch applied.

thanks

adam

>
> Best Regards
> Richard Zhu
> >
> > adam
> >
> > > > ---
> > > Hi Frank:
> > > Thanks a lot for your kindly help.
> > > Since my server is down, I can't send out this v2 in the past days.
> > >
> > > Hi Vinod:
> > > Sorry for the late reply, and bring you inconvenience.
> > >
> > > Best Regards
> > > Richard Zhu
> > >
> > > >  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > index 11fcb1867118c..e98361dcdeadf 100644
> > > > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > @@ -141,11 +141,6 @@ static int imx8_pcie_phy_power_on(struct phy
> > > > *phy)
> > > >                          IMX8MM_GPR_PCIE_REF_CLK_PLL);
> > > >       usleep_range(100, 200);
> > > >
> > > > -     /* Do the PHY common block reset */
> > > > -     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > > > -                        IMX8MM_GPR_PCIE_CMN_RST,
> > > > -                        IMX8MM_GPR_PCIE_CMN_RST);
> > > > -
> > > >       switch (imx8_phy->drvdata->variant) {
> > > >       case IMX8MP:
> > > >               reset_control_deassert(imx8_phy->perst);
> > > > @@ -156,6 +151,11 @@ static int imx8_pcie_phy_power_on(struct phy
> > > > *phy)
> > > >               break;
> > > >       }
> > > >
> > > > +     /* Do the PHY common block reset */
> > > > +     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > > > +                        IMX8MM_GPR_PCIE_CMN_RST,
> > > > +                        IMX8MM_GPR_PCIE_CMN_RST);
> > > > +
> > > >       /* Polling to check the phy is ready or not. */
> > > >       ret = readl_poll_timeout(imx8_phy->base +
> > > > IMX8MM_PCIE_PHY_CMN_REG075,
> > > >                                val, val == ANA_PLL_DONE, 10,
> > 20000);
> > > > --
> > > > 2.34.1
> > >
> > > --
> > > linux-phy mailing list
> > > [e-mail]
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> > >
> > s.infradead.org%2Fmailman%2Flistinfo%2Flinux-phy&data=05%7C02%7Chongxi
> > >
> > ng.zhu%40nxp.com%7C666c8968b3094147ed4408dcfaaec631%7C686ea1d3bc
> > 2b4c6f
> > >
> > a92cd99c5c301635%7C0%7C0%7C638660875771545687%7CUnknown%7CTWF
> > pbGZsb3d8
> > >
> > eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> > 7C0
> > > %7C%7C%7C&sdata=3Mg4SqcaA%2FlbScveriGBBMOq1YOTt3okydgHmjmdLps
> > %3D&reser
> > > ved=0

Richard Zhu Nov. 5, 2024, 2:49 a.m. UTC | #6

> -----Original Message-----
> From: Adam Ford <[e-mail]>
> Sent: 2024年11月4日 22:14
> To: Hongxing Zhu <[e-mail]>
> Cc: Frank Li <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; Marcel Ziswiler
> <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]
> Subject: Re: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just
> before PHY PLL lock check
> 
> On Sun, Nov 3, 2024 at 11:19 PM Hongxing Zhu <[e-mail]>
> wrote:
> >
> > > -----Original Message-----
> > > From: Adam Ford <[e-mail]>
> > > Sent: 2024年11月2日 3:53
> > > To: Hongxing Zhu <[e-mail]>
> > > Cc: Frank Li <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; Marcel
> > > Ziswiler <[e-mail]>; [e-mail];
> > > [e-mail]; [e-mail]
> > > Subject: Re: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST
> > > just before PHY PLL lock check
> > >
> > > On Mon, Oct 21, 2024 at 11:06 PM Hongxing Zhu
> <[e-mail]>
> > > wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Frank Li <[e-mail]>
> > > > > Sent: 2024年10月21日 23:53
> > > > > To: [e-mail]
> > > > > Cc: Frank Li <[e-mail]>; [e-mail]; Hongxing
> > > > > Zhu <[e-mail]>; [e-mail];
> > > > > [e-mail]; [e-mail];
> > > > > [e-mail];
> > > > > [e-mail]; [e-mail];
> > > > > Marcel Ziswiler <[e-mail]>;
> > > > > [e-mail]; [e-mail];
> > > > > [e-mail]
> > > > > Subject: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST
> > > > > just before PHY PLL lock check
> > > > >
> > > > > From: Richard Zhu <[e-mail]>
> > > > >
> > > > > When enable initcall_debug together with higher debug level below.
> > > > > CONFIG_CONSOLE_LOGLEVEL_DEFAULT=9
> > > > > CONFIG_CONSOLE_LOGLEVEL_QUIET=9
> > > > > CONFIG_MESSAGE_LOGLEVEL_DEFAULT=7
> > > > >
> > > > > The initialization of i.MX8MP PCIe PHY might be timeout failed
> randomly.
> > > > > To fix this issue, adjust the sequence of the resets refer to
> > > > > the power up sequence listed below.
> > > > >
> > > > > i.MX8MP PCIe PHY power up sequence:
> > > > >
> /---------------------------------------------
> > > > > 1.8v supply     ---------/
> > > > >                     /---------------------------------------------------
> > > > > 0.8v supply     ---/
> > > > >
> > > > >                 ---\ /--------------------------------------------------
> > > > >                     X        REFCLK Valid
> > > > > Reference Clock ---/ \--------------------------------------------------
> > > > >
> -------------------------------------------
> > > > >                              |
> > > > > i_init_restn    --------------
> > > > >
> ------------------------------------
> > > > >                                     |
> > > > > i_cmn_rstn      ---------------------
> > > > >
> -------------------------------
> > > > >                                          | o_pll_lock_done
> > > > > --------------------------
> > > > >
> > > > > Logs:
> > > > > imx6q-pcie 33800000.pcie: host bridge /soc@0/pcie@33800000
> ranges:
> > > > > imx6q-pcie 33800000.pcie:       IO 0x001ff80000..0x001ff8ffff ->
> > > > > 0x0000000000
> > > > > imx6q-pcie 33800000.pcie:      MEM 0x0018000000..0x001fefffff
> ->
> > > > > 0x0018000000
> > > > > probe of clk_imx8mp_audiomix.reset.0 returned 0 after 1052 usecs
> > > > > probe of 30e20000.clock-controller returned 0 after 32971 usecs
> > > > > phy
> > > > > phy-32f00000.pcie-phy.4: phy poweron failed --> -110 probe of
> > > > > 30e10000.dma-controller returned 0 after 10235 usecs imx6q-pcie
> > > > > 33800000.pcie: waiting for PHY ready timeout!
> > > > > dwhdmi-imx 32fd8000.hdmi: Detected HDMI TX controller v2.13a
> > > > > with HDCP
> > > > > (samsung_dw_hdmi_phy2) imx6q-pcie 33800000.pcie: probe with
> > > > > driver imx6q-pcie failed with error -110
> > > > >
> > > > > Fixes: dce9edff16ee ("phy: freescale: imx8m-pcie: Add i.MX8MP
> > > > > PCIe PHY
> > > > > support")
> > > > > Cc: [e-mail]
> > > > > Signed-off-by: Richard Zhu <[e-mail]>
> > > > > Signed-off-by: Frank Li <[e-mail]>
> > > > >
> > > > > v2 changes:
> > > > > - Rebase to latest fixes branch of linux-phy git repo.
> > > > > - Richard's environment have problem and can't sent out patch.
> > > > > So I help post this fix patch.
> > >
> > > Even with this patch, I am still seeing an occasional timeout on 8MP.
> > > I looked at some logs on a similarly functioning 8MM and I can't get
> > > this error to appear on Mini that I see on Plus.
> > >
> > > The TRM doesn't document the timing of the startup sequence, like
> > > this e-mail patch did nor does it state how long a reasonable
> > > timeout should take. So, I started looking through the code and I
> > > noticed that the Mini asserts the reset at the beginning, then makes
> > > all the changes, and de-asserts the resets toward the end.  Is there
> > > any reason we should not assert one or both of the resets on 8MP before
> setting up the reset of the registers like the way Mini does it?
> > Yes, I had the similar confusions when I try to bring up i.MX8MP PCIe.
> > i.MX8MP PCIe resets have the different designs but I don't know the
> > reason and the details. These resets shouldn't be asserted/de-asserted
> > as Mini does during  the initialization.
> >
> > On i.MX8MP, these resets should be configured one-shot. I used to
> > toggle them in my own experiments. Unfortunately, the PCIe PHY wouldn't
> be functional.
> 
> I started testing adding the resets before I asked, because it appears to be
> working ok, but if you're suggesting it's a bad idea, I won't continue down that
> path.  Do you have any other suggestions on how to eliminate the occasional
> timeout error?  I still see it at times on a cold-boot even with this latest
> patch applied.
I didn't re-produce the "phy poweron failed --> -110" after this commit applied
 in my local stress reboot tests. BTW, do you populate one device on the PCIe
 port? If yes, please remove it in the tests. Since it's only the PCIe PHY
 initialization tests. I forget to mention this in commit message, sorry
 about that.

Best Regards
Richard Zhu
> 
> thanks
> 
> adam
> 
> >
> > Best Regards
> > Richard Zhu
> > >
> > > adam
> > >
> > > > > ---
> > > > Hi Frank:
> > > > Thanks a lot for your kindly help.
> > > > Since my server is down, I can't send out this v2 in the past days.
> > > >
> > > > Hi Vinod:
> > > > Sorry for the late reply, and bring you inconvenience.
> > > >
> > > > Best Regards
> > > > Richard Zhu
> > > >
> > > > >  drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 10 +++++-----
> > > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > index 11fcb1867118c..e98361dcdeadf 100644
> > > > > --- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > +++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
> > > > > @@ -141,11 +141,6 @@ static int imx8_pcie_phy_power_on(struct
> > > > > phy
> > > > > *phy)
> > > > >                          IMX8MM_GPR_PCIE_REF_CLK_PLL);
> > > > >       usleep_range(100, 200);
> > > > >
> > > > > -     /* Do the PHY common block reset */
> > > > > -     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > > > > -                        IMX8MM_GPR_PCIE_CMN_RST,
> > > > > -                        IMX8MM_GPR_PCIE_CMN_RST);
> > > > > -
> > > > >       switch (imx8_phy->drvdata->variant) {
> > > > >       case IMX8MP:
> > > > >               reset_control_deassert(imx8_phy->perst);
> > > > > @@ -156,6 +151,11 @@ static int imx8_pcie_phy_power_on(struct
> > > > > phy
> > > > > *phy)
> > > > >               break;
> > > > >       }
> > > > >
> > > > > +     /* Do the PHY common block reset */
> > > > > +     regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
> > > > > +                        IMX8MM_GPR_PCIE_CMN_RST,
> > > > > +                        IMX8MM_GPR_PCIE_CMN_RST);
> > > > > +
> > > > >       /* Polling to check the phy is ready or not. */
> > > > >       ret = readl_poll_timeout(imx8_phy->base +
> > > > > IMX8MM_PCIE_PHY_CMN_REG075,
> > > > >                                val, val == ANA_PLL_DONE, 10,
> > > 20000);
> > > > > --
> > > > > 2.34.1
> > > >
> > > > --
> > > > linux-phy mailing list
> > > > [e-mail]
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > >
> list%2F&data=05%7C02%7Chongxing.zhu%40nxp.com%7Ce0c6fd581d424106
> 28
> > > >
> b608dcfcdb06d9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> 38663
> > > >
> 264864097845%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> QIjoiV
> > > >
> 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=iN2Fo
> ya54
> > > > ihEQ7ziMhmJqepGevofWA8%2FHviB%2BxXPCJY%3D&reserved=0
> > > >
> > >
> s.infradead.org%2Fmailman%2Flistinfo%2Flinux-phy&data=05%7C02%7Chon
> g
> > > xi
> > > >
> > >
> ng.zhu%40nxp.com%7C666c8968b3094147ed4408dcfaaec631%7C686ea1d3
> bc
> > > 2b4c6f
> > > >
> > >
> a92cd99c5c301635%7C0%7C0%7C638660875771545687%7CUnknown%7CT
> WF
> > > pbGZsb3d8
> > > >
> > >
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%
> > > 7C0
> > > > %7C%7C%7C&sdata=3Mg4SqcaA%2FlbScveriGBBMOq1YOTt3okydgHmj
> mdLps
> > > %3D&reser
> > > > ved=0

Patch

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index 11fcb1867118c..e98361dcdeadf 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -141,11 +141,6 @@  static int imx8_pcie_phy_power_on(struct phy *phy)
 			   IMX8MM_GPR_PCIE_REF_CLK_PLL);
 	usleep_range(100, 200);
 
-	/* Do the PHY common block reset */
-	regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
-			   IMX8MM_GPR_PCIE_CMN_RST,
-			   IMX8MM_GPR_PCIE_CMN_RST);
-
 	switch (imx8_phy->drvdata->variant) {
 	case IMX8MP:
 		reset_control_deassert(imx8_phy->perst);
@@ -156,6 +151,11 @@  static int imx8_pcie_phy_power_on(struct phy *phy)
 		break;
 	}
 
+	/* Do the PHY common block reset */
+	regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+			   IMX8MM_GPR_PCIE_CMN_RST,
+			   IMX8MM_GPR_PCIE_CMN_RST);
+
 	/* Polling to check the phy is ready or not. */
 	ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG075,
 				 val, val == ANA_PLL_DONE, 10, 20000);