diff mbox series

[v8,01/11] PCI: imx6: Fix establish link failure in EP mode for iMX8MM and iMX8MP

Message ID 20240729-pci2_upstream-v8-1-b68ee5ef2b4d@nxp.com (mailing list archive)
State Accepted
Delegated to: Krzysztof Wilczyński
Headers show
Series PCI: imx6: Fix\rename\clean up and add lut information for imx95 | expand

Commit Message

Frank Li July 29, 2024, 8:18 p.m. UTC 
From: Richard Zhu <hongxing.zhu@nxp.com>

Add IMX6_PCIE_FLAG_HAS_APP_RESET flag to IMX8MM_EP and IMX8MP_EP drvdata.
This flag was overlooked during code restructuring. It is crucial to
release the app-reset from the System Reset Controller before initiating
LTSSM to rectify the issue

Fixes: 0c9651c21f2a ("PCI: imx6: Simplify reset handling by using *_FLAG_HAS_*_RESET")
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Bjorn Helgaas Sept. 2, 2024, 8:59 p.m. UTC | #1 
On Mon, Jul 29, 2024 at 04:18:08PM -0400, Frank Li wrote:
> From: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Add IMX6_PCIE_FLAG_HAS_APP_RESET flag to IMX8MM_EP and IMX8MP_EP drvdata.
> This flag was overlooked during code restructuring. It is crucial to
> release the app-reset from the System Reset Controller before initiating
> LTSSM to rectify the issue

What exactly is the issue?  What does it look like to a user?  The
endpoint doesn't establish a link correctly?

> Fixes: 0c9651c21f2a ("PCI: imx6: Simplify reset handling by using *_FLAG_HAS_*_RESET")
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Does this need a -stable tag?

0c9651c21f2a appeared in v6.9, but this could arguably be v6.11
material if it fixes a serious issue.

> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 964d67756eb2b..42fd17fbadfa5 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1562,7 +1562,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  	},
>  	[IMX8MM_EP] = {
>  		.variant = IMX8MM_EP,
> -		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
> +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> +			 IMX6_PCIE_FLAG_HAS_PHYDRV,
>  		.mode = DW_PCIE_EP_TYPE,
>  		.gpr = "fsl,imx8mm-iomuxc-gpr",
>  		.clk_names = imx8mm_clks,
> @@ -1573,7 +1574,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
>  	},
>  	[IMX8MP_EP] = {
>  		.variant = IMX8MP_EP,
> -		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
> +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> +			 IMX6_PCIE_FLAG_HAS_PHYDRV,
>  		.mode = DW_PCIE_EP_TYPE,
>  		.gpr = "fsl,imx8mp-iomuxc-gpr",
>  		.clk_names = imx8mm_clks,
> 
> -- 
> 2.34.1
>
Bjorn Helgaas Sept. 2, 2024, 9:12 p.m. UTC | #2 
On Mon, Jul 29, 2024 at 04:18:08PM -0400, Frank Li wrote:
> From: Richard Zhu <hongxing.zhu@nxp.com>

Maybe "iMX8MP" in this subject should be "i.MX8MP" as in the subject
of the next patch?

And if so, maybe it should be "i.MX8MM" here, too?

That seems to match usage in the rest of the series (although "PCI:
imx6: Add i.MX8Q PCIe Root Complex (RC) support" uses "iMX8MP" once in
the commit log).
Frank Li Sept. 2, 2024, 10:51 p.m. UTC | #3 
On Mon, Sep 02, 2024 at 04:12:40PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 29, 2024 at 04:18:08PM -0400, Frank Li wrote:
> > From: Richard Zhu <hongxing.zhu@nxp.com>
>
> Maybe "iMX8MP" in this subject should be "i.MX8MP" as in the subject
> of the next patch?
>
> And if so, maybe it should be "i.MX8MM" here, too?

i.MX8MP and i.MX8MM is more formal. Many other place in kernel tree also
use iMX8MP and iMX8MM.

Do you need me repost it?

Frank

>
> That seems to match usage in the rest of the series (although "PCI:
> imx6: Add i.MX8Q PCIe Root Complex (RC) support" uses "iMX8MP" once in
> the commit log).
Frank Li Sept. 2, 2024, 10:57 p.m. UTC | #4 
On Mon, Sep 02, 2024 at 03:59:34PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 29, 2024 at 04:18:08PM -0400, Frank Li wrote:
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> >
> > Add IMX6_PCIE_FLAG_HAS_APP_RESET flag to IMX8MM_EP and IMX8MP_EP drvdata.
> > This flag was overlooked during code restructuring. It is crucial to
> > release the app-reset from the System Reset Controller before initiating
> > LTSSM to rectify the issue
>
> What exactly is the issue?  What does it look like to a user?  The
> endpoint doesn't establish a link correctly?

Yes. Link can't establish.

>
> > Fixes: 0c9651c21f2a ("PCI: imx6: Simplify reset handling by using *_FLAG_HAS_*_RESET")
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> Does this need a -stable tag?

Yes.

Do I need repost it?

Frank

>
> 0c9651c21f2a appeared in v6.9, but this could arguably be v6.11
> material if it fixes a serious issue.
>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 964d67756eb2b..42fd17fbadfa5 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1562,7 +1562,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> >  	},
> >  	[IMX8MM_EP] = {
> >  		.variant = IMX8MM_EP,
> > -		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
> > +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> > +			 IMX6_PCIE_FLAG_HAS_PHYDRV,
> >  		.mode = DW_PCIE_EP_TYPE,
> >  		.gpr = "fsl,imx8mm-iomuxc-gpr",
> >  		.clk_names = imx8mm_clks,
> > @@ -1573,7 +1574,8 @@ static const struct imx6_pcie_drvdata drvdata[] = {
> >  	},
> >  	[IMX8MP_EP] = {
> >  		.variant = IMX8MP_EP,
> > -		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
> > +		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
> > +			 IMX6_PCIE_FLAG_HAS_PHYDRV,
> >  		.mode = DW_PCIE_EP_TYPE,
> >  		.gpr = "fsl,imx8mp-iomuxc-gpr",
> >  		.clk_names = imx8mm_clks,
> >
> > --
> > 2.34.1
> >
Bjorn Helgaas Sept. 2, 2024, 10:59 p.m. UTC | #5 
On Mon, Sep 02, 2024 at 06:51:36PM -0400, Frank Li wrote:
> On Mon, Sep 02, 2024 at 04:12:40PM -0500, Bjorn Helgaas wrote:
> > On Mon, Jul 29, 2024 at 04:18:08PM -0400, Frank Li wrote:
> > > From: Richard Zhu <hongxing.zhu@nxp.com>
> >
> > Maybe "iMX8MP" in this subject should be "i.MX8MP" as in the subject
> > of the next patch?
> >
> > And if so, maybe it should be "i.MX8MM" here, too?
> 
> i.MX8MP and i.MX8MM is more formal. Many other place in kernel tree also
> use iMX8MP and iMX8MM.
> 
> Do you need me repost it?

No, no point it that, we can tidy that easily.

Same for the stable tag, we can add that.
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 964d67756eb2b..42fd17fbadfa5 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1562,7 +1562,8 @@  static const struct imx6_pcie_drvdata drvdata[] = {
 	},
 	[IMX8MM_EP] = {
 		.variant = IMX8MM_EP,
-		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
+		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
+			 IMX6_PCIE_FLAG_HAS_PHYDRV,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mm-iomuxc-gpr",
 		.clk_names = imx8mm_clks,
@@ -1573,7 +1574,8 @@  static const struct imx6_pcie_drvdata drvdata[] = {
 	},
 	[IMX8MP_EP] = {
 		.variant = IMX8MP_EP,
-		.flags = IMX6_PCIE_FLAG_HAS_PHYDRV,
+		.flags = IMX6_PCIE_FLAG_HAS_APP_RESET |
+			 IMX6_PCIE_FLAG_HAS_PHYDRV,
 		.mode = DW_PCIE_EP_TYPE,
 		.gpr = "fsl,imx8mp-iomuxc-gpr",
 		.clk_names = imx8mm_clks,