linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
@ 2022-11-01  7:59 Richard Zhu
  2022-11-02 23:10 ` Bjorn Helgaas
  2022-11-25  2:07 ` Hongxing Zhu
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Zhu @ 2022-11-01  7:59 UTC (permalink / raw)
  To: l.stach, bhelgaas, lorenzo.pieralisi
  Cc: hongxing.zhu, linux-pci, linux-arm-kernel, linux-kernel, kernel,
	linux-imx

The MSI Enable bit controls delivery of MSI interrupts from components
below the Root Port. This bit might lost during the suspend, should be
re-configured during resume.

Encapsulate the MSI enable set into a standalone function, and invoke it
in both probe and resume.

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

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 2616585ca5f8..dba15546075f 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1041,6 +1041,21 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
 	usleep_range(1000, 10000);
 }
 
+static void pci_imx_set_msi_en(struct dw_pcie *pci)
+{
+	u8 offset;
+	u16 val;
+
+	if (pci_msi_enabled()) {
+		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
+		dw_pcie_dbi_ro_wr_en(pci);
+		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
+		val |= PCI_MSI_FLAGS_ENABLE;
+		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
+		dw_pcie_dbi_ro_wr_dis(pci);
+	}
+}
+
 static int imx6_pcie_suspend_noirq(struct device *dev)
 {
 	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
@@ -1073,6 +1088,7 @@ static int imx6_pcie_resume_noirq(struct device *dev)
 	if (imx6_pcie->link_is_up)
 		imx6_pcie_start_link(imx6_pcie->pci);
 
+	pci_imx_set_msi_en(imx6_pcie->pci);
 	return 0;
 }
 
@@ -1090,7 +1106,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	struct resource *dbi_base;
 	struct device_node *node = dev->of_node;
 	int ret;
-	u16 val;
 
 	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
 	if (!imx6_pcie)
@@ -1282,12 +1297,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	if (pci_msi_enabled()) {
-		u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
-		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
-		val |= PCI_MSI_FLAGS_ENABLE;
-		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
-	}
+	pci_imx_set_msi_en(pci);
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
  2022-11-01  7:59 [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume Richard Zhu
@ 2022-11-02 23:10 ` Bjorn Helgaas
  2022-11-03  6:26   ` Hongxing Zhu
  2022-11-25  2:07 ` Hongxing Zhu
  1 sibling, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2022-11-02 23:10 UTC (permalink / raw)
  To: Richard Zhu
  Cc: l.stach, bhelgaas, lorenzo.pieralisi, linux-pci,
	linux-arm-kernel, linux-kernel, kernel, linux-imx

On Tue, Nov 01, 2022 at 03:59:55PM +0800, Richard Zhu wrote:
> The MSI Enable bit controls delivery of MSI interrupts from components
> below the Root Port. This bit might lost during the suspend, should be
> re-configured during resume.

Just out of curiosity, why would this bit "get lost" during suspend?

Don't the normal save and restore in the suspend/resume paths take
care of this?  Are there other bits that might get lost?

> Encapsulate the MSI enable set into a standalone function, and invoke it
> in both probe and resume.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 2616585ca5f8..dba15546075f 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1041,6 +1041,21 @@ static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
>  	usleep_range(1000, 10000);
>  }
>  
> +static void pci_imx_set_msi_en(struct dw_pcie *pci)
> +{
> +	u8 offset;
> +	u16 val;
> +
> +	if (pci_msi_enabled()) {
> +		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +		dw_pcie_dbi_ro_wr_en(pci);
> +		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> +		val |= PCI_MSI_FLAGS_ENABLE;
> +		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> +		dw_pcie_dbi_ro_wr_dis(pci);
> +	}
> +}
> +
>  static int imx6_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> @@ -1073,6 +1088,7 @@ static int imx6_pcie_resume_noirq(struct device *dev)
>  	if (imx6_pcie->link_is_up)
>  		imx6_pcie_start_link(imx6_pcie->pci);
>  
> +	pci_imx_set_msi_en(imx6_pcie->pci);
>  	return 0;
>  }
>  
> @@ -1090,7 +1106,6 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  	struct resource *dbi_base;
>  	struct device_node *node = dev->of_node;
>  	int ret;
> -	u16 val;
>  
>  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
>  	if (!imx6_pcie)
> @@ -1282,12 +1297,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	if (pci_msi_enabled()) {
> -		u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> -		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> -		val |= PCI_MSI_FLAGS_ENABLE;
> -		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> -	}
> +	pci_imx_set_msi_en(pci);
>  
>  	return 0;
>  }
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
  2022-11-02 23:10 ` Bjorn Helgaas
@ 2022-11-03  6:26   ` Hongxing Zhu
  0 siblings, 0 replies; 6+ messages in thread
From: Hongxing Zhu @ 2022-11-03  6:26 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: l.stach, bhelgaas, lorenzo.pieralisi, linux-pci,
	linux-arm-kernel, linux-kernel, kernel, dl-linux-imx

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: 2022年11月3日 7:11
> To: Hongxing Zhu <hongxing.zhu@nxp.com>
> Cc: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.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: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
> 
> On Tue, Nov 01, 2022 at 03:59:55PM +0800, Richard Zhu wrote:
> > The MSI Enable bit controls delivery of MSI interrupts from components
> > below the Root Port. This bit might lost during the suspend, should be
> > re-configured during resume.
> 
> Just out of curiosity, why would this bit "get lost" during suspend?
> 
> Don't the normal save and restore in the suspend/resume paths take care of
> this?  Are there other bits that might get lost?
Hi Bjorn:
Thanks for your care.
The controller might be reset to the known stat during the
re-initialization in the resume path, I think.

dw_pcie_setup_rc() configures most of the controller setting during resume.
It seems that this bit is i.MX PCIe special, should be configured in both the
 probe and suspend/resume.

Best Regards
Richard Zhu
> 
> > Encapsulate the MSI enable set into a standalone function, and invoke
> > it in both probe and resume.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 2616585ca5f8..dba15546075f 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1041,6 +1041,21 @@ static void imx6_pcie_pm_turnoff(struct
> imx6_pcie *imx6_pcie)
> >  	usleep_range(1000, 10000);
> >  }
> >
> > +static void pci_imx_set_msi_en(struct dw_pcie *pci) {
> > +	u8 offset;
> > +	u16 val;
> > +
> > +	if (pci_msi_enabled()) {
> > +		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > +		dw_pcie_dbi_ro_wr_en(pci);
> > +		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > +		val |= PCI_MSI_FLAGS_ENABLE;
> > +		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > +		dw_pcie_dbi_ro_wr_dis(pci);
> > +	}
> > +}
> > +
> >  static int imx6_pcie_suspend_noirq(struct device *dev)  {
> >  	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); @@ -1073,6
> > +1088,7 @@ static int imx6_pcie_resume_noirq(struct device *dev)
> >  	if (imx6_pcie->link_is_up)
> >  		imx6_pcie_start_link(imx6_pcie->pci);
> >
> > +	pci_imx_set_msi_en(imx6_pcie->pci);
> >  	return 0;
> >  }
> >
> > @@ -1090,7 +1106,6 @@ static int imx6_pcie_probe(struct platform_device
> *pdev)
> >  	struct resource *dbi_base;
> >  	struct device_node *node = dev->of_node;
> >  	int ret;
> > -	u16 val;
> >
> >  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
> >  	if (!imx6_pcie)
> > @@ -1282,12 +1297,7 @@ static int imx6_pcie_probe(struct
> platform_device *pdev)
> >  	if (ret < 0)
> >  		return ret;
> >
> > -	if (pci_msi_enabled()) {
> > -		u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > -		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > -		val |= PCI_MSI_FLAGS_ENABLE;
> > -		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > -	}
> > +	pci_imx_set_msi_en(pci);
> >
> >  	return 0;
> >  }
> > --
> > 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%7C4454f7f5d1d0438d009908dabd27767e%
> 7C686ea1
> >
> d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638030274437186510%7CUnk
> nown%7CTW
> >
> FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> CI6
> >
> Mn0%3D%7C3000%7C%7C%7C&amp;sdata=gfiPPlezSSocyNTpq0oKnk5rwwm
> hpmWCszaHa
> > YWLhoI%3D&amp;reserved=0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
  2022-11-01  7:59 [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume Richard Zhu
  2022-11-02 23:10 ` Bjorn Helgaas
@ 2022-11-25  2:07 ` Hongxing Zhu
  2022-11-25 10:19   ` Lucas Stach
  1 sibling, 1 reply; 6+ messages in thread
From: Hongxing Zhu @ 2022-11-25  2:07 UTC (permalink / raw)
  To: Hongxing Zhu, l.stach, bhelgaas, lorenzo.pieralisi
  Cc: linux-pci, linux-arm-kernel, linux-kernel, kernel, dl-linux-imx

Friendly ping.
Anymore comments are very appreciated.

BTW, I had verified on i.MX7D/i.MX6QP platforms that MSI wouldn’t be functional
 after resume without this patch.

Best Regards
Richard Zhu

> -----Original Message-----
> From: Richard Zhu <hongxing.zhu@nxp.com>
> Sent: 2022年11月1日 16:00
> To: l.stach@pengutronix.de; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com
> Cc: Hongxing Zhu <hongxing.zhu@nxp.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: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
> 
> The MSI Enable bit controls delivery of MSI interrupts from components below
> the Root Port. This bit might lost during the suspend, should be re-configured
> during resume.
> 
> Encapsulate the MSI enable set into a standalone function, and invoke it in
> both probe and resume.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 2616585ca5f8..dba15546075f 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1041,6 +1041,21 @@ static void imx6_pcie_pm_turnoff(struct
> imx6_pcie *imx6_pcie)
>  	usleep_range(1000, 10000);
>  }
> 
> +static void pci_imx_set_msi_en(struct dw_pcie *pci) {
> +	u8 offset;
> +	u16 val;
> +
> +	if (pci_msi_enabled()) {
> +		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +		dw_pcie_dbi_ro_wr_en(pci);
> +		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> +		val |= PCI_MSI_FLAGS_ENABLE;
> +		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> +		dw_pcie_dbi_ro_wr_dis(pci);
> +	}
> +}
> +
>  static int imx6_pcie_suspend_noirq(struct device *dev)  {
>  	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); @@ -1073,6
> +1088,7 @@ static int imx6_pcie_resume_noirq(struct device *dev)
>  	if (imx6_pcie->link_is_up)
>  		imx6_pcie_start_link(imx6_pcie->pci);
> 
> +	pci_imx_set_msi_en(imx6_pcie->pci);
>  	return 0;
>  }
> 
> @@ -1090,7 +1106,6 @@ static int imx6_pcie_probe(struct platform_device
> *pdev)
>  	struct resource *dbi_base;
>  	struct device_node *node = dev->of_node;
>  	int ret;
> -	u16 val;
> 
>  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
>  	if (!imx6_pcie)
> @@ -1282,12 +1297,7 @@ static int imx6_pcie_probe(struct platform_device
> *pdev)
>  	if (ret < 0)
>  		return ret;
> 
> -	if (pci_msi_enabled()) {
> -		u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> -		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> -		val |= PCI_MSI_FLAGS_ENABLE;
> -		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> -	}
> +	pci_imx_set_msi_en(pci);
> 
>  	return 0;
>  }
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
  2022-11-25  2:07 ` Hongxing Zhu
@ 2022-11-25 10:19   ` Lucas Stach
  2022-11-28  3:45     ` Hongxing Zhu
  0 siblings, 1 reply; 6+ messages in thread
From: Lucas Stach @ 2022-11-25 10:19 UTC (permalink / raw)
  To: Hongxing Zhu, bhelgaas, lorenzo.pieralisi
  Cc: linux-pci, linux-arm-kernel, linux-kernel, kernel, dl-linux-imx

Am Freitag, dem 25.11.2022 um 02:07 +0000 schrieb Hongxing Zhu:
> Friendly ping.
> Anymore comments are very appreciated.
> 
> BTW, I had verified on i.MX7D/i.MX6QP platforms that MSI wouldn’t be functional
>  after resume without this patch.

Instead of playing whack-a-mole and restoring individual config setting
after resume, shouldn't we just do a pci_save_state() on suspend and
then restore the complete config by calling pci_restore_state() on
resume?

Regards,
Lucas

> 
> Best Regards
> Richard Zhu
> 
> > -----Original Message-----
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> > Sent: 2022年11月1日 16:00
> > To: l.stach@pengutronix.de; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com
> > Cc: Hongxing Zhu <hongxing.zhu@nxp.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: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
> > 
> > The MSI Enable bit controls delivery of MSI interrupts from components below
> > the Root Port. This bit might lost during the suspend, should be re-configured
> > during resume.
> > 
> > Encapsulate the MSI enable set into a standalone function, and invoke it in
> > both probe and resume.
> > 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 24 +++++++++++++++++-------
> >  1 file changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 2616585ca5f8..dba15546075f 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1041,6 +1041,21 @@ static void imx6_pcie_pm_turnoff(struct
> > imx6_pcie *imx6_pcie)
> >  	usleep_range(1000, 10000);
> >  }
> > 
> > +static void pci_imx_set_msi_en(struct dw_pcie *pci) {
> > +	u8 offset;
> > +	u16 val;
> > +
> > +	if (pci_msi_enabled()) {
> > +		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > +		dw_pcie_dbi_ro_wr_en(pci);
> > +		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > +		val |= PCI_MSI_FLAGS_ENABLE;
> > +		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > +		dw_pcie_dbi_ro_wr_dis(pci);
> > +	}
> > +}
> > +
> >  static int imx6_pcie_suspend_noirq(struct device *dev)  {
> >  	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); @@ -1073,6
> > +1088,7 @@ static int imx6_pcie_resume_noirq(struct device *dev)
> >  	if (imx6_pcie->link_is_up)
> >  		imx6_pcie_start_link(imx6_pcie->pci);
> > 
> > +	pci_imx_set_msi_en(imx6_pcie->pci);
> >  	return 0;
> >  }
> > 
> > @@ -1090,7 +1106,6 @@ static int imx6_pcie_probe(struct platform_device
> > *pdev)
> >  	struct resource *dbi_base;
> >  	struct device_node *node = dev->of_node;
> >  	int ret;
> > -	u16 val;
> > 
> >  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
> >  	if (!imx6_pcie)
> > @@ -1282,12 +1297,7 @@ static int imx6_pcie_probe(struct platform_device
> > *pdev)
> >  	if (ret < 0)
> >  		return ret;
> > 
> > -	if (pci_msi_enabled()) {
> > -		u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > -		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > -		val |= PCI_MSI_FLAGS_ENABLE;
> > -		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > -	}
> > +	pci_imx_set_msi_en(pci);
> > 
> >  	return 0;
> >  }
> > --
> > 2.25.1
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
  2022-11-25 10:19   ` Lucas Stach
@ 2022-11-28  3:45     ` Hongxing Zhu
  0 siblings, 0 replies; 6+ messages in thread
From: Hongxing Zhu @ 2022-11-28  3:45 UTC (permalink / raw)
  To: Lucas Stach, bhelgaas, lorenzo.pieralisi
  Cc: linux-pci, linux-arm-kernel, linux-kernel, kernel, dl-linux-imx

> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: 2022年11月25日 18:19
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com
> Cc: 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: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
> 
> Am Freitag, dem 25.11.2022 um 02:07 +0000 schrieb Hongxing Zhu:
> > Friendly ping.
> > Anymore comments are very appreciated.
> >
> > BTW, I had verified on i.MX7D/i.MX6QP platforms that MSI wouldn’t be
> > functional  after resume without this patch.
> 
> Instead of playing whack-a-mole and restoring individual config setting after
> resume, shouldn't we just do a pci_save_state() on suspend and then restore
> the complete config by calling pci_restore_state() on resume?
> 
Hi Lucas:
Thanks for your comments.
Refer to my understanding, only the i.MX PCIe specific configs that might
 be lost during suspend/resume should be saved/restored here.

Because that most of the configs had been re-configured in the
 suspend/resume paths already.
So, only the PCI_MSI_FLAGS should be saved/restored here, right?

Best Regards
Richard Zhu

> Regards,
> Lucas
> 
> >
> > Best Regards
> > Richard Zhu
> >
> > > -----Original Message-----
> > > From: Richard Zhu <hongxing.zhu@nxp.com>
> > > Sent: 2022年11月1日 16:00
> > > To: l.stach@pengutronix.de; bhelgaas@google.com;
> > > lorenzo.pieralisi@arm.com
> > > Cc: Hongxing Zhu <hongxing.zhu@nxp.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: [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume
> > >
> > > The MSI Enable bit controls delivery of MSI interrupts from
> > > components below the Root Port. This bit might lost during the
> > > suspend, should be re-configured during resume.
> > >
> > > Encapsulate the MSI enable set into a standalone function, and
> > > invoke it in both probe and resume.
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 24 +++++++++++++++++-------
> > >  1 file changed, 17 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 2616585ca5f8..dba15546075f 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -1041,6 +1041,21 @@ static void imx6_pcie_pm_turnoff(struct
> > > imx6_pcie *imx6_pcie)
> > >  	usleep_range(1000, 10000);
> > >  }
> > >
> > > +static void pci_imx_set_msi_en(struct dw_pcie *pci) {
> > > +	u8 offset;
> > > +	u16 val;
> > > +
> > > +	if (pci_msi_enabled()) {
> > > +		offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > > +		dw_pcie_dbi_ro_wr_en(pci);
> > > +		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > > +		val |= PCI_MSI_FLAGS_ENABLE;
> > > +		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > > +		dw_pcie_dbi_ro_wr_dis(pci);
> > > +	}
> > > +}
> > > +
> > >  static int imx6_pcie_suspend_noirq(struct device *dev)  {
> > >  	struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev); @@ -1073,6
> > > +1088,7 @@ static int imx6_pcie_resume_noirq(struct device *dev)
> > >  	if (imx6_pcie->link_is_up)
> > >  		imx6_pcie_start_link(imx6_pcie->pci);
> > >
> > > +	pci_imx_set_msi_en(imx6_pcie->pci);
> > >  	return 0;
> > >  }
> > >
> > > @@ -1090,7 +1106,6 @@ static int imx6_pcie_probe(struct
> > > platform_device
> > > *pdev)
> > >  	struct resource *dbi_base;
> > >  	struct device_node *node = dev->of_node;
> > >  	int ret;
> > > -	u16 val;
> > >
> > >  	imx6_pcie = devm_kzalloc(dev, sizeof(*imx6_pcie), GFP_KERNEL);
> > >  	if (!imx6_pcie)
> > > @@ -1282,12 +1297,7 @@ static int imx6_pcie_probe(struct
> > > platform_device
> > > *pdev)
> > >  	if (ret < 0)
> > >  		return ret;
> > >
> > > -	if (pci_msi_enabled()) {
> > > -		u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> > > -		val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> > > -		val |= PCI_MSI_FLAGS_ENABLE;
> > > -		dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> > > -	}
> > > +	pci_imx_set_msi_en(pci);
> > >
> > >  	return 0;
> > >  }
> > > --
> > > 2.25.1
> >
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-28  3:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-01  7:59 [PATCH v1] PCI: imx6: Set MSI enable bit of RC in resume Richard Zhu
2022-11-02 23:10 ` Bjorn Helgaas
2022-11-03  6:26   ` Hongxing Zhu
2022-11-25  2:07 ` Hongxing Zhu
2022-11-25 10:19   ` Lucas Stach
2022-11-28  3:45     ` Hongxing Zhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).