| Message ID | 20220104072658.69756-13-marcan@marcan.st (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: Support Apple T2 and M1 platforms | expand |
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).
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.
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(-)
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 --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;
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(-)