lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHCN7xKj9XYnsaTjFfE_jn7rsN86wv0bxKq3o83WNkamrqeU1g@mail.gmail.com>
Date: Mon, 4 Nov 2024 08:14:28 -0600
From: Adam Ford <aford173@...il.com>
To: Hongxing Zhu <hongxing.zhu@....com>
Cc: Frank Li <frank.li@....com>, "vkoul@...nel.org" <vkoul@...nel.org>, 
	"festevam@...il.com" <festevam@...il.com>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>, 
	"kernel@...gutronix.de" <kernel@...gutronix.de>, "kishon@...nel.org" <kishon@...nel.org>, 
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>, 
	Marcel Ziswiler <marcel.ziswiler@...adex.com>, 
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>, "shawnguo@...nel.org" <shawnguo@...nel.org>, 
	"stable@...r.kernel.org" <stable@...r.kernel.org>
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 <hongxing.zhu@....com> wrote:
>
> > -----Original Message-----
> > From: Adam Ford <aford173@...il.com>
> > Sent: 2024年11月2日 3:53
> > To: Hongxing Zhu <hongxing.zhu@....com>
> > Cc: Frank Li <frank.li@....com>; vkoul@...nel.org; festevam@...il.com;
> > imx@...ts.linux.dev; kernel@...gutronix.de; kishon@...nel.org;
> > linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org;
> > linux-phy@...ts.infradead.org; Marcel Ziswiler <marcel.ziswiler@...adex.com>;
> > s.hauer@...gutronix.de; shawnguo@...nel.org; stable@...r.kernel.org
> > 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 <hongxing.zhu@....com>
> > wrote:
> > >
> > > > -----Original Message-----
> > > > From: Frank Li <frank.li@....com>
> > > > Sent: 2024年10月21日 23:53
> > > > To: vkoul@...nel.org
> > > > Cc: Frank Li <frank.li@....com>; festevam@...il.com; Hongxing Zhu
> > > > <hongxing.zhu@....com>; imx@...ts.linux.dev; kernel@...gutronix.de;
> > > > kishon@...nel.org; linux-arm-kernel@...ts.infradead.org;
> > > > linux-kernel@...r.kernel.org; linux-phy@...ts.infradead.org; Marcel
> > > > Ziswiler <marcel.ziswiler@...adex.com>; s.hauer@...gutronix.de;
> > > > shawnguo@...nel.org; stable@...r.kernel.org
> > > > Subject: [PATCH v2 1/1] phy: freescale: imx8m-pcie: Do CMN_RST just
> > > > before PHY PLL lock check
> > > >
> > > > From: Richard Zhu <hongxing.zhu@....com>
> > > >
> > > > 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@...cie@...00000 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: stable@...r.kernel.org
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> > > > Signed-off-by: Frank Li <Frank.Li@....com>
> > > >
> > > > 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
> > > linux-phy@...ts.infradead.org
> > > 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ