Hi, On 14/11/18 4:27 AM, Marc Zyngier wrote: > The write to the status register is really an ACK for the HW, > and should be treated as such by the driver. Let's move it to the > irq_ack callback, which will prevent people from moving it around > in order to paper over other bugs. > > Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 0a76948ed49e..f06e67c60593 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -99,9 +99,6 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) > (i * MAX_MSI_IRQS_PER_CTRL) + > pos); > generic_handle_irq(irq); > - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + > - (i * MSI_REG_CTRL_BLOCK_SIZE), > - 4, 1 << pos); > pos++; > } > } > @@ -200,14 +197,18 @@ static void dw_pci_bottom_unmask(struct irq_data *data) > > static void dw_pci_bottom_ack(struct irq_data *d) > { > - struct msi_desc *msi = irq_data_get_msi_desc(d); > - struct pcie_port *pp; > + struct pcie_port *pp = irq_data_get_irq_chip_data(d); > + unsigned int res, bit, ctrl; > unsigned long flags; > > - pp = msi_desc_to_pci_sysdata(msi); > + ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL; > + res = ctrl * MSI_REG_CTRL_BLOCK_SIZE; > + bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL; > > raw_spin_lock_irqsave(&pp->lock, flags); > > + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, 1 << bit); This register should be written only if msi_irq_ack callback is not populated similar to other dw_pci_bottom_*() functions. Thanks Kishon