All of lore.kernel.org
 help / color / mirror / Atom feed
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

             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: link
Be 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.