| Message ID | 20191226092033.12600-1-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8c8e60fb86a90a30721bbd797f58f96b3980dcc1 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362 | expand |
On December 26, 2019 10:23:41 AM Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling > brcmf_bus_started()") changed the initialization order of the brcmfmac > SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now > called before the sdiodev->bus_if initialization, it reads the wrong > chip ID and fails to initialize the GPIO on brcm43362. Thus the chip > cannot send interrupts and fails to probe: > > [ 12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout > [ 12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110 > [ 12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110 > [ 12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed > > Initialize the bus interface earlier to ensure that > brcmf_sdiod_intr_register() properly sets up the OOB interrupt. > > BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438 > Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling > brcmf_bus_started()") Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > --- > A workaround [1] disabling the OOB interrupt is being discussed. It > works for me, but this patch fixes the wifi problem on my cubietruck. I missed that one. Too bad it was not sent to linux-wireless as well. Good find here. I did see another patch dealing with the OOB interrupt on Nvidia Tegra. Now I wonder if this is the same issue. Regards, Arend > [1] > https://lore.kernel.org/linux-arm-kernel/20180930150927.12076-1-hdegoede@redhat.com/ > --- > .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-)
26.12.2019 12:47, Arend Van Spriel пишет: > On December 26, 2019 10:23:41 AM Jean-Philippe Brucker > <jean-philippe@linaro.org> wrote: > >> Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling >> brcmf_bus_started()") changed the initialization order of the brcmfmac >> SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now >> called before the sdiodev->bus_if initialization, it reads the wrong >> chip ID and fails to initialize the GPIO on brcm43362. Thus the chip >> cannot send interrupts and fails to probe: >> >> [ 12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout >> [ 12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110 >> [ 12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding: >> err=-110 >> [ 12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach >> failed >> >> Initialize the bus interface earlier to ensure that >> brcmf_sdiod_intr_register() properly sets up the OOB interrupt. >> >> BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438 >> Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before >> calling brcmf_bus_started()") > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > >> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> >> --- >> A workaround [1] disabling the OOB interrupt is being discussed. It >> works for me, but this patch fixes the wifi problem on my cubietruck. > > I missed that one. Too bad it was not sent to linux-wireless as well. > Good find here. I did see another patch dealing with the OOB interrupt > on Nvidia Tegra. Now I wonder if this is the same issue. > > Regards, > Arend > >> [1] >> https://lore.kernel.org/linux-arm-kernel/20180930150927.12076-1-hdegoede@redhat.com/ >> >> --- >> .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) I haven't seen any driver probe failures due to OOB on NVIDIA Tegra, only suspend-resume was problematic due to the unbalanced OOB interrupt-wake enabling. But maybe checking whether OOB interrupt-wake works by invoking enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for the cubietruck board. @Jean-Philippe, could you please try this change (on top of recent linux-next): diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index b684a5b6d904..80d7106b10a9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev) } sdiodev->oob_irq_requested = true; - ret = enable_irq_wake(pdata->oob_irq_nr); - if (ret != 0) { - brcmf_err("enable_irq_wake failed %d\n", ret); - return ret; - } - disable_irq_wake(pdata->oob_irq_nr); - sdio_claim_host(sdiodev->func1); if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
Hi Dmitry, On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote: > I haven't seen any driver probe failures due to OOB on NVIDIA Tegra, > only suspend-resume was problematic due to the unbalanced OOB > interrupt-wake enabling. > > But maybe checking whether OOB interrupt-wake works by invoking > enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for > the cubietruck board. > > @Jean-Philippe, could you please try this change (on top of recent > linux-next): Sorry for the delay, linux-next doesn't boot for me at the moment and I have little time to investigate why, so I might retry closer to the merge window. However, isn't the interrupt-wake issue independent from the problem (introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it doesn't seem to cause a regression, but the wifi only works if I apply my patch as well. Thanks, Jean > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > index b684a5b6d904..80d7106b10a9 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev > *sdiodev) > } > sdiodev->oob_irq_requested = true; > > - ret = enable_irq_wake(pdata->oob_irq_nr); > - if (ret != 0) { > - brcmf_err("enable_irq_wake failed %d\n", ret); > - return ret; > - } > - disable_irq_wake(pdata->oob_irq_nr); > - > sdio_claim_host(sdiodev->func1); > > if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
06.01.2020 22:19, Jean-Philippe Brucker пишет: > Hi Dmitry, > > On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote: >> I haven't seen any driver probe failures due to OOB on NVIDIA Tegra, >> only suspend-resume was problematic due to the unbalanced OOB >> interrupt-wake enabling. >> >> But maybe checking whether OOB interrupt-wake works by invoking >> enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for >> the cubietruck board. >> >> @Jean-Philippe, could you please try this change (on top of recent >> linux-next): > > Sorry for the delay, linux-next doesn't boot for me at the moment and I > have little time to investigate why, so I might retry closer to the merge > window. > > However, isn't the interrupt-wake issue independent from the problem > (introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB > wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it > doesn't seem to cause a regression, but the wifi only works if I apply my > patch as well. > > Thanks, > Jean > >> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >> index b684a5b6d904..80d7106b10a9 100644 >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >> @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev >> *sdiodev) >> } >> sdiodev->oob_irq_requested = true; >> >> - ret = enable_irq_wake(pdata->oob_irq_nr); >> - if (ret != 0) { >> - brcmf_err("enable_irq_wake failed %d\n", ret); >> - return ret; >> - } >> - disable_irq_wake(pdata->oob_irq_nr); >> - >> sdio_claim_host(sdiodev->func1); >> >> if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) { Hello Jean, Could you please clarify whether you applied [1] and then the above snippet on top of it or you only applied [1] without the snippet? [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled
On Tue, Jan 07, 2020 at 02:15:18AM +0300, Dmitry Osipenko wrote: > 06.01.2020 22:19, Jean-Philippe Brucker пишет: > > Hi Dmitry, > > > > On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote: > >> I haven't seen any driver probe failures due to OOB on NVIDIA Tegra, > >> only suspend-resume was problematic due to the unbalanced OOB > >> interrupt-wake enabling. > >> > >> But maybe checking whether OOB interrupt-wake works by invoking > >> enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for > >> the cubietruck board. > >> > >> @Jean-Philippe, could you please try this change (on top of recent > >> linux-next): > > > > Sorry for the delay, linux-next doesn't boot for me at the moment and I > > have little time to investigate why, so I might retry closer to the merge > > window. > > > > However, isn't the interrupt-wake issue independent from the problem > > (introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB > > wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it > > doesn't seem to cause a regression, but the wifi only works if I apply my > > patch as well. > > > > Thanks, > > Jean > > > >> > >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > >> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > >> index b684a5b6d904..80d7106b10a9 100644 > >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c > >> @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev > >> *sdiodev) > >> } > >> sdiodev->oob_irq_requested = true; > >> > >> - ret = enable_irq_wake(pdata->oob_irq_nr); > >> - if (ret != 0) { > >> - brcmf_err("enable_irq_wake failed %d\n", ret); > >> - return ret; > >> - } > >> - disable_irq_wake(pdata->oob_irq_nr); > >> - > >> sdio_claim_host(sdiodev->func1); > >> > >> if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) { > > Hello Jean, > > Could you please clarify whether you applied [1] and then the above > snippet on top of it or you only applied [1] without the snippet? I applied [1] without the snippet Thanks, Jean > > [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled
07.01.2020 10:23, Jean-Philippe Brucker пишет: > On Tue, Jan 07, 2020 at 02:15:18AM +0300, Dmitry Osipenko wrote: >> 06.01.2020 22:19, Jean-Philippe Brucker пишет: >>> Hi Dmitry, >>> >>> On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote: >>>> I haven't seen any driver probe failures due to OOB on NVIDIA Tegra, >>>> only suspend-resume was problematic due to the unbalanced OOB >>>> interrupt-wake enabling. >>>> >>>> But maybe checking whether OOB interrupt-wake works by invoking >>>> enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for >>>> the cubietruck board. >>>> >>>> @Jean-Philippe, could you please try this change (on top of recent >>>> linux-next): >>> >>> Sorry for the delay, linux-next doesn't boot for me at the moment and I >>> have little time to investigate why, so I might retry closer to the merge >>> window. >>> >>> However, isn't the interrupt-wake issue independent from the problem >>> (introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB >>> wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it >>> doesn't seem to cause a regression, but the wifi only works if I apply my >>> patch as well. >>> >>> Thanks, >>> Jean >>> >>>> >>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >>>> index b684a5b6d904..80d7106b10a9 100644 >>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c >>>> @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev >>>> *sdiodev) >>>> } >>>> sdiodev->oob_irq_requested = true; >>>> >>>> - ret = enable_irq_wake(pdata->oob_irq_nr); >>>> - if (ret != 0) { >>>> - brcmf_err("enable_irq_wake failed %d\n", ret); >>>> - return ret; >>>> - } >>>> - disable_irq_wake(pdata->oob_irq_nr); >>>> - >>>> sdio_claim_host(sdiodev->func1); >>>> >>>> if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) { >> >> Hello Jean, >> >> Could you please clarify whether you applied [1] and then the above >> snippet on top of it or you only applied [1] without the snippet? > > I applied [1] without the snippet > > Thanks, > Jean > >> >> [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled Will you be able to test *with* the snippet? I guess chances that it will make any difference are not high, nevertheless will be good to know for sure.
On Tue, Jan 07, 2020 at 07:23:32PM +0300, Dmitry Osipenko wrote: > >> Hello Jean, > >> > >> Could you please clarify whether you applied [1] and then the above > >> snippet on top of it or you only applied [1] without the snippet? > > > > I applied [1] without the snippet > > > > Thanks, > > Jean > > > >> > >> [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled > > Will you be able to test *with* the snippet? I guess chances that it > will make any difference are not high, nevertheless will be good to know > for sure. I tested it with the snippet and didn't notice a difference Thanks, Jean
08.01.2020 10:39, Jean-Philippe Brucker пишет: > On Tue, Jan 07, 2020 at 07:23:32PM +0300, Dmitry Osipenko wrote: >>>> Hello Jean, >>>> >>>> Could you please clarify whether you applied [1] and then the above >>>> snippet on top of it or you only applied [1] without the snippet? >>> >>> I applied [1] without the snippet >>> >>> Thanks, >>> Jean >>> >>>> >>>> [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled >> >> Will you be able to test *with* the snippet? I guess chances that it >> will make any difference are not high, nevertheless will be good to know >> for sure. > > I tested it with the snippet and didn't notice a difference Okay
Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling > brcmf_bus_started()") changed the initialization order of the brcmfmac > SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now > called before the sdiodev->bus_if initialization, it reads the wrong > chip ID and fails to initialize the GPIO on brcm43362. Thus the chip > cannot send interrupts and fails to probe: > > [ 12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout > [ 12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110 > [ 12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110 > [ 12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed > > Initialize the bus interface earlier to ensure that > brcmf_sdiod_intr_register() properly sets up the OOB interrupt. > > BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438 > Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling brcmf_bus_started()") > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> Patch applied to wireless-drivers-next.git, thanks. 8c8e60fb86a9 brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 264ad63232f8..058069a03693 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -4220,38 +4220,38 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err, brcmf_sdio_sr_init(bus); } else { /* Restore previous clock setting */ brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_CHIPCLKCSR, saveclk, &err); } if (err == 0) { + /* Assign bus interface call back */ + sdiod->bus_if->dev = sdiod->dev; + sdiod->bus_if->ops = &brcmf_sdio_bus_ops; + sdiod->bus_if->chip = bus->ci->chip; + sdiod->bus_if->chiprev = bus->ci->chiprev; + /* Allow full data communication using DPC from now on. */ brcmf_sdiod_change_state(bus->sdiodev, BRCMF_SDIOD_DATA); err = brcmf_sdiod_intr_register(sdiod); if (err != 0) brcmf_err("intr register failed:%d\n", err); } /* If we didn't come up, turn off backplane clock */ if (err != 0) { brcmf_sdio_clkctl(bus, CLK_NONE, false); goto checkdied; } sdio_release_host(sdiod->func1); - /* Assign bus interface call back */ - sdiod->bus_if->dev = sdiod->dev; - sdiod->bus_if->ops = &brcmf_sdio_bus_ops; - sdiod->bus_if->chip = bus->ci->chip; - sdiod->bus_if->chiprev = bus->ci->chiprev; - err = brcmf_alloc(sdiod->dev, sdiod->settings); if (err) { brcmf_err("brcmf_alloc failed\n"); goto claim; } /* Attach to the common layer, reserve hdr space */ err = brcmf_attach(sdiod->dev);
Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling brcmf_bus_started()") changed the initialization order of the brcmfmac SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now called before the sdiodev->bus_if initialization, it reads the wrong chip ID and fails to initialize the GPIO on brcm43362. Thus the chip cannot send interrupts and fails to probe: [ 12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout [ 12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110 [ 12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110 [ 12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed Initialize the bus interface earlier to ensure that brcmf_sdiod_intr_register() properly sets up the OOB interrupt. BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438 Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling brcmf_bus_started()") Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- A workaround [1] disabling the OOB interrupt is being discussed. It works for me, but this patch fixes the wifi problem on my cubietruck. [1] https://lore.kernel.org/linux-arm-kernel/20180930150927.12076-1-hdegoede@redhat.com/ --- .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)