PCI: dwc: Fix integrated MSI Receiver mask reg setting during resume

[PATCH] PCI: dwc: Fix integrated MSI Receiver mask reg setting during resume

Posted by Jisheng Zhang 4 years, 6 months ago

If the host which makes use of the IP's integrated MSI Receiver losts
power during suspend, we call dw_pcie_setup_rc() to reinit the RC. But
dw_pcie_setup_rc() always set the pp->irq_mask[ctrl] as ~0, so the mask
register is always set as 0xffffffff incorrectly, thus the MSI can't
work after resume.

Fix this issue by moving pp->irq_mask[ctrl] initialization to
dw_pcie_host_init(), so we can correctly set the mask reg during both
boot and resume.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index f4755f3a03be..2fa86f32d964 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -362,6 +362,12 @@ int dw_pcie_host_init(struct pcie_port *pp)
 			if (ret < 0)
 				return ret;
 		} else if (pp->has_msi_ctrl) {
+			u32 ctrl, num_ctrls;
+
+			num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
+			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
+				pp->irq_mask[ctrl] = ~0;
+
 			if (!pp->msi_irq) {
 				pp->msi_irq = platform_get_irq_byname_optional(pdev, "msi");
 				if (pp->msi_irq < 0) {
@@ -541,7 +547,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 
 		/* Initialize IRQ Status array */
 		for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
-			pp->irq_mask[ctrl] = ~0;
 			dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
 					    (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
 					    pp->irq_mask[ctrl]);
-- 
2.34.1

RE: [PATCH] PCI: dwc: Fix integrated MSI Receiver mask reg setting during resume

Posted by Hongxing Zhu 4 years, 5 months ago

> -----Original Message-----
> From: Jisheng Zhang <jszhang@kernel.org>
> Sent: Sunday, December 26, 2021 3:40 PM
> To: Jingoo Han <jingoohan1@gmail.com>; Gustavo Pimentel
> <gustavo.pimentel@synopsys.com>; Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com>; Rob Herring <robh@kernel.org>; Krzysztof
> Wilczyński <kw@linux.com>; Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH] PCI: dwc: Fix integrated MSI Receiver mask reg setting
> during resume
> 
> If the host which makes use of the IP's integrated MSI Receiver losts
> power during suspend, we call dw_pcie_setup_rc() to reinit the RC. But
> dw_pcie_setup_rc() always set the pp->irq_mask[ctrl] as ~0, so the mask
> register is always set as 0xffffffff incorrectly, thus the MSI can't work after
> resume.
> 
> Fix this issue by moving pp->irq_mask[ctrl] initialization to
> dw_pcie_host_init(), so we can correctly set the mask reg during both
> boot and resume.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
[Richard Zhu] Tested-by: Richard Zhu <hongxing.zhu@nxp.com>
Thanks.

Best Regards
Richard Zhu

> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f4755f3a03be..2fa86f32d964 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -362,6 +362,12 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			if (ret < 0)
>  				return ret;
>  		} else if (pp->has_msi_ctrl) {
> +			u32 ctrl, num_ctrls;
> +
> +			num_ctrls = pp->num_vectors /
> MAX_MSI_IRQS_PER_CTRL;
> +			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> +				pp->irq_mask[ctrl] = ~0;
> +
>  			if (!pp->msi_irq) {
>  				pp->msi_irq =
> platform_get_irq_byname_optional(pdev, "msi");
>  				if (pp->msi_irq < 0) {
> @@ -541,7 +547,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> 
>  		/* Initialize IRQ Status array */
>  		for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> -			pp->irq_mask[ctrl] = ~0;
>  			dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
>  					    (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>  					    pp->irq_mask[ctrl]);
> --
> 2.34.1

Re: [PATCH] PCI: dwc: Fix integrated MSI Receiver mask reg setting during resume

Posted by Jisheng Zhang 4 years, 5 months ago

On Sun, Dec 26, 2021 at 03:40:19PM +0800, Jisheng Zhang wrote:
> If the host which makes use of the IP's integrated MSI Receiver losts
> power during suspend, we call dw_pcie_setup_rc() to reinit the RC. But
> dw_pcie_setup_rc() always set the pp->irq_mask[ctrl] as ~0, so the mask
> register is always set as 0xffffffff incorrectly, thus the MSI can't
> work after resume.
> 
> Fix this issue by moving pp->irq_mask[ctrl] initialization to
> dw_pcie_host_init(), so we can correctly set the mask reg during both
> boot and resume.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>

Hi all,

This patch can still be applied to the latest linus tree. Do you want
me to rebase and send out a new version?

Without this patch, dwc host MSI interrupt(if use the IP's integrated
MSI receiver) can't work after resume. Could it be picked up as a fix
for v5.17?

Thanks

> ---
>  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f4755f3a03be..2fa86f32d964 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -362,6 +362,12 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			if (ret < 0)
>  				return ret;
>  		} else if (pp->has_msi_ctrl) {
> +			u32 ctrl, num_ctrls;
> +
> +			num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> +			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> +				pp->irq_mask[ctrl] = ~0;
> +
>  			if (!pp->msi_irq) {
>  				pp->msi_irq = platform_get_irq_byname_optional(pdev, "msi");
>  				if (pp->msi_irq < 0) {
> @@ -541,7 +547,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
>  
>  		/* Initialize IRQ Status array */
>  		for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> -			pp->irq_mask[ctrl] = ~0;
>  			dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
>  					    (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
>  					    pp->irq_mask[ctrl]);
> -- 
> 2.34.1
>

Re: [PATCH] PCI: dwc: Fix integrated MSI Receiver mask reg setting during resume

Posted by Lorenzo Pieralisi 4 years, 4 months ago

On Sun, Jan 23, 2022 at 08:02:42PM +0800, Jisheng Zhang wrote:
> On Sun, Dec 26, 2021 at 03:40:19PM +0800, Jisheng Zhang wrote:
> > If the host which makes use of the IP's integrated MSI Receiver losts
> > power during suspend, we call dw_pcie_setup_rc() to reinit the RC. But
> > dw_pcie_setup_rc() always set the pp->irq_mask[ctrl] as ~0, so the mask
> > register is always set as 0xffffffff incorrectly, thus the MSI can't
> > work after resume.
> > 
> > Fix this issue by moving pp->irq_mask[ctrl] initialization to
> > dw_pcie_host_init(), so we can correctly set the mask reg during both
> > boot and resume.
> > 
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> 
> Hi all,
> 
> This patch can still be applied to the latest linus tree. Do you want
> me to rebase and send out a new version?
> 
> Without this patch, dwc host MSI interrupt(if use the IP's integrated
> MSI receiver) can't work after resume. Could it be picked up as a fix
> for v5.17?

The tricky bit with this patch is that it is not clear what piece of
logic is lost on power down and what not. IIUC MSI interrupt controller
logic is kept so it does not need to be saved/restored (but in
dw_pcie_setup_rc() we overwrite PCIE_MSI_INTR0_ENABLE even if it
is not needed on resume - actually, it can even be destructive).

Maybe we need to write suspend/resume hooks for the dwc core instead
of moving code around to fix these bugs ?

Lorenzo

> 
> Thanks
> 
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-host.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index f4755f3a03be..2fa86f32d964 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -362,6 +362,12 @@ int dw_pcie_host_init(struct pcie_port *pp)
> >  			if (ret < 0)
> >  				return ret;
> >  		} else if (pp->has_msi_ctrl) {
> > +			u32 ctrl, num_ctrls;
> > +
> > +			num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> > +			for (ctrl = 0; ctrl < num_ctrls; ctrl++)
> > +				pp->irq_mask[ctrl] = ~0;
> > +
> >  			if (!pp->msi_irq) {
> >  				pp->msi_irq = platform_get_irq_byname_optional(pdev, "msi");
> >  				if (pp->msi_irq < 0) {
> > @@ -541,7 +547,6 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> >  
> >  		/* Initialize IRQ Status array */
> >  		for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> > -			pp->irq_mask[ctrl] = ~0;
> >  			dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
> >  					    (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> >  					    pp->irq_mask[ctrl]);
> > -- 
> > 2.34.1
> >

Re: [PATCH] PCI: dwc: Fix integrated MSI Receiver mask reg setting during resume

Posted by Lorenzo Pieralisi 4 years, 4 months ago

On Sun, 26 Dec 2021 15:40:19 +0800, Jisheng Zhang wrote:
> If the host which makes use of the IP's integrated MSI Receiver losts
> power during suspend, we call dw_pcie_setup_rc() to reinit the RC. But
> dw_pcie_setup_rc() always set the pp->irq_mask[ctrl] as ~0, so the mask
> register is always set as 0xffffffff incorrectly, thus the MSI can't
> work after resume.
> 
> Fix this issue by moving pp->irq_mask[ctrl] initialization to
> dw_pcie_host_init(), so we can correctly set the mask reg during both
> boot and resume.
> 
> [...]

Applied to pci/dwc, thanks!

[1/1] PCI: dwc: Fix integrated MSI Receiver mask reg setting during resume
      https://git.kernel.org/lpieralisi/pci/c/84edd0090e

Thanks,
Lorenzo