diff mbox series

[v2,12/35] brcmfmac: pcie: Fix crashes due to early IRQs

Message ID 20220104072658.69756-13-marcan@marcan.st (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series brcmfmac: Support Apple T2 and M1 platforms | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Hector Martin Jan. 4, 2022, 7:26 a.m. UTC
The driver was enabling IRQs before the message processing was
initialized. This could cause IRQs to come in too early and crash the
driver. Instead, move the IRQ enable and hostready to a bus preinit
function, at which point everything is properly initialized.

Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../wireless/broadcom/brcm80211/brcmfmac/pcie.c  | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Andy Shevchenko Jan. 4, 2022, 2:12 p.m. UTC | #1
On Tue, Jan 4, 2022 at 9:29 AM Hector Martin <marcan@marcan.st> wrote:
>
> The driver was enabling IRQs before the message processing was
> initialized. This could cause IRQs to come in too early and crash the
> driver. Instead, move the IRQ enable and hostready to a bus preinit
> function, at which point everything is properly initialized.
>
> Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")

You should gather fixes at the beginning of the series, and even
possible to send them as a separate series. In the current state it's
unclear if there are dependencies on your new feature (must not be for
fixes that meant to be backported).
Hector Martin Jan. 6, 2022, 1:10 p.m. UTC | #2
On 04/01/2022 23.12, Andy Shevchenko wrote:
> On Tue, Jan 4, 2022 at 9:29 AM Hector Martin <marcan@marcan.st> wrote:
>>
>> The driver was enabling IRQs before the message processing was
>> initialized. This could cause IRQs to come in too early and crash the
>> driver. Instead, move the IRQ enable and hostready to a bus preinit
>> function, at which point everything is properly initialized.
>>
>> Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
> 
> You should gather fixes at the beginning of the series, and even
> possible to send them as a separate series. In the current state it's
> unclear if there are dependencies on your new feature (must not be for
> fixes that meant to be backported).
> 

Thanks, I wasn't sure what order you wanted those in. I'll put them at
the top for v3. I think none of those should have any dependencies on
the rest of the patches, modulo some trivial rebase wrangling.
Arend van Spriel Jan. 10, 2022, 7:19 a.m. UTC | #3
On 1/4/2022 8:26 AM, Hector Martin wrote:
> The driver was enabling IRQs before the message processing was
> initialized. This could cause IRQs to come in too early and crash the
> driver. Instead, move the IRQ enable and hostready to a bus preinit
> function, at which point everything is properly initialized.
> 
> Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   .../wireless/broadcom/brcm80211/brcmfmac/pcie.c  | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
Kalle Valo Jan. 10, 2022, 1:54 p.m. UTC | #4
Hector Martin <marcan@marcan.st> writes:

> On 04/01/2022 23.12, Andy Shevchenko wrote:
>> On Tue, Jan 4, 2022 at 9:29 AM Hector Martin <marcan@marcan.st> wrote:
>>>
>>> The driver was enabling IRQs before the message processing was
>>> initialized. This could cause IRQs to come in too early and crash the
>>> driver. Instead, move the IRQ enable and hostready to a bus preinit
>>> function, at which point everything is properly initialized.
>>>
>>> Fixes: 9e37f045d5e7 ("brcmfmac: Adding PCIe bus layer support.")
>> 
>> You should gather fixes at the beginning of the series, and even
>> possible to send them as a separate series. In the current state it's
>> unclear if there are dependencies on your new feature (must not be for
>> fixes that meant to be backported).
>> 
>
> Thanks, I wasn't sure what order you wanted those in. I'll put them at
> the top for v3. I think none of those should have any dependencies on
> the rest of the patches, modulo some trivial rebase wrangling.

If there are no dependencies, please send the brcmfmac fixes separately
so that I can apply them earlier.
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 250e0bd40cb3..595815164e18 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1378,6 +1378,18 @@  static void brcmf_pcie_down(struct device *dev)
 {
 }
 
+static int brcmf_pcie_preinit(struct device *dev)
+{
+	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
+	struct brcmf_pciedev *buspub = bus_if->bus_priv.pcie;
+
+	brcmf_dbg(PCIE, "Enter\n");
+
+	brcmf_pcie_intr_enable(buspub->devinfo);
+	brcmf_pcie_hostready(buspub->devinfo);
+
+	return 0;
+}
 
 static int brcmf_pcie_tx(struct device *dev, struct sk_buff *skb)
 {
@@ -1488,6 +1500,7 @@  static int brcmf_pcie_reset(struct device *dev)
 }
 
 static const struct brcmf_bus_ops brcmf_pcie_bus_ops = {
+	.preinit = brcmf_pcie_preinit,
 	.txdata = brcmf_pcie_tx,
 	.stop = brcmf_pcie_down,
 	.txctl = brcmf_pcie_tx_ctlpkt,
@@ -2053,9 +2066,6 @@  static void brcmf_pcie_setup(struct device *dev, int ret,
 
 	init_waitqueue_head(&devinfo->mbdata_resp_wait);
 
-	brcmf_pcie_intr_enable(devinfo);
-	brcmf_pcie_hostready(devinfo);
-
 	ret = brcmf_attach(&devinfo->pdev->dev);
 	if (ret)
 		goto fail;