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

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

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
> >