[RFC 1/2] PCI: imx6: Make sure the DBI register can be changed

[RFC 1/2] PCI: imx6: Make sure the DBI register can be changed

[Richard Zhu]

The PCIE_DBI_RO_WR_EN bit should be set when write some DBI registers.
To make sure that the DBI registers are writable, set the
PCIE_DBI_RO_WR_EN properly when touch the DBI registers.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 6619e3caffe2..30641d2dda14 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -797,10 +797,12 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
 	 * started in Gen2 mode, there is a possibility the devices on the
 	 * bus will not be detected at all.  This happens with PCIe switches.
 	 */
+	dw_pcie_dbi_ro_wr_en(pci);
 	tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 	tmp &= ~PCI_EXP_LNKCAP_SLS;
 	tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
 	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
+	dw_pcie_dbi_ro_wr_dis(pci);
 
 	/* Start LTSSM. */
 	imx6_pcie_ltssm_enable(dev);
@@ -809,6 +811,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
 
 	if (pci->link_gen == 2) {
 		/* Allow Gen2 mode after the link is up. */
+		dw_pcie_dbi_ro_wr_en(pci);
 		tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
 		tmp &= ~PCI_EXP_LNKCAP_SLS;
 		tmp |= PCI_EXP_LNKCAP_SLS_5_0GB;
@@ -821,6 +824,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
 		tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
 		tmp |= PORT_LOGIC_SPEED_CHANGE;
 		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
+		dw_pcie_dbi_ro_wr_dis(pci);
 
 		if (imx6_pcie->drvdata->flags &
 		    IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE) {
-- 
2.25.1

[RFC 2/2] PCI: imx6: Support more than Gen2 speed link mode

[Richard Zhu]

Support more than Gen2 speed link mode, since i.MX8MP PCIe supports up
to Gen3 link speed.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 30641d2dda14..37012f9a33a0 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -809,8 +809,8 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
 
 	dw_pcie_wait_for_link(pci);
 
-	if (pci->link_gen == 2) {
-		/* Allow Gen2 mode after the link is up. */
+	if (pci->link_gen > 1) {
+		/* 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);
 		tmp &= ~PCI_EXP_LNKCAP_SLS;
-- 
2.25.1

Re: [RFC 1/2] PCI: imx6: Make sure the DBI register can be changed

[Bjorn Helgaas]

Hi Richard,

On Wed, May 18, 2022 at 05:35:27PM +0800, Richard Zhu wrote:
> The PCIE_DBI_RO_WR_EN bit should be set when write some DBI registers.
> To make sure that the DBI registers are writable, set the
> PCIE_DBI_RO_WR_EN properly when touch the DBI registers.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 6619e3caffe2..30641d2dda14 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -797,10 +797,12 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
>  	 * started in Gen2 mode, there is a possibility the devices on the
>  	 * bus will not be detected at all.  This happens with PCIe switches.
>  	 */
> +	dw_pcie_dbi_ro_wr_en(pci);

What's the status of this patch?  I don't see this change included in
your v14 series [1].  That series has a lot of imx6 updates, so I
would have thought you'd include this change in it.  Or maybe this
change turned out not to be needed?

What about the 2/2 patch?

They're both marked "RFC" which is sometimes a hint that the author
isn't really committed to the patch, so sometimes they don't get the
attention they deserve.

Bjorn

[1] https://lore.kernel.org/r/1656645935-1370-1-git-send-email-hongxing.zhu@nxp.com

>  	tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
>  	tmp &= ~PCI_EXP_LNKCAP_SLS;
>  	tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
>  	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
> +	dw_pcie_dbi_ro_wr_dis(pci);
>  
>  	/* Start LTSSM. */
>  	imx6_pcie_ltssm_enable(dev);
> @@ -809,6 +811,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
>  
>  	if (pci->link_gen == 2) {
>  		/* Allow Gen2 mode after the link is up. */
> +		dw_pcie_dbi_ro_wr_en(pci);
>  		tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
>  		tmp &= ~PCI_EXP_LNKCAP_SLS;
>  		tmp |= PCI_EXP_LNKCAP_SLS_5_0GB;
> @@ -821,6 +824,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
>  		tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>  		tmp |= PORT_LOGIC_SPEED_CHANGE;
>  		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
> +		dw_pcie_dbi_ro_wr_dis(pci);
>  
>  		if (imx6_pcie->drvdata->flags &
>  		    IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE) {
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [RFC 1/2] PCI: imx6: Make sure the DBI register can be changed

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年7月12日 6:17
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com; festevam@gmail.com; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [RFC 1/2] PCI: imx6: Make sure the DBI register can be changed
> 
> Hi Richard,
> 
> On Wed, May 18, 2022 at 05:35:27PM +0800, Richard Zhu wrote:
> > The PCIE_DBI_RO_WR_EN bit should be set when write some DBI registers.
> > To make sure that the DBI registers are writable, set the
> > PCIE_DBI_RO_WR_EN properly when touch the DBI registers.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 6619e3caffe2..30641d2dda14 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -797,10 +797,12 @@ static int imx6_pcie_start_link(struct dw_pcie
> *pci)
> >  	 * started in Gen2 mode, there is a possibility the devices on the
> >  	 * bus will not be detected at all.  This happens with PCIe switches.
> >  	 */
> > +	dw_pcie_dbi_ro_wr_en(pci);
> 
> What's the status of this patch?  I don't see this change included in your v14
> series [1].  That series has a lot of imx6 updates, so I would have thought
> you'd include this change in it.  Or maybe this change turned out not to be
> needed?
Hi Bjorn:
Thanks for your kindly help.
The v14 series[1] had been reviewing for a quite time. I'm afraid that
 this series might miss the L5.20 merge window if I add new patch
 into it from time to time.
If you don't think so, I can merge the first one, and re-issue the v15 a
moment later.
> 
> What about the 2/2 patch?
i.MX8MP PCIe supports the PCIe GEN3 speed, the second patch is used to extend
 the link speed support capability, and prepared for i.MX8MP PCIe support.
I assumed that these two patches can be contained in i.MX8MP PCIe support
patch-set before.

> 
> They're both marked "RFC" which is sometimes a hint that the author isn't
> really committed to the patch, so sometimes they don't get the attention they
> deserve.
> 
I see. Would remove the "RFC" when issue the patch next time.
Thanks.

Best Regards
Richard Zhu

> Bjorn
> 
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Fr%2F1656645935-1370-1-git-send-email-hongxing.zhu%40nxp.com
> &amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C62e9d070313048589
> 90308da638b18b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 637931746310188774%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> 7C&amp;sdata=Oz2Zgdch0gI2Qd66AiM5iWmp23uwn1SkH07gDAQhGQY%3D
> &amp;reserved=0
> 
> >  	tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> >  	tmp &= ~PCI_EXP_LNKCAP_SLS;
> >  	tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
> >  	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
> > +	dw_pcie_dbi_ro_wr_dis(pci);
> >
> >  	/* Start LTSSM. */
> >  	imx6_pcie_ltssm_enable(dev);
> > @@ -809,6 +811,7 @@ static int imx6_pcie_start_link(struct dw_pcie
> > *pci)
> >
> >  	if (pci->link_gen == 2) {
> >  		/* Allow Gen2 mode after the link is up. */
> > +		dw_pcie_dbi_ro_wr_en(pci);
> >  		tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> >  		tmp &= ~PCI_EXP_LNKCAP_SLS;
> >  		tmp |= PCI_EXP_LNKCAP_SLS_5_0GB;
> > @@ -821,6 +824,7 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
> >  		tmp = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> >  		tmp |= PORT_LOGIC_SPEED_CHANGE;
> >  		dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, tmp);
> > +		dw_pcie_dbi_ro_wr_dis(pci);
> >
> >  		if (imx6_pcie->drvdata->flags &
> >  		    IMX6_PCIE_FLAG_IMX6_SPEED_CHANGE) {
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;data=05%7
> C0
> >
> 1%7Chongxing.zhu%40nxp.com%7C62e9d07031304858990308da638b18b4%
> 7C686ea1
> >
> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637931746310188774%7CUnk
> nown%7CTW
> >
> FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> CI6
> >
> Mn0%3D%7C3000%7C%7C%7C&amp;sdata=mG6ix8VqqWocpp62GIWxgQOpE
> xSCMDJivto%2
> > BmqgOZHs%3D&amp;reserved=0

Re: [RFC 1/2] PCI: imx6: Make sure the DBI register can be changed

[Bjorn Helgaas]

On Tue, Jul 12, 2022 at 01:30:02AM +0000, Hongxing Zhu wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@kernel.org>
> > Sent: 2022年7月12日 6:17
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com; festevam@gmail.com; linux-pci@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [RFC 1/2] PCI: imx6: Make sure the DBI register can be changed
> > 
> > Hi Richard,
> > 
> > On Wed, May 18, 2022 at 05:35:27PM +0800, Richard Zhu wrote:
> > > The PCIE_DBI_RO_WR_EN bit should be set when write some DBI registers.
> > > To make sure that the DBI registers are writable, set the
> > > PCIE_DBI_RO_WR_EN properly when touch the DBI registers.
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 6619e3caffe2..30641d2dda14 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -797,10 +797,12 @@ static int imx6_pcie_start_link(struct dw_pcie
> > *pci)
> > >  	 * started in Gen2 mode, there is a possibility the devices on the
> > >  	 * bus will not be detected at all.  This happens with PCIe switches.
> > >  	 */
> > > +	dw_pcie_dbi_ro_wr_en(pci);
> > 
> > What's the status of this patch?  I don't see this change included in your v14
> > series [1].  That series has a lot of imx6 updates, so I would have thought
> > you'd include this change in it.  Or maybe this change turned out not to be
> > needed?
> Hi Bjorn:
> Thanks for your kindly help.
> The v14 series[1] had been reviewing for a quite time. I'm afraid that
>  this series might miss the L5.20 merge window if I add new patch
>  into it from time to time.
> If you don't think so, I can merge the first one, and re-issue the v15 a
> moment later.
> > 
> > What about the 2/2 patch?
> i.MX8MP PCIe supports the PCIe GEN3 speed, the second patch is used to extend
>  the link speed support capability, and prepared for i.MX8MP PCIe support.
> I assumed that these two patches can be contained in i.MX8MP PCIe support
> patch-set before.

I applied both these patches on pci/ctrl/imx6 for v5.20, thanks!

Bjorn

Re: [RFC 2/2] PCI: imx6: Support more than Gen2 speed link mode

[Tim Harvey]

On Wed, May 18, 2022 at 2:49 AM Richard Zhu <hongxing.zhu@nxp.com> wrote:
>
> Support more than Gen2 speed link mode, since i.MX8MP PCIe supports up
> to Gen3 link speed.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 30641d2dda14..37012f9a33a0 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -809,8 +809,8 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
>
>         dw_pcie_wait_for_link(pci);
>
> -       if (pci->link_gen == 2) {
> -               /* Allow Gen2 mode after the link is up. */
> +       if (pci->link_gen > 1) {
> +               /* 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);
>                 tmp &= ~PCI_EXP_LNKCAP_SLS;
> --

Richard,

I noticed that your imx8mp pcie series [1] will force the imx8mp to
link only at gen1 speeds unless support like the above is added. I
believe you would also need the following:
-               tmp |= PCI_EXP_LNKCAP_SLS_5_0GB;
+               tmp |= pci->link_gen;

When I used this along with your imx8mp series however I only get a gen1 link.

Have you made any progress on a v3 of your imx8mp series?

Do you know if the downstream NXP vendor kernel [2] supports imx8mp Gen3 links?

Best Regards,

Tim
[1] https://patchwork.kernel.org/project/linux-pci/list/?series=620887&state=*
[2] https://source.codeaurora.org/external/imx/linux-imx/

RE: [RFC 2/2] PCI: imx6: Support more than Gen2 speed link mode

[Hongxing Zhu]

> -----Original Message-----
> From: Tim Harvey <tharvey@gateworks.com>
> Sent: 2022年7月20日 8:59
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>; bhelgaas@google.com; Lorenzo
> Pieralisi <lorenzo.pieralisi@arm.com>; Fabio Estevam <festevam@gmail.com>;
> linux-pci@vger.kernel.org; Linux ARM Mailing List
> <linux-arm-kernel@lists.infradead.org>; open list
> <linux-kernel@vger.kernel.org>; Sascha Hauer <kernel@pengutronix.de>;
> dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [RFC 2/2] PCI: imx6: Support more than Gen2 speed link mode
> 
> On Wed, May 18, 2022 at 2:49 AM Richard Zhu <hongxing.zhu@nxp.com>
> wrote:
> >
> > Support more than Gen2 speed link mode, since i.MX8MP PCIe supports up
> > to Gen3 link speed.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 30641d2dda14..37012f9a33a0 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -809,8 +809,8 @@ static int imx6_pcie_start_link(struct dw_pcie
> > *pci)
> >
> >         dw_pcie_wait_for_link(pci);
> >
> > -       if (pci->link_gen == 2) {
> > -               /* Allow Gen2 mode after the link is up. */
> > +       if (pci->link_gen > 1) {
> > +               /* 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);
> >                 tmp &= ~PCI_EXP_LNKCAP_SLS;
> > --
> 
> Richard,
> 
> I noticed that your imx8mp pcie series [1] will force the imx8mp to link only at
> gen1 speeds unless support like the above is added. I believe you would also
> need the following:
> -               tmp |= PCI_EXP_LNKCAP_SLS_5_0GB;
> +               tmp |= pci->link_gen;
> 
> When I used this along with your imx8mp series however I only get a gen1 link.
> 
> Have you made any progress on a v3 of your imx8mp series?
> 
> Do you know if the downstream NXP vendor kernel [2] supports imx8mp Gen3
> links?
Hi Tim:
Thanks for your timely reminder. I just was puzzled a while why only Gen1 is
 linked up on i.MX8MP.
Root cause is that I forget merge some local changes to the up-stream codes.
BTW, the local codes supports the Gen3 link on i.MX8MP A1 chip or later.
Since the A0 chip PCIe only supports up to Gen2 link speed.

Best Regards
Richard Zhu

> 
> Best Regards,
> 
> Tim
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.kernel.org%2Fproject%2Flinux-pci%2Flist%2F%3Fseries%3D620887%26sta
> te%3D*&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C9c7b11f2599
> 645f62dd608da69eb1455%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C637938755629962307%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7
> C%7C%7C&amp;sdata=I3PwnRO56jV7yKW0Al16C%2F%2FY5GupiyIifdK%2FE2
> 2AiSs%3D&amp;reserved=0
> [2]
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.
> codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2F&amp;data=05%7C01%7
> Chongxing.zhu%40nxp.com%7C9c7b11f2599645f62dd608da69eb1455%7C68
> 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637938755629962307%7
> CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=PUul%2F8b
> 6%2FtRjJMHOOHcm7Jb1BXwGi%2FVnLnWmKCeNeo4%3D&amp;reserved=0

RE: [RFC 2/2] PCI: imx6: Support more than Gen2 speed link mode

[Hongxing Zhu]

> -----Original Message-----
> From: Hongxing Zhu <hongxing.zhu@nxp.com>
> Sent: 2022年7月20日 9:44
> To: tharvey@gateworks.com
> Cc: Lucas Stach <l.stach@pengutronix.de>; bhelgaas@google.com; Lorenzo
> Pieralisi <lorenzo.pieralisi@arm.com>; Fabio Estevam <festevam@gmail.com>;
> linux-pci@vger.kernel.org; Linux ARM Mailing List
> <linux-arm-kernel@lists.infradead.org>; open list
> <linux-kernel@vger.kernel.org>; Sascha Hauer <kernel@pengutronix.de>;
> dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [RFC 2/2] PCI: imx6: Support more than Gen2 speed link mode
> 
> > -----Original Message-----
> > From: Tim Harvey <tharvey@gateworks.com>
> > Sent: 2022年7月20日 8:59
> > To: Hongxing Zhu <hongxing.zhu@nxp.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>; bhelgaas@google.com; Lorenzo
> > Pieralisi <lorenzo.pieralisi@arm.com>; Fabio Estevam
> > <festevam@gmail.com>; linux-pci@vger.kernel.org; Linux ARM Mailing
> > List <linux-arm-kernel@lists.infradead.org>; open list
> > <linux-kernel@vger.kernel.org>; Sascha Hauer <kernel@pengutronix.de>;
> > dl-linux-imx <linux-imx@nxp.com>
> > Subject: Re: [RFC 2/2] PCI: imx6: Support more than Gen2 speed link
> > mode
> >
> > On Wed, May 18, 2022 at 2:49 AM Richard Zhu <hongxing.zhu@nxp.com>
> > wrote:
> > >
> > > Support more than Gen2 speed link mode, since i.MX8MP PCIe supports
> > > up to Gen3 link speed.
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 30641d2dda14..37012f9a33a0 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -809,8 +809,8 @@ static int imx6_pcie_start_link(struct dw_pcie
> > > *pci)
> > >
> > >         dw_pcie_wait_for_link(pci);
> > >
> > > -       if (pci->link_gen == 2) {
> > > -               /* Allow Gen2 mode after the link is up. */
> > > +       if (pci->link_gen > 1) {
> > > +               /* 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);
> > >                 tmp &= ~PCI_EXP_LNKCAP_SLS;
> > > --
> >
> > Richard,
> >
> > I noticed that your imx8mp pcie series [1] will force the imx8mp to
> > link only at
> > gen1 speeds unless support like the above is added. I believe you
> > would also need the following:
> > -               tmp |= PCI_EXP_LNKCAP_SLS_5_0GB;
> > +               tmp |= pci->link_gen;
> >
> > When I used this along with your imx8mp series however I only get a gen1
> link.
> >
> > Have you made any progress on a v3 of your imx8mp series?
Missing one in previous reply.
I still didn't know how to encapsulate the PLL bits related operations of
 HSIOMIX to one clock suggested by Lucas [1].
[1] https://patchwork.ozlabs.org/project/linux-pci/patch/1646644054-24421-4-git-send-email-hongxing.zhu@nxp.com/
I'm prepare the i.MX EP RC patch-set reviewing, and plan to look at i.MX8MP
 series after the EP RC patch-set is settle down.

Best Regards
Richard Zhu
> >
> > Do you know if the downstream NXP vendor kernel [2] supports imx8mp
> > Gen3 links?
> Hi Tim:
> Thanks for your timely reminder. I just was puzzled a while why only Gen1 is
> linked up on i.MX8MP.
> Root cause is that I forget merge some local changes to the up-stream codes.
> BTW, the local codes supports the Gen3 link on i.MX8MP A1 chip or later.
> Since the A0 chip PCIe only supports up to Gen2 link speed.
> 
> Best Regards
> Richard Zhu
> 
> >
> > Best Regards,
> >
> > Tim
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> > hw
> >
> ork.kernel.org%2Fproject%2Flinux-pci%2Flist%2F%3Fseries%3D620887%26sta
> >
> te%3D*&amp;data=05%7C01%7Chongxing.zhu%40nxp.com%7C9c7b11f2599
> >
> 645f62dd608da69eb1455%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> >
> C0%7C637938755629962307%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> >
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7
> >
> C%7C%7C&amp;sdata=I3PwnRO56jV7yKW0Al16C%2F%2FY5GupiyIifdK%2FE2
> > 2AiSs%3D&amp;reserved=0
> > [2]
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.
> >
> codeaurora.org%2Fexternal%2Fimx%2Flinux-imx%2F&amp;data=05%7C01%7
> >
> Chongxing.zhu%40nxp.com%7C9c7b11f2599645f62dd608da69eb1455%7C68
> >
> 6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637938755629962307%7
> >
> CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT
> >
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=PUul%2F8b
> > 6%2FtRjJMHOOHcm7Jb1BXwGi%2FVnLnWmKCeNeo4%3D&amp;reserved=0