Hi brian,
On 07/07/2017 08:53 AM, Brian Norris wrote:
On Mon, Jul 03, 2017 at 06:46:21PM +0800, Jeffy Chen wrote:
Hi guys,
with this patch, the pci device's irq might be override by this
wakeup irq when not using msi:
Hmm, good point. I believe I noticed this one at some point and then
didn't get to investigate further...
It kind of seems like we inadvertently conflicted with the PCI OF
interrupt spec [1]. There, the "interrupts" property for a device (if
present) is supposed to represent INT{A...D} with values of {1...4}.
IIUC, there should only be a single entry in this property.
If we were to extend this properly, I guess that would mean we'd need a
second "interrupts" entry, with a different parent. I think we can use
"interrupts-extended" for that.
So we'd need to document an optional "interrupt-names" for Marvell, and
have the driver try that first. The rough outline would be something
like this.
For the device tree (e.g., rk3399-gru):
- interrupt-parent = <&gpio0>;
- interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
+ interrupts-extended = <&pcie0 1>, <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "int-A", "wake";
This is a great idea.
And how about also add a property to tell of_pci_irq to ignore of irq
and force using PCI_INTERRUPT_PIN? Since there might be devices don't
use pci irq, but using other irq(wowlan for example).
Then we can specify this property and add a name("wake") to the wifi
wake irq here. And interrupts-extended would still be an available option.
Then mwifiex would need to check "byname" before trying "by index":
adapter->irq_wakeup = of_irq_get_byname(adapter->dt_node, "wake");
if (!adapter->irq_wakeup) {
adapter->irq_wakeup = irq_of_parse_and_map(adapter->dt_node, 0);
if (!adapter->irq_wakeup) {
dev_dbg(dev, "fail to parse irq_wakeup from device tree\n");
goto err_exit;
}
}
Or if we want to suggest the original binding was wrong and that we
should just ignore existing device trees that tried to use it, we can
skip the by-index fallback.
Brian
[1] Documentation/devicetree/bindings/pci/pci.txt points to
http://www.firmware.org/1275/practice/imap/imap0_9d.pdf
except that link is also dead now. I found the same doc here:
https://www.openfirmware.info/data/docs/rec.intmap.d09.pdf
Might want to update the binding doc... I've sent a patch for that
separately.
