linux-kernel - RE: [PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not required

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:
 <AS8PR04MB8676752D7F3A11679FFA485B8CAF2@AS8PR04MB8676.eurprd04.prod.outlook.com>
Date: Wed, 2 Apr 2025 07:39:27 +0000
From: Hongxing Zhu <hongxing.zhu@....com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
CC: Frank Li <frank.li@....com>, "l.stach@...gutronix.de"
	<l.stach@...gutronix.de>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
	"kw@...ux.com" <kw@...ux.com>, "robh@...nel.org" <robh@...nel.org>,
	"bhelgaas@...gle.com" <bhelgaas@...gle.com>, "shawnguo@...nel.org"
	<shawnguo@...nel.org>, "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>, "festevam@...il.com"
	<festevam@...il.com>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "imx@...ts.linux.dev"
	<imx@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 1/6] PCI: imx6: Start link directly when workaround is
 not required

> -----Original Message-----
> From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> Sent: 2025年4月2日 14:27
> To: Hongxing Zhu <hongxing.zhu@....com>
> Cc: Frank Li <frank.li@....com>; l.stach@...gutronix.de; lpieralisi@...nel.org;
> kw@...ux.com; robh@...nel.org; bhelgaas@...gle.com;
> shawnguo@...nel.org; s.hauer@...gutronix.de; kernel@...gutronix.de;
> festevam@...il.com; linux-pci@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; imx@...ts.linux.dev;
> linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v3 1/6] PCI: imx6: Start link directly when workaround is not
> required
> 
> On Fri, Mar 28, 2025 at 11:02:08AM +0800, Richard Zhu wrote:
> > The current link setup procedure is one workaround to detect the
> > device behind PCIe switches on some i.MX6 platforms.
> >
> > To describe more accurately, change the flag name from
> > IMX_PCIE_FLAG_IMX_SPEED_CHANGE to
> IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND.
> >
> > Start PCIe link directly when this flag is not set on i.MX7 or later
> > platforms to simple and speed up link training.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> 
> One observation below (not related to this change).
> 
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 34
> > +++++++++++----------------
> >  1 file changed, 14 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index c1f7904e3600..57aa777231ae 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -91,7 +91,7 @@ enum imx_pcie_variants {  };
> >
> >  #define IMX_PCIE_FLAG_IMX_PHY			BIT(0)
> > -#define IMX_PCIE_FLAG_IMX_SPEED_CHANGE		BIT(1)
> > +#define IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND	BIT(1)
> >  #define IMX_PCIE_FLAG_SUPPORTS_SUSPEND		BIT(2)
> >  #define IMX_PCIE_FLAG_HAS_PHYDRV		BIT(3)
> >  #define IMX_PCIE_FLAG_HAS_APP_RESET		BIT(4)
> > @@ -860,6 +860,12 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
> >  	u32 tmp;
> >  	int ret;
> >
> > +	if (!(imx_pcie->drvdata->flags &
> > +	    IMX_PCIE_FLAG_SPEED_CHANGE_WORKAROUND)) {
> > +		imx_pcie_ltssm_enable(dev);
> > +		return 0;
> 
> While looking into the code, I realized that we could skip waiting for link during
> the workaround in atleast one instance:
> 
> ```
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 5f267dd261b5..9dbfbd9de2da 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -875,11 +875,11 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>         /* Start LTSSM. */
>         imx_pcie_ltssm_enable(dev);
> 
> -       ret = dw_pcie_wait_for_link(pci);
> -       if (ret)
> -               goto err_reset_phy;
> -
>         if (pci->max_link_speed > 1) {
> +               ret = dw_pcie_wait_for_link(pci);
> +               if (ret)
> +                       goto err_reset_phy;
> +
>                 /* Allow faster modes after the link is up */
>                 dw_pcie_dbi_ro_wr_en(pci);
>                 tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> @@ -913,17 +913,10 @@ static int imx_pcie_start_link(struct dw_pcie *pci)
>                                 goto err_reset_phy;
>                         }
>                 }
> -
> -               /* Make sure link training is finished as well! */
> -               ret = dw_pcie_wait_for_link(pci);
> -               if (ret)
> -                       goto err_reset_phy;
>         } else {
>                 dev_info(dev, "Link: Only Gen1 is enabled\n");
>         }
> 
> -       tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
> -       dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS);
>         return 0;
> 
>  err_reset_phy:
> ```
> 
> dw_pcie_wait_for_link() in DWC core should serve the purpose.
Good suggestion. Thanks.
I can add another patch to optimize the workaround procedure without
 function changes.

Best Regards
Richard Zhu
> 
> - Mani
> 
> --
> மணிவண்ணன் சதாசிவம்