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
WARNING: multiple messages have this Message-ID (diff)
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: devicetree@vger.kernel.org, netdev@vger.kernel.org, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, "David S. Miller" <davem@davemloft.net>, linux-arm-kernel@lists.infradead.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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2019-02-24 14:05 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-24 14:04 Marc Zyngier [this message] 2019-02-24 14:04 ` [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes Marc Zyngier 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 ` 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 ` 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-24 14:04 ` Marc Zyngier 2019-02-26 23:31 ` Brian Norris 2019-02-26 23:31 ` Brian Norris 2019-02-26 23:34 ` Brian Norris 2019-02-26 23:34 ` Brian Norris 2019-04-04 10:22 ` Kalle Valo 2019-04-04 10:22 ` Kalle Valo 2019-04-04 10:22 ` Kalle Valo 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-24 14:04 ` Marc Zyngier 2019-02-25 12:45 ` [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes Ard Biesheuvel 2019-02-25 12:45 ` Ard Biesheuvel 2019-02-25 14:52 ` Marc Zyngier 2019-02-25 14:52 ` Marc Zyngier 2019-02-26 16:21 ` Ard Biesheuvel 2019-02-26 16:21 ` Ard Biesheuvel 2019-02-26 16:21 ` Ard Biesheuvel 2019-02-26 17:14 ` Marc Zyngier 2019-02-26 17:14 ` Marc Zyngier 2019-02-26 23:44 ` Brian Norris 2019-02-26 23:44 ` Brian Norris 2019-02-26 23:44 ` Brian Norris 2019-02-27 9:27 ` Marc Zyngier 2019-02-27 9:27 ` Marc Zyngier 2019-02-27 9:27 ` Marc Zyngier 2019-02-26 23:28 ` Brian Norris 2019-02-26 23:28 ` Brian Norris 2019-02-27 10:02 ` Marc Zyngier 2019-02-27 10:02 ` Marc Zyngier 2019-02-27 10:16 ` Ard Biesheuvel 2019-02-27 10:16 ` Ard Biesheuvel 2019-02-27 10:16 ` Ard Biesheuvel 2019-02-27 10:16 ` Ard Biesheuvel 2019-02-27 20:57 ` Brian Norris 2019-02-27 20:57 ` Brian Norris 2019-02-27 20:57 ` Brian Norris 2019-02-27 23:03 ` Rafael J. Wysocki 2019-02-27 23:03 ` Rafael J. Wysocki 2019-02-27 23:03 ` Rafael J. Wysocki 2019-02-28 2:29 ` Brian Norris 2019-02-28 2:29 ` Brian Norris 2019-02-28 2:29 ` Brian Norris 2019-02-28 11:03 ` Rafael J. Wysocki 2019-02-28 11:03 ` Rafael J. Wysocki 2019-02-28 11:03 ` Rafael J. Wysocki 2019-02-27 20:51 ` Brian Norris 2019-02-27 20:51 ` Brian Norris 2019-03-08 8:26 ` Kalle Valo 2019-03-08 8:26 ` Kalle Valo 2019-03-08 8:26 ` Kalle Valo 2019-03-08 9:02 ` Marc Zyngier 2019-03-08 9:02 ` Marc Zyngier 2019-03-08 9:36 ` Kalle Valo 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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.