From: Marc Zyngier <marc.zyngier@arm.com>
To: Amitkumar Karwar <amitkarwar@gmail.com>,
Enric Balletbo i Serra <enric.balletbo@collabora.com>,
Ganapathi Bhat <gbhat@marvell.com>,
Heiko Stuebner <heiko@sntech.de>,
Kalle Valo <kvalo@codeaurora.org>,
Nishant Sarmukadam <nishants@marvell.com>,
Rob Herring <robh+dt@kernel.org>,
Xinming Hu <huxinming820@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org
Subject: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
Date: Sun, 24 Feb 2019 14:04:22 +0000 [thread overview]
Message-ID: <20190224140426.3267-1-marc.zyngier@arm.com> (raw)
For quite some time, I wondered why the PCI mwifiex device built in my
Chromebook was unable to use the good old legacy interrupts. But as MSIs
were working fine, I never really bothered investigating. I finally had a
look, and the result isn't very pretty.
On this machine (rk3399-based kevin), the wake-up interrupt is described as
such:
&pci_rootport {
mvl_wifi: wifi@0,0 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
interrupt-parent = <&gpio0>;
interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
wakeup-source;
};
};
Note how the interrupt is part of the properties directly attached to the
PCI node. And yet, this interrupt has nothing to do with a PCI legacy
interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
altogether (Yay for the broken design!). This is in total violation of the
IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
specifiers describe the PCI device interrupts, and must obey the
INT-{A,B,C,D} mapping. Oops!
The net effect of the above is that Linux tries to do something vaguely
sensible, and uses the same interrupt for both the wake-up widget and the
PCI device. This doesn't work for two reasons: (1) the wake-up widget grabs
the interrupt in exclusive mode, and (2) the PCI interrupt is still routed
to the RC, leading to a screaming interrupt. This simply cannot work.
To sort out this mess, we need to lift the confusion between the two
interrupts. This is done by extending the DT binding to allow the wake-up
interrupt to be described in a 'wake-up' subnode, sidestepping the issue
completely. On my Chromebook, it now looks like this:
&pci_rootport {
mvl_wifi: wifi@0,0 {
compatible = "pci1b4b,2b42";
reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
0x83010000 0x0 0x00100000 0x0 0x00100000>;
pinctrl-names = "default";
pinctrl-0 = <&wlan_host_wake_l>;
wake-up {
interrupt-parent = <&gpio0>;
interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
wakeup-source;
};
};
};
The driver is then updated to look for this subnode first, and fallback to
the original, broken behaviour (spitting out a warning in the offending
configuration).
For good measure, there are two additional patches:
- The wake-up interrupt requesting is horribly racy, and could lead to
unpredictable behaviours. Let's fix that properly.
- A final patch implementing the above transformation for the whole
RK3399-based Chromebook range, which all use the same broken
configuration.
With all that, I finally have PCI legacy interrupts working with the mwifiex
driver on my Chromebook.
[1] http://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf
Marc Zyngier (4):
dt-bindings/marvell-8xxx: Allow wake-up interrupt to be placed in a
separate node
mwifiex: Fetch wake-up interrupt from 'wake-up' subnode when it exists
mwifiex: Flag wake-up interrupt as IRQ_NOAUTOEN rather than disabling
it too late
arm64: dts: rockchip: gru: Move wifi wake-up interrupt into its own
subnode
.../bindings/net/wireless/marvell-8xxx.txt | 23 ++++++++++++++++++-
.../dts/rockchip/rk3399-gru-chromebook.dtsi | 8 ++++---
drivers/net/wireless/marvell/mwifiex/main.c | 13 +++++++++--
3 files changed, 38 insertions(+), 6 deletions(-)
--
2.20.1
next reply other threads:[~2019-02-24 14:05 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-24 14:04 Marc Zyngier [this message]
2019-02-24 14:04 ` [PATCH 1/4] dt-bindings/marvell-8xxx: Allow wake-up interrupt to be placed in a separate node Marc Zyngier
2019-02-24 14:04 ` [PATCH 2/4] mwifiex: Fetch wake-up interrupt from 'wake-up' subnode when it exists Marc Zyngier
2019-02-24 14:04 ` [PATCH 3/4] mwifiex: Flag wake-up interrupt as IRQ_NOAUTOEN rather than disabling it too late Marc Zyngier
2019-02-26 23:31 ` Brian Norris
2019-02-26 23:34 ` Brian Norris
2019-04-04 10:22 ` Kalle Valo
2019-02-24 14:04 ` [PATCH 4/4] arm64: dts: rockchip: gru: Move wifi wake-up interrupt into its own subnode Marc Zyngier
2019-02-25 12:45 ` [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes Ard Biesheuvel
2019-02-25 14:52 ` Marc Zyngier
2019-02-26 16:21 ` Ard Biesheuvel
2019-02-26 17:14 ` Marc Zyngier
2019-02-26 23:44 ` Brian Norris
2019-02-27 9:27 ` Marc Zyngier
2019-02-26 23:28 ` Brian Norris
2019-02-27 10:02 ` Marc Zyngier
2019-02-27 10:16 ` Ard Biesheuvel
2019-02-27 20:57 ` Brian Norris
2019-02-27 23:03 ` Rafael J. Wysocki
2019-02-28 2:29 ` Brian Norris
2019-02-28 11:03 ` Rafael J. Wysocki
2019-02-27 20:51 ` Brian Norris
2019-03-08 8:26 ` Kalle Valo
2019-03-08 9:02 ` Marc Zyngier
2019-03-08 9:36 ` Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190224140426.3267-1-marc.zyngier@arm.com \
--to=marc.zyngier@arm.com \
--cc=amitkarwar@gmail.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=enric.balletbo@collabora.com \
--cc=gbhat@marvell.com \
--cc=heiko@sntech.de \
--cc=huxinming820@gmail.com \
--cc=kvalo@codeaurora.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nishants@marvell.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).