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@xxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx>.
>>>
>>> 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.
>
> 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...
