On Fri, Jan 18, 2019 at 04:22:15PM +0000, Marc Zyngier wrote:
On 18/01/2019 16:00, Niklas Cassel wrote:
On Thu, Jan 17, 2019 at 09:58:43AM +0000, Marc Zyngier wrote:
On 16/01/2019 23:43, Niklas Cassel wrote:
On Tue, Jan 15, 2019 at 09:24:37AM +0100, gregkh@linuxfoundation.org wrote:
The patch below does not apply to the 4.14-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
Hello Marc,
You said that you were going to backport this to 4.14, right?
Me, or anyone else. Preferably someone who, like you, has the HW at hand (I don't).
IIRC, you said something about creating a simple dw_pci_bottom_ack() (since 4.14 lacks a ack() function).
Indeed. Something like this:
diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c index bc3e2d8d0cce..f4f3eeee10af 100644 --- a/drivers/pci/dwc/pcie-designware-host.c +++ b/drivers/pci/dwc/pcie-designware-host.c @@ -45,8 +45,19 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size, return dw_pcie_write(pci->dbi_base + where, size, val); } +static void dwc_irq_ack(struct irq_data *d) +{
- struct msi_desc *msi = irq_data_get_msi_desc(d);
- struct pcie_port *pp = msi_desc_to_pci_sysdata(msi);
- int pos = d->hwirq % 32;
- int i = d->hwirq / 32;
- dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4, BIT(pos));
+}
Thanks Marc.
This does not seem to work. It appears that the ack function is never called. I assume that this is because of:
irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
Ah, indeed, I forgot about that particular nugget.
is it safe to simply change this to handle_edge_irq? (Which seems to be the case after this after this driver's MSI handling was heavily refactored in 4.17.)
Yes, this should be just a matter of using the right interrupt flow. Please let me know if that works for you.
Hello Marc,
I ran a simple stress test during the night with your patch plus:
- irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq); + irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_edge_irq);
without any issues. (Feel free to add my Tested-by).
So this looks good to me :) Thanks!
Kind regards, Niklas
I know that you were against reverting 8c934095fa2f ("PCI: dwc: Clear MSI interrupt status after it is handled, not before") on mainline, since that wouldn't hinder people from moving around stuff in the future, but perhaps reverting this commit on 4.14 is safer than changing the irq handler type?
I still don't think that it is a good idea. Using the correct flow is just as important in order to properly honors the interrupt masking which the "simple" flow completely ignores.
Thanks,
M.
Jazz is not dead. It just smells funny...